Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eb489f69 authored by liulijun's avatar liulijun Committed by Chavi Weingarten
Browse files

Add producerId so we know when the BBQ producer has been changed.



If BBQ producer changes but the SC remains the same, the frame numbers
for the SC will get reset. This causes issues if there's a barrier layer
set because the barrier is waiting for a particular frame number before
applying the transaction. Since the frame numbers have been reset, the
barrier will be greater than the incoming frame numbers. The change adds
a producerId to the buffer being sent so it can check if the producerId
is older than what's currently set on the Layer. If there's a barriers
set from the old producer, the buffer can be released and not applied
and will stop SF from waiting indefinitely.

Bug: 251971691
Test: Builds, hard to repro

Signed-off-by: default avatarLiu <Lijun&lt;liulijun@xiaomi.corp-partner.google.com>
Change-Id: If37171de4693a73f36f8de43e29c129b352eb55f
parent 858a6a88
Loading
Loading
Loading
Loading
+8 −6
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
//#define LOG_NDEBUG 0

#include <cutils/atomic.h>
#include <gui/BLASTBufferQueue.h>
#include <gui/BufferItemConsumer.h>
#include <gui/BufferQueueConsumer.h>
@@ -157,11 +158,11 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati
                                                      GraphicBuffer::USAGE_HW_COMPOSER |
                                                              GraphicBuffer::USAGE_HW_TEXTURE,
                                                      1, false, this);
    static int32_t id = 0;
    mName = name + "#" + std::to_string(id);
    auto consumerName = mName + "(BLAST Consumer)" + std::to_string(id);
    mQueuedBufferTrace = "QueuedBuffer - " + mName + "BLAST#" + std::to_string(id);
    id++;
    static std::atomic<uint32_t> nextId = 0;
    mProducerId = nextId++;
    mName = name + "#" + std::to_string(mProducerId);
    auto consumerName = mName + "(BLAST Consumer)" + std::to_string(mProducerId);
    mQueuedBufferTrace = "QueuedBuffer - " + mName + "BLAST#" + std::to_string(mProducerId);
    mBufferItemConsumer->setName(String8(consumerName.c_str()));
    mBufferItemConsumer->setFrameAvailableListener(this);

@@ -572,7 +573,8 @@ status_t BLASTBufferQueue::acquireNextBufferLocked(
            std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */,
                      std::placeholders::_1, std::placeholders::_2, std::placeholders::_3);
    sp<Fence> fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE;
    t->setBuffer(mSurfaceControl, buffer, fence, bufferItem.mFrameNumber, releaseBufferCallback);
    t->setBuffer(mSurfaceControl, buffer, fence, bufferItem.mFrameNumber, mProducerId,
                 releaseBufferCallback);
    t->setDataspace(mSurfaceControl, static_cast<ui::Dataspace>(bufferItem.mDataSpace));
    t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata);
    t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage);
+2 −0
Original line number Diff line number Diff line
@@ -984,6 +984,7 @@ status_t BufferData::writeToParcel(Parcel* output) const {
    SAFE_PARCEL(output->writeUint64, cachedBuffer.id);
    SAFE_PARCEL(output->writeBool, hasBarrier);
    SAFE_PARCEL(output->writeUint64, barrierFrameNumber);
    SAFE_PARCEL(output->writeUint32, producerId);

    return NO_ERROR;
}
@@ -1022,6 +1023,7 @@ status_t BufferData::readFromParcel(const Parcel* input) {

    SAFE_PARCEL(input->readBool, &hasBarrier);
    SAFE_PARCEL(input->readUint64, &barrierFrameNumber);
    SAFE_PARCEL(input->readUint32, &producerId);

    return NO_ERROR;
}
+2 −1
Original line number Diff line number Diff line
@@ -1632,7 +1632,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer(
        const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
        const std::optional<sp<Fence>>& fence, const std::optional<uint64_t>& optFrameNumber,
        ReleaseBufferCallback callback) {
        uint32_t producerId, ReleaseBufferCallback callback) {
    layer_state_t* s = getLayerState(sc);
    if (!s) {
        mStatus = BAD_INDEX;
@@ -1651,6 +1651,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe
    bufferData->buffer = buffer;
    uint64_t frameNumber = sc->resolveFrameNumber(optFrameNumber);
    bufferData->frameNumber = frameNumber;
    bufferData->producerId = producerId;
    bufferData->flags |= BufferData::BufferDataChange::frameNumberChanged;
    if (fence) {
        bufferData->acquireFence = *fence;
+5 −0
Original line number Diff line number Diff line
@@ -162,6 +162,11 @@ private:
    int32_t mNumFrameAvailable GUARDED_BY(mMutex) = 0;
    int32_t mNumAcquired GUARDED_BY(mMutex) = 0;

    // 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.
    uint32_t mProducerId = 0;

    // 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<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mSubmitted
+1 −0
Original line number Diff line number Diff line
@@ -111,6 +111,7 @@ public:
    uint64_t frameNumber = 0;
    bool hasBarrier = false;
    uint64_t barrierFrameNumber = 0;
    uint32_t producerId = 0;

    // Listens to when the buffer is safe to be released. This is used for blast
    // layers only. The callback includes a release fence as well as the graphic
Loading