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

Commit cac35380 authored by Dan Stoza's avatar Dan Stoza
Browse files

SF: Modify transaction/buffer sync logic 

Reorganizes the transaction/buffer synchronization logic to only
consider the head of the incoming BufferQueue instead of all buffers,
and fixes some potential race conditions.

Bug: 25951593
Change-Id: Ib2b08e7be571eb8bbd8c364fb85acf0e046b439c
parent 9219ecae

services/surfaceflinger/Layer.cpp

+97 −75
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client, 
        mCurrentTransform(0),

        mCurrentScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE),

        mCurrentOpacity(true),

        mCurrentFrameNumber(0),

        mRefreshPending(false),

        mFrameLatencyNeeded(false),

        mFiltering(false),
@@ -147,6 +148,9 @@ void Layer::onFirstRef() { 
}



Layer::~Layer() {

    for (auto& point : mRemoteSyncPoints) {

        point->setTransactionApplied();

    }

    mFlinger->deleteTextureAsync(mTextureName);

    mFrameTracker.logAndResetStats(mName);

}
@@ -163,20 +167,6 @@ void Layer::onLayerDisplayed(const sp<const DisplayDevice>& /* hw */, 
    }

}



void Layer::markSyncPointsAvailable(const BufferItem& item) {

    auto pointIter = mLocalSyncPoints.begin();

    while (pointIter != mLocalSyncPoints.end()) {

        if ((*pointIter)->getFrameNumber() == item.mFrameNumber) {

            auto syncPoint = *pointIter;

            pointIter = mLocalSyncPoints.erase(pointIter);

            Mutex::Autolock lock(mAvailableFrameMutex);

            mAvailableFrames.push_back(std::move(syncPoint));

        } else {

            ++pointIter;

        }

    }

}



void Layer::onFrameAvailable(const BufferItem& item) {

    // Add this buffer from our internal queue tracker

    { // Autolock scope
@@ -205,8 +195,6 @@ void Layer::onFrameAvailable(const BufferItem& item) { 
        mQueueItemCondition.broadcast();

    }



    markSyncPointsAvailable(item);



    mFlinger->signalLayerUpdate();

}
@@ -233,8 +221,6 @@ void Layer::onFrameReplaced(const BufferItem& item) { 
        mLastFrameNumberReceived = item.mFrameNumber;

        mQueueItemCondition.broadcast();

    }



    markSyncPointsAvailable(item);

}



