From ac70bc579028cdc00a2f14b77718080b26b93c7d Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Tue, 9 Jul 2024 17:11:28 -0500 Subject: Optimize BLAST buffer releases via Unix sockets Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: BLASTBufferQueueTest Change-Id: Ia183452198dadc7f8e540f7219bd44d8b5823458 --- libs/gui/BLASTBufferQueue.cpp | 326 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 307 insertions(+), 19 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 739c3c2a41..f13d499b50 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -38,13 +38,17 @@ #include #include +#include #include +#include +#include #include #include using namespace com::android::graphics::libgui; using namespace std::chrono_literals; +using android::base::unique_fd; namespace { inline const char* boolToString(bool b) { @@ -179,8 +183,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati // explicitly so that dequeueBuffer will block mProducer->setDequeueTimeout(std::numeric_limits::max()); - // safe default, most producers are expected to override this - mProducer->setMaxDequeuedBufferCount(2); mBufferItemConsumer = new BLASTBufferItemConsumer(mConsumer, GraphicBuffer::USAGE_HW_COMPOSER | GraphicBuffer::USAGE_HW_TEXTURE, @@ -210,6 +212,12 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + std::unique_ptr bufferReleaseConsumer; + gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); + mBufferReleaseReader.emplace(std::move(bufferReleaseConsumer)); +#endif + BQA_LOGV("BLASTBufferQueue created"); } @@ -259,6 +267,9 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); + if (mBufferReleaseProducer) { + t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); + } applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -439,6 +450,21 @@ void BLASTBufferQueue::releaseBufferCallback( BBQ_TRACE(); releaseBufferCallbackLocked(id, releaseFence, currentMaxAcquiredBufferCount, false /* fakeRelease */); + if (!mBufferReleaseReader) { + return; + } + // Drain the buffer release channel socket + while (true) { + ReleaseCallbackId releaseCallbackId; + sp releaseFence; + if (status_t status = + mBufferReleaseReader->readNonBlocking(releaseCallbackId, releaseFence); + status != OK) { + break; + } + releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, + false /* fakeRelease */); + } } void BLASTBufferQueue::releaseBufferCallbackLocked( @@ -495,11 +521,11 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, const sp& releaseFence) { auto it = mSubmitted.find(callbackId); if (it == mSubmitted.end()) { - BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s", - callbackId.to_string().c_str()); return; } mNumAcquired--; + updateDequeueShouldBlockLocked(); + mBufferReleaseReader->interruptBlockingRead(); BBQ_TRACE("frame=%" PRIu64, callbackId.framenumber); BQA_LOGV("released %s", callbackId.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseFence); @@ -564,6 +590,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( auto buffer = bufferItem.mGraphicBuffer; mNumFrameAvailable--; + updateDequeueShouldBlockLocked(); BBQ_TRACE("frame=%" PRIu64, bufferItem.mFrameNumber); if (buffer == nullptr) { @@ -582,6 +609,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( } mNumAcquired++; + updateDequeueShouldBlockLocked(); mLastAcquiredFrameNumber = bufferItem.mFrameNumber; ReleaseCallbackId releaseCallbackId(buffer->getId(), mLastAcquiredFrameNumber); mSubmitted[releaseCallbackId] = bufferItem; @@ -708,6 +736,7 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() { return; } mNumFrameAvailable--; + updateDequeueShouldBlockLocked(); mBufferItemConsumer->releaseBuffer(bufferItem, bufferItem.mFence); } @@ -761,7 +790,9 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { } // add to shadow queue + mNumDequeued--; mNumFrameAvailable++; + updateDequeueShouldBlockLocked(); if (waitForTransactionCallback && mNumFrameAvailable >= 2) { acquireAndReleaseBuffer(); } @@ -812,11 +843,21 @@ void BLASTBufferQueue::onFrameReplaced(const BufferItem& item) { void BLASTBufferQueue::onFrameDequeued(const uint64_t bufferId) { std::lock_guard _lock{mTimestampMutex}; mDequeueTimestamps[bufferId] = systemTime(); -}; + mNumDequeued++; +} void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { - std::lock_guard _lock{mTimestampMutex}; - mDequeueTimestamps.erase(bufferId); + { + std::lock_guard _lock{mTimestampMutex}; + mDequeueTimestamps.erase(bufferId); + } + + { + std::lock_guard lock{mMutex}; + mNumDequeued--; + updateDequeueShouldBlockLocked(); + } + mBufferReleaseReader->interruptBlockingRead(); }; bool BLASTBufferQueue::syncNextTransaction( @@ -888,6 +929,22 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { return mSize != bufferSize; } +void BLASTBufferQueue::updateDequeueShouldBlockLocked() { + int32_t buffersInUse = mNumDequeued + mNumFrameAvailable + mNumAcquired; + int32_t maxBufferCount = std::min(mMaxAcquiredBuffers + mMaxDequeuedBuffers, kMaxBufferCount); + bool bufferAvailable = buffersInUse < maxBufferCount; + // BLASTBufferQueueProducer should block until a buffer is released if + // (1) There are no free buffers available. + // (2) We're not in async mode. In async mode, BufferQueueProducer::dequeueBuffer returns + // WOULD_BLOCK instead of blocking when there are no free buffers. + // (3) We're not in shared buffer mode. In shared buffer mode, both the producer and consumer + // can access the same buffer simultaneously. BufferQueueProducer::dequeueBuffer returns + // the shared buffer immediately instead of blocking. + mDequeueShouldBlock = !(bufferAvailable || mAsyncMode || mSharedBufferMode); + ATRACE_INT("Dequeued", mNumDequeued); + ATRACE_INT("DequeueShouldBlock", mDequeueShouldBlock); +} + class BBQSurface : public Surface { private: std::mutex mMutex; @@ -1116,24 +1173,58 @@ public: producerControlledByApp, output); } + status_t disconnect(int api, DisconnectMode mode) override { + if (status_t status = BufferQueueProducer::disconnect(api, mode); status != OK) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mNumDequeued = 0; + bbq->mNumFrameAvailable = 0; + bbq->mNumAcquired = 0; + bbq->mSubmitted.clear(); + bbq->updateDequeueShouldBlockLocked(); + } + bbq->mBufferReleaseReader->interruptBlockingRead(); + + return OK; + } + // We want to resize the frame history when changing the size of the buffer queue status_t setMaxDequeuedBufferCount(int maxDequeuedBufferCount) override { int maxBufferCount; status_t status = BufferQueueProducer::setMaxDequeuedBufferCount(maxDequeuedBufferCount, &maxBufferCount); - // if we can't determine the max buffer count, then just skip growing the history size - if (status == OK) { - size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering - // optimize away resizing the frame history unless it will grow - if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { - sp bbq = mBLASTBufferQueue.promote(); - if (bbq != nullptr) { - ALOGV("increasing frame history size to %zu", newFrameHistorySize); - bbq->resizeFrameEventHistory(newFrameHistorySize); - } - } + if (status != OK) { + return status; } - return status; + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mMaxDequeuedBuffers = maxDequeuedBufferCount; + bbq->updateDequeueShouldBlockLocked(); + } + bbq->mBufferReleaseReader->interruptBlockingRead(); + + size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering + // optimize away resizing the frame history unless it will grow + if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { + ALOGV("increasing frame history size to %zu", newFrameHistorySize); + bbq->resizeFrameEventHistory(newFrameHistorySize); + } + + return OK; } int query(int what, int* value) override { @@ -1144,6 +1235,125 @@ public: return BufferQueueProducer::query(what, value); } + status_t setAsyncMode(bool asyncMode) override { + if (status_t status = BufferQueueProducer::setAsyncMode(asyncMode); status != NO_ERROR) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return NO_ERROR; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mAsyncMode = asyncMode; + bbq->updateDequeueShouldBlockLocked(); + } + + bbq->mBufferReleaseReader->interruptBlockingRead(); + return NO_ERROR; + } + + status_t setSharedBufferMode(bool sharedBufferMode) override { + if (status_t status = BufferQueueProducer::setSharedBufferMode(sharedBufferMode); + status != NO_ERROR) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return NO_ERROR; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mSharedBufferMode = sharedBufferMode; + bbq->updateDequeueShouldBlockLocked(); + } + + bbq->mBufferReleaseReader->interruptBlockingRead(); + return NO_ERROR; + } + + status_t detachBuffer(int slot) override { + if (status_t status = BufferQueueProducer::detachBuffer(slot); status != NO_ERROR) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return NO_ERROR; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mNumDequeued--; + bbq->updateDequeueShouldBlockLocked(); + } + + bbq->mBufferReleaseReader->interruptBlockingRead(); + return NO_ERROR; + } + + // Override dequeueBuffer to block if there are no free buffers. + // + // Buffer releases are communicated via the BufferReleaseChannel. When dequeueBuffer determines + // a free buffer is not available, it blocks on an epoll file descriptor. Epoll is configured to + // detect messages on the BufferReleaseChannel's socket and an eventfd. The eventfd is signaled + // whenever an event other than a buffer release occurs that may change the number of free + // buffers. dequeueBuffer uses epoll in a similar manner as a condition variable by testing for + // the availability of a free buffer in a loop, breaking the loop once a free buffer is + // available. + // + // This is an optimization implemented to reduce thread scheduling delays in the previously + // existing binder release callback. The binder buffer release callback is still used and there + // are no guarantees around order between buffer releases via binder and the + // BufferReleaseChannel. If we attempt to a release a buffer here that has already been released + // via binder, the release is ignored. + status_t dequeueBuffer(int* outSlot, sp* outFence, uint32_t width, uint32_t height, + PixelFormat format, uint64_t usage, uint64_t* outBufferAge, + FrameEventHistoryDelta* outTimestamps) { + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq || !bbq->mBufferReleaseReader) { + return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, + usage, outBufferAge, outTimestamps); + } + + if (bbq->mDequeueShouldBlock) { + ATRACE_FORMAT("waiting for free buffer"); + auto maxWaitTime = std::chrono::steady_clock::now() + 1s; + do { + auto timeout = std::chrono::duration_cast( + maxWaitTime - std::chrono::steady_clock::now()); + if (timeout <= 0ms) { + break; + } + + ReleaseCallbackId releaseCallbackId; + sp releaseFence; + status_t status = bbq->mBufferReleaseReader->readBlocking(releaseCallbackId, + releaseFence, timeout); + if (status == WOULD_BLOCK) { + // readBlocking was interrupted. The loop will test if we have a free buffer. + continue; + } + + if (status != OK) { + // An error occurred or readBlocking timed out. + break; + } + + std::lock_guard lock{bbq->mMutex}; + bbq->releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, + false); + } while (bbq->mDequeueShouldBlock); + } + + return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, usage, + outBufferAge, outTimestamps); + } + private: const wp mBLASTBufferQueue; }; @@ -1173,6 +1383,16 @@ void BLASTBufferQueue::createBufferQueue(sp* outProducer *outConsumer = consumer; } +void BLASTBufferQueue::onFirstRef() { + // safe default, most producers are expected to override this + // + // This is done in onFirstRef instead of BLASTBufferQueue's constructor because + // BBQBufferQueueProducer::setMaxDequeuedBufferCount promotes a weak pointer to BLASTBufferQueue + // to a strong pointer. If this is done in the constructor, then when the strong pointer goes + // out of scope, it's the last reference so BLASTBufferQueue is deleted. + mProducer->setMaxDequeuedBufferCount(2); +} + void BLASTBufferQueue::resizeFrameEventHistory(size_t newSize) { // This can be null during creation of the buffer queue, but resizing won't do anything at that // point in time, so just ignore. This can go away once the class relationships and lifetimes of @@ -1222,4 +1442,72 @@ void BLASTBufferQueue::setTransactionHangCallback( mTransactionHangCallback = callback; } +BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( + std::unique_ptr endpoint) + : mEndpoint(std::move(endpoint)) { + mEpollFd = android::base::unique_fd(epoll_create1(0)); + if (!mEpollFd.ok()) { + ALOGE("Failed to create buffer release epoll file descriptor. errno=%d message='%s'", errno, + strerror(errno)); + } + + epoll_event event; + event.events = EPOLLIN; + event.data.fd = mEndpoint->getFd(); + if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), &event) == -1) { + ALOGE("Failed to register buffer release consumer file descriptor with epoll. errno=%d " + "message='%s'", + errno, strerror(errno)); + } + + mEventFd = android::base::unique_fd(eventfd(0, EFD_NONBLOCK)); + event.data.fd = mEventFd.get(); + if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), &event) == -1) { + ALOGE("Failed to register buffer release eventfd with epoll. errno=%d message='%s'", errno, + strerror(errno)); + } +} + +status_t BLASTBufferQueue::BufferReleaseReader::readNonBlocking(ReleaseCallbackId& outId, + sp& outFence) { + std::lock_guard lock{mMutex}; + return mEndpoint->readReleaseFence(outId, outFence); +} + +status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, + sp& outFence, + std::chrono::milliseconds timeout) { + epoll_event event; + int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, timeout.count()); + + if (eventCount == -1) { + ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, + strerror(errno)); + return UNKNOWN_ERROR; + } + + if (eventCount == 0) { + return TIMED_OUT; + } + + if (event.data.fd == mEventFd.get()) { + uint64_t value; + if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { + ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, + strerror(errno)); + } + return WOULD_BLOCK; + } + + std::lock_guard lock{mMutex}; + return mEndpoint->readReleaseFence(outId, outFence); +} + +void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { + uint64_t value = 1; + if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { + ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); + } +} + } // namespace android -- cgit v1.2.3-59-g8ed1b From 8f71501b795d3891279e4930b879856b56c12429 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Wed, 24 Jul 2024 15:31:03 -0500 Subject: Fix BufferReleaseChannel flagging. Bug: 294133380 Test: presubmits Flag: com.android.graphics.libgui.flags.buffer_release_channel Change-Id: Iad39b9eb77b57e32521f7ba447a66510967c8209 --- libs/gui/BLASTBufferQueue.cpp | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index f13d499b50..767f3e8c16 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -525,7 +525,9 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, } mNumAcquired--; updateDequeueShouldBlockLocked(); - mBufferReleaseReader->interruptBlockingRead(); + if (mBufferReleaseReader) { + mBufferReleaseReader->interruptBlockingRead(); + } BBQ_TRACE("frame=%" PRIu64, callbackId.framenumber); BQA_LOGV("released %s", callbackId.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseFence); @@ -857,7 +859,10 @@ void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { mNumDequeued--; updateDequeueShouldBlockLocked(); } - mBufferReleaseReader->interruptBlockingRead(); + + if (mBufferReleaseReader) { + mBufferReleaseReader->interruptBlockingRead(); + } }; bool BLASTBufferQueue::syncNextTransaction( @@ -1191,7 +1196,10 @@ public: bbq->mSubmitted.clear(); bbq->updateDequeueShouldBlockLocked(); } - bbq->mBufferReleaseReader->interruptBlockingRead(); + + if (bbq->mBufferReleaseReader) { + bbq->mBufferReleaseReader->interruptBlockingRead(); + } return OK; } @@ -1215,7 +1223,10 @@ public: bbq->mMaxDequeuedBuffers = maxDequeuedBufferCount; bbq->updateDequeueShouldBlockLocked(); } - bbq->mBufferReleaseReader->interruptBlockingRead(); + + if (bbq->mBufferReleaseReader) { + bbq->mBufferReleaseReader->interruptBlockingRead(); + } size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering // optimize away resizing the frame history unless it will grow @@ -1251,7 +1262,9 @@ public: bbq->updateDequeueShouldBlockLocked(); } - bbq->mBufferReleaseReader->interruptBlockingRead(); + if (bbq->mBufferReleaseReader) { + bbq->mBufferReleaseReader->interruptBlockingRead(); + } return NO_ERROR; } @@ -1272,7 +1285,9 @@ public: bbq->updateDequeueShouldBlockLocked(); } - bbq->mBufferReleaseReader->interruptBlockingRead(); + if (bbq->mBufferReleaseReader) { + bbq->mBufferReleaseReader->interruptBlockingRead(); + } return NO_ERROR; } @@ -1292,7 +1307,9 @@ public: bbq->updateDequeueShouldBlockLocked(); } - bbq->mBufferReleaseReader->interruptBlockingRead(); + if (bbq->mBufferReleaseReader) { + bbq->mBufferReleaseReader->interruptBlockingRead(); + } return NO_ERROR; } -- cgit v1.2.3-59-g8ed1b From 4b9507d5a4586ecd2e14db88eb8674e204387ddd Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 25 Jul 2024 09:55:52 -0500 Subject: Revert "Optimize BLAST buffer releases via Unix sockets" Reverting due to b/355260320 Change-Id: I8a32f73b6805d3f2bceb2948925be6a9baaa3015 --- libs/gui/Android.bp | 1 - libs/gui/BLASTBufferQueue.cpp | 343 ++----------------- libs/gui/BufferReleaseChannel.cpp | 364 --------------------- libs/gui/LayerState.cpp | 17 - libs/gui/SurfaceComposerClient.cpp | 16 - libs/gui/include/gui/BLASTBufferQueue.h | 56 +--- libs/gui/include/gui/BufferReleaseChannel.h | 127 ------- libs/gui/include/gui/LayerState.h | 4 - libs/gui/include/gui/SurfaceComposerClient.h | 5 - libs/gui/tests/Android.bp | 1 - libs/gui/tests/BufferReleaseChannel_test.cpp | 121 ------- services/surfaceflinger/Layer.cpp | 15 +- services/surfaceflinger/Layer.h | 3 - services/surfaceflinger/SurfaceFlinger.cpp | 4 - .../surfaceflinger/TransactionCallbackInvoker.cpp | 11 - .../surfaceflinger/TransactionCallbackInvoker.h | 9 - 16 files changed, 26 insertions(+), 1071 deletions(-) delete mode 100644 libs/gui/BufferReleaseChannel.cpp delete mode 100644 libs/gui/include/gui/BufferReleaseChannel.h delete mode 100644 libs/gui/tests/BufferReleaseChannel_test.cpp (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 1243b214d3..51d2e5305a 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -255,7 +255,6 @@ filegroup { "BitTube.cpp", "BLASTBufferQueue.cpp", "BufferItemConsumer.cpp", - "BufferReleaseChannel.cpp", "Choreographer.cpp", "CompositorTiming.cpp", "ConsumerBase.cpp", diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 767f3e8c16..739c3c2a41 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -38,17 +38,13 @@ #include #include -#include #include -#include -#include #include #include using namespace com::android::graphics::libgui; using namespace std::chrono_literals; -using android::base::unique_fd; namespace { inline const char* boolToString(bool b) { @@ -183,6 +179,8 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati // explicitly so that dequeueBuffer will block mProducer->setDequeueTimeout(std::numeric_limits::max()); + // safe default, most producers are expected to override this + mProducer->setMaxDequeuedBufferCount(2); mBufferItemConsumer = new BLASTBufferItemConsumer(mConsumer, GraphicBuffer::USAGE_HW_COMPOSER | GraphicBuffer::USAGE_HW_TEXTURE, @@ -212,12 +210,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - std::unique_ptr bufferReleaseConsumer; - gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); - mBufferReleaseReader.emplace(std::move(bufferReleaseConsumer)); -#endif - BQA_LOGV("BLASTBufferQueue created"); } @@ -267,9 +259,6 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); - if (mBufferReleaseProducer) { - t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); - } applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -450,21 +439,6 @@ void BLASTBufferQueue::releaseBufferCallback( BBQ_TRACE(); releaseBufferCallbackLocked(id, releaseFence, currentMaxAcquiredBufferCount, false /* fakeRelease */); - if (!mBufferReleaseReader) { - return; - } - // Drain the buffer release channel socket - while (true) { - ReleaseCallbackId releaseCallbackId; - sp releaseFence; - if (status_t status = - mBufferReleaseReader->readNonBlocking(releaseCallbackId, releaseFence); - status != OK) { - break; - } - releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, - false /* fakeRelease */); - } } void BLASTBufferQueue::releaseBufferCallbackLocked( @@ -521,13 +495,11 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, const sp& releaseFence) { auto it = mSubmitted.find(callbackId); if (it == mSubmitted.end()) { + BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s", + callbackId.to_string().c_str()); return; } mNumAcquired--; - updateDequeueShouldBlockLocked(); - if (mBufferReleaseReader) { - mBufferReleaseReader->interruptBlockingRead(); - } BBQ_TRACE("frame=%" PRIu64, callbackId.framenumber); BQA_LOGV("released %s", callbackId.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseFence); @@ -592,7 +564,6 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( auto buffer = bufferItem.mGraphicBuffer; mNumFrameAvailable--; - updateDequeueShouldBlockLocked(); BBQ_TRACE("frame=%" PRIu64, bufferItem.mFrameNumber); if (buffer == nullptr) { @@ -611,7 +582,6 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( } mNumAcquired++; - updateDequeueShouldBlockLocked(); mLastAcquiredFrameNumber = bufferItem.mFrameNumber; ReleaseCallbackId releaseCallbackId(buffer->getId(), mLastAcquiredFrameNumber); mSubmitted[releaseCallbackId] = bufferItem; @@ -738,7 +708,6 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() { return; } mNumFrameAvailable--; - updateDequeueShouldBlockLocked(); mBufferItemConsumer->releaseBuffer(bufferItem, bufferItem.mFence); } @@ -792,9 +761,7 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { } // add to shadow queue - mNumDequeued--; mNumFrameAvailable++; - updateDequeueShouldBlockLocked(); if (waitForTransactionCallback && mNumFrameAvailable >= 2) { acquireAndReleaseBuffer(); } @@ -845,24 +812,11 @@ void BLASTBufferQueue::onFrameReplaced(const BufferItem& item) { void BLASTBufferQueue::onFrameDequeued(const uint64_t bufferId) { std::lock_guard _lock{mTimestampMutex}; mDequeueTimestamps[bufferId] = systemTime(); - mNumDequeued++; -} +}; void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { - { - std::lock_guard _lock{mTimestampMutex}; - mDequeueTimestamps.erase(bufferId); - } - - { - std::lock_guard lock{mMutex}; - mNumDequeued--; - updateDequeueShouldBlockLocked(); - } - - if (mBufferReleaseReader) { - mBufferReleaseReader->interruptBlockingRead(); - } + std::lock_guard _lock{mTimestampMutex}; + mDequeueTimestamps.erase(bufferId); }; bool BLASTBufferQueue::syncNextTransaction( @@ -934,22 +888,6 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { return mSize != bufferSize; } -void BLASTBufferQueue::updateDequeueShouldBlockLocked() { - int32_t buffersInUse = mNumDequeued + mNumFrameAvailable + mNumAcquired; - int32_t maxBufferCount = std::min(mMaxAcquiredBuffers + mMaxDequeuedBuffers, kMaxBufferCount); - bool bufferAvailable = buffersInUse < maxBufferCount; - // BLASTBufferQueueProducer should block until a buffer is released if - // (1) There are no free buffers available. - // (2) We're not in async mode. In async mode, BufferQueueProducer::dequeueBuffer returns - // WOULD_BLOCK instead of blocking when there are no free buffers. - // (3) We're not in shared buffer mode. In shared buffer mode, both the producer and consumer - // can access the same buffer simultaneously. BufferQueueProducer::dequeueBuffer returns - // the shared buffer immediately instead of blocking. - mDequeueShouldBlock = !(bufferAvailable || mAsyncMode || mSharedBufferMode); - ATRACE_INT("Dequeued", mNumDequeued); - ATRACE_INT("DequeueShouldBlock", mDequeueShouldBlock); -} - class BBQSurface : public Surface { private: std::mutex mMutex; @@ -1178,64 +1116,24 @@ public: producerControlledByApp, output); } - status_t disconnect(int api, DisconnectMode mode) override { - if (status_t status = BufferQueueProducer::disconnect(api, mode); status != OK) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return OK; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mNumDequeued = 0; - bbq->mNumFrameAvailable = 0; - bbq->mNumAcquired = 0; - bbq->mSubmitted.clear(); - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - - return OK; - } - // We want to resize the frame history when changing the size of the buffer queue status_t setMaxDequeuedBufferCount(int maxDequeuedBufferCount) override { int maxBufferCount; status_t status = BufferQueueProducer::setMaxDequeuedBufferCount(maxDequeuedBufferCount, &maxBufferCount); - if (status != OK) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return OK; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mMaxDequeuedBuffers = maxDequeuedBufferCount; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - - size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering - // optimize away resizing the frame history unless it will grow - if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { - ALOGV("increasing frame history size to %zu", newFrameHistorySize); - bbq->resizeFrameEventHistory(newFrameHistorySize); + // if we can't determine the max buffer count, then just skip growing the history size + if (status == OK) { + size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering + // optimize away resizing the frame history unless it will grow + if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { + sp bbq = mBLASTBufferQueue.promote(); + if (bbq != nullptr) { + ALOGV("increasing frame history size to %zu", newFrameHistorySize); + bbq->resizeFrameEventHistory(newFrameHistorySize); + } + } } - - return OK; + return status; } int query(int what, int* value) override { @@ -1246,131 +1144,6 @@ public: return BufferQueueProducer::query(what, value); } - status_t setAsyncMode(bool asyncMode) override { - if (status_t status = BufferQueueProducer::setAsyncMode(asyncMode); status != NO_ERROR) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return NO_ERROR; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mAsyncMode = asyncMode; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - return NO_ERROR; - } - - status_t setSharedBufferMode(bool sharedBufferMode) override { - if (status_t status = BufferQueueProducer::setSharedBufferMode(sharedBufferMode); - status != NO_ERROR) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return NO_ERROR; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mSharedBufferMode = sharedBufferMode; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - return NO_ERROR; - } - - status_t detachBuffer(int slot) override { - if (status_t status = BufferQueueProducer::detachBuffer(slot); status != NO_ERROR) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return NO_ERROR; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mNumDequeued--; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - return NO_ERROR; - } - - // Override dequeueBuffer to block if there are no free buffers. - // - // Buffer releases are communicated via the BufferReleaseChannel. When dequeueBuffer determines - // a free buffer is not available, it blocks on an epoll file descriptor. Epoll is configured to - // detect messages on the BufferReleaseChannel's socket and an eventfd. The eventfd is signaled - // whenever an event other than a buffer release occurs that may change the number of free - // buffers. dequeueBuffer uses epoll in a similar manner as a condition variable by testing for - // the availability of a free buffer in a loop, breaking the loop once a free buffer is - // available. - // - // This is an optimization implemented to reduce thread scheduling delays in the previously - // existing binder release callback. The binder buffer release callback is still used and there - // are no guarantees around order between buffer releases via binder and the - // BufferReleaseChannel. If we attempt to a release a buffer here that has already been released - // via binder, the release is ignored. - status_t dequeueBuffer(int* outSlot, sp* outFence, uint32_t width, uint32_t height, - PixelFormat format, uint64_t usage, uint64_t* outBufferAge, - FrameEventHistoryDelta* outTimestamps) { - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq || !bbq->mBufferReleaseReader) { - return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, - usage, outBufferAge, outTimestamps); - } - - if (bbq->mDequeueShouldBlock) { - ATRACE_FORMAT("waiting for free buffer"); - auto maxWaitTime = std::chrono::steady_clock::now() + 1s; - do { - auto timeout = std::chrono::duration_cast( - maxWaitTime - std::chrono::steady_clock::now()); - if (timeout <= 0ms) { - break; - } - - ReleaseCallbackId releaseCallbackId; - sp releaseFence; - status_t status = bbq->mBufferReleaseReader->readBlocking(releaseCallbackId, - releaseFence, timeout); - if (status == WOULD_BLOCK) { - // readBlocking was interrupted. The loop will test if we have a free buffer. - continue; - } - - if (status != OK) { - // An error occurred or readBlocking timed out. - break; - } - - std::lock_guard lock{bbq->mMutex}; - bbq->releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, - false); - } while (bbq->mDequeueShouldBlock); - } - - return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, usage, - outBufferAge, outTimestamps); - } - private: const wp mBLASTBufferQueue; }; @@ -1400,16 +1173,6 @@ void BLASTBufferQueue::createBufferQueue(sp* outProducer *outConsumer = consumer; } -void BLASTBufferQueue::onFirstRef() { - // safe default, most producers are expected to override this - // - // This is done in onFirstRef instead of BLASTBufferQueue's constructor because - // BBQBufferQueueProducer::setMaxDequeuedBufferCount promotes a weak pointer to BLASTBufferQueue - // to a strong pointer. If this is done in the constructor, then when the strong pointer goes - // out of scope, it's the last reference so BLASTBufferQueue is deleted. - mProducer->setMaxDequeuedBufferCount(2); -} - void BLASTBufferQueue::resizeFrameEventHistory(size_t newSize) { // This can be null during creation of the buffer queue, but resizing won't do anything at that // point in time, so just ignore. This can go away once the class relationships and lifetimes of @@ -1459,72 +1222,4 @@ void BLASTBufferQueue::setTransactionHangCallback( mTransactionHangCallback = callback; } -BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( - std::unique_ptr endpoint) - : mEndpoint(std::move(endpoint)) { - mEpollFd = android::base::unique_fd(epoll_create1(0)); - if (!mEpollFd.ok()) { - ALOGE("Failed to create buffer release epoll file descriptor. errno=%d message='%s'", errno, - strerror(errno)); - } - - epoll_event event; - event.events = EPOLLIN; - event.data.fd = mEndpoint->getFd(); - if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), &event) == -1) { - ALOGE("Failed to register buffer release consumer file descriptor with epoll. errno=%d " - "message='%s'", - errno, strerror(errno)); - } - - mEventFd = android::base::unique_fd(eventfd(0, EFD_NONBLOCK)); - event.data.fd = mEventFd.get(); - if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), &event) == -1) { - ALOGE("Failed to register buffer release eventfd with epoll. errno=%d message='%s'", errno, - strerror(errno)); - } -} - -status_t BLASTBufferQueue::BufferReleaseReader::readNonBlocking(ReleaseCallbackId& outId, - sp& outFence) { - std::lock_guard lock{mMutex}; - return mEndpoint->readReleaseFence(outId, outFence); -} - -status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, - sp& outFence, - std::chrono::milliseconds timeout) { - epoll_event event; - int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, timeout.count()); - - if (eventCount == -1) { - ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, - strerror(errno)); - return UNKNOWN_ERROR; - } - - if (eventCount == 0) { - return TIMED_OUT; - } - - if (event.data.fd == mEventFd.get()) { - uint64_t value; - if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { - ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, - strerror(errno)); - } - return WOULD_BLOCK; - } - - std::lock_guard lock{mMutex}; - return mEndpoint->readReleaseFence(outId, outFence); -} - -void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { - uint64_t value = 1; - if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { - ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); - } -} - } // namespace android diff --git a/libs/gui/BufferReleaseChannel.cpp b/libs/gui/BufferReleaseChannel.cpp deleted file mode 100644 index 183b25a4cb..0000000000 --- a/libs/gui/BufferReleaseChannel.cpp +++ /dev/null @@ -1,364 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "BufferReleaseChannel" -#define ATRACE_TAG ATRACE_TAG_GRAPHICS - -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include - -using android::base::Result; - -namespace android::gui { - -namespace { - -template -static void readAligned(const void*& buffer, size_t& size, T& value) { - size -= FlattenableUtils::align(buffer); - FlattenableUtils::read(buffer, size, value); -} - -template -static void writeAligned(void*& buffer, size_t& size, T value) { - size -= FlattenableUtils::align(buffer); - FlattenableUtils::write(buffer, size, value); -} - -template -static void addAligned(size_t& size, T /* value */) { - size = FlattenableUtils::align(size); - size += sizeof(T); -} - -template -static inline constexpr uint32_t low32(const T n) { - return static_cast(static_cast(n)); -} - -template -static inline constexpr uint32_t high32(const T n) { - return static_cast(static_cast(n) >> 32); -} - -template -static inline constexpr T to64(const uint32_t lo, const uint32_t hi) { - return static_cast(static_cast(hi) << 32 | lo); -} - -} // namespace - -size_t BufferReleaseChannel::Message::getPodSize() const { - size_t size = 0; - addAligned(size, low32(releaseCallbackId.bufferId)); - addAligned(size, high32(releaseCallbackId.bufferId)); - addAligned(size, low32(releaseCallbackId.framenumber)); - addAligned(size, high32(releaseCallbackId.framenumber)); - return size; -} - -size_t BufferReleaseChannel::Message::getFlattenedSize() const { - size_t size = releaseFence->getFlattenedSize(); - size = FlattenableUtils::align<4>(size); - size += getPodSize(); - return size; -} - -status_t BufferReleaseChannel::Message::flatten(void*& buffer, size_t& size, int*& fds, - size_t& count) const { - if (status_t err = releaseFence->flatten(buffer, size, fds, count); err != OK) { - return err; - } - size -= FlattenableUtils::align<4>(buffer); - - // Check we still have enough space - if (size < getPodSize()) { - return NO_MEMORY; - } - - writeAligned(buffer, size, low32(releaseCallbackId.bufferId)); - writeAligned(buffer, size, high32(releaseCallbackId.bufferId)); - writeAligned(buffer, size, low32(releaseCallbackId.framenumber)); - writeAligned(buffer, size, high32(releaseCallbackId.framenumber)); - return OK; -} - -status_t BufferReleaseChannel::Message::unflatten(void const*& buffer, size_t& size, - int const*& fds, size_t& count) { - releaseFence = new Fence(); - if (status_t err = releaseFence->unflatten(buffer, size, fds, count); err != OK) { - return err; - } - size -= FlattenableUtils::align<4>(buffer); - - // Check we still have enough space - if (size < getPodSize()) { - return OK; - } - - uint32_t bufferIdLo = 0, bufferIdHi = 0; - uint32_t frameNumberLo = 0, frameNumberHi = 0; - - readAligned(buffer, size, bufferIdLo); - readAligned(buffer, size, bufferIdHi); - releaseCallbackId.bufferId = to64(bufferIdLo, bufferIdHi); - readAligned(buffer, size, frameNumberLo); - readAligned(buffer, size, frameNumberHi); - releaseCallbackId.framenumber = to64(frameNumberLo, frameNumberHi); - - return NO_ERROR; -} - -status_t BufferReleaseChannel::ConsumerEndpoint::readReleaseFence( - ReleaseCallbackId& outReleaseCallbackId, sp& outReleaseFence) { - Message releaseBufferMessage; - mFlattenedBuffer.resize(releaseBufferMessage.getFlattenedSize()); - std::array controlMessageBuffer; - - iovec iov{ - .iov_base = mFlattenedBuffer.data(), - .iov_len = mFlattenedBuffer.size(), - }; - - msghdr msg{ - .msg_iov = &iov, - .msg_iovlen = 1, - .msg_control = controlMessageBuffer.data(), - .msg_controllen = controlMessageBuffer.size(), - }; - - int result; - do { - result = recvmsg(mFd, &msg, 0); - } while (result == -1 && errno == EINTR); - if (result == -1) { - if (errno == EWOULDBLOCK || errno == EAGAIN) { - return WOULD_BLOCK; - } - ALOGE("Error reading release fence from socket: error %#x (%s)", errno, strerror(errno)); - return -errno; - } - - if (msg.msg_iovlen != 1) { - ALOGE("Error reading release fence from socket: bad data length"); - return UNKNOWN_ERROR; - } - - if (msg.msg_controllen % sizeof(int) != 0) { - ALOGE("Error reading release fence from socket: bad fd length"); - return UNKNOWN_ERROR; - } - - size_t dataLen = msg.msg_iov->iov_len; - const void* data = static_cast(msg.msg_iov->iov_base); - if (!data) { - ALOGE("Error reading release fence from socket: no buffer data"); - return UNKNOWN_ERROR; - } - - size_t fdCount = 0; - const int* fdData = nullptr; - if (cmsghdr* cmsg = CMSG_FIRSTHDR(&msg)) { - fdData = reinterpret_cast(CMSG_DATA(cmsg)); - fdCount = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); - } - - if (status_t err = releaseBufferMessage.unflatten(data, dataLen, fdData, fdCount); err != OK) { - return err; - } - - outReleaseCallbackId = releaseBufferMessage.releaseCallbackId; - outReleaseFence = std::move(releaseBufferMessage.releaseFence); - - return OK; -} - -int BufferReleaseChannel::ProducerEndpoint::writeReleaseFence(const ReleaseCallbackId& callbackId, - const sp& fence) { - Message releaseBufferMessage(callbackId, fence ? fence : Fence::NO_FENCE); - mFlattenedBuffer.resize(releaseBufferMessage.getFlattenedSize()); - int flattenedFd; - { - // Make copies of needed items since flatten modifies them, and we don't - // want to send anything if there's an error during flatten. - void* flattenedBufferPtr = mFlattenedBuffer.data(); - size_t flattenedBufferSize = mFlattenedBuffer.size(); - int* flattenedFdPtr = &flattenedFd; - size_t flattenedFdCount = 1; - if (status_t err = releaseBufferMessage.flatten(flattenedBufferPtr, flattenedBufferSize, - flattenedFdPtr, flattenedFdCount); - err != OK) { - ALOGE("Failed to flatten BufferReleaseChannel message."); - return err; - } - } - - iovec iov{ - .iov_base = mFlattenedBuffer.data(), - .iov_len = mFlattenedBuffer.size(), - }; - - msghdr msg{ - .msg_iov = &iov, - .msg_iovlen = 1, - }; - - std::array controlMessageBuffer; - if (fence && fence->isValid()) { - msg.msg_control = controlMessageBuffer.data(); - msg.msg_controllen = controlMessageBuffer.size(); - - cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - cmsg->cmsg_len = CMSG_LEN(sizeof(int)); - memcpy(CMSG_DATA(cmsg), &flattenedFd, sizeof(int)); - } - - int result; - do { - result = sendmsg(mFd, &msg, 0); - } while (result == -1 && errno == EINTR); - if (result == -1) { - ALOGD("Error writing release fence to socket: error %#x (%s)", errno, strerror(errno)); - return -errno; - } - - return OK; -} - -status_t BufferReleaseChannel::ProducerEndpoint::readFromParcel(const android::Parcel* parcel) { - if (!parcel) return STATUS_BAD_VALUE; - SAFE_PARCEL(parcel->readUtf8FromUtf16, &mName); - SAFE_PARCEL(parcel->readUniqueFileDescriptor, &mFd); - return STATUS_OK; -} - -status_t BufferReleaseChannel::ProducerEndpoint::writeToParcel(android::Parcel* parcel) const { - if (!parcel) return STATUS_BAD_VALUE; - SAFE_PARCEL(parcel->writeUtf8AsUtf16, mName); - SAFE_PARCEL(parcel->writeUniqueFileDescriptor, mFd); - return STATUS_OK; -} - -status_t BufferReleaseChannel::open(std::string name, - std::unique_ptr& outConsumer, - std::shared_ptr& outProducer) { - outConsumer.reset(); - outProducer.reset(); - - int sockets[2]; - if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sockets)) { - ALOGE("[%s] Failed to create socket pair. errorno=%d message='%s'", name.c_str(), errno, - strerror(errno)); - return -errno; - } - - android::base::unique_fd consumerFd(sockets[0]); - android::base::unique_fd producerFd(sockets[1]); - - // Socket buffer size. The default is typically about 128KB, which is much larger than - // we really need. So we make it smaller. It just needs to be big enough to hold - // a few dozen release fences. - const int bufferSize = 32 * 1024; - if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set consumer socket send buffer size. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set consumer socket receive buffer size. errno=%d " - "message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - if (setsockopt(producerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set producer socket send buffer size. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set producer socket receive buffer size. errno=%d " - "message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - // Configure the consumer socket to be non-blocking. - int flags = fcntl(consumerFd.get(), F_GETFL, 0); - if (flags == -1) { - ALOGE("[%s] Failed to get consumer socket flags. errno=%d message='%s'", name.c_str(), - errno, strerror(errno)); - return -errno; - } - if (fcntl(consumerFd.get(), F_SETFL, flags | O_NONBLOCK) == -1) { - ALOGE("[%s] Failed to set consumer socket to non-blocking mode. errno=%d " - "message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - // Configure a timeout for the producer socket. - const timeval timeout{.tv_sec = 1, .tv_usec = 0}; - if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeval)) == -1) { - ALOGE("[%s] Failed to set producer socket timeout. errno=%d message='%s'", name.c_str(), - errno, strerror(errno)); - return -errno; - } - - // Make the consumer read-only - if (shutdown(consumerFd.get(), SHUT_WR) == -1) { - ALOGE("[%s] Failed to shutdown writing on consumer socket. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - // Make the producer write-only - if (shutdown(producerFd.get(), SHUT_RD) == -1) { - ALOGE("[%s] Failed to shutdown reading on producer socket. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - outConsumer = std::make_unique(name, std::move(consumerFd)); - outProducer = std::make_shared(std::move(name), std::move(producerFd)); - return STATUS_OK; -} - -} // namespace android::gui diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index adf1646b83..307ae3990e 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -194,12 +194,6 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeFloat, currentHdrSdrRatio); SAFE_PARCEL(output.writeFloat, desiredHdrSdrRatio); SAFE_PARCEL(output.writeInt32, static_cast(cachingHint)); - - const bool hasBufferReleaseChannel = (bufferReleaseChannel != nullptr); - SAFE_PARCEL(output.writeBool, hasBufferReleaseChannel); - if (hasBufferReleaseChannel) { - SAFE_PARCEL(output.writeParcelable, *bufferReleaseChannel); - } return NO_ERROR; } @@ -345,12 +339,6 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(input.readInt32, &tmpInt32); cachingHint = static_cast(tmpInt32); - bool hasBufferReleaseChannel; - SAFE_PARCEL(input.readBool, &hasBufferReleaseChannel); - if (hasBufferReleaseChannel) { - bufferReleaseChannel = std::make_shared(); - SAFE_PARCEL(input.readParcelable, bufferReleaseChannel.get()); - } return NO_ERROR; } @@ -730,10 +718,6 @@ void layer_state_t::merge(const layer_state_t& other) { if (other.what & eFlushJankData) { what |= eFlushJankData; } - if (other.what & eBufferReleaseChannelChanged) { - what |= eBufferReleaseChannelChanged; - bufferReleaseChannel = other.bufferReleaseChannel; - } if ((other.what & what) != other.what) { ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? " "other.what=0x%" PRIX64 " what=0x%" PRIX64 " unmerged flags=0x%" PRIX64, @@ -813,7 +797,6 @@ uint64_t layer_state_t::diff(const layer_state_t& other) const { CHECK_DIFF(diff, eColorChanged, other, color.rgb); CHECK_DIFF(diff, eColorSpaceAgnosticChanged, other, colorSpaceAgnostic); CHECK_DIFF(diff, eDimmingEnabledChanged, other, dimmingEnabled); - if (other.what & eBufferReleaseChannelChanged) diff |= eBufferReleaseChannelChanged; return diff; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index cdf57ffd61..e86e13d3ee 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -2395,22 +2395,6 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setDropI return *this; } -SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBufferReleaseChannel( - const sp& sc, - const std::shared_ptr& channel) { - layer_state_t* s = getLayerState(sc); - if (!s) { - mStatus = BAD_INDEX; - return *this; - } - - s->what |= layer_state_t::eBufferReleaseChannelChanged; - s->bufferReleaseChannel = channel; - - registerSurfaceControlForCallback(sc); - return *this; -} - // --------------------------------------------------------------------------- DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp& token) { diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index e9a350c301..0e1a505c69 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -19,7 +19,7 @@ #include #include -#include + #include #include @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -130,8 +131,6 @@ public: virtual ~BLASTBufferQueue(); - void onFirstRef() override; - private: friend class BLASTBufferQueueHelper; friend class BBQBufferQueueProducer; @@ -171,23 +170,11 @@ private: // BufferQueue internally allows 1 more than // the max to be acquired - int32_t mMaxAcquiredBuffers GUARDED_BY(mMutex) = 1; - int32_t mMaxDequeuedBuffers GUARDED_BY(mMutex) = 1; - static constexpr int32_t kMaxBufferCount = BufferQueueDefs::NUM_BUFFER_SLOTS; - - // mNumDequeued is an atomic instead of being guarded by mMutex so that it can be incremented in - // onFrameDequeued while avoiding lock contention. updateDequeueShouldBlockLocked is not called - // after mNumDequeued is incremented for this reason. This means mDequeueShouldBlock may be - // temporarily false when it should be true. This can happen if multiple threads are dequeuing - // buffers or if dequeueBuffers is called multiple times in a row without queuing a buffer in - // between. As mDequeueShouldBlock is only used for optimization, this is OK. - std::atomic_int mNumDequeued; + int32_t mMaxAcquiredBuffers = 1; + int32_t mNumFrameAvailable GUARDED_BY(mMutex) = 0; int32_t mNumAcquired GUARDED_BY(mMutex) = 0; - bool mAsyncMode GUARDED_BY(mMutex) = false; - bool mSharedBufferMode GUARDED_BY(mMutex) = false; - // A value used to identify if a producer has been changed for the same SurfaceControl. // This is needed to know when the frame number has been reset to make sure we don't // latch stale buffers and that we don't wait on barriers from an old producer. @@ -313,41 +300,6 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); - - class BufferReleaseReader { - public: - BufferReleaseReader(std::unique_ptr); - - BufferReleaseReader(const BufferReleaseReader&) = delete; - BufferReleaseReader& operator=(const BufferReleaseReader&) = delete; - - // Block until we can release a buffer. - // - // Returns: - // * OK if a ReleaseCallbackId and Fence were successfully read. - // * TIMED_OUT if the specified timeout was reached. - // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. - // * UNKNOWN_ERROR if something went wrong. - status_t readBlocking(ReleaseCallbackId&, sp&, std::chrono::milliseconds timeout); - - status_t readNonBlocking(ReleaseCallbackId&, sp&); - - void interruptBlockingRead(); - - private: - std::mutex mMutex; - std::unique_ptr mEndpoint GUARDED_BY(mMutex); - android::base::unique_fd mEpollFd; - android::base::unique_fd mEventFd; - }; - - // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to - // the client. See BBQBufferQueueProducer::dequeueBuffer for details. - std::optional mBufferReleaseReader; - std::shared_ptr mBufferReleaseProducer; - - std::atomic_bool mDequeueShouldBlock{false}; - void updateDequeueShouldBlockLocked() REQUIRES(mMutex); }; } // namespace android diff --git a/libs/gui/include/gui/BufferReleaseChannel.h b/libs/gui/include/gui/BufferReleaseChannel.h deleted file mode 100644 index cb0b261240..0000000000 --- a/libs/gui/include/gui/BufferReleaseChannel.h +++ /dev/null @@ -1,127 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include - -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace android::gui { - -/** - * IPC wrapper to pass release fences from SurfaceFlinger to apps via a local unix domain socket. - */ -class BufferReleaseChannel { -private: - class Endpoint { - public: - Endpoint(std::string name, android::base::unique_fd fd) - : mName(std::move(name)), mFd(std::move(fd)) {} - Endpoint() {} - - Endpoint(Endpoint&&) noexcept = default; - Endpoint& operator=(Endpoint&&) noexcept = default; - - Endpoint(const Endpoint&) = delete; - void operator=(const Endpoint&) = delete; - - const android::base::unique_fd& getFd() const { return mFd; } - - protected: - std::string mName; - android::base::unique_fd mFd; - }; - -public: - class ConsumerEndpoint : public Endpoint { - public: - ConsumerEndpoint(std::string name, android::base::unique_fd fd) - : Endpoint(std::move(name), std::move(fd)) {} - - /** - * Reads a release fence from the BufferReleaseChannel. - * - * Returns OK on success. - * Returns WOULD_BLOCK if there is no fence present. - * Other errors probably indicate that the channel is broken. - */ - status_t readReleaseFence(ReleaseCallbackId& outReleaseCallbackId, - sp& outReleaseFence); - - private: - std::vector mFlattenedBuffer; - }; - - class ProducerEndpoint : public Endpoint, public Parcelable { - public: - ProducerEndpoint(std::string name, android::base::unique_fd fd) - : Endpoint(std::move(name), std::move(fd)) {} - ProducerEndpoint() {} - - status_t readFromParcel(const android::Parcel* parcel) override; - status_t writeToParcel(android::Parcel* parcel) const override; - - status_t writeReleaseFence(const ReleaseCallbackId&, const sp& releaseFence); - - private: - std::vector mFlattenedBuffer; - }; - - /** - * Create two endpoints that make up the BufferReleaseChannel. - * - * Return OK on success. - */ - static status_t open(const std::string name, std::unique_ptr& outConsumer, - std::shared_ptr& outProducer); - - struct Message : public Flattenable { - ReleaseCallbackId releaseCallbackId; - sp releaseFence = Fence::NO_FENCE; - - Message() = default; - Message(ReleaseCallbackId releaseCallbackId, sp releaseFence) - : releaseCallbackId(releaseCallbackId), releaseFence(std::move(releaseFence)) {} - - // Flattenable protocol - size_t getFlattenedSize() const; - - size_t getFdCount() const { return releaseFence->getFdCount(); } - - status_t flatten(void*& buffer, size_t& size, int*& fds, size_t& count) const; - - status_t unflatten(void const*& buffer, size_t& size, int const*& fds, size_t& count); - - private: - size_t getPodSize() const; - }; -}; - -} // namespace android::gui diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index d41994589f..3fb1894585 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -34,7 +34,6 @@ #include #include -#include #include #include #include @@ -221,7 +220,6 @@ struct layer_state_t { eDropInputModeChanged = 0x8000'00000000, eExtendedRangeBrightnessChanged = 0x10000'00000000, eEdgeExtensionChanged = 0x20000'00000000, - eBufferReleaseChannelChanged = 0x40000'00000000, }; layer_state_t(); @@ -414,8 +412,6 @@ struct layer_state_t { TrustedPresentationThresholds trustedPresentationThresholds; TrustedPresentationListener trustedPresentationListener; - - std::shared_ptr bufferReleaseChannel; }; class ComposerState { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 8c1d6443c5..95574ee30e 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -45,7 +45,6 @@ #include #include -#include #include #include #include @@ -764,10 +763,6 @@ public: const Rect& destinationFrame); Transaction& setDropInputMode(const sp& sc, gui::DropInputMode mode); - Transaction& setBufferReleaseChannel( - const sp& sc, - const std::shared_ptr& channel); - status_t setDisplaySurface(const sp& token, const sp& bufferProducer); diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index daaddfbc15..ea8acbbb72 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -32,7 +32,6 @@ cc_test { "BLASTBufferQueue_test.cpp", "BufferItemConsumer_test.cpp", "BufferQueue_test.cpp", - "BufferReleaseChannel_test.cpp", "Choreographer_test.cpp", "CompositorTiming_test.cpp", "CpuConsumer_test.cpp", diff --git a/libs/gui/tests/BufferReleaseChannel_test.cpp b/libs/gui/tests/BufferReleaseChannel_test.cpp deleted file mode 100644 index f3e962c192..0000000000 --- a/libs/gui/tests/BufferReleaseChannel_test.cpp +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include - -#include -#include - -using namespace std::string_literals; -using android::gui::BufferReleaseChannel; - -namespace android { - -namespace { - -// Helper function to check if two file descriptors point to the same file. -bool is_same_file(int fd1, int fd2) { - struct stat stat1; - if (fstat(fd1, &stat1) != 0) { - return false; - } - struct stat stat2; - if (fstat(fd2, &stat2) != 0) { - return false; - } - return (stat1.st_dev == stat2.st_dev) && (stat1.st_ino == stat2.st_ino); -} - -} // namespace - -TEST(BufferReleaseChannelTest, MessageFlattenable) { - ReleaseCallbackId releaseCallbackId{1, 2}; - sp releaseFence = sp::make(memfd_create("fake-fence-fd", 0)); - - std::vector dataBuffer; - std::vector fdBuffer; - - // Verify that we can flatten a message - { - BufferReleaseChannel::Message message{releaseCallbackId, releaseFence}; - - dataBuffer.resize(message.getFlattenedSize()); - void* dataPtr = dataBuffer.data(); - size_t dataSize = dataBuffer.size(); - - fdBuffer.resize(message.getFdCount()); - int* fdPtr = fdBuffer.data(); - size_t fdSize = fdBuffer.size(); - - ASSERT_EQ(OK, message.flatten(dataPtr, dataSize, fdPtr, fdSize)); - - // Fence's unique_fd uses fdsan to check ownership of the file descriptor. Normally the file - // descriptor is passed through the Unix socket and duplicated (and sent to another process) - // so there's no problem with duplicate file descriptor ownership. For this unit test, we - // need to set up a duplicate file descriptor to avoid crashing due to duplicate ownership. - ASSERT_EQ(releaseFence->get(), fdBuffer[0]); - fdBuffer[0] = message.releaseFence->dup(); - } - - // Verify that we can unflatten a message - { - BufferReleaseChannel::Message message; - - const void* dataPtr = dataBuffer.data(); - size_t dataSize = dataBuffer.size(); - - const int* fdPtr = fdBuffer.data(); - size_t fdSize = fdBuffer.size(); - - ASSERT_EQ(OK, message.unflatten(dataPtr, dataSize, fdPtr, fdSize)); - ASSERT_EQ(releaseCallbackId, message.releaseCallbackId); - ASSERT_TRUE(is_same_file(releaseFence->get(), message.releaseFence->get())); - } -} - -// Verify that the BufferReleaseChannel consume returns WOULD_BLOCK when there's no message -// available. -TEST(BufferReleaseChannelTest, ConsumerEndpointIsNonBlocking) { - std::unique_ptr consumer; - std::shared_ptr producer; - ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); - - ReleaseCallbackId releaseCallbackId; - sp releaseFence; - ASSERT_EQ(WOULD_BLOCK, consumer->readReleaseFence(releaseCallbackId, releaseFence)); -} - -// Verify that we can write a message to the BufferReleaseChannel producer and read that message -// using the BufferReleaseChannel consumer. -TEST(BufferReleaseChannelTest, ProduceAndConsume) { - std::unique_ptr consumer; - std::shared_ptr producer; - ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); - - ReleaseCallbackId producerReleaseCallbackId{1, 2}; - sp producerReleaseFence = sp::make(memfd_create("fake-fence-fd", 0)); - ASSERT_EQ(OK, producer->writeReleaseFence(producerReleaseCallbackId, producerReleaseFence)); - - ReleaseCallbackId consumerReleaseCallbackId; - sp consumerReleaseFence; - ASSERT_EQ(OK, consumer->readReleaseFence(consumerReleaseCallbackId, consumerReleaseFence)); - - ASSERT_EQ(producerReleaseCallbackId, consumerReleaseCallbackId); - ASSERT_TRUE(is_same_file(producerReleaseFence->get(), consumerReleaseFence->get())); -} - -} // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 0682fdb639..0eb6cc32c5 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2775,14 +2775,11 @@ void Layer::callReleaseBufferCallback(const sp& l return; } SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); - ReleaseCallbackId callbackId{buffer->getId(), framenumber}; - const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); - if (mBufferReleaseChannel) { - mBufferReleaseChannel->writeReleaseFence(callbackId, fence); - } + listener->onReleaseBuffer({buffer->getId(), framenumber}, + releaseFence ? releaseFence : Fence::NO_FENCE, + currentMaxAcquiredBufferCount); } sp Layer::findCallbackHandle() { @@ -2900,7 +2897,6 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { - handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -4372,11 +4368,6 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } -void Layer::setBufferReleaseChannel( - const std::shared_ptr& channel) { - mBufferReleaseChannel = channel; -} - void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index d4707388b2..52f169ebee 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -550,7 +550,6 @@ public: }; BufferInfo mBufferInfo; - std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -814,8 +813,6 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); - void setBufferReleaseChannel( - const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f61763880e..84553cab39 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5789,10 +5789,6 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } - if (what & layer_state_t::eBufferReleaseChannelChanged) { - layer->setBufferReleaseChannel(s.bufferReleaseChannel); - } - const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 9ea0f5f4ca..881bf35b58 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,12 +149,6 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); - if (handle->bufferReleaseChannel && - handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { - mBufferReleases.emplace_back(handle->bufferReleaseChannel, - handle->previousReleaseCallbackId, - handle->previousReleaseFence); - } } return NO_ERROR; } @@ -164,11 +158,6 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { - for (const auto& bufferRelease : mBufferReleases) { - bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence); - } - mBufferReleases.clear(); - // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index f74f9644b6..7853a9f359 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -60,7 +59,6 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; - std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -88,13 +86,6 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; - struct BufferRelease { - std::shared_ptr channel; - ReleaseCallbackId callbackId; - sp fence; - }; - std::vector mBufferReleases; - sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From f5b42de949b928d5b08854a592c2af0600bbca7b Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 1 Aug 2024 16:08:51 -0500 Subject: Track BBQ dequeue count and mode Counts, shared buffer mode, and async mode will be used to determine when dequeueBuffer should block. Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: BLASTBufferQueueTest Change-Id: I23cd5606f84ac5e0c29c470d05b28153fc4c9ce8 --- libs/gui/BLASTBufferQueue.cpp | 173 ++++++++++++++++++++++++++++---- libs/gui/include/gui/BLASTBufferQueue.h | 13 ++- 2 files changed, 164 insertions(+), 22 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 739c3c2a41..7f0e80e0f0 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -39,7 +39,6 @@ #include #include -#include #include @@ -179,8 +178,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati // explicitly so that dequeueBuffer will block mProducer->setDequeueTimeout(std::numeric_limits::max()); - // safe default, most producers are expected to override this - mProducer->setMaxDequeuedBufferCount(2); mBufferItemConsumer = new BLASTBufferItemConsumer(mConsumer, GraphicBuffer::USAGE_HW_COMPOSER | GraphicBuffer::USAGE_HW_TEXTURE, @@ -236,6 +233,11 @@ BLASTBufferQueue::~BLASTBufferQueue() { } } +void BLASTBufferQueue::onFirstRef() { + // safe default, most producers are expected to override this + mProducer->setMaxDequeuedBufferCount(2); +} + void BLASTBufferQueue::update(const sp& surface, uint32_t width, uint32_t height, int32_t format) { LOG_ALWAYS_FATAL_IF(surface == nullptr, "BLASTBufferQueue: mSurfaceControl must not be NULL"); @@ -499,7 +501,13 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, callbackId.to_string().c_str()); return; } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + if (!it->second.mIsStale) { + mNumAcquired--; + } +#else mNumAcquired--; +#endif BBQ_TRACE("frame=%" PRIu64, callbackId.framenumber); BQA_LOGV("released %s", callbackId.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseFence); @@ -761,6 +769,9 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { } // add to shadow queue +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + mNumDequeued--; +#endif mNumFrameAvailable++; if (waitForTransactionCallback && mNumFrameAvailable >= 2) { acquireAndReleaseBuffer(); @@ -815,9 +826,18 @@ void BLASTBufferQueue::onFrameDequeued(const uint64_t bufferId) { }; void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { - std::lock_guard _lock{mTimestampMutex}; - mDequeueTimestamps.erase(bufferId); -}; + { + std::lock_guard _lock{mTimestampMutex}; + mDequeueTimestamps.erase(bufferId); + } + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + { + std::lock_guard lock{mMutex}; + mNumDequeued--; + } +#endif +} bool BLASTBufferQueue::syncNextTransaction( std::function callback, @@ -1116,30 +1136,143 @@ public: producerControlledByApp, output); } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + status_t disconnect(int api, DisconnectMode mode) override { + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return BufferQueueProducer::disconnect(api, mode); + } + + std::lock_guard lock{bbq->mMutex}; + if (status_t status = BufferQueueProducer::disconnect(api, mode); status != OK) { + return status; + } + + bbq->mNumDequeued = 0; + bbq->mNumAcquired = 0; + for (auto& [releaseId, bufferItem] : bbq->mSubmitted) { + bufferItem.mIsStale = true; + } + + return OK; + } + + status_t setAsyncMode(bool asyncMode) override { + if (status_t status = BufferQueueProducer::setAsyncMode(asyncMode); status != OK) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mAsyncMode = asyncMode; + } + + return OK; + } + + status_t setSharedBufferMode(bool sharedBufferMode) override { + if (status_t status = BufferQueueProducer::setSharedBufferMode(sharedBufferMode); + status != OK) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mSharedBufferMode = sharedBufferMode; + } + + return OK; + } + + status_t detachBuffer(int slot) override { + if (status_t status = BufferQueueProducer::detachBuffer(slot); status != OK) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mNumDequeued--; + } + + return OK; + } + + status_t dequeueBuffer(int* outSlot, sp* outFence, uint32_t width, uint32_t height, + PixelFormat format, uint64_t usage, uint64_t* outBufferAge, + FrameEventHistoryDelta* outTimestamps) override { + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, + usage, outBufferAge, outTimestamps); + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mNumDequeued++; + } + + status_t status = + BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, usage, + outBufferAge, outTimestamps); + if (status < 0) { + std::lock_guard lock{bbq->mMutex}; + bbq->mNumDequeued--; + } + return status; + } +#endif + // We want to resize the frame history when changing the size of the buffer queue status_t setMaxDequeuedBufferCount(int maxDequeuedBufferCount) override { int maxBufferCount; - status_t status = BufferQueueProducer::setMaxDequeuedBufferCount(maxDequeuedBufferCount, - &maxBufferCount); + if (status_t status = BufferQueueProducer::setMaxDequeuedBufferCount(maxDequeuedBufferCount, + &maxBufferCount); + status != OK) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + // if we can't determine the max buffer count, then just skip growing the history size - if (status == OK) { - size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering - // optimize away resizing the frame history unless it will grow - if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { - sp bbq = mBLASTBufferQueue.promote(); - if (bbq != nullptr) { - ALOGV("increasing frame history size to %zu", newFrameHistorySize); - bbq->resizeFrameEventHistory(newFrameHistorySize); - } - } + size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering + // optimize away resizing the frame history unless it will grow + if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { + ALOGV("increasing frame history size to %zu", newFrameHistorySize); + bbq->resizeFrameEventHistory(newFrameHistorySize); } - return status; + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + { + std::lock_guard lock{bbq->mMutex}; + bbq->mMaxDequeuedBuffers = maxDequeuedBufferCount; + } +#endif + + return OK; } int query(int what, int* value) override { if (what == NATIVE_WINDOW_QUEUES_TO_WINDOW_COMPOSER) { *value = 1; - return NO_ERROR; + return OK; } return BufferQueueProducer::query(what, value); } diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 0e1a505c69..266729abe5 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -28,7 +28,6 @@ #include #include -#include #include #include @@ -131,6 +130,8 @@ public: virtual ~BLASTBufferQueue(); + void onFirstRef() override; + private: friend class BLASTBufferQueueHelper; friend class BBQBufferQueueProducer; @@ -170,8 +171,16 @@ private: // BufferQueue internally allows 1 more than // the max to be acquired - int32_t mMaxAcquiredBuffers = 1; + int32_t mMaxAcquiredBuffers GUARDED_BY(mMutex) = 1; +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + int32_t mMaxDequeuedBuffers GUARDED_BY(mMutex) = 1; + static constexpr int32_t kMaxBufferCount = BufferQueueDefs::NUM_BUFFER_SLOTS; + + bool mAsyncMode GUARDED_BY(mMutex) = false; + bool mSharedBufferMode GUARDED_BY(mMutex) = false; + int32_t mNumDequeued GUARDED_BY(mMutex) = 0; +#endif int32_t mNumFrameAvailable GUARDED_BY(mMutex) = 0; int32_t mNumAcquired GUARDED_BY(mMutex) = 0; -- cgit v1.2.3-59-g8ed1b From d30823a16ea4c210d96ac983d2a04bec74b2c35a Mon Sep 17 00:00:00 2001 From: Jim Shargo Date: Sat, 27 Jul 2024 02:49:39 +0000 Subject: libgui: ConsumerBase-based classes now create their own BufferQueues Using ConsumerBase-based classes is now the recommended way to create BufferQueues. This also includes a few new methods that are used by downstream classes to avoid calling methods on raw IGBP/IGBCs. I decided to keep and deprecate the old ctors temporarily. Before I roll the flag out I'll remove them, but this way I can build the whole build with or without the flag. This is an important step for go/warren-buffers, because it consolidates usages of BufferQueues to supported APIs and reduces the libgui API surface that exposes IGBP/IGBC. BYPASS_IGBP_IGBC_API_REASON: this CL is part of the migration. Bug: 340933754 Flag: com.android.graphics.libgui.flags.wb_consumer_base_owns_bq Test: atest, presubmit, compiles Change-Id: I977165f3e50bc343df396a4c5ecc97fe31a67d5a --- libs/gui/BLASTBufferQueue.cpp | 16 +++-- libs/gui/BufferItemConsumer.cpp | 27 ++++++-- libs/gui/ConsumerBase.cpp | 68 +++++++++++++++++-- libs/gui/CpuConsumer.cpp | 29 ++++++--- libs/gui/GLConsumer.cpp | 97 ++++++++++++++++++++++------ libs/gui/include/gui/BLASTBufferQueue.h | 13 +++- libs/gui/include/gui/BufferItemConsumer.h | 18 ++++++ libs/gui/include/gui/ConsumerBase.h | 42 +++++++++++- libs/gui/include/gui/CpuConsumer.h | 12 +++- libs/gui/include/gui/GLConsumer.h | 24 +++++-- libs/gui/tests/Android.bp | 1 + libs/gui/tests/BufferItemConsumer_test.cpp | 5 +- libs/gui/tests/BufferQueue_test.cpp | 26 ++------ libs/gui/tests/CpuConsumer_test.cpp | 7 +- libs/gui/tests/MultiTextureConsumer_test.cpp | 8 +-- libs/gui/tests/SurfaceTextureClient_test.cpp | 16 ++--- libs/gui/tests/SurfaceTextureGL.h | 8 +-- libs/gui/tests/SurfaceTextureGL_test.cpp | 4 +- libs/gui/tests/Surface_test.cpp | 53 ++++----------- 19 files changed, 331 insertions(+), 143 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 7f0e80e0f0..cda49850f2 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -20,6 +20,7 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS //#define LOG_NDEBUG 0 +#include #include #include #include @@ -174,14 +175,21 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati mSyncTransaction(nullptr), mUpdateDestinationFrame(updateDestinationFrame) { createBufferQueue(&mProducer, &mConsumer); - // since the adapter is in the client process, set dequeue timeout - // explicitly so that dequeueBuffer will block - mProducer->setDequeueTimeout(std::numeric_limits::max()); - +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + mBufferItemConsumer = new BLASTBufferItemConsumer(mProducer, mConsumer, + GraphicBuffer::USAGE_HW_COMPOSER | + GraphicBuffer::USAGE_HW_TEXTURE, + 1, false, this); +#else mBufferItemConsumer = new BLASTBufferItemConsumer(mConsumer, GraphicBuffer::USAGE_HW_COMPOSER | GraphicBuffer::USAGE_HW_TEXTURE, 1, false, this); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // since the adapter is in the client process, set dequeue timeout + // explicitly so that dequeueBuffer will block + mProducer->setDequeueTimeout(std::numeric_limits::max()); + static std::atomic nextId = 0; mProducerId = nextId++; mName = name + "#" + std::to_string(mProducerId); diff --git a/libs/gui/BufferItemConsumer.cpp b/libs/gui/BufferItemConsumer.cpp index eeb8f196e6..bfe3d6e023 100644 --- a/libs/gui/BufferItemConsumer.cpp +++ b/libs/gui/BufferItemConsumer.cpp @@ -35,18 +35,37 @@ namespace android { +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +BufferItemConsumer::BufferItemConsumer(uint64_t consumerUsage, int bufferCount, + bool controlledByApp, bool isConsumerSurfaceFlinger) + : ConsumerBase(controlledByApp, isConsumerSurfaceFlinger) { + initialize(consumerUsage, bufferCount); +} + +BufferItemConsumer::BufferItemConsumer(const sp& producer, + const sp& consumer, + uint64_t consumerUsage, int bufferCount, + bool controlledByApp) + : ConsumerBase(producer, consumer, controlledByApp) { + initialize(consumerUsage, bufferCount); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + BufferItemConsumer::BufferItemConsumer( const sp& consumer, uint64_t consumerUsage, int bufferCount, bool controlledByApp) : ConsumerBase(consumer, controlledByApp) { + initialize(consumerUsage, bufferCount); +} + +void BufferItemConsumer::initialize(uint64_t consumerUsage, int bufferCount) { status_t err = mConsumer->setConsumerUsageBits(consumerUsage); - LOG_ALWAYS_FATAL_IF(err != OK, - "Failed to set consumer usage bits to %#" PRIx64, consumerUsage); + LOG_ALWAYS_FATAL_IF(err != OK, "Failed to set consumer usage bits to %#" PRIx64, consumerUsage); if (bufferCount != DEFAULT_MAX_BUFFERS) { err = mConsumer->setMaxAcquiredBufferCount(bufferCount); - LOG_ALWAYS_FATAL_IF(err != OK, - "Failed to set max acquired buffer count to %d", bufferCount); + LOG_ALWAYS_FATAL_IF(err != OK, "Failed to set max acquired buffer count to %d", + bufferCount); } } diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index 19b9c8b09d..602bba8dab 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -14,8 +14,6 @@ * limitations under the License. */ -#include - #define LOG_TAG "ConsumerBase" #define ATRACE_TAG ATRACE_TAG_GRAPHICS //#define LOG_NDEBUG 0 @@ -29,19 +27,22 @@ #include +#include #include +#include #include #include +#include #include -#include #include +#include #include #include #include -#include +#include // Macros for including the ConsumerBase name in log messages #define CB_LOGV(x, ...) ALOGV("[%s] " x, mName.c_str(), ##__VA_ARGS__) @@ -62,6 +63,30 @@ ConsumerBase::ConsumerBase(const sp& bufferQueue, bool c mAbandoned(false), mConsumer(bufferQueue), mPrevFinalReleaseFence(Fence::NO_FENCE) { + initialize(controlledByApp); +} + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +ConsumerBase::ConsumerBase(bool controlledByApp, bool consumerIsSurfaceFlinger) + : mAbandoned(false), mPrevFinalReleaseFence(Fence::NO_FENCE) { + sp producer; + BufferQueue::createBufferQueue(&producer, &mConsumer, consumerIsSurfaceFlinger); + mSurface = sp::make(producer, controlledByApp); + initialize(controlledByApp); +} + +ConsumerBase::ConsumerBase(const sp& producer, + const sp& consumer, bool controlledByApp) + : mAbandoned(false), + mConsumer(consumer), + mSurface(sp::make(producer, controlledByApp)), + mPrevFinalReleaseFence(Fence::NO_FENCE) { + initialize(controlledByApp); +} + +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + +void ConsumerBase::initialize(bool controlledByApp) { // Choose a name using the PID and a process-unique ID. mName = String8::format("unnamed-%d-%d", getpid(), createProcessUniqueId()); @@ -355,6 +380,17 @@ status_t ConsumerBase::setTransformHint(uint32_t hint) { return mConsumer->setTransformHint(hint); } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +status_t ConsumerBase::setMaxBufferCount(int bufferCount) { + Mutex::Autolock lock(mMutex); + if (mAbandoned) { + CB_LOGE("setMaxBufferCount: ConsumerBase is abandoned!"); + return NO_INIT; + } + return mConsumer->setMaxBufferCount(bufferCount); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + status_t ConsumerBase::setMaxAcquiredBufferCount(int maxAcquiredBuffers) { Mutex::Autolock lock(mMutex); if (mAbandoned) { @@ -364,6 +400,17 @@ status_t ConsumerBase::setMaxAcquiredBufferCount(int maxAcquiredBuffers) { return mConsumer->setMaxAcquiredBufferCount(maxAcquiredBuffers); } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +status_t ConsumerBase::setConsumerIsProtected(bool isProtected) { + Mutex::Autolock lock(mMutex); + if (mAbandoned) { + CB_LOGE("setConsumerIsProtected: ConsumerBase is abandoned!"); + return NO_INIT; + } + return mConsumer->setConsumerIsProtected(isProtected); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + sp ConsumerBase::getSidebandStream() const { Mutex::Autolock _l(mMutex); if (mAbandoned) { @@ -430,6 +477,19 @@ void ConsumerBase::dumpLocked(String8& result, const char* prefix) const { } } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +sp ConsumerBase::getSurface() const { + LOG_ALWAYS_FATAL_IF(mSurface == nullptr, + "It's illegal to get the surface of a Consumer that does not own it. This " + "should be impossible once the old CTOR is removed."); + return mSurface; +} + +sp ConsumerBase::getIGraphicBufferConsumer() const { + return mConsumer; +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + status_t ConsumerBase::acquireBufferLocked(BufferItem *item, nsecs_t presentWhen, uint64_t maxFrameNumber) { if (mAbandoned) { diff --git a/libs/gui/CpuConsumer.cpp b/libs/gui/CpuConsumer.cpp index 3031fa11fc..23b432e1f4 100644 --- a/libs/gui/CpuConsumer.cpp +++ b/libs/gui/CpuConsumer.cpp @@ -18,9 +18,9 @@ #define LOG_TAG "CpuConsumer" //#define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include - +#include #include +#include #include #define CC_LOGV(x, ...) ALOGV("[%s] " x, mName.c_str(), ##__VA_ARGS__) @@ -31,12 +31,25 @@ namespace android { -CpuConsumer::CpuConsumer(const sp& bq, - size_t maxLockedBuffers, bool controlledByApp) : - ConsumerBase(bq, controlledByApp), - mMaxLockedBuffers(maxLockedBuffers), - mCurrentLockedBuffers(0) -{ +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +CpuConsumer::CpuConsumer(size_t maxLockedBuffers, bool controlledByApp, + bool isConsumerSurfaceFlinger) + : ConsumerBase(controlledByApp, isConsumerSurfaceFlinger), + mMaxLockedBuffers(maxLockedBuffers), + mCurrentLockedBuffers(0) { + // Create tracking entries for locked buffers + mAcquiredBuffers.insertAt(0, maxLockedBuffers); + + mConsumer->setConsumerUsageBits(GRALLOC_USAGE_SW_READ_OFTEN); + mConsumer->setMaxAcquiredBufferCount(static_cast(maxLockedBuffers)); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + +CpuConsumer::CpuConsumer(const sp& bq, size_t maxLockedBuffers, + bool controlledByApp) + : ConsumerBase(bq, controlledByApp), + mMaxLockedBuffers(maxLockedBuffers), + mCurrentLockedBuffers(0) { // Create tracking entries for locked buffers mAcquiredBuffers.insertAt(0, maxLockedBuffers); diff --git a/libs/gui/GLConsumer.cpp b/libs/gui/GLConsumer.cpp index d49489c5a8..95cce5c1df 100644 --- a/libs/gui/GLConsumer.cpp +++ b/libs/gui/GLConsumer.cpp @@ -101,6 +101,34 @@ static bool hasEglProtectedContent() { return hasIt; } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +GLConsumer::GLConsumer(uint32_t tex, uint32_t texTarget, bool useFenceSync, bool isControlledByApp) + : ConsumerBase(isControlledByApp, /* isConsumerSurfaceFlinger */ false), + mCurrentCrop(Rect::EMPTY_RECT), + mCurrentTransform(0), + mCurrentScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE), + mCurrentFence(Fence::NO_FENCE), + mCurrentTimestamp(0), + mCurrentDataSpace(HAL_DATASPACE_UNKNOWN), + mCurrentFrameNumber(0), + mDefaultWidth(1), + mDefaultHeight(1), + mFilteringEnabled(true), + mTexName(tex), + mUseFenceSync(useFenceSync), + mTexTarget(texTarget), + mEglDisplay(EGL_NO_DISPLAY), + mEglContext(EGL_NO_CONTEXT), + mCurrentTexture(BufferQueue::INVALID_BUFFER_SLOT), + mAttached(true) { + GLC_LOGV("GLConsumer"); + + memcpy(mCurrentTransformMatrix, mtxIdentity.asArray(), sizeof(mCurrentTransformMatrix)); + + mConsumer->setConsumerUsageBits(DEFAULT_USAGE_FLAGS); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + GLConsumer::GLConsumer(const sp& bq, uint32_t tex, uint32_t texTarget, bool useFenceSync, bool isControlledByApp) : ConsumerBase(bq, isControlledByApp), @@ -130,27 +158,54 @@ GLConsumer::GLConsumer(const sp& bq, uint32_t tex, mConsumer->setConsumerUsageBits(DEFAULT_USAGE_FLAGS); } -GLConsumer::GLConsumer(const sp& bq, uint32_t texTarget, - bool useFenceSync, bool isControlledByApp) : - ConsumerBase(bq, isControlledByApp), - mCurrentCrop(Rect::EMPTY_RECT), - mCurrentTransform(0), - mCurrentScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE), - mCurrentFence(Fence::NO_FENCE), - mCurrentTimestamp(0), - mCurrentDataSpace(HAL_DATASPACE_UNKNOWN), - mCurrentFrameNumber(0), - mDefaultWidth(1), - mDefaultHeight(1), - mFilteringEnabled(true), - mTexName(0), - mUseFenceSync(useFenceSync), - mTexTarget(texTarget), - mEglDisplay(EGL_NO_DISPLAY), - mEglContext(EGL_NO_CONTEXT), - mCurrentTexture(BufferQueue::INVALID_BUFFER_SLOT), - mAttached(false) -{ +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) +GLConsumer::GLConsumer(uint32_t texTarget, bool useFenceSync, bool isControlledByApp) + : ConsumerBase(isControlledByApp, /* isConsumerSurfaceFlinger */ false), + mCurrentCrop(Rect::EMPTY_RECT), + mCurrentTransform(0), + mCurrentScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE), + mCurrentFence(Fence::NO_FENCE), + mCurrentTimestamp(0), + mCurrentDataSpace(HAL_DATASPACE_UNKNOWN), + mCurrentFrameNumber(0), + mDefaultWidth(1), + mDefaultHeight(1), + mFilteringEnabled(true), + mTexName(0), + mUseFenceSync(useFenceSync), + mTexTarget(texTarget), + mEglDisplay(EGL_NO_DISPLAY), + mEglContext(EGL_NO_CONTEXT), + mCurrentTexture(BufferQueue::INVALID_BUFFER_SLOT), + mAttached(false) { + GLC_LOGV("GLConsumer"); + + memcpy(mCurrentTransformMatrix, mtxIdentity.asArray(), sizeof(mCurrentTransformMatrix)); + + mConsumer->setConsumerUsageBits(DEFAULT_USAGE_FLAGS); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + +GLConsumer::GLConsumer(const sp& bq, uint32_t texTarget, bool useFenceSync, + bool isControlledByApp) + : ConsumerBase(bq, isControlledByApp), + mCurrentCrop(Rect::EMPTY_RECT), + mCurrentTransform(0), + mCurrentScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE), + mCurrentFence(Fence::NO_FENCE), + mCurrentTimestamp(0), + mCurrentDataSpace(HAL_DATASPACE_UNKNOWN), + mCurrentFrameNumber(0), + mDefaultWidth(1), + mDefaultHeight(1), + mFilteringEnabled(true), + mTexName(0), + mUseFenceSync(useFenceSync), + mTexTarget(texTarget), + mEglDisplay(EGL_NO_DISPLAY), + mEglContext(EGL_NO_CONTEXT), + mCurrentTexture(BufferQueue::INVALID_BUFFER_SLOT), + mAttached(false) { GLC_LOGV("GLConsumer"); memcpy(mCurrentTransformMatrix, mtxIdentity.asArray(), diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 266729abe5..4dafc57734 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -17,9 +17,10 @@ #ifndef ANDROID_GUI_BLAST_BUFFER_QUEUE_H #define ANDROID_GUI_BLAST_BUFFER_QUEUE_H +#include #include #include - +#include #include #include @@ -39,12 +40,20 @@ class BufferItemConsumer; class BLASTBufferItemConsumer : public BufferItemConsumer { public: +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + BLASTBufferItemConsumer(const sp& producer, + const sp& consumer, uint64_t consumerUsage, + int bufferCount, bool controlledByApp, wp bbq) + : BufferItemConsumer(producer, consumer, consumerUsage, bufferCount, controlledByApp), +#else BLASTBufferItemConsumer(const sp& consumer, uint64_t consumerUsage, int bufferCount, bool controlledByApp, wp bbq) : BufferItemConsumer(consumer, consumerUsage, bufferCount, controlledByApp), +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) mBLASTBufferQueue(std::move(bbq)), mCurrentlyConnected(false), - mPreviouslyConnected(false) {} + mPreviouslyConnected(false) { + } void onDisconnect() override EXCLUDES(mMutex); void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, diff --git a/libs/gui/include/gui/BufferItemConsumer.h b/libs/gui/include/gui/BufferItemConsumer.h index e383a407dc..6810edaf7c 100644 --- a/libs/gui/include/gui/BufferItemConsumer.h +++ b/libs/gui/include/gui/BufferItemConsumer.h @@ -53,9 +53,17 @@ class BufferItemConsumer: public ConsumerBase // access at the same time. // controlledByApp tells whether this consumer is controlled by the // application. +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + BufferItemConsumer(uint64_t consumerUsage, int bufferCount = DEFAULT_MAX_BUFFERS, + bool controlledByApp = false, bool isConsumerSurfaceFlinger = false); + BufferItemConsumer(const sp& consumer, uint64_t consumerUsage, + int bufferCount = DEFAULT_MAX_BUFFERS, bool controlledByApp = false) + __attribute((deprecated("Prefer ctors that create their own surface and consumer."))); +#else BufferItemConsumer(const sp& consumer, uint64_t consumerUsage, int bufferCount = DEFAULT_MAX_BUFFERS, bool controlledByApp = false); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) ~BufferItemConsumer() override; @@ -92,7 +100,17 @@ class BufferItemConsumer: public ConsumerBase const sp& releaseFence = Fence::NO_FENCE); #endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) +protected: +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // This should only be used by BLASTBufferQueue: + BufferItemConsumer(const sp& producer, + const sp& consumer, uint64_t consumerUsage, + int bufferCount = DEFAULT_MAX_BUFFERS, bool controlledByApp = false); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + private: + void initialize(uint64_t consumerUsage, int bufferCount); + status_t releaseBufferSlotLocked(int slotIndex, const sp& buffer, const sp& releaseFence); diff --git a/libs/gui/include/gui/ConsumerBase.h b/libs/gui/include/gui/ConsumerBase.h index a031e66f68..e976aa48be 100644 --- a/libs/gui/include/gui/ConsumerBase.h +++ b/libs/gui/include/gui/ConsumerBase.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -74,6 +75,16 @@ public: void dumpState(String8& result) const; void dumpState(String8& result, const char* prefix) const; +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // Returns a Surface that can be used as the producer for this consumer. + sp getSurface() const; + + // DEPRECATED, DO NOT USE. Returns the underlying IGraphicBufferConsumer + // that backs this ConsumerBase. + sp getIGraphicBufferConsumer() const + __attribute((deprecated("DO NOT USE: Temporary hack for refactoring"))); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // setFrameAvailableListener sets the listener object that will be notified // when a new frame becomes available. void setFrameAvailableListener(const wp& listener); @@ -102,9 +113,18 @@ public: // See IGraphicBufferConsumer::setTransformHint status_t setTransformHint(uint32_t hint); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // See IGraphicBufferConsumer::setMaxBufferCount + status_t setMaxBufferCount(int bufferCount); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // See IGraphicBufferConsumer::setMaxAcquiredBufferCount status_t setMaxAcquiredBufferCount(int maxAcquiredBuffers); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + status_t setConsumerIsProtected(bool isProtected); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // See IGraphicBufferConsumer::getSidebandStream sp getSidebandStream() const; @@ -119,12 +139,24 @@ private: ConsumerBase(const ConsumerBase&); void operator=(const ConsumerBase&); + void initialize(bool controlledByApp); + protected: // ConsumerBase constructs a new ConsumerBase object to consume image // buffers from the given IGraphicBufferConsumer. // The controlledByApp flag indicates that this consumer is under the application's // control. +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + explicit ConsumerBase(bool controlledByApp = false, bool consumerIsSurfaceFlinger = false); + explicit ConsumerBase(const sp& producer, + const sp& consumer, bool controlledByApp = false); + + explicit ConsumerBase(const sp& consumer, bool controlledByApp = false) + __attribute((deprecated("ConsumerBase should own its own producer, and constructing it " + "without one is fragile! This method is going away soon."))); +#else explicit ConsumerBase(const sp& consumer, bool controlledByApp = false); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) // onLastStrongRef gets called by RefBase just before the dtor of the most // derived class. It is used to clean up the buffers so that ConsumerBase @@ -272,10 +304,16 @@ protected: Mutex mFrameAvailableMutex; wp mFrameAvailableListener; - // The ConsumerBase has-a BufferQueue and is responsible for creating this object - // if none is supplied + // The ConsumerBase has-a BufferQueue and is responsible for creating these + // objects if not supplied. sp mConsumer; +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // This Surface wraps the IGraphicBufferConsumer created for this + // ConsumerBase. + sp mSurface; +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + // The final release fence of the most recent buffer released by // releaseBufferLocked. sp mPrevFinalReleaseFence; diff --git a/libs/gui/include/gui/CpuConsumer.h b/libs/gui/include/gui/CpuConsumer.h index 806fbe8aa0..2bba61bbe8 100644 --- a/libs/gui/include/gui/CpuConsumer.h +++ b/libs/gui/include/gui/CpuConsumer.h @@ -19,8 +19,9 @@ #include -#include +#include #include +#include #include @@ -91,8 +92,17 @@ class CpuConsumer : public ConsumerBase // Create a new CPU consumer. The maxLockedBuffers parameter specifies // how many buffers can be locked for user access at the same time. +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + CpuConsumer(size_t maxLockedBuffers, bool controlledByApp = false, + bool isConsumerSurfaceFlinger = false); + + CpuConsumer(const sp& bq, size_t maxLockedBuffers, + bool controlledByApp = false) + __attribute((deprecated("Prefer ctors that create their own surface and consumer."))); +#else CpuConsumer(const sp& bq, size_t maxLockedBuffers, bool controlledByApp = false); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) // Gets the next graphics buffer from the producer and locks it for CPU use, // filling out the passed-in locked buffer structure with the native pointer diff --git a/libs/gui/include/gui/GLConsumer.h b/libs/gui/include/gui/GLConsumer.h index ba268ab17a..bfe3eb31e8 100644 --- a/libs/gui/include/gui/GLConsumer.h +++ b/libs/gui/include/gui/GLConsumer.h @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -82,12 +83,25 @@ public: // If the constructor without the tex parameter is used, the GLConsumer is // created in a detached state, and attachToContext must be called before // calls to updateTexImage. - GLConsumer(const sp& bq, - uint32_t tex, uint32_t texureTarget, bool useFenceSync, - bool isControlledByApp); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) + GLConsumer(uint32_t tex, uint32_t textureTarget, bool useFenceSync, bool isControlledByApp); - GLConsumer(const sp& bq, uint32_t texureTarget, - bool useFenceSync, bool isControlledByApp); + GLConsumer(uint32_t textureTarget, bool useFenceSync, bool isControlledByApp); + + GLConsumer(const sp& bq, uint32_t tex, uint32_t textureTarget, + bool useFenceSync, bool isControlledByApp) + __attribute((deprecated("Prefer ctors that create their own surface and consumer."))); + + GLConsumer(const sp& bq, uint32_t textureTarget, bool useFenceSync, + bool isControlledByApp) + __attribute((deprecated("Prefer ctors that create their own surface and consumer."))); +#else + GLConsumer(const sp& bq, uint32_t tex, uint32_t textureTarget, + bool useFenceSync, bool isControlledByApp); + + GLConsumer(const sp& bq, uint32_t textureTarget, bool useFenceSync, + bool isControlledByApp); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ) // updateTexImage acquires the most recently queued buffer, and sets the // image contents of the target texture to it. diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index 9558edab87..1b216e9063 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -25,6 +25,7 @@ cc_test { "-Wthread-safety", "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_BQ_SETFRAMERATE=true", "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_BQ_EXTENDEDALLOCATE=true", + "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_WB_CONSUMER_BASE_OWNS_BQ=true", "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_WB_PLATFORM_API_IMPROVEMENTS=true", ], diff --git a/libs/gui/tests/BufferItemConsumer_test.cpp b/libs/gui/tests/BufferItemConsumer_test.cpp index 7d33f283aa..845a1caf21 100644 --- a/libs/gui/tests/BufferItemConsumer_test.cpp +++ b/libs/gui/tests/BufferItemConsumer_test.cpp @@ -56,15 +56,14 @@ class BufferItemConsumerTest : public ::testing::Test { }; void SetUp() override { - BufferQueue::createBufferQueue(&mProducer, &mConsumer); - mBIC = - new BufferItemConsumer(mConsumer, kFormat, kMaxLockedBuffers, true); + mBIC = new BufferItemConsumer(kFormat, kMaxLockedBuffers, true); String8 name("BufferItemConsumer_Under_Test"); mBIC->setName(name); mBFL = new BufferFreedListener(this); mBIC->setBufferFreedListener(mBFL); sp producerListener = new TrackingProducerListener(this); + mProducer = mBIC->getSurface()->getIGraphicBufferProducer(); IGraphicBufferProducer::QueueBufferOutput bufferOutput; ASSERT_EQ(NO_ERROR, mProducer->connect(producerListener, NATIVE_WINDOW_API_CPU, diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index 590e2c87c9..2e6ffcb57f 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -1430,19 +1430,15 @@ TEST_F(BufferQueueTest, TestBqSetFrameRateFlagBuildTimeIsSet) { } struct BufferItemConsumerSetFrameRateListener : public BufferItemConsumer { - BufferItemConsumerSetFrameRateListener(const sp& consumer) - : BufferItemConsumer(consumer, GRALLOC_USAGE_SW_READ_OFTEN, 1) {} + BufferItemConsumerSetFrameRateListener() : BufferItemConsumer(GRALLOC_USAGE_SW_READ_OFTEN, 1) {} MOCK_METHOD(void, onSetFrameRate, (float, int8_t, int8_t), (override)); }; TEST_F(BufferQueueTest, TestSetFrameRate) { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp bufferConsumer = - sp::make(consumer); + sp::make(); + sp producer = bufferConsumer->getSurface()->getIGraphicBufferProducer(); EXPECT_CALL(*bufferConsumer, onSetFrameRate(12.34f, 1, 0)).Times(1); producer->setFrameRate(12.34f, 1, 0); @@ -1493,14 +1489,10 @@ struct OneshotOnDequeuedListener final : public BufferItemConsumer::FrameAvailab // See b/270004534 TEST(BufferQueueThreading, TestProducerDequeueConsumerDestroy) { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp bufferConsumer = - sp::make(consumer, GRALLOC_USAGE_SW_READ_OFTEN, 2); + sp::make(GRALLOC_USAGE_SW_READ_OFTEN, 2); ASSERT_NE(nullptr, bufferConsumer.get()); - sp surface = sp::make(producer); + sp surface = bufferConsumer->getSurface(); native_window_set_buffers_format(surface.get(), PIXEL_FORMAT_RGBA_8888); native_window_set_buffers_dimensions(surface.get(), 100, 100); @@ -1531,14 +1523,10 @@ TEST(BufferQueueThreading, TestProducerDequeueConsumerDestroy) { } TEST_F(BufferQueueTest, TestAdditionalOptions) { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp bufferConsumer = - sp::make(consumer, GRALLOC_USAGE_SW_READ_OFTEN, 2); + sp::make(GRALLOC_USAGE_SW_READ_OFTEN, 2); ASSERT_NE(nullptr, bufferConsumer.get()); - sp surface = sp::make(producer); + sp surface = bufferConsumer->getSurface(); native_window_set_buffers_format(surface.get(), PIXEL_FORMAT_RGBA_8888); native_window_set_buffers_dimensions(surface.get(), 100, 100); diff --git a/libs/gui/tests/CpuConsumer_test.cpp b/libs/gui/tests/CpuConsumer_test.cpp index d80bd9c62a..f4239cb69e 100644 --- a/libs/gui/tests/CpuConsumer_test.cpp +++ b/libs/gui/tests/CpuConsumer_test.cpp @@ -66,13 +66,10 @@ protected: test_info->name(), params.width, params.height, params.maxLockedBuffers, params.format); - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - mCC = new CpuConsumer(consumer, params.maxLockedBuffers); + mCC = new CpuConsumer(params.maxLockedBuffers); String8 name("CpuConsumer_Under_Test"); mCC->setName(name); - mSTC = new Surface(producer); + mSTC = mCC->getSurface(); mANW = mSTC; } diff --git a/libs/gui/tests/MultiTextureConsumer_test.cpp b/libs/gui/tests/MultiTextureConsumer_test.cpp index 7d3d4aa412..2428bb3110 100644 --- a/libs/gui/tests/MultiTextureConsumer_test.cpp +++ b/libs/gui/tests/MultiTextureConsumer_test.cpp @@ -34,12 +34,8 @@ protected: virtual void SetUp() { GLTest::SetUp(); - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - mGlConsumer = new GLConsumer(consumer, TEX_ID, - GLConsumer::TEXTURE_EXTERNAL, true, false); - mSurface = new Surface(producer); + mGlConsumer = new GLConsumer(TEX_ID, GLConsumer::TEXTURE_EXTERNAL, true, false); + mSurface = mGlConsumer->getSurface(); mANW = mSurface.get(); } diff --git a/libs/gui/tests/SurfaceTextureClient_test.cpp b/libs/gui/tests/SurfaceTextureClient_test.cpp index b28dca8ab4..59d05b673c 100644 --- a/libs/gui/tests/SurfaceTextureClient_test.cpp +++ b/libs/gui/tests/SurfaceTextureClient_test.cpp @@ -40,12 +40,8 @@ protected: } virtual void SetUp() { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - mST = new GLConsumer(consumer, 123, GLConsumer::TEXTURE_EXTERNAL, true, - false); - mSTC = new Surface(producer); + mST = new GLConsumer(123, GLConsumer::TEXTURE_EXTERNAL, true, false); + mSTC = mST->getSurface(); mANW = mSTC; // We need a valid GL context so we can test updateTexImage() @@ -731,12 +727,8 @@ protected: ASSERT_NE(EGL_NO_CONTEXT, mEglContext); for (int i = 0; i < NUM_SURFACE_TEXTURES; i++) { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp st(new GLConsumer(consumer, i, - GLConsumer::TEXTURE_EXTERNAL, true, false)); - sp stc(new Surface(producer)); + sp st(new GLConsumer(i, GLConsumer::TEXTURE_EXTERNAL, true, false)); + sp stc = st->getSurface(); mEglSurfaces[i] = eglCreateWindowSurface(mEglDisplay, myConfig, static_cast(stc.get()), nullptr); ASSERT_EQ(EGL_SUCCESS, eglGetError()); diff --git a/libs/gui/tests/SurfaceTextureGL.h b/libs/gui/tests/SurfaceTextureGL.h index 9d8af5d0f5..1309635afd 100644 --- a/libs/gui/tests/SurfaceTextureGL.h +++ b/libs/gui/tests/SurfaceTextureGL.h @@ -38,11 +38,8 @@ protected: void SetUp() { GLTest::SetUp(); - sp producer; - BufferQueue::createBufferQueue(&producer, &mConsumer); - mST = new GLConsumer(mConsumer, TEX_ID, GLConsumer::TEXTURE_EXTERNAL, - true, false); - mSTC = new Surface(producer); + mST = new GLConsumer(TEX_ID, GLConsumer::TEXTURE_EXTERNAL, true, false); + mSTC = mST->getSurface(); mANW = mSTC; ASSERT_EQ(NO_ERROR, native_window_set_usage(mANW.get(), TEST_PRODUCER_USAGE_BITS)); mTextureRenderer = new TextureRenderer(TEX_ID, mST); @@ -63,7 +60,6 @@ protected: mTextureRenderer->drawTexture(); } - sp mConsumer; sp mST; sp mSTC; sp mANW; diff --git a/libs/gui/tests/SurfaceTextureGL_test.cpp b/libs/gui/tests/SurfaceTextureGL_test.cpp index f76c0be265..449533aa57 100644 --- a/libs/gui/tests/SurfaceTextureGL_test.cpp +++ b/libs/gui/tests/SurfaceTextureGL_test.cpp @@ -480,8 +480,8 @@ TEST_F(SurfaceTextureGLTest, DisconnectStressTest) { }; sp dw(new DisconnectWaiter()); - mConsumer->consumerConnect(dw, false); - + sp consumer = mST->getIGraphicBufferConsumer(); + consumer->consumerConnect(dw, false); sp pt(new ProducerThread(mANW)); pt->run("ProducerThread"); diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 85f4a42b89..8ab8783c4c 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -272,13 +272,9 @@ TEST_F(SurfaceTest, LayerCountIsOne) { TEST_F(SurfaceTest, QueryConsumerUsage) { const int TEST_USAGE_FLAGS = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_HW_RENDER; - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp c = new BufferItemConsumer(consumer, - TEST_USAGE_FLAGS); - sp s = new Surface(producer); + sp c = new BufferItemConsumer(TEST_USAGE_FLAGS); + sp s = c->getSurface(); sp anw(s); int flags = -1; @@ -290,15 +286,11 @@ TEST_F(SurfaceTest, QueryConsumerUsage) { TEST_F(SurfaceTest, QueryDefaultBuffersDataSpace) { const android_dataspace TEST_DATASPACE = HAL_DATASPACE_V0_SRGB; - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp cpuConsumer = new CpuConsumer(consumer, 1); + sp cpuConsumer = new CpuConsumer(1); cpuConsumer->setDefaultBufferDataSpace(TEST_DATASPACE); - sp s = new Surface(producer); - + sp s = cpuConsumer->getSurface(); sp anw(s); android_dataspace dataSpace; @@ -311,11 +303,8 @@ TEST_F(SurfaceTest, QueryDefaultBuffersDataSpace) { } TEST_F(SurfaceTest, SettingGenerationNumber) { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp cpuConsumer = new CpuConsumer(consumer, 1); - sp surface = new Surface(producer); + sp cpuConsumer = new CpuConsumer(1); + sp surface = cpuConsumer->getSurface(); sp window(surface); // Allocate a buffer with a generation number of 0 @@ -2155,12 +2144,9 @@ TEST_F(SurfaceTest, DefaultMaxBufferCountSetAndUpdated) { TEST_F(SurfaceTest, BatchOperations) { const int BUFFER_COUNT = 16; const int BATCH_SIZE = 8; - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp cpuConsumer = new CpuConsumer(consumer, 1); - sp surface = new Surface(producer); + sp cpuConsumer = new CpuConsumer(1); + sp surface = cpuConsumer->getSurface(); sp window(surface); sp listener = new StubSurfaceListener(); @@ -2207,12 +2193,9 @@ TEST_F(SurfaceTest, BatchOperations) { TEST_F(SurfaceTest, BatchIllegalOperations) { const int BUFFER_COUNT = 16; const int BATCH_SIZE = 8; - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - sp cpuConsumer = new CpuConsumer(consumer, 1); - sp surface = new Surface(producer); + sp cpuConsumer = new CpuConsumer(1); + sp surface = cpuConsumer->getSurface(); sp window(surface); sp listener = new StubSurfaceListener(); @@ -2237,12 +2220,8 @@ TEST_F(SurfaceTest, BatchIllegalOperations) { #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) TEST_F(SurfaceTest, PlatformBufferMethods) { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - - sp cpuConsumer = sp::make(consumer, 1); - sp surface = sp::make(producer); + sp cpuConsumer = sp::make(1); + sp surface = cpuConsumer->getSurface(); sp listener = sp::make(); sp buffer; sp fence; @@ -2294,13 +2273,9 @@ TEST_F(SurfaceTest, PlatformBufferMethods) { } TEST_F(SurfaceTest, AllowAllocation) { - sp producer; - sp consumer; - BufferQueue::createBufferQueue(&producer, &consumer); - // controlledByApp must be true to disable blocking - sp cpuConsumer = sp::make(consumer, 1, /*controlledByApp*/ true); - sp surface = sp::make(producer, /*controlledByApp*/ true); + sp cpuConsumer = sp::make(1, /*controlledByApp*/ true); + sp surface = cpuConsumer->getSurface(); sp listener = sp::make(); sp buffer; sp fence; -- cgit v1.2.3-59-g8ed1b From ca81c05b5abe743d41687417139039aa25771fd5 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 15 Aug 2024 12:38:34 -0500 Subject: Fix stale buffer release issue. BufferItemConsumer sometimes returns BufferItems where mIsStale is already set to true, breaking the logic for ignoring buffers that were released via a disconnect. This CL adds a new field for BLAST to use for this purpose. Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: CtsMediaDecoderTestCases, CtsDeqpTestCases Change-Id: Ie32c9c6e0e26e68b794a1ca78cdd3c2395ac0966 --- libs/gui/BLASTBufferQueue.cpp | 19 ++++++++++++++----- libs/gui/include/gui/BLASTBufferQueue.h | 9 ++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index cda49850f2..f065ffa611 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -510,7 +510,7 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, return; } #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - if (!it->second.mIsStale) { + if (!it->second.disconnectedAfterAcquired) { mNumAcquired--; } #else @@ -566,7 +566,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( applyTransaction = false; } - BufferItem bufferItem; + BLASTBufferItem bufferItem; status_t status = mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false); @@ -1130,9 +1130,9 @@ public: // can be non-blocking when the producer is in the client process. class BBQBufferQueueProducer : public BufferQueueProducer { public: - BBQBufferQueueProducer(const sp& core, wp bbq) + BBQBufferQueueProducer(const sp& core, const wp& bbq) : BufferQueueProducer(core, false /* consumerIsSurfaceFlinger*/), - mBLASTBufferQueue(std::move(bbq)) {} + mBLASTBufferQueue(bbq) {} status_t connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output) override { @@ -1156,10 +1156,19 @@ public: return status; } + // We need to reset dequeued and acquired counts because BufferQueueProducer::disconnect + // calls BufferQueueCore::freeAllBuffersLocked which frees all dequeued and acquired + // buffers. We don't reset mNumFrameAvailable because these buffers are still available + // in BufferItemConsumer. bbq->mNumDequeued = 0; bbq->mNumAcquired = 0; + // SurfaceFlinger sends release callbacks for buffers that have been acquired after a + // disconnect. We set disconnectedAfterAcquired to true so that we can ignore any stale + // releases that come in after the producer is disconnected. Otherwise, releaseBuffer will + // decrement mNumAcquired for a buffer that was acquired before we reset mNumAcquired to + // zero. for (auto& [releaseId, bufferItem] : bbq->mSubmitted) { - bufferItem.mIsStale = true; + bufferItem.disconnectedAfterAcquired = true; } return OK; diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 4dafc57734..c2dcd2579c 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -198,9 +198,16 @@ private: // latch stale buffers and that we don't wait on barriers from an old producer. uint32_t mProducerId = 0; + class BLASTBufferItem : public BufferItem { + public: + // True if BBQBufferQueueProducer is disconnected after the buffer is acquried but + // before it is released. + bool disconnectedAfterAcquired{false}; + }; + // Keep a reference to the submitted buffers so we can release when surfaceflinger drops the // buffer or the buffer has been presented and a new buffer is ready to be presented. - std::unordered_map mSubmitted + std::unordered_map mSubmitted GUARDED_BY(mMutex); // Keep a queue of the released buffers instead of immediately releasing -- cgit v1.2.3-59-g8ed1b From 1a37b731c88f79f81c4add642b2a06b06d38ea2d Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 22 Aug 2024 15:00:50 -0500 Subject: Fix transactionCallbackThunk lifetime issue TestBLASTBufferQueue overrides transactionCallback but BLASTBufferQueue::transactionCallback can trigger the destruction of BLASTBufferQueue, causing native crashes. This CL fixes the issue by moving the decStrong call after transactionCallback has completed. Bug: 294133380 Flag: EXEMPT bugfix Test: BLASTBufferQueueTest Change-Id: Ia2344ed6d14e89026f464b32c95c76b3a96a19d0 --- libs/gui/BLASTBufferQueue.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index f065ffa611..1fb59fd8e0 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -356,8 +356,9 @@ static void transactionCallbackThunk(void* context, nsecs_t latchTime, if (context == nullptr) { return; } - sp bq = static_cast(context); + auto bq = static_cast(context); bq->transactionCallback(latchTime, presentFence, stats); + bq->decStrong((void*)transactionCallbackThunk); } void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp& /*presentFence*/, @@ -413,8 +414,6 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp Date: Fri, 23 Aug 2024 16:11:10 -0500 Subject: Replace BBQ callback thunks with lambdas The lambdas allows for better StrongPointer/RefBase safety by replacing explicit incStrong/decStrong calls with RAII. Captured StrongPointers are created using sp::fromExisting which helps detect RefBase errors such as creating a StrongPointer from an already deleted RefBase object. Bug: 361588984 Flag: EXEMPT refactor Test: presubmits Change-Id: I1a0c0df882ec2a3f91f857c446418f3fb51689c1 --- libs/gui/BLASTBufferQueue.cpp | 65 ++++++++++++++------------------- libs/gui/SurfaceComposerClient.cpp | 11 ++++-- libs/gui/include/gui/BLASTBufferQueue.h | 6 +++ 3 files changed, 41 insertions(+), 41 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 1fb59fd8e0..f8e3fd0a5c 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -306,14 +306,12 @@ static std::optional findMatchingStat( return std::nullopt; } -static void transactionCommittedCallbackThunk(void* context, nsecs_t latchTime, - const sp& presentFence, - const std::vector& stats) { - if (context == nullptr) { - return; - } - sp bq = static_cast(context); - bq->transactionCommittedCallback(latchTime, presentFence, stats); +TransactionCompletedCallbackTakesContext BLASTBufferQueue::makeTransactionCommittedCallbackThunk() { + return [bbq = sp::fromExisting( + this)](void* /*context*/, nsecs_t latchTime, const sp& presentFence, + const std::vector& stats) { + bbq->transactionCommittedCallback(latchTime, presentFence, stats); + }; } void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/, @@ -346,19 +344,15 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/, BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was " "empty."); } - decStrong((void*)transactionCommittedCallbackThunk); } } -static void transactionCallbackThunk(void* context, nsecs_t latchTime, - const sp& presentFence, - const std::vector& stats) { - if (context == nullptr) { - return; - } - auto bq = static_cast(context); - bq->transactionCallback(latchTime, presentFence, stats); - bq->decStrong((void*)transactionCallbackThunk); +TransactionCompletedCallbackTakesContext BLASTBufferQueue::makeTransactionCallbackThunk() { + return [bbq = sp::fromExisting( + this)](void* /*context*/, nsecs_t latchTime, const sp& presentFence, + const std::vector& stats) { + bbq->transactionCallback(latchTime, presentFence, stats); + }; } void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp& /*presentFence*/, @@ -421,15 +415,17 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp context, const ReleaseCallbackId& id, - const sp& releaseFence, - std::optional currentMaxAcquiredBufferCount) { - sp blastBufferQueue = context.promote(); - if (blastBufferQueue) { - blastBufferQueue->releaseBufferCallback(id, releaseFence, currentMaxAcquiredBufferCount); - } else { - ALOGV("releaseBufferCallbackThunk %s blastBufferQueue is dead", id.to_string().c_str()); - } +ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() { + return [weakBbq = wp::fromExisting( + this)](const ReleaseCallbackId& id, const sp& releaseFence, + std::optional currentMaxAcquiredBufferCount) { + sp bbq = weakBbq.promote(); + if (!bbq) { + ALOGV("releaseBufferCallbackThunk %s blastBufferQueue is dead", id.to_string().c_str()); + return; + } + bbq->releaseBufferCallback(id, releaseFence, currentMaxAcquiredBufferCount); + }; } void BLASTBufferQueue::flushShadowQueue() { @@ -609,9 +605,6 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( t->notifyProducerDisconnect(mSurfaceControl); } - // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback. - incStrong((void*)transactionCallbackThunk); - // Only update mSize for destination bounds if the incoming buffer matches the requested size. // Otherwise, it could cause stretching since the destination bounds will update before the // buffer with the new size is acquired. @@ -624,9 +617,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); - auto releaseBufferCallback = - std::bind(releaseBufferCallbackThunk, wp(this) /* callbackContext */, - std::placeholders::_1, std::placeholders::_2, std::placeholders::_3); + auto releaseBufferCallback = makeReleaseBufferCallbackThunk(); sp fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE; nsecs_t dequeueTime = -1; @@ -644,7 +635,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( t->setDataspace(mSurfaceControl, static_cast(bufferItem.mDataSpace)); t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata); t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage); - t->addTransactionCompletedCallback(transactionCallbackThunk, static_cast(this)); + t->addTransactionCompletedCallback(makeTransactionCallbackThunk(), nullptr); mSurfaceControlsWithPendingCallback.push(mSurfaceControl); @@ -804,9 +795,9 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { // Only need a commit callback when syncing to ensure the buffer that's synced has been // sent to SF - incStrong((void*)transactionCommittedCallbackThunk); - mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk, - static_cast(this)); + mSyncTransaction + ->addTransactionCommittedCallback(makeTransactionCommittedCallbackThunk(), + nullptr); if (mAcquireSingleBuffer) { prevCallback = mTransactionReadyCallback; prevTransaction = mSyncTransaction; diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 7d3e5c166f..df58df43be 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -2051,8 +2051,9 @@ SurfaceComposerClient::Transaction::setFrameRateSelectionPriority(const sp& presentFence, const std::vector& stats); + + TransactionCompletedCallbackTakesContext makeTransactionCallbackThunk(); virtual void transactionCallback(nsecs_t latchTime, const sp& presentFence, const std::vector& stats); + + ReleaseBufferCallback makeReleaseBufferCallbackThunk(); void releaseBufferCallback(const ReleaseCallbackId& id, const sp& releaseFence, std::optional currentMaxAcquiredBufferCount); void releaseBufferCallbackLocked(const ReleaseCallbackId& id, const sp& releaseFence, std::optional currentMaxAcquiredBufferCount, bool fakeRelease) REQUIRES(mMutex); + bool syncNextTransaction(std::function callback, bool acquireSingleBuffer = true); void stopContinuousSyncTransaction(); -- cgit v1.2.3-59-g8ed1b From 5ab65e9ef047bc802885396f0e1d960901e86a5c Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Wed, 21 Aug 2024 13:30:38 -0500 Subject: Read from BufferReleaseChannel in background thread Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: BLASTBufferQueueTest Change-Id: I18d2cf5457f615e832352ff9d03729ffe82cf4b9 --- libs/gui/BLASTBufferQueue.cpp | 161 +++++++++++++++++++-- libs/gui/include/gui/BLASTBufferQueue.h | 45 ++++++ services/surfaceflinger/Layer.cpp | 23 ++- services/surfaceflinger/Layer.h | 3 + services/surfaceflinger/SurfaceFlinger.cpp | 4 + .../surfaceflinger/TransactionCallbackInvoker.cpp | 13 ++ .../surfaceflinger/TransactionCallbackInvoker.h | 15 +- 7 files changed, 245 insertions(+), 19 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index f8e3fd0a5c..8f6dd840aa 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -22,11 +22,14 @@ #include #include +#include #include #include #include #include #include +#include +#include #include #include @@ -74,6 +77,12 @@ namespace android { std::unique_lock _lock{mutex}; \ base::ScopedLockAssertion assumeLocked(mutex); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) +static ReleaseBufferCallback EMPTY_RELEASE_CALLBACK = + [](const ReleaseCallbackId&, const sp& /*releaseFence*/, + std::optional /*currentMaxAcquiredBufferCount*/) {}; +#endif + void BLASTBufferItemConsumer::onDisconnect() { Mutex::Autolock lock(mMutex); mPreviouslyConnected = mCurrentlyConnected; @@ -215,6 +224,12 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + std::unique_ptr bufferReleaseConsumer; + gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); + mBufferReleaseReader = std::make_shared(std::move(bufferReleaseConsumer)); +#endif + BQA_LOGV("BLASTBufferQueue created"); } @@ -244,6 +259,9 @@ BLASTBufferQueue::~BLASTBufferQueue() { void BLASTBufferQueue::onFirstRef() { // safe default, most producers are expected to override this mProducer->setMaxDequeuedBufferCount(2); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + mBufferReleaseThread.start(sp::fromExisting(this)); +#endif } void BLASTBufferQueue::update(const sp& surface, uint32_t width, uint32_t height, @@ -269,6 +287,9 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); +#endif applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -386,6 +407,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp staleReleases; for (const auto& [key, value]: mSubmitted) { @@ -401,6 +423,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp 0) { + acquireNextBufferLocked(std::nullopt); + numFramesToFlush--; + } +} + // Unlike transactionCallbackThunk the release buffer callback does not extend the life of the // BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client. // So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer. @@ -428,15 +460,6 @@ ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() { }; } -void BLASTBufferQueue::flushShadowQueue() { - BQA_LOGV("flushShadowQueue"); - int numFramesToFlush = mNumFrameAvailable; - while (numFramesToFlush > 0) { - acquireNextBufferLocked(std::nullopt); - numFramesToFlush--; - } -} - void BLASTBufferQueue::releaseBufferCallback( const ReleaseCallbackId& id, const sp& releaseFence, std::optional currentMaxAcquiredBufferCount) { @@ -617,7 +640,12 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + ReleaseBufferCallback releaseBufferCallback = + applyTransaction ? EMPTY_RELEASE_CALLBACK : makeReleaseBufferCallbackThunk(); +#else auto releaseBufferCallback = makeReleaseBufferCallbackThunk(); +#endif sp fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE; nsecs_t dequeueTime = -1; @@ -1359,7 +1387,120 @@ bool BLASTBufferQueue::isSameSurfaceControl(const sp& surfaceCon void BLASTBufferQueue::setTransactionHangCallback( std::function callback) { std::lock_guard _lock{mMutex}; - mTransactionHangCallback = callback; + mTransactionHangCallback = std::move(callback); +} + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + +BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( + std::unique_ptr endpoint) + : mEndpoint{std::move(endpoint)} { + mEpollFd = android::base::unique_fd{epoll_create1(0)}; + LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(), + "Failed to create buffer release epoll file descriptor. errno=%d " + "message='%s'", + errno, strerror(errno)); + + epoll_event registerEndpointFd{}; + registerEndpointFd.events = EPOLLIN; + registerEndpointFd.data.fd = mEndpoint->getFd(); + status_t status = + epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), ®isterEndpointFd); + LOG_ALWAYS_FATAL_IF(status == -1, + "Failed to register buffer release consumer file descriptor with epoll. " + "errno=%d message='%s'", + errno, strerror(errno)); + + mEventFd = android::base::unique_fd(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); + LOG_ALWAYS_FATAL_IF(!mEventFd.ok(), + "Failed to create buffer release event file descriptor. errno=%d " + "message='%s'", + errno, strerror(errno)); + + epoll_event registerEventFd{}; + registerEventFd.events = EPOLLIN; + registerEventFd.data.fd = mEventFd.get(); + status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), ®isterEventFd); + LOG_ALWAYS_FATAL_IF(status == -1, + "Failed to register buffer release event file descriptor with epoll. " + "errno=%d message='%s'", + errno, strerror(errno)); +} + +BLASTBufferQueue::BufferReleaseReader& BLASTBufferQueue::BufferReleaseReader::operator=( + BufferReleaseReader&& other) { + if (this != &other) { + ftl::FakeGuard guard{mMutex}; + ftl::FakeGuard otherGuard{other.mMutex}; + mEndpoint = std::move(other.mEndpoint); + mEpollFd = std::move(other.mEpollFd); + mEventFd = std::move(other.mEventFd); + } + return *this; } +status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, + sp& outFence, + uint32_t& outMaxAcquiredBufferCount) { + epoll_event event{}; + while (true) { + int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, -1 /* timeout */); + if (eventCount == 1) { + break; + } + if (eventCount == -1 && errno != EINTR) { + ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, + strerror(errno)); + } + } + + if (event.data.fd == mEventFd.get()) { + uint64_t value; + if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { + ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, + strerror(errno)); + } + return WOULD_BLOCK; + } + + std::lock_guard lock{mMutex}; + return mEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount); +} + +void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { + uint64_t value = 1; + if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { + ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); + } +} + +void BLASTBufferQueue::BufferReleaseThread::start(const sp& bbq) { + mRunning = std::make_shared(true); + mReader = bbq->mBufferReleaseReader; + std::thread([running = mRunning, reader = mReader, weakBbq = wp(bbq)]() { + pthread_setname_np(pthread_self(), "BufferReleaseThread"); + while (*running) { + ReleaseCallbackId id; + sp fence; + uint32_t maxAcquiredBufferCount; + if (status_t status = reader->readBlocking(id, fence, maxAcquiredBufferCount); + status != OK) { + continue; + } + sp bbq = weakBbq.promote(); + if (!bbq) { + return; + } + bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount); + } + }).detach(); +} + +BLASTBufferQueue::BufferReleaseThread::~BufferReleaseThread() { + *mRunning = false; + mReader->interruptBlockingRead(); +} + +#endif + } // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 729d46a387..0a81126c7d 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -331,6 +331,51 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + class BufferReleaseReader { + public: + BufferReleaseReader() = default; + BufferReleaseReader(std::unique_ptr); + BufferReleaseReader& operator=(BufferReleaseReader&&); + + // Block until we can read a buffer release message. + // + // Returns: + // * OK if a ReleaseCallbackId and Fence were successfully read. + // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. + // * UNKNOWN_ERROR if something went wrong. + status_t readBlocking(ReleaseCallbackId& outId, sp& outReleaseFence, + uint32_t& outMaxAcquiredBufferCount); + + // Signals the reader's eventfd to wake up any threads waiting on readBlocking. + void interruptBlockingRead(); + + private: + std::mutex mMutex; + std::unique_ptr mEndpoint GUARDED_BY(mMutex); + android::base::unique_fd mEpollFd; + android::base::unique_fd mEventFd; + }; + + // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to + // the client. See BBQBufferQueueProducer::dequeueBuffer for details. + std::shared_ptr mBufferReleaseReader; + std::shared_ptr mBufferReleaseProducer; + + class BufferReleaseThread { + public: + BufferReleaseThread() = default; + ~BufferReleaseThread(); + void start(const sp&); + + private: + std::shared_ptr mRunning; + std::shared_ptr mReader; + }; + + BufferReleaseThread mBufferReleaseThread; +#endif }; } // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 636f7bdabf..5d6e5ba232 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2529,15 +2529,24 @@ void Layer::cloneDrawingState(const Layer* from) { void Layer::callReleaseBufferCallback(const sp& listener, const sp& buffer, uint64_t framenumber, const sp& releaseFence) { - if (!listener) { + if (!listener && !mBufferReleaseChannel) { return; } + SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); + + ReleaseCallbackId callbackId{buffer->getId(), framenumber}; + const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - listener->onReleaseBuffer({buffer->getId(), framenumber}, - releaseFence ? releaseFence : Fence::NO_FENCE, - currentMaxAcquiredBufferCount); + + if (listener) { + listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); + } + + if (mBufferReleaseChannel) { + mBufferReleaseChannel->writeReleaseFence(callbackId, fence, currentMaxAcquiredBufferCount); + } } sp Layer::findCallbackHandle() { @@ -2655,6 +2664,7 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { + handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -4093,6 +4103,11 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } +void Layer::setBufferReleaseChannel( + const std::shared_ptr& channel) { + mBufferReleaseChannel = channel; +} + void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index f6eed6332b..32dbe34408 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -543,6 +543,7 @@ public: }; BufferInfo mBufferInfo; + std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -798,6 +799,8 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); + void setBufferReleaseChannel( + const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 0c297d9b77..69f109cc89 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5115,6 +5115,10 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } + if (what & layer_state_t::eBufferReleaseChannelChanged) { + layer->setBufferReleaseChannel(s.bufferReleaseChannel); + } + const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 881bf35b58..c6856aea75 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,6 +149,13 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); + if (handle->bufferReleaseChannel && + handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { + mBufferReleases.emplace_back(handle->bufferReleaseChannel, + handle->previousReleaseCallbackId, + handle->previousReleaseFence, + handle->currentMaxAcquiredBufferCount); + } } return NO_ERROR; } @@ -158,6 +165,12 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { + for (const auto& bufferRelease : mBufferReleases) { + bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence, + bufferRelease.currentMaxAcquiredBufferCount); + } + mBufferReleases.clear(); + // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 7853a9f359..14a7487156 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -16,18 +16,14 @@ #pragma once -#include #include -#include #include -#include -#include #include -#include #include #include #include +#include #include #include #include @@ -59,6 +55,7 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; + std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -86,6 +83,14 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; + struct BufferRelease { + std::shared_ptr channel; + ReleaseCallbackId callbackId; + sp fence; + uint32_t currentMaxAcquiredBufferCount; + }; + std::vector mBufferReleases; + sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From 8af62f245bfc575a00f08db450067d172ef7c825 Mon Sep 17 00:00:00 2001 From: "Priyanka Advani (xWF)" Date: Wed, 28 Aug 2024 20:50:11 +0000 Subject: Revert "Read from BufferReleaseChannel in background thread" This reverts commit 5ab65e9ef047bc802885396f0e1d960901e86a5c. Reason for revert: Droidmonitor created revert due to b/362800004. Will be verifying through ABTD before submission. Change-Id: I440606b3da7f4d4c85f5900ac9b9e6fb18ccd8fa --- libs/gui/BLASTBufferQueue.cpp | 161 ++------------------- libs/gui/include/gui/BLASTBufferQueue.h | 45 ------ services/surfaceflinger/Layer.cpp | 23 +-- services/surfaceflinger/Layer.h | 3 - services/surfaceflinger/SurfaceFlinger.cpp | 4 - .../surfaceflinger/TransactionCallbackInvoker.cpp | 13 -- .../surfaceflinger/TransactionCallbackInvoker.h | 15 +- 7 files changed, 19 insertions(+), 245 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 8f6dd840aa..f8e3fd0a5c 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -22,14 +22,11 @@ #include #include -#include #include #include #include #include #include -#include -#include #include #include @@ -77,12 +74,6 @@ namespace android { std::unique_lock _lock{mutex}; \ base::ScopedLockAssertion assumeLocked(mutex); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) -static ReleaseBufferCallback EMPTY_RELEASE_CALLBACK = - [](const ReleaseCallbackId&, const sp& /*releaseFence*/, - std::optional /*currentMaxAcquiredBufferCount*/) {}; -#endif - void BLASTBufferItemConsumer::onDisconnect() { Mutex::Autolock lock(mMutex); mPreviouslyConnected = mCurrentlyConnected; @@ -224,12 +215,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - std::unique_ptr bufferReleaseConsumer; - gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); - mBufferReleaseReader = std::make_shared(std::move(bufferReleaseConsumer)); -#endif - BQA_LOGV("BLASTBufferQueue created"); } @@ -259,9 +244,6 @@ BLASTBufferQueue::~BLASTBufferQueue() { void BLASTBufferQueue::onFirstRef() { // safe default, most producers are expected to override this mProducer->setMaxDequeuedBufferCount(2); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - mBufferReleaseThread.start(sp::fromExisting(this)); -#endif } void BLASTBufferQueue::update(const sp& surface, uint32_t width, uint32_t height, @@ -287,9 +269,6 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); -#endif applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -407,7 +386,6 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp staleReleases; for (const auto& [key, value]: mSubmitted) { @@ -423,7 +401,6 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp 0) { - acquireNextBufferLocked(std::nullopt); - numFramesToFlush--; - } -} - // Unlike transactionCallbackThunk the release buffer callback does not extend the life of the // BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client. // So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer. @@ -460,6 +428,15 @@ ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() { }; } +void BLASTBufferQueue::flushShadowQueue() { + BQA_LOGV("flushShadowQueue"); + int numFramesToFlush = mNumFrameAvailable; + while (numFramesToFlush > 0) { + acquireNextBufferLocked(std::nullopt); + numFramesToFlush--; + } +} + void BLASTBufferQueue::releaseBufferCallback( const ReleaseCallbackId& id, const sp& releaseFence, std::optional currentMaxAcquiredBufferCount) { @@ -640,12 +617,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - ReleaseBufferCallback releaseBufferCallback = - applyTransaction ? EMPTY_RELEASE_CALLBACK : makeReleaseBufferCallbackThunk(); -#else auto releaseBufferCallback = makeReleaseBufferCallbackThunk(); -#endif sp fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE; nsecs_t dequeueTime = -1; @@ -1387,120 +1359,7 @@ bool BLASTBufferQueue::isSameSurfaceControl(const sp& surfaceCon void BLASTBufferQueue::setTransactionHangCallback( std::function callback) { std::lock_guard _lock{mMutex}; - mTransactionHangCallback = std::move(callback); -} - -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - -BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( - std::unique_ptr endpoint) - : mEndpoint{std::move(endpoint)} { - mEpollFd = android::base::unique_fd{epoll_create1(0)}; - LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(), - "Failed to create buffer release epoll file descriptor. errno=%d " - "message='%s'", - errno, strerror(errno)); - - epoll_event registerEndpointFd{}; - registerEndpointFd.events = EPOLLIN; - registerEndpointFd.data.fd = mEndpoint->getFd(); - status_t status = - epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), ®isterEndpointFd); - LOG_ALWAYS_FATAL_IF(status == -1, - "Failed to register buffer release consumer file descriptor with epoll. " - "errno=%d message='%s'", - errno, strerror(errno)); - - mEventFd = android::base::unique_fd(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); - LOG_ALWAYS_FATAL_IF(!mEventFd.ok(), - "Failed to create buffer release event file descriptor. errno=%d " - "message='%s'", - errno, strerror(errno)); - - epoll_event registerEventFd{}; - registerEventFd.events = EPOLLIN; - registerEventFd.data.fd = mEventFd.get(); - status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), ®isterEventFd); - LOG_ALWAYS_FATAL_IF(status == -1, - "Failed to register buffer release event file descriptor with epoll. " - "errno=%d message='%s'", - errno, strerror(errno)); -} - -BLASTBufferQueue::BufferReleaseReader& BLASTBufferQueue::BufferReleaseReader::operator=( - BufferReleaseReader&& other) { - if (this != &other) { - ftl::FakeGuard guard{mMutex}; - ftl::FakeGuard otherGuard{other.mMutex}; - mEndpoint = std::move(other.mEndpoint); - mEpollFd = std::move(other.mEpollFd); - mEventFd = std::move(other.mEventFd); - } - return *this; + mTransactionHangCallback = callback; } -status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, - sp& outFence, - uint32_t& outMaxAcquiredBufferCount) { - epoll_event event{}; - while (true) { - int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, -1 /* timeout */); - if (eventCount == 1) { - break; - } - if (eventCount == -1 && errno != EINTR) { - ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, - strerror(errno)); - } - } - - if (event.data.fd == mEventFd.get()) { - uint64_t value; - if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { - ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, - strerror(errno)); - } - return WOULD_BLOCK; - } - - std::lock_guard lock{mMutex}; - return mEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount); -} - -void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { - uint64_t value = 1; - if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { - ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); - } -} - -void BLASTBufferQueue::BufferReleaseThread::start(const sp& bbq) { - mRunning = std::make_shared(true); - mReader = bbq->mBufferReleaseReader; - std::thread([running = mRunning, reader = mReader, weakBbq = wp(bbq)]() { - pthread_setname_np(pthread_self(), "BufferReleaseThread"); - while (*running) { - ReleaseCallbackId id; - sp fence; - uint32_t maxAcquiredBufferCount; - if (status_t status = reader->readBlocking(id, fence, maxAcquiredBufferCount); - status != OK) { - continue; - } - sp bbq = weakBbq.promote(); - if (!bbq) { - return; - } - bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount); - } - }).detach(); -} - -BLASTBufferQueue::BufferReleaseThread::~BufferReleaseThread() { - *mRunning = false; - mReader->interruptBlockingRead(); -} - -#endif - } // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 0a81126c7d..729d46a387 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -331,51 +331,6 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); - -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - class BufferReleaseReader { - public: - BufferReleaseReader() = default; - BufferReleaseReader(std::unique_ptr); - BufferReleaseReader& operator=(BufferReleaseReader&&); - - // Block until we can read a buffer release message. - // - // Returns: - // * OK if a ReleaseCallbackId and Fence were successfully read. - // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. - // * UNKNOWN_ERROR if something went wrong. - status_t readBlocking(ReleaseCallbackId& outId, sp& outReleaseFence, - uint32_t& outMaxAcquiredBufferCount); - - // Signals the reader's eventfd to wake up any threads waiting on readBlocking. - void interruptBlockingRead(); - - private: - std::mutex mMutex; - std::unique_ptr mEndpoint GUARDED_BY(mMutex); - android::base::unique_fd mEpollFd; - android::base::unique_fd mEventFd; - }; - - // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to - // the client. See BBQBufferQueueProducer::dequeueBuffer for details. - std::shared_ptr mBufferReleaseReader; - std::shared_ptr mBufferReleaseProducer; - - class BufferReleaseThread { - public: - BufferReleaseThread() = default; - ~BufferReleaseThread(); - void start(const sp&); - - private: - std::shared_ptr mRunning; - std::shared_ptr mReader; - }; - - BufferReleaseThread mBufferReleaseThread; -#endif }; } // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 5d6e5ba232..636f7bdabf 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2529,24 +2529,15 @@ void Layer::cloneDrawingState(const Layer* from) { void Layer::callReleaseBufferCallback(const sp& listener, const sp& buffer, uint64_t framenumber, const sp& releaseFence) { - if (!listener && !mBufferReleaseChannel) { + if (!listener) { return; } - SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); - - ReleaseCallbackId callbackId{buffer->getId(), framenumber}; - const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - - if (listener) { - listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); - } - - if (mBufferReleaseChannel) { - mBufferReleaseChannel->writeReleaseFence(callbackId, fence, currentMaxAcquiredBufferCount); - } + listener->onReleaseBuffer({buffer->getId(), framenumber}, + releaseFence ? releaseFence : Fence::NO_FENCE, + currentMaxAcquiredBufferCount); } sp Layer::findCallbackHandle() { @@ -2664,7 +2655,6 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { - handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -4103,11 +4093,6 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } -void Layer::setBufferReleaseChannel( - const std::shared_ptr& channel) { - mBufferReleaseChannel = channel; -} - void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 32dbe34408..f6eed6332b 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -543,7 +543,6 @@ public: }; BufferInfo mBufferInfo; - std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -799,8 +798,6 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); - void setBufferReleaseChannel( - const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 69f109cc89..0c297d9b77 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5115,10 +5115,6 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } - if (what & layer_state_t::eBufferReleaseChannelChanged) { - layer->setBufferReleaseChannel(s.bufferReleaseChannel); - } - const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index c6856aea75..881bf35b58 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,13 +149,6 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); - if (handle->bufferReleaseChannel && - handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { - mBufferReleases.emplace_back(handle->bufferReleaseChannel, - handle->previousReleaseCallbackId, - handle->previousReleaseFence, - handle->currentMaxAcquiredBufferCount); - } } return NO_ERROR; } @@ -165,12 +158,6 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { - for (const auto& bufferRelease : mBufferReleases) { - bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence, - bufferRelease.currentMaxAcquiredBufferCount); - } - mBufferReleases.clear(); - // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 14a7487156..7853a9f359 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -16,14 +16,18 @@ #pragma once +#include #include +#include #include +#include +#include #include +#include #include #include #include -#include #include #include #include @@ -55,7 +59,6 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; - std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -83,14 +86,6 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; - struct BufferRelease { - std::shared_ptr channel; - ReleaseCallbackId callbackId; - sp fence; - uint32_t currentMaxAcquiredBufferCount; - }; - std::vector mBufferReleases; - sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From 7a34bdcde80fbeb55b507cc7975b1f2a5125e562 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 29 Aug 2024 15:32:27 +0000 Subject: Revert^2 "Read from BufferReleaseChannel in background thread" This reverts commit 8af62f245bfc575a00f08db450067d172ef7c825. Reason for revert: The feature flag was enabled when this change was merged. I've disabled the feature flag so this change shouldn't cause issues. I'll add the missing SELinux permissions in AOSP. Change-Id: I9b07a854b0e0514d8036066f7b05a37a9701991b --- libs/gui/BLASTBufferQueue.cpp | 161 +++++++++++++++++++-- libs/gui/include/gui/BLASTBufferQueue.h | 45 ++++++ services/surfaceflinger/Layer.cpp | 23 ++- services/surfaceflinger/Layer.h | 3 + services/surfaceflinger/SurfaceFlinger.cpp | 4 + .../surfaceflinger/TransactionCallbackInvoker.cpp | 13 ++ .../surfaceflinger/TransactionCallbackInvoker.h | 15 +- 7 files changed, 245 insertions(+), 19 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index f8e3fd0a5c..8f6dd840aa 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -22,11 +22,14 @@ #include #include +#include #include #include #include #include #include +#include +#include #include #include @@ -74,6 +77,12 @@ namespace android { std::unique_lock _lock{mutex}; \ base::ScopedLockAssertion assumeLocked(mutex); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) +static ReleaseBufferCallback EMPTY_RELEASE_CALLBACK = + [](const ReleaseCallbackId&, const sp& /*releaseFence*/, + std::optional /*currentMaxAcquiredBufferCount*/) {}; +#endif + void BLASTBufferItemConsumer::onDisconnect() { Mutex::Autolock lock(mMutex); mPreviouslyConnected = mCurrentlyConnected; @@ -215,6 +224,12 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + std::unique_ptr bufferReleaseConsumer; + gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); + mBufferReleaseReader = std::make_shared(std::move(bufferReleaseConsumer)); +#endif + BQA_LOGV("BLASTBufferQueue created"); } @@ -244,6 +259,9 @@ BLASTBufferQueue::~BLASTBufferQueue() { void BLASTBufferQueue::onFirstRef() { // safe default, most producers are expected to override this mProducer->setMaxDequeuedBufferCount(2); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + mBufferReleaseThread.start(sp::fromExisting(this)); +#endif } void BLASTBufferQueue::update(const sp& surface, uint32_t width, uint32_t height, @@ -269,6 +287,9 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); +#endif applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -386,6 +407,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp staleReleases; for (const auto& [key, value]: mSubmitted) { @@ -401,6 +423,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp 0) { + acquireNextBufferLocked(std::nullopt); + numFramesToFlush--; + } +} + // Unlike transactionCallbackThunk the release buffer callback does not extend the life of the // BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client. // So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer. @@ -428,15 +460,6 @@ ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() { }; } -void BLASTBufferQueue::flushShadowQueue() { - BQA_LOGV("flushShadowQueue"); - int numFramesToFlush = mNumFrameAvailable; - while (numFramesToFlush > 0) { - acquireNextBufferLocked(std::nullopt); - numFramesToFlush--; - } -} - void BLASTBufferQueue::releaseBufferCallback( const ReleaseCallbackId& id, const sp& releaseFence, std::optional currentMaxAcquiredBufferCount) { @@ -617,7 +640,12 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + ReleaseBufferCallback releaseBufferCallback = + applyTransaction ? EMPTY_RELEASE_CALLBACK : makeReleaseBufferCallbackThunk(); +#else auto releaseBufferCallback = makeReleaseBufferCallbackThunk(); +#endif sp fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE; nsecs_t dequeueTime = -1; @@ -1359,7 +1387,120 @@ bool BLASTBufferQueue::isSameSurfaceControl(const sp& surfaceCon void BLASTBufferQueue::setTransactionHangCallback( std::function callback) { std::lock_guard _lock{mMutex}; - mTransactionHangCallback = callback; + mTransactionHangCallback = std::move(callback); +} + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + +BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( + std::unique_ptr endpoint) + : mEndpoint{std::move(endpoint)} { + mEpollFd = android::base::unique_fd{epoll_create1(0)}; + LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(), + "Failed to create buffer release epoll file descriptor. errno=%d " + "message='%s'", + errno, strerror(errno)); + + epoll_event registerEndpointFd{}; + registerEndpointFd.events = EPOLLIN; + registerEndpointFd.data.fd = mEndpoint->getFd(); + status_t status = + epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), ®isterEndpointFd); + LOG_ALWAYS_FATAL_IF(status == -1, + "Failed to register buffer release consumer file descriptor with epoll. " + "errno=%d message='%s'", + errno, strerror(errno)); + + mEventFd = android::base::unique_fd(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); + LOG_ALWAYS_FATAL_IF(!mEventFd.ok(), + "Failed to create buffer release event file descriptor. errno=%d " + "message='%s'", + errno, strerror(errno)); + + epoll_event registerEventFd{}; + registerEventFd.events = EPOLLIN; + registerEventFd.data.fd = mEventFd.get(); + status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), ®isterEventFd); + LOG_ALWAYS_FATAL_IF(status == -1, + "Failed to register buffer release event file descriptor with epoll. " + "errno=%d message='%s'", + errno, strerror(errno)); +} + +BLASTBufferQueue::BufferReleaseReader& BLASTBufferQueue::BufferReleaseReader::operator=( + BufferReleaseReader&& other) { + if (this != &other) { + ftl::FakeGuard guard{mMutex}; + ftl::FakeGuard otherGuard{other.mMutex}; + mEndpoint = std::move(other.mEndpoint); + mEpollFd = std::move(other.mEpollFd); + mEventFd = std::move(other.mEventFd); + } + return *this; } +status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, + sp& outFence, + uint32_t& outMaxAcquiredBufferCount) { + epoll_event event{}; + while (true) { + int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, -1 /* timeout */); + if (eventCount == 1) { + break; + } + if (eventCount == -1 && errno != EINTR) { + ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, + strerror(errno)); + } + } + + if (event.data.fd == mEventFd.get()) { + uint64_t value; + if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { + ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, + strerror(errno)); + } + return WOULD_BLOCK; + } + + std::lock_guard lock{mMutex}; + return mEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount); +} + +void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { + uint64_t value = 1; + if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { + ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); + } +} + +void BLASTBufferQueue::BufferReleaseThread::start(const sp& bbq) { + mRunning = std::make_shared(true); + mReader = bbq->mBufferReleaseReader; + std::thread([running = mRunning, reader = mReader, weakBbq = wp(bbq)]() { + pthread_setname_np(pthread_self(), "BufferReleaseThread"); + while (*running) { + ReleaseCallbackId id; + sp fence; + uint32_t maxAcquiredBufferCount; + if (status_t status = reader->readBlocking(id, fence, maxAcquiredBufferCount); + status != OK) { + continue; + } + sp bbq = weakBbq.promote(); + if (!bbq) { + return; + } + bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount); + } + }).detach(); +} + +BLASTBufferQueue::BufferReleaseThread::~BufferReleaseThread() { + *mRunning = false; + mReader->interruptBlockingRead(); +} + +#endif + } // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 729d46a387..0a81126c7d 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -331,6 +331,51 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + class BufferReleaseReader { + public: + BufferReleaseReader() = default; + BufferReleaseReader(std::unique_ptr); + BufferReleaseReader& operator=(BufferReleaseReader&&); + + // Block until we can read a buffer release message. + // + // Returns: + // * OK if a ReleaseCallbackId and Fence were successfully read. + // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. + // * UNKNOWN_ERROR if something went wrong. + status_t readBlocking(ReleaseCallbackId& outId, sp& outReleaseFence, + uint32_t& outMaxAcquiredBufferCount); + + // Signals the reader's eventfd to wake up any threads waiting on readBlocking. + void interruptBlockingRead(); + + private: + std::mutex mMutex; + std::unique_ptr mEndpoint GUARDED_BY(mMutex); + android::base::unique_fd mEpollFd; + android::base::unique_fd mEventFd; + }; + + // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to + // the client. See BBQBufferQueueProducer::dequeueBuffer for details. + std::shared_ptr mBufferReleaseReader; + std::shared_ptr mBufferReleaseProducer; + + class BufferReleaseThread { + public: + BufferReleaseThread() = default; + ~BufferReleaseThread(); + void start(const sp&); + + private: + std::shared_ptr mRunning; + std::shared_ptr mReader; + }; + + BufferReleaseThread mBufferReleaseThread; +#endif }; } // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 7b12a807c9..3c8af19015 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -1951,15 +1951,24 @@ bool Layer::setDropInputMode(gui::DropInputMode mode) { void Layer::callReleaseBufferCallback(const sp& listener, const sp& buffer, uint64_t framenumber, const sp& releaseFence) { - if (!listener) { + if (!listener && !mBufferReleaseChannel) { return; } + SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); + + ReleaseCallbackId callbackId{buffer->getId(), framenumber}; + const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - listener->onReleaseBuffer({buffer->getId(), framenumber}, - releaseFence ? releaseFence : Fence::NO_FENCE, - currentMaxAcquiredBufferCount); + + if (listener) { + listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); + } + + if (mBufferReleaseChannel) { + mBufferReleaseChannel->writeReleaseFence(callbackId, fence, currentMaxAcquiredBufferCount); + } } sp Layer::findCallbackHandle() { @@ -2077,6 +2086,7 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { + handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -3508,6 +3518,11 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } +void Layer::setBufferReleaseChannel( + const std::shared_ptr& channel) { + mBufferReleaseChannel = channel; +} + void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 3692fb88f1..1e4f2dc296 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -537,6 +537,7 @@ public: }; BufferInfo mBufferInfo; + std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -724,6 +725,8 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); + void setBufferReleaseChannel( + const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index ce83475c4c..f4bff8f972 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5063,6 +5063,10 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } + if (what & layer_state_t::eBufferReleaseChannelChanged) { + layer->setBufferReleaseChannel(s.bufferReleaseChannel); + } + const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 881bf35b58..c6856aea75 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,6 +149,13 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); + if (handle->bufferReleaseChannel && + handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { + mBufferReleases.emplace_back(handle->bufferReleaseChannel, + handle->previousReleaseCallbackId, + handle->previousReleaseFence, + handle->currentMaxAcquiredBufferCount); + } } return NO_ERROR; } @@ -158,6 +165,12 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { + for (const auto& bufferRelease : mBufferReleases) { + bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence, + bufferRelease.currentMaxAcquiredBufferCount); + } + mBufferReleases.clear(); + // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 7853a9f359..14a7487156 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -16,18 +16,14 @@ #pragma once -#include #include -#include #include -#include -#include #include -#include #include #include #include +#include #include #include #include @@ -59,6 +55,7 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; + std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -86,6 +83,14 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; + struct BufferRelease { + std::shared_ptr channel; + ReleaseCallbackId callbackId; + sp fence; + uint32_t currentMaxAcquiredBufferCount; + }; + std::vector mBufferReleases; + sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From 3ced538a4a45d60cefb5b3ed582713f8e861e9b1 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Wed, 21 Aug 2024 15:39:32 -0500 Subject: Remove unnecessary buffer count changes Reverting buffer count changes in favor of abstracting methods out in BufferQueue to determine when to block. Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: presubmits Change-Id: I9922c2c5668054330df7137e7b2eafeb8d8fe7c9 --- libs/gui/BLASTBufferQueue.cpp | 141 +------------------------------- libs/gui/include/gui/BLASTBufferQueue.h | 18 +--- 2 files changed, 4 insertions(+), 155 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 8f6dd840aa..3c1971fd81 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -527,13 +527,7 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, callbackId.to_string().c_str()); return; } -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - if (!it->second.disconnectedAfterAcquired) { - mNumAcquired--; - } -#else mNumAcquired--; -#endif BBQ_TRACE("frame=%" PRIu64, callbackId.framenumber); BQA_LOGV("released %s", callbackId.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseFence); @@ -584,7 +578,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( applyTransaction = false; } - BLASTBufferItem bufferItem; + BufferItem bufferItem; status_t status = mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false); @@ -795,9 +789,6 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { } // add to shadow queue -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - mNumDequeued--; -#endif mNumFrameAvailable++; if (waitForTransactionCallback && mNumFrameAvailable >= 2) { acquireAndReleaseBuffer(); @@ -852,17 +843,8 @@ void BLASTBufferQueue::onFrameDequeued(const uint64_t bufferId) { }; void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { - { - std::lock_guard _lock{mTimestampMutex}; - mDequeueTimestamps.erase(bufferId); - } - -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - { - std::lock_guard lock{mMutex}; - mNumDequeued--; - } -#endif + std::lock_guard _lock{mTimestampMutex}; + mDequeueTimestamps.erase(bufferId); } bool BLASTBufferQueue::syncNextTransaction( @@ -1162,116 +1144,6 @@ public: producerControlledByApp, output); } -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - status_t disconnect(int api, DisconnectMode mode) override { - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return BufferQueueProducer::disconnect(api, mode); - } - - std::lock_guard lock{bbq->mMutex}; - if (status_t status = BufferQueueProducer::disconnect(api, mode); status != OK) { - return status; - } - - // We need to reset dequeued and acquired counts because BufferQueueProducer::disconnect - // calls BufferQueueCore::freeAllBuffersLocked which frees all dequeued and acquired - // buffers. We don't reset mNumFrameAvailable because these buffers are still available - // in BufferItemConsumer. - bbq->mNumDequeued = 0; - bbq->mNumAcquired = 0; - // SurfaceFlinger sends release callbacks for buffers that have been acquired after a - // disconnect. We set disconnectedAfterAcquired to true so that we can ignore any stale - // releases that come in after the producer is disconnected. Otherwise, releaseBuffer will - // decrement mNumAcquired for a buffer that was acquired before we reset mNumAcquired to - // zero. - for (auto& [releaseId, bufferItem] : bbq->mSubmitted) { - bufferItem.disconnectedAfterAcquired = true; - } - - return OK; - } - - status_t setAsyncMode(bool asyncMode) override { - if (status_t status = BufferQueueProducer::setAsyncMode(asyncMode); status != OK) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return OK; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mAsyncMode = asyncMode; - } - - return OK; - } - - status_t setSharedBufferMode(bool sharedBufferMode) override { - if (status_t status = BufferQueueProducer::setSharedBufferMode(sharedBufferMode); - status != OK) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return OK; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mSharedBufferMode = sharedBufferMode; - } - - return OK; - } - - status_t detachBuffer(int slot) override { - if (status_t status = BufferQueueProducer::detachBuffer(slot); status != OK) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return OK; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mNumDequeued--; - } - - return OK; - } - - status_t dequeueBuffer(int* outSlot, sp* outFence, uint32_t width, uint32_t height, - PixelFormat format, uint64_t usage, uint64_t* outBufferAge, - FrameEventHistoryDelta* outTimestamps) override { - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, - usage, outBufferAge, outTimestamps); - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mNumDequeued++; - } - - status_t status = - BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, usage, - outBufferAge, outTimestamps); - if (status < 0) { - std::lock_guard lock{bbq->mMutex}; - bbq->mNumDequeued--; - } - return status; - } -#endif - // We want to resize the frame history when changing the size of the buffer queue status_t setMaxDequeuedBufferCount(int maxDequeuedBufferCount) override { int maxBufferCount; @@ -1294,13 +1166,6 @@ public: bbq->resizeFrameEventHistory(newFrameHistorySize); } -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - { - std::lock_guard lock{bbq->mMutex}; - bbq->mMaxDequeuedBuffers = maxDequeuedBufferCount; - } -#endif - return OK; } diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 0a81126c7d..d787d6cc45 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -187,15 +187,6 @@ private: // BufferQueue internally allows 1 more than // the max to be acquired int32_t mMaxAcquiredBuffers GUARDED_BY(mMutex) = 1; -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - int32_t mMaxDequeuedBuffers GUARDED_BY(mMutex) = 1; - static constexpr int32_t kMaxBufferCount = BufferQueueDefs::NUM_BUFFER_SLOTS; - - bool mAsyncMode GUARDED_BY(mMutex) = false; - bool mSharedBufferMode GUARDED_BY(mMutex) = false; - - int32_t mNumDequeued GUARDED_BY(mMutex) = 0; -#endif int32_t mNumFrameAvailable GUARDED_BY(mMutex) = 0; int32_t mNumAcquired GUARDED_BY(mMutex) = 0; @@ -204,16 +195,9 @@ private: // latch stale buffers and that we don't wait on barriers from an old producer. uint32_t mProducerId = 0; - class BLASTBufferItem : public BufferItem { - public: - // True if BBQBufferQueueProducer is disconnected after the buffer is acquried but - // before it is released. - bool disconnectedAfterAcquired{false}; - }; - // Keep a reference to the submitted buffers so we can release when surfaceflinger drops the // buffer or the buffer has been presented and a new buffer is ready to be presented. - std::unordered_map mSubmitted + std::unordered_map mSubmitted GUARDED_BY(mMutex); // Keep a queue of the released buffers instead of immediately releasing -- cgit v1.2.3-59-g8ed1b From 4aface1c7583f8593ad40534f65218c590bd419e Mon Sep 17 00:00:00 2001 From: Satish Yalla Date: Fri, 30 Aug 2024 09:25:35 +0000 Subject: Revert^3 "Read from BufferReleaseChannel in background thread" This reverts commit 7a34bdcde80fbeb55b507cc7975b1f2a5125e562. Reason for revert: Droidmonitor created revert due to b/363109809. Will be verifying through ABTD before submission. Change-Id: I933a9b7602732bcd8c152978bbb2c7ee88a1533b --- libs/gui/BLASTBufferQueue.cpp | 161 ++------------------- libs/gui/include/gui/BLASTBufferQueue.h | 45 ------ services/surfaceflinger/Layer.cpp | 23 +-- services/surfaceflinger/Layer.h | 3 - services/surfaceflinger/SurfaceFlinger.cpp | 4 - .../surfaceflinger/TransactionCallbackInvoker.cpp | 13 -- .../surfaceflinger/TransactionCallbackInvoker.h | 15 +- 7 files changed, 19 insertions(+), 245 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 8f6dd840aa..f8e3fd0a5c 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -22,14 +22,11 @@ #include #include -#include #include #include #include #include #include -#include -#include #include #include @@ -77,12 +74,6 @@ namespace android { std::unique_lock _lock{mutex}; \ base::ScopedLockAssertion assumeLocked(mutex); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) -static ReleaseBufferCallback EMPTY_RELEASE_CALLBACK = - [](const ReleaseCallbackId&, const sp& /*releaseFence*/, - std::optional /*currentMaxAcquiredBufferCount*/) {}; -#endif - void BLASTBufferItemConsumer::onDisconnect() { Mutex::Autolock lock(mMutex); mPreviouslyConnected = mCurrentlyConnected; @@ -224,12 +215,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - std::unique_ptr bufferReleaseConsumer; - gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); - mBufferReleaseReader = std::make_shared(std::move(bufferReleaseConsumer)); -#endif - BQA_LOGV("BLASTBufferQueue created"); } @@ -259,9 +244,6 @@ BLASTBufferQueue::~BLASTBufferQueue() { void BLASTBufferQueue::onFirstRef() { // safe default, most producers are expected to override this mProducer->setMaxDequeuedBufferCount(2); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - mBufferReleaseThread.start(sp::fromExisting(this)); -#endif } void BLASTBufferQueue::update(const sp& surface, uint32_t width, uint32_t height, @@ -287,9 +269,6 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); -#endif applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -407,7 +386,6 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp staleReleases; for (const auto& [key, value]: mSubmitted) { @@ -423,7 +401,6 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp 0) { - acquireNextBufferLocked(std::nullopt); - numFramesToFlush--; - } -} - // Unlike transactionCallbackThunk the release buffer callback does not extend the life of the // BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client. // So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer. @@ -460,6 +428,15 @@ ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() { }; } +void BLASTBufferQueue::flushShadowQueue() { + BQA_LOGV("flushShadowQueue"); + int numFramesToFlush = mNumFrameAvailable; + while (numFramesToFlush > 0) { + acquireNextBufferLocked(std::nullopt); + numFramesToFlush--; + } +} + void BLASTBufferQueue::releaseBufferCallback( const ReleaseCallbackId& id, const sp& releaseFence, std::optional currentMaxAcquiredBufferCount) { @@ -640,12 +617,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - ReleaseBufferCallback releaseBufferCallback = - applyTransaction ? EMPTY_RELEASE_CALLBACK : makeReleaseBufferCallbackThunk(); -#else auto releaseBufferCallback = makeReleaseBufferCallbackThunk(); -#endif sp fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE; nsecs_t dequeueTime = -1; @@ -1387,120 +1359,7 @@ bool BLASTBufferQueue::isSameSurfaceControl(const sp& surfaceCon void BLASTBufferQueue::setTransactionHangCallback( std::function callback) { std::lock_guard _lock{mMutex}; - mTransactionHangCallback = std::move(callback); -} - -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - -BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( - std::unique_ptr endpoint) - : mEndpoint{std::move(endpoint)} { - mEpollFd = android::base::unique_fd{epoll_create1(0)}; - LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(), - "Failed to create buffer release epoll file descriptor. errno=%d " - "message='%s'", - errno, strerror(errno)); - - epoll_event registerEndpointFd{}; - registerEndpointFd.events = EPOLLIN; - registerEndpointFd.data.fd = mEndpoint->getFd(); - status_t status = - epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), ®isterEndpointFd); - LOG_ALWAYS_FATAL_IF(status == -1, - "Failed to register buffer release consumer file descriptor with epoll. " - "errno=%d message='%s'", - errno, strerror(errno)); - - mEventFd = android::base::unique_fd(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); - LOG_ALWAYS_FATAL_IF(!mEventFd.ok(), - "Failed to create buffer release event file descriptor. errno=%d " - "message='%s'", - errno, strerror(errno)); - - epoll_event registerEventFd{}; - registerEventFd.events = EPOLLIN; - registerEventFd.data.fd = mEventFd.get(); - status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), ®isterEventFd); - LOG_ALWAYS_FATAL_IF(status == -1, - "Failed to register buffer release event file descriptor with epoll. " - "errno=%d message='%s'", - errno, strerror(errno)); -} - -BLASTBufferQueue::BufferReleaseReader& BLASTBufferQueue::BufferReleaseReader::operator=( - BufferReleaseReader&& other) { - if (this != &other) { - ftl::FakeGuard guard{mMutex}; - ftl::FakeGuard otherGuard{other.mMutex}; - mEndpoint = std::move(other.mEndpoint); - mEpollFd = std::move(other.mEpollFd); - mEventFd = std::move(other.mEventFd); - } - return *this; + mTransactionHangCallback = callback; } -status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, - sp& outFence, - uint32_t& outMaxAcquiredBufferCount) { - epoll_event event{}; - while (true) { - int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, -1 /* timeout */); - if (eventCount == 1) { - break; - } - if (eventCount == -1 && errno != EINTR) { - ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, - strerror(errno)); - } - } - - if (event.data.fd == mEventFd.get()) { - uint64_t value; - if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { - ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, - strerror(errno)); - } - return WOULD_BLOCK; - } - - std::lock_guard lock{mMutex}; - return mEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount); -} - -void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { - uint64_t value = 1; - if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { - ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); - } -} - -void BLASTBufferQueue::BufferReleaseThread::start(const sp& bbq) { - mRunning = std::make_shared(true); - mReader = bbq->mBufferReleaseReader; - std::thread([running = mRunning, reader = mReader, weakBbq = wp(bbq)]() { - pthread_setname_np(pthread_self(), "BufferReleaseThread"); - while (*running) { - ReleaseCallbackId id; - sp fence; - uint32_t maxAcquiredBufferCount; - if (status_t status = reader->readBlocking(id, fence, maxAcquiredBufferCount); - status != OK) { - continue; - } - sp bbq = weakBbq.promote(); - if (!bbq) { - return; - } - bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount); - } - }).detach(); -} - -BLASTBufferQueue::BufferReleaseThread::~BufferReleaseThread() { - *mRunning = false; - mReader->interruptBlockingRead(); -} - -#endif - } // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 0a81126c7d..729d46a387 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -331,51 +331,6 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); - -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - class BufferReleaseReader { - public: - BufferReleaseReader() = default; - BufferReleaseReader(std::unique_ptr); - BufferReleaseReader& operator=(BufferReleaseReader&&); - - // Block until we can read a buffer release message. - // - // Returns: - // * OK if a ReleaseCallbackId and Fence were successfully read. - // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. - // * UNKNOWN_ERROR if something went wrong. - status_t readBlocking(ReleaseCallbackId& outId, sp& outReleaseFence, - uint32_t& outMaxAcquiredBufferCount); - - // Signals the reader's eventfd to wake up any threads waiting on readBlocking. - void interruptBlockingRead(); - - private: - std::mutex mMutex; - std::unique_ptr mEndpoint GUARDED_BY(mMutex); - android::base::unique_fd mEpollFd; - android::base::unique_fd mEventFd; - }; - - // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to - // the client. See BBQBufferQueueProducer::dequeueBuffer for details. - std::shared_ptr mBufferReleaseReader; - std::shared_ptr mBufferReleaseProducer; - - class BufferReleaseThread { - public: - BufferReleaseThread() = default; - ~BufferReleaseThread(); - void start(const sp&); - - private: - std::shared_ptr mRunning; - std::shared_ptr mReader; - }; - - BufferReleaseThread mBufferReleaseThread; -#endif }; } // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 3c8af19015..7b12a807c9 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -1951,24 +1951,15 @@ bool Layer::setDropInputMode(gui::DropInputMode mode) { void Layer::callReleaseBufferCallback(const sp& listener, const sp& buffer, uint64_t framenumber, const sp& releaseFence) { - if (!listener && !mBufferReleaseChannel) { + if (!listener) { return; } - SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); - - ReleaseCallbackId callbackId{buffer->getId(), framenumber}; - const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - - if (listener) { - listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); - } - - if (mBufferReleaseChannel) { - mBufferReleaseChannel->writeReleaseFence(callbackId, fence, currentMaxAcquiredBufferCount); - } + listener->onReleaseBuffer({buffer->getId(), framenumber}, + releaseFence ? releaseFence : Fence::NO_FENCE, + currentMaxAcquiredBufferCount); } sp Layer::findCallbackHandle() { @@ -2086,7 +2077,6 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { - handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -3518,11 +3508,6 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } -void Layer::setBufferReleaseChannel( - const std::shared_ptr& channel) { - mBufferReleaseChannel = channel; -} - void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 1e4f2dc296..3692fb88f1 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -537,7 +537,6 @@ public: }; BufferInfo mBufferInfo; - std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -725,8 +724,6 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); - void setBufferReleaseChannel( - const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f4bff8f972..ce83475c4c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5063,10 +5063,6 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } - if (what & layer_state_t::eBufferReleaseChannelChanged) { - layer->setBufferReleaseChannel(s.bufferReleaseChannel); - } - const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index c6856aea75..881bf35b58 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,13 +149,6 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); - if (handle->bufferReleaseChannel && - handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { - mBufferReleases.emplace_back(handle->bufferReleaseChannel, - handle->previousReleaseCallbackId, - handle->previousReleaseFence, - handle->currentMaxAcquiredBufferCount); - } } return NO_ERROR; } @@ -165,12 +158,6 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { - for (const auto& bufferRelease : mBufferReleases) { - bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence, - bufferRelease.currentMaxAcquiredBufferCount); - } - mBufferReleases.clear(); - // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 14a7487156..7853a9f359 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -16,14 +16,18 @@ #pragma once +#include #include +#include #include +#include +#include #include +#include #include #include #include -#include #include #include #include @@ -55,7 +59,6 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; - std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -83,14 +86,6 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; - struct BufferRelease { - std::shared_ptr channel; - ReleaseCallbackId callbackId; - sp fence; - uint32_t currentMaxAcquiredBufferCount; - }; - std::vector mBufferReleases; - sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From 7c9fa270062a2ea0f9aacdf17835231e8878bc41 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Fri, 30 Aug 2024 12:38:43 +0000 Subject: Read from BufferReleaseChannel in background thread Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: BLASTBufferQueueTest Change-Id: I99c5da3bd99d6893ae24f5d55e7553b02bfc07ab --- libs/gui/BLASTBufferQueue.cpp | 161 +++++++++++++++++++-- libs/gui/include/gui/BLASTBufferQueue.h | 45 ++++++ services/surfaceflinger/Layer.cpp | 23 ++- services/surfaceflinger/Layer.h | 3 + services/surfaceflinger/SurfaceFlinger.cpp | 4 + .../surfaceflinger/TransactionCallbackInvoker.cpp | 13 ++ .../surfaceflinger/TransactionCallbackInvoker.h | 15 +- 7 files changed, 245 insertions(+), 19 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index b8d2b5ffbf..3c1971fd81 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -22,11 +22,14 @@ #include #include +#include #include #include #include #include #include +#include +#include #include #include @@ -74,6 +77,12 @@ namespace android { std::unique_lock _lock{mutex}; \ base::ScopedLockAssertion assumeLocked(mutex); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) +static ReleaseBufferCallback EMPTY_RELEASE_CALLBACK = + [](const ReleaseCallbackId&, const sp& /*releaseFence*/, + std::optional /*currentMaxAcquiredBufferCount*/) {}; +#endif + void BLASTBufferItemConsumer::onDisconnect() { Mutex::Autolock lock(mMutex); mPreviouslyConnected = mCurrentlyConnected; @@ -215,6 +224,12 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + std::unique_ptr bufferReleaseConsumer; + gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); + mBufferReleaseReader = std::make_shared(std::move(bufferReleaseConsumer)); +#endif + BQA_LOGV("BLASTBufferQueue created"); } @@ -244,6 +259,9 @@ BLASTBufferQueue::~BLASTBufferQueue() { void BLASTBufferQueue::onFirstRef() { // safe default, most producers are expected to override this mProducer->setMaxDequeuedBufferCount(2); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + mBufferReleaseThread.start(sp::fromExisting(this)); +#endif } void BLASTBufferQueue::update(const sp& surface, uint32_t width, uint32_t height, @@ -269,6 +287,9 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); +#endif applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -386,6 +407,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp staleReleases; for (const auto& [key, value]: mSubmitted) { @@ -401,6 +423,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp 0) { + acquireNextBufferLocked(std::nullopt); + numFramesToFlush--; + } +} + // Unlike transactionCallbackThunk the release buffer callback does not extend the life of the // BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client. // So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer. @@ -428,15 +460,6 @@ ReleaseBufferCallback BLASTBufferQueue::makeReleaseBufferCallbackThunk() { }; } -void BLASTBufferQueue::flushShadowQueue() { - BQA_LOGV("flushShadowQueue"); - int numFramesToFlush = mNumFrameAvailable; - while (numFramesToFlush > 0) { - acquireNextBufferLocked(std::nullopt); - numFramesToFlush--; - } -} - void BLASTBufferQueue::releaseBufferCallback( const ReleaseCallbackId& id, const sp& releaseFence, std::optional currentMaxAcquiredBufferCount) { @@ -611,7 +634,12 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + ReleaseBufferCallback releaseBufferCallback = + applyTransaction ? EMPTY_RELEASE_CALLBACK : makeReleaseBufferCallbackThunk(); +#else auto releaseBufferCallback = makeReleaseBufferCallbackThunk(); +#endif sp fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE; nsecs_t dequeueTime = -1; @@ -1224,7 +1252,120 @@ bool BLASTBufferQueue::isSameSurfaceControl(const sp& surfaceCon void BLASTBufferQueue::setTransactionHangCallback( std::function callback) { std::lock_guard _lock{mMutex}; - mTransactionHangCallback = callback; + mTransactionHangCallback = std::move(callback); } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + +BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( + std::unique_ptr endpoint) + : mEndpoint{std::move(endpoint)} { + mEpollFd = android::base::unique_fd{epoll_create1(0)}; + LOG_ALWAYS_FATAL_IF(!mEpollFd.ok(), + "Failed to create buffer release epoll file descriptor. errno=%d " + "message='%s'", + errno, strerror(errno)); + + epoll_event registerEndpointFd{}; + registerEndpointFd.events = EPOLLIN; + registerEndpointFd.data.fd = mEndpoint->getFd(); + status_t status = + epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), ®isterEndpointFd); + LOG_ALWAYS_FATAL_IF(status == -1, + "Failed to register buffer release consumer file descriptor with epoll. " + "errno=%d message='%s'", + errno, strerror(errno)); + + mEventFd = android::base::unique_fd(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); + LOG_ALWAYS_FATAL_IF(!mEventFd.ok(), + "Failed to create buffer release event file descriptor. errno=%d " + "message='%s'", + errno, strerror(errno)); + + epoll_event registerEventFd{}; + registerEventFd.events = EPOLLIN; + registerEventFd.data.fd = mEventFd.get(); + status = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), ®isterEventFd); + LOG_ALWAYS_FATAL_IF(status == -1, + "Failed to register buffer release event file descriptor with epoll. " + "errno=%d message='%s'", + errno, strerror(errno)); +} + +BLASTBufferQueue::BufferReleaseReader& BLASTBufferQueue::BufferReleaseReader::operator=( + BufferReleaseReader&& other) { + if (this != &other) { + ftl::FakeGuard guard{mMutex}; + ftl::FakeGuard otherGuard{other.mMutex}; + mEndpoint = std::move(other.mEndpoint); + mEpollFd = std::move(other.mEpollFd); + mEventFd = std::move(other.mEventFd); + } + return *this; +} + +status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, + sp& outFence, + uint32_t& outMaxAcquiredBufferCount) { + epoll_event event{}; + while (true) { + int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, -1 /* timeout */); + if (eventCount == 1) { + break; + } + if (eventCount == -1 && errno != EINTR) { + ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, + strerror(errno)); + } + } + + if (event.data.fd == mEventFd.get()) { + uint64_t value; + if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { + ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, + strerror(errno)); + } + return WOULD_BLOCK; + } + + std::lock_guard lock{mMutex}; + return mEndpoint->readReleaseFence(outId, outFence, outMaxAcquiredBufferCount); +} + +void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { + uint64_t value = 1; + if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { + ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); + } +} + +void BLASTBufferQueue::BufferReleaseThread::start(const sp& bbq) { + mRunning = std::make_shared(true); + mReader = bbq->mBufferReleaseReader; + std::thread([running = mRunning, reader = mReader, weakBbq = wp(bbq)]() { + pthread_setname_np(pthread_self(), "BufferReleaseThread"); + while (*running) { + ReleaseCallbackId id; + sp fence; + uint32_t maxAcquiredBufferCount; + if (status_t status = reader->readBlocking(id, fence, maxAcquiredBufferCount); + status != OK) { + continue; + } + sp bbq = weakBbq.promote(); + if (!bbq) { + return; + } + bbq->releaseBufferCallback(id, fence, maxAcquiredBufferCount); + } + }).detach(); +} + +BLASTBufferQueue::BufferReleaseThread::~BufferReleaseThread() { + *mRunning = false; + mReader->interruptBlockingRead(); +} + +#endif + } // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 0e7dd201f6..d787d6cc45 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -315,6 +315,51 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); + +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + class BufferReleaseReader { + public: + BufferReleaseReader() = default; + BufferReleaseReader(std::unique_ptr); + BufferReleaseReader& operator=(BufferReleaseReader&&); + + // Block until we can read a buffer release message. + // + // Returns: + // * OK if a ReleaseCallbackId and Fence were successfully read. + // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. + // * UNKNOWN_ERROR if something went wrong. + status_t readBlocking(ReleaseCallbackId& outId, sp& outReleaseFence, + uint32_t& outMaxAcquiredBufferCount); + + // Signals the reader's eventfd to wake up any threads waiting on readBlocking. + void interruptBlockingRead(); + + private: + std::mutex mMutex; + std::unique_ptr mEndpoint GUARDED_BY(mMutex); + android::base::unique_fd mEpollFd; + android::base::unique_fd mEventFd; + }; + + // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to + // the client. See BBQBufferQueueProducer::dequeueBuffer for details. + std::shared_ptr mBufferReleaseReader; + std::shared_ptr mBufferReleaseProducer; + + class BufferReleaseThread { + public: + BufferReleaseThread() = default; + ~BufferReleaseThread(); + void start(const sp&); + + private: + std::shared_ptr mRunning; + std::shared_ptr mReader; + }; + + BufferReleaseThread mBufferReleaseThread; +#endif }; } // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 7b12a807c9..3c8af19015 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -1951,15 +1951,24 @@ bool Layer::setDropInputMode(gui::DropInputMode mode) { void Layer::callReleaseBufferCallback(const sp& listener, const sp& buffer, uint64_t framenumber, const sp& releaseFence) { - if (!listener) { + if (!listener && !mBufferReleaseChannel) { return; } + SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); + + ReleaseCallbackId callbackId{buffer->getId(), framenumber}; + const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - listener->onReleaseBuffer({buffer->getId(), framenumber}, - releaseFence ? releaseFence : Fence::NO_FENCE, - currentMaxAcquiredBufferCount); + + if (listener) { + listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); + } + + if (mBufferReleaseChannel) { + mBufferReleaseChannel->writeReleaseFence(callbackId, fence, currentMaxAcquiredBufferCount); + } } sp Layer::findCallbackHandle() { @@ -2077,6 +2086,7 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { + handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -3508,6 +3518,11 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } +void Layer::setBufferReleaseChannel( + const std::shared_ptr& channel) { + mBufferReleaseChannel = channel; +} + void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 3692fb88f1..1e4f2dc296 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -537,6 +537,7 @@ public: }; BufferInfo mBufferInfo; + std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -724,6 +725,8 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); + void setBufferReleaseChannel( + const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index ce83475c4c..f4bff8f972 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5063,6 +5063,10 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } + if (what & layer_state_t::eBufferReleaseChannelChanged) { + layer->setBufferReleaseChannel(s.bufferReleaseChannel); + } + const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 881bf35b58..c6856aea75 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,6 +149,13 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); + if (handle->bufferReleaseChannel && + handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { + mBufferReleases.emplace_back(handle->bufferReleaseChannel, + handle->previousReleaseCallbackId, + handle->previousReleaseFence, + handle->currentMaxAcquiredBufferCount); + } } return NO_ERROR; } @@ -158,6 +165,12 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { + for (const auto& bufferRelease : mBufferReleases) { + bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence, + bufferRelease.currentMaxAcquiredBufferCount); + } + mBufferReleases.clear(); + // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 7853a9f359..14a7487156 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -16,18 +16,14 @@ #pragma once -#include #include -#include #include -#include -#include #include -#include #include #include #include +#include #include #include #include @@ -59,6 +55,7 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; + std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -86,6 +83,14 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; + struct BufferRelease { + std::shared_ptr channel; + ReleaseCallbackId callbackId; + sp fence; + uint32_t currentMaxAcquiredBufferCount; + }; + std::vector mBufferReleases; + sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From 3118f969f488b738de1976917a0944705c209173 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Fri, 13 Sep 2024 22:34:11 +0000 Subject: Add some tracing for release fences * Fix FenceMonitor to cleanly teardown * App-side tracing for BBQ release callbacks * Trace future compleition in TxnCallbackInvoker Bug: 360932099 Bug: 357762453 Flag: EXEMPT debugging Test: builds Test: perfetto Change-Id: I188c04da1e249b672b428837c328568d9e7a1e66 --- libs/gui/BLASTBufferQueue.cpp | 11 +++++++ libs/gui/FenceMonitor.cpp | 36 +++++++++++++++------- libs/gui/include/gui/BLASTBufferQueue.h | 3 ++ libs/gui/include/gui/FenceMonitor.h | 8 +++-- services/surfaceflinger/Layer.cpp | 4 +++ .../surfaceflinger/TransactionCallbackInvoker.cpp | 4 +++ 6 files changed, 53 insertions(+), 13 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 3c1971fd81..94998e5ab7 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -475,6 +476,16 @@ void BLASTBufferQueue::releaseBufferCallbackLocked( ATRACE_CALL(); BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str()); + if (CC_UNLIKELY(atrace_is_tag_enabled(ATRACE_TAG_GRAPHICS))) { + if (!mFenceMonitor) { + std::string monitorName = "release :"; + monitorName.append(mName.c_str()); + mFenceMonitor.emplace(monitorName.c_str()); + } + + mFenceMonitor->queueFence(releaseFence); + } + // Calculate how many buffers we need to hold before we release them back // to the buffer queue. This will prevent higher latency when we are running // on a lower refresh rate than the max supported. We only do that for EGL diff --git a/libs/gui/FenceMonitor.cpp b/libs/gui/FenceMonitor.cpp index 230c81a0b3..e38f1a8ce6 100644 --- a/libs/gui/FenceMonitor.cpp +++ b/libs/gui/FenceMonitor.cpp @@ -25,9 +25,18 @@ namespace android::gui { FenceMonitor::FenceMonitor(const char* name) : mName(name), mFencesQueued(0), mFencesSignaled(0) { - std::thread thread(&FenceMonitor::loop, this); - pthread_setname_np(thread.native_handle(), mName); - thread.detach(); + mThread = std::thread(&FenceMonitor::loop, this); +} + +FenceMonitor::~FenceMonitor() { + { + std::lock_guard lock(mMutex); + mStopped = true; + mCondition.notify_one(); + } + if (mThread.joinable()) { + mThread.join(); + } } void FenceMonitor::queueFence(const sp& fence) { @@ -35,24 +44,26 @@ void FenceMonitor::queueFence(const sp& fence) { std::lock_guard lock(mMutex); if (fence->getSignalTime() != Fence::SIGNAL_TIME_PENDING) { - snprintf(message, sizeof(message), "%s fence %u has signaled", mName, mFencesQueued); + snprintf(message, sizeof(message), "%s fence %u has signaled", mName.c_str(), + mFencesQueued); ATRACE_NAME(message); // Need an increment on both to make the trace number correct. mFencesQueued++; mFencesSignaled++; return; } - snprintf(message, sizeof(message), "Trace %s fence %u", mName, mFencesQueued); + snprintf(message, sizeof(message), "Trace %s fence %u", mName.c_str(), mFencesQueued); ATRACE_NAME(message); mQueue.push_back(fence); mCondition.notify_one(); mFencesQueued++; - ATRACE_INT(mName, int32_t(mQueue.size())); + ATRACE_INT(mName.c_str(), int32_t(mQueue.size())); } void FenceMonitor::loop() { - while (true) { + pthread_setname_np(pthread_self(), mName.c_str()); + while (!mStopped) { threadLoop(); } } @@ -62,15 +73,18 @@ void FenceMonitor::threadLoop() { uint32_t fenceNum; { std::unique_lock lock(mMutex); - while (mQueue.empty()) { + while (mQueue.empty() && !mStopped) { mCondition.wait(lock); } + if (mStopped) { + return; + } fence = mQueue[0]; fenceNum = mFencesSignaled; } { char message[64]; - snprintf(message, sizeof(message), "waiting for %s %u", mName, fenceNum); + snprintf(message, sizeof(message), "waiting for %s %u", mName.c_str(), fenceNum); ATRACE_NAME(message); status_t result = fence->waitForever(message); @@ -82,8 +96,8 @@ void FenceMonitor::threadLoop() { std::lock_guard lock(mMutex); mQueue.pop_front(); mFencesSignaled++; - ATRACE_INT(mName, int32_t(mQueue.size())); + ATRACE_INT(mName.c_str(), int32_t(mQueue.size())); } } -} // namespace android::gui \ No newline at end of file +} // namespace android::gui diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index d787d6cc45..868dbd06ec 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -316,6 +317,8 @@ private: std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); + std::optional mFenceMonitor GUARDED_BY(mMutex); + #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) class BufferReleaseReader { public: diff --git a/libs/gui/include/gui/FenceMonitor.h b/libs/gui/include/gui/FenceMonitor.h index 62ceddee5f..ac5cc0a574 100644 --- a/libs/gui/include/gui/FenceMonitor.h +++ b/libs/gui/include/gui/FenceMonitor.h @@ -19,6 +19,7 @@ #include #include #include +#include #include @@ -28,17 +29,20 @@ class FenceMonitor { public: explicit FenceMonitor(const char* name); void queueFence(const sp& fence); + ~FenceMonitor(); private: void loop(); void threadLoop(); - const char* mName; + std::string mName; uint32_t mFencesQueued; uint32_t mFencesSignaled; std::deque> mQueue; std::condition_variable mCondition; std::mutex mMutex; + std::thread mThread; + std::atomic_bool mStopped = false; }; -} // namespace android::gui \ No newline at end of file +} // namespace android::gui diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index c8bb068fa9..c17ea3b579 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -769,6 +769,10 @@ void Layer::prepareReleaseCallbacks(ftl::Future futureFenceResult, // Older fences for the same layer stack can be dropped when a new fence arrives. // An assumption here is that RenderEngine performs work sequentially, so an // incoming fence will not fire before an existing fence. + SFTRACE_NAME( + ftl::Concat("Adding additional fence for: ", ftl::truncated<20>(mName.c_str()), + ", Replacing?: ", mAdditionalPreviousReleaseFences.contains(layerStack)) + .c_str()); mAdditionalPreviousReleaseFences.emplace_or_replace(layerStack, std::move(futureFenceResult)); } diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index c6856aea75..2b20648e42 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include namespace android { @@ -129,6 +130,9 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& if (FlagManager::getInstance().ce_fence_promise()) { for (auto& future : handle->previousReleaseFences) { + SFTRACE_NAME(ftl::Concat("Merging fence for layer: ", + ftl::truncated<20>(handle->name.c_str())) + .c_str()); mergeFence(handle->name.c_str(), future.get().value_or(Fence::NO_FENCE), prevFence); } } else { -- cgit v1.2.3-59-g8ed1b From af15fabf36de6b9a5ca9ed5ba1076bc9a9aa2da2 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Tue, 30 Jul 2024 08:59:26 -0700 Subject: Pass an apply token to BBQ Allow BBQ to use a passed in apply token for applying transactions. This allows us to order transactions when a BBQ is recreated by sharing the same apply token between the BBQs. Flag: EXEMPT bugfix Test: presubmit Fixes: 353332587 Change-Id: Icbadce96e9f72f13a26c4430cdfadf6ea315ce84 --- libs/gui/BLASTBufferQueue.cpp | 5 +++ libs/gui/include/gui/BLASTBufferQueue.h | 4 +- libs/gui/tests/BLASTBufferQueue_test.cpp | 67 ++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 94998e5ab7..c65eafa541 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -1266,6 +1266,11 @@ void BLASTBufferQueue::setTransactionHangCallback( mTransactionHangCallback = std::move(callback); } +void BLASTBufferQueue::setApplyToken(sp applyToken) { + std::lock_guard _lock{mMutex}; + mApplyToken = std::move(applyToken); +} + #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 868dbd06ec..ba58a15e17 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -143,7 +143,7 @@ public: * indicates the reason for the hang. */ void setTransactionHangCallback(std::function callback); - + void setApplyToken(sp); virtual ~BLASTBufferQueue(); void onFirstRef() override; @@ -272,7 +272,7 @@ private: // Queues up transactions using this token in SurfaceFlinger. This prevents queued up // transactions from other parts of the client from blocking this transaction. - const sp mApplyToken GUARDED_BY(mMutex) = sp::make(); + sp mApplyToken GUARDED_BY(mMutex) = sp::make(); // Guards access to mDequeueTimestamps since we cannot hold to mMutex in onFrameDequeued or // we will deadlock. diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index eb2a61d151..53f4a36c42 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -186,6 +186,10 @@ public: mBlastBufferQueueAdapter->mergeWithNextTransaction(merge, frameNumber); } + void setApplyToken(sp applyToken) { + mBlastBufferQueueAdapter->setApplyToken(std::move(applyToken)); + } + private: sp mBlastBufferQueueAdapter; }; @@ -511,6 +515,69 @@ TEST_F(BLASTBufferQueueTest, TripleBuffering) { adapter.waitForCallbacks(); } +class WaitForCommittedCallback { +public: + WaitForCommittedCallback() = default; + ~WaitForCommittedCallback() = default; + + void wait() { + std::unique_lock lock(mMutex); + cv.wait(lock, [this] { return mCallbackReceived; }); + } + + void notify() { + std::unique_lock lock(mMutex); + mCallbackReceived = true; + cv.notify_one(); + mCallbackReceivedTimeStamp = std::chrono::system_clock::now(); + } + auto getCallback() { + return [this](void* /* unused context */, nsecs_t /* latchTime */, + const sp& /* presentFence */, + const std::vector& /* stats */) { notify(); }; + } + std::chrono::time_point mCallbackReceivedTimeStamp; + +private: + std::mutex mMutex; + std::condition_variable cv; + bool mCallbackReceived = false; +}; + +TEST_F(BLASTBufferQueueTest, setApplyToken) { + sp applyToken = sp::make(); + WaitForCommittedCallback firstTransaction; + WaitForCommittedCallback secondTransaction; + { + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + adapter.setApplyToken(applyToken); + sp igbProducer; + setUpProducer(adapter, igbProducer); + + Transaction t; + t.addTransactionCommittedCallback(firstTransaction.getCallback(), nullptr); + adapter.mergeWithNextTransaction(&t, 1); + queueBuffer(igbProducer, 127, 127, 127, + /*presentTimeDelay*/ std::chrono::nanoseconds(500ms).count()); + } + { + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + adapter.setApplyToken(applyToken); + sp igbProducer; + setUpProducer(adapter, igbProducer); + + Transaction t; + t.addTransactionCommittedCallback(secondTransaction.getCallback(), nullptr); + adapter.mergeWithNextTransaction(&t, 1); + queueBuffer(igbProducer, 127, 127, 127, /*presentTimeDelay*/ 0); + } + + firstTransaction.wait(); + secondTransaction.wait(); + EXPECT_GT(secondTransaction.mCallbackReceivedTimeStamp, + firstTransaction.mCallbackReceivedTimeStamp); +} + TEST_F(BLASTBufferQueueTest, SetCrop_Item) { uint8_t r = 255; uint8_t g = 0; -- cgit v1.2.3-59-g8ed1b From 454c55240d50de730f023e429ab40b82fd3c440f Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Mon, 23 Sep 2024 14:05:56 +0000 Subject: Revert "Add some tracing for release fences" This reverts commit 3118f969f488b738de1976917a0944705c209173. Reason for revert: b/368514217 Change-Id: Id69a0bd8850d2b6c093f7ce78817cdf5705215b1 --- libs/gui/BLASTBufferQueue.cpp | 11 ------- libs/gui/FenceMonitor.cpp | 36 +++++++--------------- libs/gui/include/gui/BLASTBufferQueue.h | 3 -- libs/gui/include/gui/FenceMonitor.h | 8 ++--- services/surfaceflinger/Layer.cpp | 4 --- .../surfaceflinger/TransactionCallbackInvoker.cpp | 4 --- 6 files changed, 13 insertions(+), 53 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 94998e5ab7..3c1971fd81 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -31,7 +31,6 @@ #include #include -#include #include #include #include @@ -476,16 +475,6 @@ void BLASTBufferQueue::releaseBufferCallbackLocked( ATRACE_CALL(); BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str()); - if (CC_UNLIKELY(atrace_is_tag_enabled(ATRACE_TAG_GRAPHICS))) { - if (!mFenceMonitor) { - std::string monitorName = "release :"; - monitorName.append(mName.c_str()); - mFenceMonitor.emplace(monitorName.c_str()); - } - - mFenceMonitor->queueFence(releaseFence); - } - // Calculate how many buffers we need to hold before we release them back // to the buffer queue. This will prevent higher latency when we are running // on a lower refresh rate than the max supported. We only do that for EGL diff --git a/libs/gui/FenceMonitor.cpp b/libs/gui/FenceMonitor.cpp index e38f1a8ce6..230c81a0b3 100644 --- a/libs/gui/FenceMonitor.cpp +++ b/libs/gui/FenceMonitor.cpp @@ -25,18 +25,9 @@ namespace android::gui { FenceMonitor::FenceMonitor(const char* name) : mName(name), mFencesQueued(0), mFencesSignaled(0) { - mThread = std::thread(&FenceMonitor::loop, this); -} - -FenceMonitor::~FenceMonitor() { - { - std::lock_guard lock(mMutex); - mStopped = true; - mCondition.notify_one(); - } - if (mThread.joinable()) { - mThread.join(); - } + std::thread thread(&FenceMonitor::loop, this); + pthread_setname_np(thread.native_handle(), mName); + thread.detach(); } void FenceMonitor::queueFence(const sp& fence) { @@ -44,26 +35,24 @@ void FenceMonitor::queueFence(const sp& fence) { std::lock_guard lock(mMutex); if (fence->getSignalTime() != Fence::SIGNAL_TIME_PENDING) { - snprintf(message, sizeof(message), "%s fence %u has signaled", mName.c_str(), - mFencesQueued); + snprintf(message, sizeof(message), "%s fence %u has signaled", mName, mFencesQueued); ATRACE_NAME(message); // Need an increment on both to make the trace number correct. mFencesQueued++; mFencesSignaled++; return; } - snprintf(message, sizeof(message), "Trace %s fence %u", mName.c_str(), mFencesQueued); + snprintf(message, sizeof(message), "Trace %s fence %u", mName, mFencesQueued); ATRACE_NAME(message); mQueue.push_back(fence); mCondition.notify_one(); mFencesQueued++; - ATRACE_INT(mName.c_str(), int32_t(mQueue.size())); + ATRACE_INT(mName, int32_t(mQueue.size())); } void FenceMonitor::loop() { - pthread_setname_np(pthread_self(), mName.c_str()); - while (!mStopped) { + while (true) { threadLoop(); } } @@ -73,18 +62,15 @@ void FenceMonitor::threadLoop() { uint32_t fenceNum; { std::unique_lock lock(mMutex); - while (mQueue.empty() && !mStopped) { + while (mQueue.empty()) { mCondition.wait(lock); } - if (mStopped) { - return; - } fence = mQueue[0]; fenceNum = mFencesSignaled; } { char message[64]; - snprintf(message, sizeof(message), "waiting for %s %u", mName.c_str(), fenceNum); + snprintf(message, sizeof(message), "waiting for %s %u", mName, fenceNum); ATRACE_NAME(message); status_t result = fence->waitForever(message); @@ -96,8 +82,8 @@ void FenceMonitor::threadLoop() { std::lock_guard lock(mMutex); mQueue.pop_front(); mFencesSignaled++; - ATRACE_INT(mName.c_str(), int32_t(mQueue.size())); + ATRACE_INT(mName, int32_t(mQueue.size())); } } -} // namespace android::gui +} // namespace android::gui \ No newline at end of file diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 868dbd06ec..d787d6cc45 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -317,8 +316,6 @@ private: std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); - std::optional mFenceMonitor GUARDED_BY(mMutex); - #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) class BufferReleaseReader { public: diff --git a/libs/gui/include/gui/FenceMonitor.h b/libs/gui/include/gui/FenceMonitor.h index ac5cc0a574..62ceddee5f 100644 --- a/libs/gui/include/gui/FenceMonitor.h +++ b/libs/gui/include/gui/FenceMonitor.h @@ -19,7 +19,6 @@ #include #include #include -#include #include @@ -29,20 +28,17 @@ class FenceMonitor { public: explicit FenceMonitor(const char* name); void queueFence(const sp& fence); - ~FenceMonitor(); private: void loop(); void threadLoop(); - std::string mName; + const char* mName; uint32_t mFencesQueued; uint32_t mFencesSignaled; std::deque> mQueue; std::condition_variable mCondition; std::mutex mMutex; - std::thread mThread; - std::atomic_bool mStopped = false; }; -} // namespace android::gui +} // namespace android::gui \ No newline at end of file diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index c17ea3b579..c8bb068fa9 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -769,10 +769,6 @@ void Layer::prepareReleaseCallbacks(ftl::Future futureFenceResult, // Older fences for the same layer stack can be dropped when a new fence arrives. // An assumption here is that RenderEngine performs work sequentially, so an // incoming fence will not fire before an existing fence. - SFTRACE_NAME( - ftl::Concat("Adding additional fence for: ", ftl::truncated<20>(mName.c_str()), - ", Replacing?: ", mAdditionalPreviousReleaseFences.contains(layerStack)) - .c_str()); mAdditionalPreviousReleaseFences.emplace_or_replace(layerStack, std::move(futureFenceResult)); } diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 2b20648e42..c6856aea75 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include namespace android { @@ -130,9 +129,6 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& if (FlagManager::getInstance().ce_fence_promise()) { for (auto& future : handle->previousReleaseFences) { - SFTRACE_NAME(ftl::Concat("Merging fence for layer: ", - ftl::truncated<20>(handle->name.c_str())) - .c_str()); mergeFence(handle->name.c_str(), future.get().value_or(Fence::NO_FENCE), prevFence); } } else { -- cgit v1.2.3-59-g8ed1b