From 4ba0c2e2a2e8a4870b4d5055bd765d5514a35ebc Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 24 Jun 2021 11:27:17 -0700 Subject: Blast: Use a unique id to track buffers When buffer queue is configured in shared buffer mode, the client can queue the same buffer over and over again and the consumer can acquire the same buffer repeatedly. BBQ tracks acquired buffers by graphic buffer id and SCC tracks release buffer callbacks by graphic buffer ids. This breaks if a buffer is reused before release. Fix this by using graphic buffer id and framenumber to track acquired buffers and buffer release callbacks. Test: atest CtsDeqpTestCases:dEQP-VK.wsi.android.shared_presentable_image.scale_none.identity.inherit.demand CtsDeqpTestCases:dEQP-VK.wsi.android.colorspace#basic Bug: 191220669 Change-Id: Ibd54df9fa6780c26cd8de769bf9e43163abbed5a --- services/surfaceflinger/BufferStateLayer.cpp | 40 ++++++++++++---------------- 1 file changed, 17 insertions(+), 23 deletions(-) (limited to 'services/surfaceflinger/BufferStateLayer.cpp') diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 6b5cf04536..8cf0d66a08 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -44,25 +44,18 @@ namespace android { using PresentState = frametimeline::SurfaceFrame::PresentState; namespace { void callReleaseBufferCallback(const sp& listener, - const sp& buffer, const sp& releaseFence, - uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) { + const sp& buffer, uint64_t framenumber, + const sp& releaseFence, uint32_t transformHint, + uint32_t currentMaxAcquiredBufferCount) { if (!listener) { return; } - listener->onReleaseBuffer(buffer->getId(), releaseFence ? releaseFence : Fence::NO_FENCE, - transformHint, currentMaxAcquiredBufferCount); + listener->onReleaseBuffer({buffer->getId(), framenumber}, + releaseFence ? releaseFence : Fence::NO_FENCE, transformHint, + currentMaxAcquiredBufferCount); } } // namespace -// clang-format off -const std::array BufferStateLayer::IDENTITY_MATRIX{ - 1, 0, 0, 0, - 0, 1, 0, 0, - 0, 0, 1, 0, - 0, 0, 0, 1 -}; -// clang-format on - BufferStateLayer::BufferStateLayer(const LayerCreationArgs& args) : BufferLayer(args), mHwcSlotGenerator(new HwcSlotGenerator()) { mDrawingState.dataspace = ui::Dataspace::V0_SRGB; @@ -75,8 +68,8 @@ BufferStateLayer::~BufferStateLayer() { // issue with the clone layer trying to use the texture. if (mBufferInfo.mBuffer != nullptr && !isClone()) { callReleaseBufferCallback(mDrawingState.releaseBufferListener, - mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFence, - mTransformHint, + mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFrameNumber, + mBufferInfo.mFence, mTransformHint, mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate( mOwnerUid)); } @@ -87,7 +80,7 @@ status_t BufferStateLayer::addReleaseFence(const sp& ch, if (ch == nullptr) { return OK; } - ch->previousBufferId = mPreviousBufferId; + ch->previousReleaseCallbackId = mPreviousReleaseCallbackId; if (!ch->previousReleaseFence.get()) { ch->previousReleaseFence = fence; return OK; @@ -214,7 +207,7 @@ void BufferStateLayer::releasePendingBuffer(nsecs_t dequeueReadyTime) { // see BufferStateLayer::onLayerDisplayed. for (auto& handle : mDrawingState.callbackHandles) { if (handle->releasePreviousBuffer) { - handle->previousBufferId = mPreviousBufferId; + handle->previousReleaseCallbackId = mPreviousReleaseCallbackId; break; } } @@ -438,8 +431,8 @@ bool BufferStateLayer::setBuffer(const std::shared_ptrgetBuffer(), mDrawingState.acquireFence, - mTransformHint, + mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber, + mDrawingState.acquireFence, mTransformHint, mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate( mOwnerUid)); decrementPendingBufferCount(); @@ -684,7 +677,7 @@ uint64_t BufferStateLayer::getFrameNumber(nsecs_t /*expectedPresentTime*/) const * DeferTransactionUntil -> frameNumber = 2 * Random other stuff * } - * Now imagine getHeadFrameNumber returned mDrawingState.mFrameNumber (or mCurrentFrameNumber). + * Now imagine mFrameNumber returned mDrawingState.frameNumber (or mCurrentFrameNumber). * Prior to doTransaction SurfaceFlinger will call notifyAvailableFrames, but because we * haven't swapped mDrawingState to mDrawingState yet we will think the sync point * is not ready. So we will return false from applyPendingState and not swap @@ -792,9 +785,10 @@ status_t BufferStateLayer::updateActiveBuffer() { decrementPendingBufferCount(); } - mPreviousBufferId = getCurrentBufferId(); + mPreviousReleaseCallbackId = {getCurrentBufferId(), mBufferInfo.mFrameNumber}; mBufferInfo.mBuffer = s.buffer; mBufferInfo.mFence = s.acquireFence; + mBufferInfo.mFrameNumber = s.frameNumber; return NO_ERROR; } @@ -969,8 +963,8 @@ void BufferStateLayer::bufferMayChange(const sp& newBuffer) { // then we will drop a buffer and should decrement the pending buffer count and // call any release buffer callbacks if set. callReleaseBufferCallback(mDrawingState.releaseBufferListener, - mDrawingState.buffer->getBuffer(), mDrawingState.acquireFence, - mTransformHint, + mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber, + mDrawingState.acquireFence, mTransformHint, mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate( mOwnerUid)); decrementPendingBufferCount(); -- cgit v1.2.3-59-g8ed1b