void Layer::onSidebandStreamChanged() {
@@ -803,22 +789,25 @@ uint32_t Layer::getProducerStickyTransform() const { 
    return static_cast<uint32_t>(producerStickyTransform);

}



void Layer::addSyncPoint(std::shared_ptr<SyncPoint> point) {

    uint64_t headFrameNumber = 0;

    {

uint64_t Layer::getHeadFrameNumber() const {

    Mutex::Autolock lock(mQueueItemLock);

    if (!mQueueItems.empty()) {

            headFrameNumber = mQueueItems[0].mFrameNumber;

        return mQueueItems[0].mFrameNumber;

    } else {

            headFrameNumber = mLastFrameNumberReceived;

        return mCurrentFrameNumber;

    }

}



    if (point->getFrameNumber() <= headFrameNumber) {

        point->setFrameAvailable();

    } else {

        mLocalSyncPoints.push_back(std::move(point));

bool Layer::addSyncPoint(const std::shared_ptr<SyncPoint>& point) {

    if (point->getFrameNumber() <= mCurrentFrameNumber) {

        // Don't bother with a SyncPoint, since we've already latched the

        // relevant frame

        return false;

    }



    Mutex::Autolock lock(mLocalSyncPointMutex);

    mLocalSyncPoints.push_back(point);

    return true;

}



void Layer::setFiltering(bool filtering) {
@@ -940,8 +929,6 @@ void Layer::pushPendingState() { 
        return;

    }



    Mutex::Autolock lock(mPendingStateMutex);



    // If this transaction is waiting on the receipt of a frame, generate a sync

    // point and send it to the remote layer.

    if (mCurrentState.handle != nullptr) {
@@ -956,8 +943,13 @@ void Layer::pushPendingState() { 
        } else {

            auto syncPoint = std::make_shared<SyncPoint>(

                    mCurrentState.frameNumber);

            handleLayer->addSyncPoint(syncPoint);

            if (handleLayer->addSyncPoint(syncPoint)) {

                mRemoteSyncPoints.push_back(std::move(syncPoint));

            } else {

                // We already missed the frame we're supposed to synchronize

                // on, so go ahead and apply the state update

                mCurrentState.handle = nullptr;

            }

        }



        // Wake us up to check if the frame has been received
@@ -976,8 +968,6 @@ void Layer::popPendingState() { 
}



bool Layer::applyPendingStates() {

    Mutex::Autolock lock(mPendingStateMutex);



    bool stateUpdateAvailable = false;

    while (!mPendingStates.empty()) {

        if (mPendingStates[0].handle != nullptr) {
@@ -991,6 +981,17 @@ bool Layer::applyPendingStates() { 
                continue;

            }



            if (mRemoteSyncPoints.front()->getFrameNumber() !=

                    mPendingStates[0].frameNumber) {

                ALOGE("[%s] Unexpected sync point frame number found",

                        mName.string());



                // Signal our end of the sync point and then dispose of it

                mRemoteSyncPoints.front()->setTransactionApplied();

                mRemoteSyncPoints.pop_front();

                continue;

            }



            if (mRemoteSyncPoints.front()->frameIsAvailable()) {

                // Apply the state update

                popPendingState();
@@ -1019,9 +1020,12 @@ bool Layer::applyPendingStates() { 
}



void Layer::notifyAvailableFrames() {

    Mutex::Autolock lock(mAvailableFrameMutex);

    for (auto frame : mAvailableFrames) {

        frame->setFrameAvailable();

    auto headFrameNumber = getHeadFrameNumber();

    Mutex::Autolock lock(mLocalSyncPointMutex);

    for (auto& point : mLocalSyncPoints) {

        if (headFrameNumber >= point->getFrameNumber()) {

            point->setFrameAvailable();

        }

    }

}
@@ -1462,37 +1466,40 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) 
        Reject r(mDrawingState, getCurrentState(), recomputeVisibleRegions,

                getProducerStickyTransform() != 0);



        uint64_t maxFrameNumber = 0;

        uint64_t headFrameNumber = 0;

        {

            Mutex::Autolock lock(mQueueItemLock);

            maxFrameNumber = mLastFrameNumberReceived;

            if (!mQueueItems.empty()) {

                headFrameNumber = mQueueItems[0].mFrameNumber;

            }

        }



        bool availableFramesEmpty = true;

        {

            Mutex::Autolock lock(mAvailableFrameMutex);

            availableFramesEmpty = mAvailableFrames.empty();

        }

        if (!availableFramesEmpty) {

            Mutex::Autolock lock(mAvailableFrameMutex);

        // Check all of our local sync points to ensure that all transactions

        // which need to have been applied prior to the frame which is about to

        // be latched have signaled



        auto headFrameNumber = getHeadFrameNumber();

        bool matchingFramesFound = false;

        bool allTransactionsApplied = true;

            for (auto& frame : mAvailableFrames) {

                if (headFrameNumber != frame->getFrameNumber()) {

        {

            Mutex::Autolock lock(mLocalSyncPointMutex);

            for (auto& point : mLocalSyncPoints) {

                if (point->getFrameNumber() > headFrameNumber) {

                    break;

                }



                matchingFramesFound = true;

                allTransactionsApplied &= frame->transactionIsApplied();



                if (!point->frameIsAvailable()) {

                    // We haven't notified the remote layer that the frame for

                    // this point is available yet. Notify it now, and then

                    // abort this attempt to latch.

                    point->setFrameAvailable();

                    allTransactionsApplied = false;

                    break;

                }



                allTransactionsApplied &= point->transactionIsApplied();

            }

        }



        if (matchingFramesFound && !allTransactionsApplied) {

            mFlinger->signalLayerUpdate();

            return outDirtyRegion;

        }

        }



        // This boolean is used to make sure that SurfaceFlinger's shadow copy

        // of the buffer queue isn't modified when the buffer queue is returning
@@ -1501,7 +1508,7 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) 
        bool queuedBuffer = false;

        status_t updateResult = mSurfaceFlingerConsumer->updateTexImage(&r,

                mFlinger->mPrimaryDispSync, &mSingleBufferMode, &queuedBuffer,

                maxFrameNumber);

                mLastFrameNumberReceived);

        if (updateResult == BufferQueue::PRESENT_LATER) {

            // Producer doesn't want buffer to be displayed yet.  Signal a

            // layer update so we check again at the next opportunity.
@@ -1560,15 +1567,6 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) 
            mFlinger->signalLayerUpdate();

        }



        if (!availableFramesEmpty) {

            Mutex::Autolock lock(mAvailableFrameMutex);

            auto frameNumber = mSurfaceFlingerConsumer->getFrameNumber();

            while (!mAvailableFrames.empty() &&

                    frameNumber == mAvailableFrames.front()->getFrameNumber()) {

                mAvailableFrames.pop_front();

            }

        }



        if (updateResult != NO_ERROR) {

            // something happened!

            recomputeVisibleRegions = true;
@@ -1617,6 +1615,30 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) 
            recomputeVisibleRegions = true;

        }



        mCurrentFrameNumber = mSurfaceFlingerConsumer->getFrameNumber();



        // Remove any sync points corresponding to the buffer which was just

        // latched

        {

            Mutex::Autolock lock(mLocalSyncPointMutex);

            auto point = mLocalSyncPoints.begin();

            while (point != mLocalSyncPoints.end()) {

                if (!(*point)->frameIsAvailable() ||

                        !(*point)->transactionIsApplied()) {

                    // This sync point must have been added since we started

                    // latching. Don't drop it yet.

                    ++point;

                    continue;

                }



                if ((*point)->getFrameNumber() <= mCurrentFrameNumber) {

                    point = mLocalSyncPoints.erase(point);

                } else {

                    ++point;

                }

            }

        }



        // FIXME: postedRegion should be dirty & bounds

        Region dirtyRegion(Rect(s.active.w, s.active.h));

services/surfaceflinger/Layer.h

+12 −10
Original line number Diff line number Diff line
@@ -356,10 +356,6 @@ private: 
    virtual void onFrameReplaced(const BufferItem& item) override;

    virtual void onSidebandStreamChanged() override;



    // Move frames made available by item in to a list which will

    // be signalled at the beginning of the next transaction

    virtual void markSyncPointsAvailable(const BufferItem& item);



    void commitTransaction();



    // needsLinearFiltering - true if this surface's state requires filtering
@@ -413,19 +409,24 @@ private: 
        std::atomic<bool> mTransactionIsApplied;

    };



    // SyncPoints which will be signaled when the correct frame is at the head

    // of the queue and dropped after the frame has been latched. Protected by

    // mLocalSyncPointMutex.

    Mutex mLocalSyncPointMutex;

    std::list<std::shared_ptr<SyncPoint>> mLocalSyncPoints;



    // Guarded by mPendingStateMutex

    // SyncPoints which will be signaled and then dropped when the transaction

    // is applied

    std::list<std::shared_ptr<SyncPoint>> mRemoteSyncPoints;



    void addSyncPoint(std::shared_ptr<SyncPoint> point);

    uint64_t getHeadFrameNumber() const;



    // Returns false if the relevant frame has already been latched

    bool addSyncPoint(const std::shared_ptr<SyncPoint>& point);



    void pushPendingState();

    void popPendingState();

    bool applyPendingStates();



    Mutex mAvailableFrameMutex;

    std::list<std::shared_ptr<SyncPoint>> mAvailableFrames;

public:

    void notifyAvailableFrames();

private:
@@ -461,6 +462,7 @@ private: 
    uint32_t mCurrentTransform;

    uint32_t mCurrentScalingMode;

    bool mCurrentOpacity;

    std::atomic<uint64_t> mCurrentFrameNumber;

    bool mRefreshPending;

    bool mFrameLatencyNeeded;

    // Whether filtering is forced on or not
@@ -488,7 +490,7 @@ private: 
    mutable Mutex mQueueItemLock;

    Condition mQueueItemCondition;

    Vector<BufferItem> mQueueItems;

    uint64_t mLastFrameNumberReceived;

    std::atomic<uint64_t> mLastFrameNumberReceived;

    bool mUpdateTexImageFailed; // This is only modified from the main thread



    bool mSingleBufferMode;