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

Commit 3e8b9044 authored by Ady Abraham's avatar Ady Abraham
Browse files

SF: fix buffer stuffing classification 

Keep track of the expected present time of SF when classifying buffer
staffing. Buffer stuffing is classified when:
1. The current frame was due to present on the previous SF frame; and
2. The current frame was ready by the time SF tried to latch the previous frame; and
3. SF latched a frame from this layer on the previous frame

Test: manual
Bug: 417070677
Flag: com.android.graphics.surfaceflinger.flags.buffer_stuffing_fix
Change-Id: I319b77376e3765388f3360513191479b041cb21b
parent d861d454

services/surfaceflinger/Layer.cpp

+14 −11
Original line number Diff line number Diff line
@@ -367,7 +367,7 @@ void Layer::commitTransaction() REQUIRES(mFlinger->mStateLock) { 
        if (surfaceFrame->getPresentState() != PresentState::Presented) {

            // With applyPendingStates, we could end up having presented surfaceframes from previous

            // states

            surfaceFrame->setPresentState(PresentState::Presented, mLastLatchTime);

            surfaceFrame->setPresentState(PresentState::Presented, mFrameTimelinePastTimestamps);

            mFlinger->mFrameTimeline->addSurfaceFrame(surfaceFrame);

        }

    }
