diff options
-rw-r--r-- | libs/gui/BLASTBufferQueue.cpp | 20 | ||||
-rw-r--r-- | libs/gui/ITransactionCompletedListener.cpp | 16 | ||||
-rw-r--r-- | libs/gui/SurfaceComposerClient.cpp | 29 | ||||
-rw-r--r-- | libs/gui/include/gui/BLASTBufferQueue.h | 10 | ||||
-rw-r--r-- | libs/gui/include/gui/ITransactionCompletedListener.h | 3 | ||||
-rw-r--r-- | libs/gui/include/gui/SurfaceComposerClient.h | 6 | ||||
-rw-r--r-- | services/surfaceflinger/ClientCache.cpp | 23 | ||||
-rw-r--r-- | services/surfaceflinger/ClientCache.h | 5 | ||||
-rw-r--r-- | services/surfaceflinger/FrontEnd/TransactionHandler.cpp | 13 | ||||
-rw-r--r-- | services/surfaceflinger/FrontEnd/TransactionHandler.h | 3 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 82 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 5 | ||||
-rw-r--r-- | services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp | 3 |
13 files changed, 123 insertions, 95 deletions
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 8b9a878598..fc8aff87af 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -167,14 +167,15 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati mNumFrameAvailable = 0; TransactionCompletedListener::getInstance()->addQueueStallListener( - [&]() { - std::function<void(bool)> callbackCopy; - { - std::unique_lock _lock{mMutex}; - callbackCopy = mTransactionHangCallback; - } - if (callbackCopy) callbackCopy(true); - }, this); + [&](const std::string& reason) { + std::function<void(const std::string&)> callbackCopy; + { + std::unique_lock _lock{mMutex}; + callbackCopy = mTransactionHangCallback; + } + if (callbackCopy) callbackCopy(reason); + }, + this); BQA_LOGV("BLASTBufferQueue created"); } @@ -1112,7 +1113,8 @@ bool BLASTBufferQueue::isSameSurfaceControl(const sp<SurfaceControl>& surfaceCon return SurfaceControl::isSameSurface(mSurfaceControl, surfaceControl); } -void BLASTBufferQueue::setTransactionHangCallback(std::function<void(bool)> callback) { +void BLASTBufferQueue::setTransactionHangCallback( + std::function<void(const std::string&)> callback) { std::unique_lock _lock{mMutex}; mTransactionHangCallback = callback; } diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp index ca91afa35d..2a114c82ee 100644 --- a/libs/gui/ITransactionCompletedListener.cpp +++ b/libs/gui/ITransactionCompletedListener.cpp @@ -273,15 +273,17 @@ public: void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence, uint32_t currentMaxAcquiredBufferCount) override { - callRemoteAsync<decltype( - &ITransactionCompletedListener::onReleaseBuffer)>(Tag::ON_RELEASE_BUFFER, - callbackId, releaseFence, - currentMaxAcquiredBufferCount); + callRemoteAsync<decltype(&ITransactionCompletedListener:: + onReleaseBuffer)>(Tag::ON_RELEASE_BUFFER, callbackId, + releaseFence, + currentMaxAcquiredBufferCount); } - void onTransactionQueueStalled() override { - callRemoteAsync<decltype(&ITransactionCompletedListener::onTransactionQueueStalled)>( - Tag::ON_TRANSACTION_QUEUE_STALLED); + void onTransactionQueueStalled(const String8& reason) override { + callRemoteAsync< + decltype(&ITransactionCompletedListener:: + onTransactionQueueStalled)>(Tag::ON_TRANSACTION_QUEUE_STALLED, + reason); } }; diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index a5879a77e1..9efdd35a52 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -455,23 +455,24 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener } } -void TransactionCompletedListener::onTransactionQueueStalled() { - std::unordered_map<void*, std::function<void()>> callbackCopy; - { - std::scoped_lock<std::mutex> lock(mMutex); - callbackCopy = mQueueStallListeners; - } - for (auto const& it : callbackCopy) { - it.second(); - } -} - -void TransactionCompletedListener::addQueueStallListener(std::function<void()> stallListener, - void* id) { +void TransactionCompletedListener::onTransactionQueueStalled(const String8& reason) { + std::unordered_map<void*, std::function<void(const std::string&)>> callbackCopy; + { + std::scoped_lock<std::mutex> lock(mMutex); + callbackCopy = mQueueStallListeners; + } + for (auto const& it : callbackCopy) { + it.second(reason.c_str()); + } +} + +void TransactionCompletedListener::addQueueStallListener( + std::function<void(const std::string&)> stallListener, void* id) { std::scoped_lock<std::mutex> lock(mMutex); mQueueStallListeners[id] = stallListener; } -void TransactionCompletedListener::removeQueueStallListener(void *id) { + +void TransactionCompletedListener::removeQueueStallListener(void* id) { std::scoped_lock<std::mutex> lock(mMutex); mQueueStallListeners.erase(id); } diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 827a6cc5a9..957652e1f1 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -113,12 +113,10 @@ public: uint64_t getLastAcquiredFrameNum(); /** - * Set a callback to be invoked when we are hung. The boolean parameter - * indicates whether the hang is due to an unfired fence. - * TODO: The boolean is always true atm, unfired fence is - * the only case we detect. + * Set a callback to be invoked when we are hung. The string parameter + * indicates the reason for the hang. */ - void setTransactionHangCallback(std::function<void(bool)> callback); + void setTransactionHangCallback(std::function<void(const std::string&)> callback); virtual ~BLASTBufferQueue(); @@ -282,7 +280,7 @@ private: bool mAppliedLastTransaction = false; uint64_t mLastAppliedFrameNumber = 0; - std::function<void(bool)> mTransactionHangCallback; + std::function<void(const std::string&)> mTransactionHangCallback; std::unordered_set<uint64_t> mSyncedFrameNumbers GUARDED_BY(mMutex); }; diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h index cc136bb40a..d2a6f40cea 100644 --- a/libs/gui/include/gui/ITransactionCompletedListener.h +++ b/libs/gui/include/gui/ITransactionCompletedListener.h @@ -194,7 +194,8 @@ public: virtual void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence, uint32_t currentMaxAcquiredBufferCount) = 0; - virtual void onTransactionQueueStalled() = 0; + + virtual void onTransactionQueueStalled(const String8& name) = 0; }; class BnTransactionCompletedListener : public SafeBnInterface<ITransactionCompletedListener> { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 36969db576..9efde6cb35 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -786,7 +786,7 @@ protected: // This is protected by mSurfaceStatsListenerMutex, but GUARDED_BY isn't supported for // std::recursive_mutex std::multimap<int32_t, SurfaceStatsCallbackEntry> mSurfaceStatsListeners; - std::unordered_map<void*, std::function<void()>> mQueueStallListeners; + std::unordered_map<void*, std::function<void(const std::string&)>> mQueueStallListeners; public: static sp<TransactionCompletedListener> getInstance(); @@ -804,7 +804,7 @@ public: const sp<SurfaceControl>& surfaceControl, const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds); - void addQueueStallListener(std::function<void()> stallListener, void* id); + void addQueueStallListener(std::function<void(const std::string&)> stallListener, void* id); void removeQueueStallListener(void *id); /* @@ -835,7 +835,7 @@ public: // For Testing Only static void setInstance(const sp<TransactionCompletedListener>&); - void onTransactionQueueStalled() override; + void onTransactionQueueStalled(const String8& reason) override; private: ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&); diff --git a/services/surfaceflinger/ClientCache.cpp b/services/surfaceflinger/ClientCache.cpp index b01932e413..2bd8f324e1 100644 --- a/services/surfaceflinger/ClientCache.cpp +++ b/services/surfaceflinger/ClientCache.cpp @@ -59,16 +59,17 @@ bool ClientCache::getBuffer(const client_cache_t& cacheId, return true; } -bool ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer) { +base::expected<std::shared_ptr<renderengine::ExternalTexture>, ClientCache::AddError> +ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer) { auto& [processToken, id] = cacheId; if (processToken == nullptr) { ALOGE_AND_TRACE("ClientCache::add - invalid (nullptr) process token"); - return false; + return base::unexpected(AddError::Unspecified); } if (!buffer) { ALOGE_AND_TRACE("ClientCache::add - invalid (nullptr) buffer"); - return false; + return base::unexpected(AddError::Unspecified); } std::lock_guard lock(mMutex); @@ -81,7 +82,7 @@ bool ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& bu token = processToken.promote(); if (!token) { ALOGE_AND_TRACE("ClientCache::add - invalid token"); - return false; + return base::unexpected(AddError::Unspecified); } // Only call linkToDeath if not a local binder @@ -89,7 +90,7 @@ bool ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& bu status_t err = token->linkToDeath(mDeathRecipient); if (err != NO_ERROR) { ALOGE_AND_TRACE("ClientCache::add - could not link to death"); - return false; + return base::unexpected(AddError::Unspecified); } } auto [itr, success] = @@ -104,17 +105,17 @@ bool ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& bu if (processBuffers.size() > BUFFER_CACHE_MAX_SIZE) { ALOGE_AND_TRACE("ClientCache::add - cache is full"); - return false; + return base::unexpected(AddError::CacheFull); } LOG_ALWAYS_FATAL_IF(mRenderEngine == nullptr, "Attempted to build the ClientCache before a RenderEngine instance was " "ready!"); - processBuffers[id].buffer = std::make_shared< - renderengine::impl::ExternalTexture>(buffer, *mRenderEngine, - renderengine::impl::ExternalTexture::Usage:: - READABLE); - return true; + + return (processBuffers[id].buffer = std::make_shared< + renderengine::impl::ExternalTexture>(buffer, *mRenderEngine, + renderengine::impl::ExternalTexture:: + Usage::READABLE)); } void ClientCache::erase(const client_cache_t& cacheId) { diff --git a/services/surfaceflinger/ClientCache.h b/services/surfaceflinger/ClientCache.h index a9b8177d70..cdeac2bbc1 100644 --- a/services/surfaceflinger/ClientCache.h +++ b/services/surfaceflinger/ClientCache.h @@ -37,7 +37,10 @@ class ClientCache : public Singleton<ClientCache> { public: ClientCache(); - bool add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer); + enum class AddError { CacheFull, Unspecified }; + + base::expected<std::shared_ptr<renderengine::ExternalTexture>, AddError> add( + const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer); void erase(const client_cache_t& cacheId); std::shared_ptr<renderengine::ExternalTexture> get(const client_cache_t& cacheId); diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp index 6c6a487b67..95961fe6a9 100644 --- a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp +++ b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp @@ -168,15 +168,16 @@ bool TransactionHandler::hasPendingTransactions() { return !mPendingTransactionQueues.empty() || !mLocklessTransactionQueue.isEmpty(); } -void TransactionHandler::onTransactionQueueStalled(const TransactionState& transaction, - sp<ITransactionCompletedListener>& listener) { - if (std::find(mStalledTransactions.begin(), mStalledTransactions.end(), transaction.id) != +void TransactionHandler::onTransactionQueueStalled(uint64_t transactionId, + sp<ITransactionCompletedListener>& listener, + const std::string& reason) { + if (std::find(mStalledTransactions.begin(), mStalledTransactions.end(), transactionId) != mStalledTransactions.end()) { return; } - mStalledTransactions.push_back(transaction.id); - listener->onTransactionQueueStalled(); + mStalledTransactions.push_back(transactionId); + listener->onTransactionQueueStalled(String8(reason.c_str())); } void TransactionHandler::removeFromStalledTransactions(uint64_t id) { @@ -185,4 +186,4 @@ void TransactionHandler::removeFromStalledTransactions(uint64_t id) { mStalledTransactions.erase(it); } } -} // namespace android
\ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.h b/services/surfaceflinger/FrontEnd/TransactionHandler.h index 237b48d55f..2b6f07d49a 100644 --- a/services/surfaceflinger/FrontEnd/TransactionHandler.h +++ b/services/surfaceflinger/FrontEnd/TransactionHandler.h @@ -54,7 +54,8 @@ public: std::vector<TransactionState> flushTransactions(); void addTransactionReadyFilter(TransactionFilter&&); void queueTransaction(TransactionState&&); - void onTransactionQueueStalled(const TransactionState&, sp<ITransactionCompletedListener>&); + void onTransactionQueueStalled(uint64_t transactionId, sp<ITransactionCompletedListener>&, + const std::string& reason); void removeFromStalledTransactions(uint64_t transactionId); private: diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 100ad43f32..d8491592d0 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3744,7 +3744,10 @@ TransactionHandler::TransactionReadiness SurfaceFlinger::transactionReadyBufferC if (listener && (flushState.queueProcessTime - transaction.postTime) > std::chrono::nanoseconds(4s).count()) { - mTransactionHandler.onTransactionQueueStalled(transaction, listener); + mTransactionHandler + .onTransactionQueueStalled(transaction.id, listener, + "Buffer processing hung up due to stuck " + "fence. Indicates GPU hang"); } return false; } @@ -3960,8 +3963,9 @@ bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin uint32_t clientStateFlags = 0; for (int i = 0; i < states.size(); i++) { ComposerState& state = states.editItemAt(i); - clientStateFlags |= setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, - isAutoTimestamp, postTime, permissions); + clientStateFlags |= + setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp, + postTime, permissions, transactionId); if ((flags & eAnimation) && state.state.surface) { if (const auto layer = fromHandle(state.state.surface).promote()) { using LayerUpdateType = scheduler::LayerHistory::LayerUpdateType; @@ -4077,7 +4081,8 @@ bool SurfaceFlinger::callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermis uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTimelineInfo, ComposerState& composerState, int64_t desiredPresentTime, bool isAutoTimestamp, - int64_t postTime, uint32_t permissions) { + int64_t postTime, uint32_t permissions, + uint64_t transactionId) { layer_state_t& s = composerState.state; s.sanitize(permissions); @@ -4370,7 +4375,8 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime if (what & layer_state_t::eBufferChanged) { std::shared_ptr<renderengine::ExternalTexture> buffer = - getExternalTextureFromBufferData(*s.bufferData, layer->getDebugName()); + getExternalTextureFromBufferData(*s.bufferData, layer->getDebugName(), + transactionId); if (layer->setBuffer(buffer, *s.bufferData, postTime, desiredPresentTime, isAutoTimestamp, dequeueBufferTimestamp, frameTimelineInfo)) { flags |= eTraversalNeeded; @@ -6955,34 +6961,44 @@ status_t SurfaceFlinger::removeWindowInfosListener( } std::shared_ptr<renderengine::ExternalTexture> SurfaceFlinger::getExternalTextureFromBufferData( - const BufferData& bufferData, const char* layerName) const { - bool cacheIdChanged = bufferData.flags.test(BufferData::BufferDataChange::cachedBufferChanged); - bool bufferSizeExceedsLimit = false; - std::shared_ptr<renderengine::ExternalTexture> buffer = nullptr; - if (cacheIdChanged && bufferData.buffer != nullptr) { - bufferSizeExceedsLimit = exceedsMaxRenderTargetSize(bufferData.buffer->getWidth(), - bufferData.buffer->getHeight()); - if (!bufferSizeExceedsLimit) { - ClientCache::getInstance().add(bufferData.cachedBuffer, bufferData.buffer); - buffer = ClientCache::getInstance().get(bufferData.cachedBuffer); - } - } else if (cacheIdChanged) { - buffer = ClientCache::getInstance().get(bufferData.cachedBuffer); - } else if (bufferData.buffer != nullptr) { - bufferSizeExceedsLimit = exceedsMaxRenderTargetSize(bufferData.buffer->getWidth(), - bufferData.buffer->getHeight()); - if (!bufferSizeExceedsLimit) { - buffer = std::make_shared< - renderengine::impl::ExternalTexture>(bufferData.buffer, getRenderEngine(), - renderengine::impl::ExternalTexture:: - Usage::READABLE); - } - } - ALOGE_IF(bufferSizeExceedsLimit, - "Attempted to create an ExternalTexture for layer %s that exceeds render target size " - "limit.", - layerName); - return buffer; + BufferData& bufferData, const char* layerName, uint64_t transactionId) { + if (bufferData.buffer && + exceedsMaxRenderTargetSize(bufferData.buffer->getWidth(), bufferData.buffer->getHeight())) { + ALOGE("Attempted to create an ExternalTexture for layer %s that exceeds render target " + "size limit.", + layerName); + return nullptr; + } + + bool cachedBufferChanged = + bufferData.flags.test(BufferData::BufferDataChange::cachedBufferChanged); + if (cachedBufferChanged && bufferData.buffer) { + auto result = ClientCache::getInstance().add(bufferData.cachedBuffer, bufferData.buffer); + if (result.ok()) { + return result.value(); + } + + if (result.error() == ClientCache::AddError::CacheFull) { + mTransactionHandler + .onTransactionQueueStalled(transactionId, bufferData.releaseBufferListener, + "Buffer processing hung due to full buffer cache"); + } + + return nullptr; + } + + if (cachedBufferChanged) { + return ClientCache::getInstance().get(bufferData.cachedBuffer); + } + + if (bufferData.buffer) { + return std::make_shared< + renderengine::impl::ExternalTexture>(bufferData.buffer, getRenderEngine(), + renderengine::impl::ExternalTexture::Usage:: + READABLE); + } + + return nullptr; } bool SurfaceFlinger::commitMirrorDisplays(VsyncId vsyncId) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 674f21f77e..3a14ba0493 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -312,7 +312,7 @@ protected: REQUIRES(mStateLock); virtual std::shared_ptr<renderengine::ExternalTexture> getExternalTextureFromBufferData( - const BufferData& bufferData, const char* layerName) const; + BufferData& bufferData, const char* layerName, uint64_t transactionId); // Returns true if any display matches a `bool(const DisplayDevice&)` predicate. template <typename Predicate> @@ -728,7 +728,8 @@ private: uint32_t setClientStateLocked(const FrameTimelineInfo&, ComposerState&, int64_t desiredPresentTime, bool isAutoTimestamp, - int64_t postTime, uint32_t permissions) REQUIRES(mStateLock); + int64_t postTime, uint32_t permissions, uint64_t transactionId) + REQUIRES(mStateLock); uint32_t getTransactionFlags() const; diff --git a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp index 276b3dad18..3b11f2383e 100644 --- a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp +++ b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp @@ -100,7 +100,8 @@ public: MockSurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SurfaceFlinger::SkipInitialization) {} std::shared_ptr<renderengine::ExternalTexture> getExternalTextureFromBufferData( - const BufferData& bufferData, const char* /* layerName */) const override { + BufferData& bufferData, const char* /* layerName */, + uint64_t /* transactionId */) override { return std::make_shared<renderengine::mock::FakeExternalTexture>(bufferData.getWidth(), bufferData.getHeight(), bufferData.getId(), |