diff options
5 files changed, 171 insertions, 65 deletions
diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp index 8629671214..a209cad612 100644 --- a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp +++ b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp @@ -64,11 +64,61 @@ std::vector<TransactionState> TransactionHandler::flushTransactions() { transactionsPendingBarrier = flushPendingTransactionQueues(transactions, flushState); } while (lastTransactionsPendingBarrier != transactionsPendingBarrier); + applyUnsignaledBufferTransaction(transactions, flushState); + mPendingTransactionCount.fetch_sub(transactions.size()); ATRACE_INT("TransactionQueue", static_cast<int>(mPendingTransactionCount.load())); return transactions; } +void TransactionHandler::applyUnsignaledBufferTransaction( + std::vector<TransactionState>& transactions, TransactionFlushState& flushState) { + // only apply an unsignaled buffer transaction if it's the first one + if (!transactions.empty()) { + return; + } + + if (!flushState.queueWithUnsignaledBuffer) { + return; + } + + auto it = mPendingTransactionQueues.find(flushState.queueWithUnsignaledBuffer); + LOG_ALWAYS_FATAL_IF(it == mPendingTransactionQueues.end(), + "Could not find queue with unsignaled buffer!"); + + auto& queue = it->second; + popTransactionFromPending(transactions, flushState, queue); + if (queue.empty()) { + it = mPendingTransactionQueues.erase(it); + } +} + +void TransactionHandler::popTransactionFromPending(std::vector<TransactionState>& transactions, + TransactionFlushState& flushState, + std::queue<TransactionState>& queue) { + auto& transaction = queue.front(); + // Transaction is ready move it from the pending queue. + flushState.firstTransaction = false; + removeFromStalledTransactions(transaction.id); + transactions.emplace_back(std::move(transaction)); + queue.pop(); + + auto& readyToApplyTransaction = transactions.back(); + readyToApplyTransaction.traverseStatesWithBuffers([&](const layer_state_t& state) { + const bool frameNumberChanged = + state.bufferData->flags.test(BufferData::BufferDataChange::frameNumberChanged); + if (frameNumberChanged) { + flushState.bufferLayersReadyToPresent.emplace_or_replace(state.surface.get(), + state.bufferData->frameNumber); + } else { + // Barrier function only used for BBQ which always includes a frame number. + // This value only used for barrier logic. + flushState.bufferLayersReadyToPresent + .emplace_or_replace(state.surface.get(), std::numeric_limits<uint64_t>::max()); + } + }); +} + TransactionHandler::TransactionReadiness TransactionHandler::applyFilters( TransactionFlushState& flushState) { auto ready = TransactionReadiness::Ready; @@ -79,8 +129,7 @@ TransactionHandler::TransactionReadiness TransactionHandler::applyFilters( case TransactionReadiness::NotReadyBarrier: return perFilterReady; - case TransactionReadiness::ReadyUnsignaled: - case TransactionReadiness::ReadyUnsignaledSingle: + case TransactionReadiness::NotReadyUnsignaled: // If one of the filters allows latching an unsignaled buffer, latch this ready // state. ready = perFilterReady; @@ -97,17 +146,7 @@ int TransactionHandler::flushPendingTransactionQueues(std::vector<TransactionSta int transactionsPendingBarrier = 0; auto it = mPendingTransactionQueues.begin(); while (it != mPendingTransactionQueues.end()) { - auto& queue = it->second; - IBinder* queueToken = it->first.get(); - - // if we have already flushed a transaction with an unsignaled buffer then stop queue - // processing - if (std::find(flushState.queuesWithUnsignaledBuffers.begin(), - flushState.queuesWithUnsignaledBuffers.end(), - queueToken) != flushState.queuesWithUnsignaledBuffers.end()) { - continue; - } - + auto& [applyToken, queue] = *it; while (!queue.empty()) { auto& transaction = queue.front(); flushState.transaction = &transaction; @@ -117,38 +156,14 @@ int TransactionHandler::flushPendingTransactionQueues(std::vector<TransactionSta break; } else if (ready == TransactionReadiness::NotReady) { break; - } - - // Transaction is ready move it from the pending queue. - flushState.firstTransaction = false; - removeFromStalledTransactions(transaction.id); - transactions.emplace_back(std::move(transaction)); - queue.pop(); - - // If the buffer is unsignaled, then we don't want to signal other transactions using - // the buffer as a barrier. - auto& readyToApplyTransaction = transactions.back(); - if (ready == TransactionReadiness::Ready) { - readyToApplyTransaction.traverseStatesWithBuffers([&](const layer_state_t& state) { - const bool frameNumberChanged = state.bufferData->flags.test( - BufferData::BufferDataChange::frameNumberChanged); - if (frameNumberChanged) { - flushState.bufferLayersReadyToPresent - .emplace_or_replace(state.surface.get(), - state.bufferData->frameNumber); - } else { - // Barrier function only used for BBQ which always includes a frame number. - // This value only used for barrier logic. - flushState.bufferLayersReadyToPresent - .emplace_or_replace(state.surface.get(), - std::numeric_limits<uint64_t>::max()); - } - }); - } else if (ready == TransactionReadiness::ReadyUnsignaledSingle) { - // Track queues with a flushed unsingaled buffer. - flushState.queuesWithUnsignaledBuffers.emplace_back(queueToken); + } else if (ready == TransactionReadiness::NotReadyUnsignaled) { + // We maybe able to latch this transaction if it's the only transaction + // ready to be applied. + flushState.queueWithUnsignaledBuffer = applyToken; break; } + // ready == TransactionReadiness::Ready + popTransactionFromPending(transactions, flushState, queue); } if (queue.empty()) { diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.h b/services/surfaceflinger/FrontEnd/TransactionHandler.h index 7fc825eba3..865835f92d 100644 --- a/services/surfaceflinger/FrontEnd/TransactionHandler.h +++ b/services/surfaceflinger/FrontEnd/TransactionHandler.h @@ -40,14 +40,20 @@ public: // Layer handles that have transactions with buffers that are ready to be applied. ftl::SmallMap<IBinder* /* binder address */, uint64_t /* framenumber */, 15> bufferLayersReadyToPresent = {}; - ftl::SmallVector<IBinder* /* queueToken */, 15> queuesWithUnsignaledBuffers; + // Tracks the queue with an unsignaled buffer. This is used to handle + // LatchUnsignaledConfig::AutoSingleLayer to ensure we only apply an unsignaled buffer + // if it's the only transaction that is ready to be applied. + sp<IBinder> queueWithUnsignaledBuffer = nullptr; }; enum class TransactionReadiness { + // Transaction is ready to be applied + Ready, + // Transaction has unmet conditions (fence, present time, etc) and cannot be applied. NotReady, + // Transaction is waiting on a barrier (another buffer to be latched first) NotReadyBarrier, - Ready, - ReadyUnsignaled, - ReadyUnsignaledSingle, + // Transaction has an unsignaled fence but can be applied if it's the only transaction + NotReadyUnsignaled, }; using TransactionFilter = std::function<TransactionReadiness(const TransactionFlushState&)>; @@ -64,6 +70,9 @@ private: friend class ::android::TestableSurfaceFlinger; int flushPendingTransactionQueues(std::vector<TransactionState>&, TransactionFlushState&); + void applyUnsignaledBufferTransaction(std::vector<TransactionState>&, TransactionFlushState&); + void popTransactionFromPending(std::vector<TransactionState>&, TransactionFlushState&, + std::queue<TransactionState>&); TransactionReadiness applyFilters(TransactionFlushState&); std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IListenerHash> mPendingTransactionQueues; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 145fd46187..b543467217 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4262,20 +4262,23 @@ TransactionHandler::TransactionReadiness SurfaceFlinger::transactionReadyBufferC return TraverseBuffersReturnValues::STOP_TRAVERSAL; } - // check fence status - const bool allowLatchUnsignaled = shouldLatchUnsignaled(layer, s, transaction.states.size(), - flushState.firstTransaction); - ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(), - allowLatchUnsignaled ? "true" : "false"); - - const bool acquireFenceChanged = s.bufferData && + // ignore the acquire fence if LatchUnsignaledConfig::Always is set. + const bool checkAcquireFence = enableLatchUnsignaledConfig != LatchUnsignaledConfig::Always; + const bool acquireFenceAvailable = s.bufferData && s.bufferData->flags.test(BufferData::BufferDataChange::fenceChanged) && s.bufferData->acquireFence; - const bool fenceSignaled = - (!acquireFenceChanged || - s.bufferData->acquireFence->getStatus() != Fence::Status::Unsignaled); + const bool fenceSignaled = !checkAcquireFence || !acquireFenceAvailable || + s.bufferData->acquireFence->getStatus() != Fence::Status::Unsignaled; if (!fenceSignaled) { - if (!allowLatchUnsignaled) { + // check fence status + const bool allowLatchUnsignaled = + shouldLatchUnsignaled(layer, s, transaction.states.size(), + flushState.firstTransaction); + ATRACE_FORMAT("%s allowLatchUnsignaled=%s", layer->getName().c_str(), + allowLatchUnsignaled ? "true" : "false"); + if (allowLatchUnsignaled) { + ready = TransactionReadiness::NotReadyUnsignaled; + } else { ready = TransactionReadiness::NotReady; auto& listener = s.bufferData->releaseBufferListener; if (listener && @@ -4288,10 +4291,6 @@ TransactionHandler::TransactionReadiness SurfaceFlinger::transactionReadyBufferC } return TraverseBuffersReturnValues::STOP_TRAVERSAL; } - - ready = enableLatchUnsignaledConfig == LatchUnsignaledConfig::AutoSingleLayer - ? TransactionReadiness::ReadyUnsignaledSingle - : TransactionReadiness::ReadyUnsignaled; } return TraverseBuffersReturnValues::CONTINUE_TRAVERSAL; }); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 35707a2682..6fa918c33a 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -174,10 +174,15 @@ enum class LatchUnsignaledConfig { // Latch unsignaled is permitted when a single layer is updated in a frame, // and the update includes just a buffer update (i.e. no sync transactions // or geometry changes). + // Latch unsignaled is also only permitted when a single transaction is ready + // to be applied. If we pass an unsignaled fence to HWC, HWC might miss presenting + // the frame if the fence does not fire in time. If we apply another transaction, + // we may penalize the other transaction unfairly. AutoSingleLayer, // All buffers are latched unsignaled. This behaviour is discouraged as it // can break sync transactions, stall the display and cause undesired side effects. + // This is equivalent to ignoring the acquire fence when applying transactions. Always, }; diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp index d4e2357b6b..03c4e713a0 100644 --- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp @@ -294,7 +294,8 @@ public: return fence; } - ComposerState createComposerState(int layerId, sp<Fence> fence, uint64_t what) { + ComposerState createComposerState(int layerId, sp<Fence> fence, uint64_t what, + std::optional<sp<IBinder>> layerHandle = std::nullopt) { ComposerState state; state.state.bufferData = std::make_shared<fake::BufferData>(/* bufferId */ 123L, /* width */ 1, @@ -302,15 +303,20 @@ public: /* outUsage */ 0); state.state.bufferData->acquireFence = std::move(fence); state.state.layerId = layerId; - state.state.surface = + state.state.surface = layerHandle.value_or( sp<Layer>::make(LayerCreationArgs(mFlinger.flinger(), nullptr, "TestLayer", 0, {})) - ->getHandle(); + ->getHandle()); state.state.bufferData->flags = BufferData::BufferDataChange::fenceChanged; state.state.what = what; if (what & layer_state_t::eCropChanged) { state.state.crop = Rect(1, 2, 3, 4); } + if (what & layer_state_t::eFlagsChanged) { + state.state.flags = layer_state_t::eEnableBackpressure; + state.state.mask = layer_state_t::eEnableBackpressure; + } + return state; } @@ -601,6 +607,41 @@ TEST_F(LatchUnsignaledAutoSingleLayerTest, DontLatchUnsignaledWhenEarlyOffset) { setTransactionStates({unsignaledTransaction}, kExpectedTransactionsPending); } +TEST_F(LatchUnsignaledAutoSingleLayerTest, UnsignaledNotAppliedWhenThereAreSignaled_SignaledFirst) { + const sp<IBinder> kApplyToken1 = + IInterface::asBinder(TransactionCompletedListener::getIInstance()); + const sp<IBinder> kApplyToken2 = sp<BBinder>::make(); + const sp<IBinder> kApplyToken3 = sp<BBinder>::make(); + const auto kLayerId1 = 1; + const auto kLayerId2 = 2; + const auto kExpectedTransactionsPending = 1u; + + const auto signaledTransaction = + createTransactionInfo(kApplyToken1, + { + createComposerState(kLayerId1, + fence(Fence::Status::Signaled), + layer_state_t::eBufferChanged), + }); + const auto signaledTransaction2 = + createTransactionInfo(kApplyToken2, + { + createComposerState(kLayerId1, + fence(Fence::Status::Signaled), + layer_state_t::eBufferChanged), + }); + const auto unsignaledTransaction = + createTransactionInfo(kApplyToken3, + { + createComposerState(kLayerId2, + fence(Fence::Status::Unsignaled), + layer_state_t::eBufferChanged), + }); + + setTransactionStates({signaledTransaction, signaledTransaction2, unsignaledTransaction}, + kExpectedTransactionsPending); +} + class LatchUnsignaledDisabledTest : public LatchUnsignaledTest { public: void SetUp() override { @@ -943,6 +984,43 @@ TEST_F(LatchUnsignaledAlwaysTest, Flush_RemovesUnsignaledFromTheQueue) { kExpectedTransactionsPending); } +TEST_F(LatchUnsignaledAlwaysTest, RespectsBackPressureFlag) { + const sp<IBinder> kApplyToken1 = + IInterface::asBinder(TransactionCompletedListener::getIInstance()); + const sp<IBinder> kApplyToken2 = sp<BBinder>::make(); + const auto kLayerId1 = 1; + const auto kExpectedTransactionsPending = 1u; + auto layer = + sp<Layer>::make(LayerCreationArgs(mFlinger.flinger(), nullptr, "TestLayer", 0, {})); + auto layerHandle = layer->getHandle(); + const auto setBackPressureFlagTransaction = + createTransactionInfo(kApplyToken1, + {createComposerState(kLayerId1, fence(Fence::Status::Unsignaled), + layer_state_t::eBufferChanged | + layer_state_t::eFlagsChanged, + {layerHandle})}); + setTransactionStates({setBackPressureFlagTransaction}, 0u); + + const auto unsignaledTransaction = + createTransactionInfo(kApplyToken1, + { + createComposerState(kLayerId1, + fence(Fence::Status::Unsignaled), + layer_state_t::eBufferChanged, + {layerHandle}), + }); + const auto unsignaledTransaction2 = + createTransactionInfo(kApplyToken1, + { + createComposerState(kLayerId1, + fence(Fence::Status::Unsignaled), + layer_state_t::eBufferChanged, + {layerHandle}), + }); + setTransactionStates({unsignaledTransaction, unsignaledTransaction2}, + kExpectedTransactionsPending); +} + TEST_F(LatchUnsignaledAlwaysTest, LatchUnsignaledWhenEarlyOffset) { const sp<IBinder> kApplyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance()); |