@@ -457,11 +457,12 @@ void Layer::addSurfaceFrameDroppedForBuffer(std::shared_ptr<scheduler::SurfaceFr 


void Layer::addSurfaceFramePresentedForBuffer(

        std::shared_ptr<scheduler::SurfaceFrame>& surfaceFrame, nsecs_t acquireFenceTime,

        nsecs_t currentLatchTime) REQUIRES(mFlinger->mStateLock) {

        nsecs_t currentLatchTime, nsecs_t expectedPresentTime) REQUIRES(mFlinger->mStateLock) {

    surfaceFrame->setAcquireFenceTime(acquireFenceTime);

    surfaceFrame->setPresentState(PresentState::Presented, mLastLatchTime);

    surfaceFrame->setPresentState(PresentState::Presented, mFrameTimelinePastTimestamps);

    mFlinger->mFrameTimeline->addSurfaceFrame(surfaceFrame);

    updateLastLatchTime(currentLatchTime);

    updateFrameTimelinePastTimestamps(

            {.latchTime = currentLatchTime, .expectedPresentTime = expectedPresentTime});

}



std::shared_ptr<scheduler::SurfaceFrame> Layer::createSurfaceFrameForTransaction(
@@ -905,7 +906,7 @@ bool Layer::setBuffer(std::shared_ptr<renderengine::ExternalTexture>& buffer, 
    } else if (buffer) {

        // if we are latching a buffer for the first time then clear the mLastLatchTime since

        // we don't want to incorrectly classify a frame if we miss the desired present time.

        updateLastLatchTime(0);

        updateFrameTimelinePastTimestamps({});

    }



    mDrawingState.desiredPresentTime = desiredPresentTime;
@@ -1204,7 +1205,8 @@ bool Layer::latchSidebandStream(bool& recomputeVisibleRegions) { 
    return false;

}



void Layer::updateTexImage(nsecs_t latchTime, bool bgColorOnly) REQUIRES(mFlinger->mStateLock) {

void Layer::updateTexImage(nsecs_t latchTime, nsecs_t expectedPresentTime, bool bgColorOnly)

        REQUIRES(mFlinger->mStateLock) {

    const State& s(getDrawingState());



    if (!s.buffer) {
@@ -1242,7 +1244,7 @@ void Layer::updateTexImage(nsecs_t latchTime, bool bgColorOnly) REQUIRES(mFlinge 
        // are processing the next state.

        addSurfaceFramePresentedForBuffer(bufferSurfaceFrame,

                                          mDrawingState.acquireFenceTime->getSignalTime(),

                                          latchTime);

                                          latchTime, expectedPresentTime);

        mDrawingState.bufferSurfaceFrameTX.reset();

    }
@@ -1469,7 +1471,8 @@ void Layer::onCompositionPresented(const DisplayDevice* display, 
    mBufferInfo.mFrameLatencyNeeded = false;

}



bool Layer::latchBufferImpl(bool& recomputeVisibleRegions, nsecs_t latchTime, bool bgColorOnly)

bool Layer::latchBufferImpl(bool& recomputeVisibleRegions, nsecs_t latchTime,

                            nsecs_t expectedPresentTime, bool bgColorOnly)

        REQUIRES(mFlinger->mStateLock) {

    SFTRACE_FORMAT_INSTANT("latchBuffer %s - %" PRIu64, getDebugName(),

                           getDrawingState().frameNumber);
@@ -1487,7 +1490,7 @@ bool Layer::latchBufferImpl(bool& recomputeVisibleRegions, nsecs_t latchTime, bo 
        mFlinger->onLayerUpdate();

        return false;

    }

    updateTexImage(latchTime, bgColorOnly);

    updateTexImage(latchTime, expectedPresentTime, bgColorOnly);



    // Capture the old state of the layer for comparisons later

    BufferInfo oldBufferInfo = mBufferInfo;
@@ -1597,8 +1600,8 @@ void Layer::setBufferReleaseChannel( 
    mBufferReleaseChannel = channel;

}



void Layer::updateLastLatchTime(nsecs_t latchTime) {

    mLastLatchTime = latchTime;

void Layer::updateFrameTimelinePastTimestamps(scheduler::SurfaceFrame::LastFrameTimestamps time) {

    mFrameTimelinePastTimestamps = time;

}



void Layer::setIsSmallDirty(frontend::LayerSnapshot* snapshot) {

services/surfaceflinger/Layer.h

+9 −7
Original line number Diff line number Diff line
@@ -54,6 +54,7 @@ 
#include "FrameTracker.h"

#include "LayerFE.h"

#include "LayerVector.h"

#include "Scheduler/FrameTimeline.h"

#include "Scheduler/LayerInfo.h"

#include "SurfaceFlinger.h"

#include "TransactionCallbackInvoker.h"
@@ -195,7 +196,7 @@ public: 
    // damage down to hardware composer. Otherwise, we must send a region with

    // one empty rect.

    Region getVisibleRegion(const DisplayDevice*) const;

    void updateLastLatchTime(nsecs_t latchtime);

    void updateFrameTimelinePastTimestamps(scheduler::SurfaceFrame::LastFrameTimestamps);



    Rect getCrop(const Layer::State& s) const { return Rect(s.crop); }
@@ -217,8 +218,8 @@ public: 
     * operation, so this should be set only if needed). Typically this is used

     * to figure out if the content or size of a surface has changed.

     */

    bool latchBufferImpl(bool& /*recomputeVisibleRegions*/, nsecs_t /*latchTime*/,

                         bool bgColorOnly);

    bool latchBufferImpl(bool& recomputeVisibleRegions, nsecs_t latchTime,

                         nsecs_t expectedPresentTime, bool bgColorOnly);



    sp<GraphicBuffer> getBuffer() const;

    /**
@@ -333,7 +334,8 @@ public: 
    void addSurfaceFrameDroppedForBuffer(std::shared_ptr<scheduler::SurfaceFrame>& surfaceFrame,

                                         nsecs_t dropTime);

    void addSurfaceFramePresentedForBuffer(std::shared_ptr<scheduler::SurfaceFrame>& surfaceFrame,

                                           nsecs_t acquireFenceTime, nsecs_t currentLatchTime);

                                           nsecs_t acquireFenceTime, nsecs_t currentLatchTime,

                                           nsecs_t expectedPresentTime);



    std::shared_ptr<scheduler::SurfaceFrame> createSurfaceFrameForTransaction(

            const FrameTimelineInfo& info, nsecs_t postTime, gui::GameMode gameMode);
@@ -464,9 +466,9 @@ protected: 


    int32_t mOwnerAppId;



    // Keeps track of the time SF latched the last buffer from this layer.

    // Keeps track of the various timestamps for the last buffer from this layer.

    // Used in buffer stuffing analysis in FrameTimeline.

    nsecs_t mLastLatchTime = 0;

    scheduler::SurfaceFrame::LastFrameTimestamps mFrameTimelinePastTimestamps;



    sp<Fence> mLastClientCompositionFence;

    bool mClearClientCompositionFenceOnLayerDisplayed = false;
@@ -493,7 +495,7 @@ private: 
    // Latch sideband stream and returns true if the dirty region should be updated.

    bool latchSidebandStream(bool& recomputeVisibleRegions);



    void updateTexImage(nsecs_t latchTime, bool bgColorOnly = false);

    void updateTexImage(nsecs_t latchTime, nsecs_t expectedPresentTime, bool bgColorOnly = false);



    // Crop that applies to the buffer

    Rect computeBufferCrop(const State& s);

services/surfaceflinger/Scheduler/FrameTimeline.cpp

+19 −6
Original line number Diff line number Diff line
@@ -388,14 +388,14 @@ void SurfaceFrame::setDropTime(nsecs_t dropTime) { 
    mDropTime = dropTime;

}



void SurfaceFrame::setPresentState(PresentState presentState, nsecs_t lastLatchTime) {

void SurfaceFrame::setPresentState(PresentState presentState, LastFrameTimestamps times) {

    std::scoped_lock lock(mMutex);

    LOG_ALWAYS_FATAL_IF(mPresentState != PresentState::Unknown,

                        "setPresentState called on a SurfaceFrame from Layer - %s, that has a "

                        "PresentState - %s set already.",

                        mDebugName.c_str(), toString(mPresentState).c_str());

    mPresentState = presentState;

    mLastLatchTime = lastLatchTime;

    mLastFrameTimestamps = times;

}



void SurfaceFrame::setRenderRate(Fps renderRate) {
@@ -536,10 +536,15 @@ void SurfaceFrame::dump(std::string& result, const std::string& indent, nsecs_t 
    StringAppendF(&result, "%s", indent.c_str());

    StringAppendF(&result, "Finish Metadata: %s\n", toString(mFrameReadyMetadata).c_str());

    std::chrono::nanoseconds latchTime(

            std::max(static_cast<int64_t>(0), mLastLatchTime - baseTime));

            std::max(static_cast<int64_t>(0), mLastFrameTimestamps.latchTime - baseTime));

    StringAppendF(&result, "%s", indent.c_str());

    StringAppendF(&result, "Last latch time: %10f\n",

                  std::chrono::duration<double, std::milli>(latchTime).count());

    std::chrono::nanoseconds latchExpectedPresentTime(

            std::max(static_cast<int64_t>(0), mLastFrameTimestamps.expectedPresentTime - baseTime));

    StringAppendF(&result, "%s", indent.c_str());

    StringAppendF(&result, "Last expected present time: %10f\n",

                  std::chrono::duration<double, std::milli>(latchTime).count());

    if (mPredictionState == PredictionState::Valid) {

        nsecs_t presentDelta = mActuals.presentTime - mPredictions.presentTime;

        std::chrono::nanoseconds presentDeltaNs(std::abs(presentDelta));
@@ -644,8 +649,15 @@ void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& r 
            // Finish late, Present early

            mJankType = JankType::Unknown;

        }

    } else {

        if (mLastLatchTime != 0 && mPredictions.endTime <= mLastLatchTime) {

    } else { // FramePresentMetadata::LatePresent

        const bool readyBeforePreviousLatch = mLastFrameTimestamps.latchTime != 0 &&

                mPredictions.endTime <= mLastFrameTimestamps.latchTime;

        const bool dueLastFrame = !FlagManager::getInstance().buffer_stuffing_fix() ||

                (mLastFrameTimestamps.expectedPresentTime != 0 &&

                 mPredictions.presentTime - presentThreshold <

                         mLastFrameTimestamps.expectedPresentTime);



        if (readyBeforePreviousLatch && dueLastFrame) {

            // Buffer Stuffing.

            mJankType |= JankType::BufferStuffing;

            // In a stuffed state, the frame could be stuck on a dequeue wait for quite some time.
@@ -653,7 +665,8 @@ void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& r 
            // We try to do this by moving the deadline. Since the queue could be stuffed by more

            // than one buffer, we take the last latch time as reference and give one vsync

            // worth of time for the frame to be ready.

            nsecs_t adjustedDeadline = mLastLatchTime + displayFrameRenderRate.getPeriodNsecs();

            nsecs_t adjustedDeadline =

                    mLastFrameTimestamps.latchTime + displayFrameRenderRate.getPeriodNsecs();

            if (adjustedDeadline > mActuals.endTime) {

                mFrameReadyMetadata = FrameReadyMetadata::OnTimeFinish;

            } else {

services/surfaceflinger/Scheduler/FrameTimeline.h

+12 −5
Original line number Diff line number Diff line
@@ -192,7 +192,14 @@ public: 
    void setAcquireFenceTime(nsecs_t acquireFenceTime);

    void setDesiredPresentTime(nsecs_t desiredPresentTime);

    void setDropTime(nsecs_t dropTime);

    void setPresentState(PresentState presentState, nsecs_t lastLatchTime = 0);



    struct LastFrameTimestamps {

        nsecs_t latchTime = 0;

        nsecs_t expectedPresentTime = 0;

    };

    void setPresentState(PresentState presentState,

                         LastFrameTimestamps lastFrameTimestamps = {.latchTime = 0,

                                                                    .expectedPresentTime = 0});

    void setRenderRate(Fps renderRate);

    // Return the render rate if it exists, otherwise returns the DisplayFrame's render rate.

    Fps getRenderRate() const;
@@ -281,10 +288,10 @@ private: 
            FramePresentMetadata::UnknownPresent;

    // Enum for the type of finish

    FrameReadyMetadata mFrameReadyMetadata GUARDED_BY(mMutex) = FrameReadyMetadata::UnknownFinish;

    // Time when the previous buffer from the same layer was latched by SF. This is used in checking

    // for BufferStuffing where the current buffer is expected to be ready but the previous buffer

    // was latched instead.

    nsecs_t mLastLatchTime GUARDED_BY(mMutex) = 0;

    // Time when the previous buffer from the same layer was latched by SF, togther with the

    // expected present time for that buffer. This is used in checking for BufferStuffing where

    // the current buffer is expected to be ready but the previous buffer was latched instead.

    LastFrameTimestamps mLastFrameTimestamps GUARDED_BY(mMutex) = {};

    // TraceCookieCounter is used to obtain the cookie for sendig trace packets to perfetto. Using a

    // reference here because the counter is owned by FrameTimeline, which outlives SurfaceFrame.

    TraceCookieCounter& mTraceCookieCounter;

services/surfaceflinger/SurfaceFlinger.cpp

+9 −5
Original line number Diff line number Diff line
@@ -2536,8 +2536,8 @@ void SurfaceFlinger::updateLayerHistory(nsecs_t now) { 
}



bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,

                                          bool flushTransactions, bool& outTransactionsAreEmpty)

        EXCLUDES(mStateLock) {

                                          nsecs_t expectedPresentTimeNs, bool flushTransactions,

                                          bool& outTransactionsAreEmpty) EXCLUDES(mStateLock) {

    using Changes = frontend::RequestedLayerState::Changes;

    SFTRACE_CALL();

    SFTRACE_NAME_FOR_TRACK(WorkloadTracer::TRACK_NAME, "Transaction Handling");
@@ -2676,7 +2676,10 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, 
                // could not latch a buffer or apply a transaction due to backpressure.

                // We only update the latch time for bufferless layers here, the latch time

                // is updated for buffer layers when the buffer is latched.

                it->second->updateLastLatchTime(latchTime);

                it->second->updateFrameTimelinePastTimestamps({

                        .latchTime = latchTime,

                        .expectedPresentTime = expectedPresentTimeNs,

                });

            }

            continue;

        }
@@ -2686,7 +2689,7 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, 
        if (willReleaseBufferOnLatch) {

            mLayersWithBuffersRemoved.emplace(it->second);

        }

        it->second->latchBufferImpl(unused, latchTime, bgColorOnly);

        it->second->latchBufferImpl(unused, latchTime, expectedPresentTimeNs, bgColorOnly);

        newDataLatched = true;



        frontend::LayerSnapshot* snapshot = mLayerSnapshotBuilder.getSnapshot(it->second->sequence);
@@ -2839,6 +2842,7 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, 
        const bool flushTransactions = clearTransactionFlags(eTransactionFlushNeeded);

        bool transactionsAreEmpty = false;

        mustComposite |= updateLayerSnapshots(vsyncId, pacesetterFrameTarget.frameBeginTime().ns(),

                                              pacesetterFrameTarget.expectedPresentTime().ns(),

                                              flushTransactions, transactionsAreEmpty);



        // Tell VsyncTracker that we are going to present this frame before scheduling