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

Commit ed1283a5 authored by Ady Abraham's avatar Ady Abraham
Browse files

SF: fix a few bugs with FrameTargeter storing N fences 

Bug: 267315508
Bug: 354007767
Bug: 308858993
Test: SF unit tests
Flag: com.android.graphics.surfaceflinger.flags.allow_n_vsyncs_in_targeter
Change-Id: Ib603c0fd29a3e4cb2fcd99dca59bbd5bfb55a787
parent 44e895b7

services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h

+12 −28
Original line number Diff line number Diff line
@@ -55,15 +55,11 @@ public: 


    std::optional<TimePoint> earliestPresentTime() const { return mEarliestPresentTime; }



    // The time of the VSYNC that preceded this frame. See `presentFenceForPastVsync` for details.

    TimePoint pastVsyncTime(Period minFramePeriod) const;



    // Equivalent to `presentFenceForPastVsync` unless running N VSYNCs ahead.

    const FenceTimePtr& presentFenceForPreviousFrame() const {

        return mPresentFences.front().fenceTime;

    }

    // Equivalent to `expectedSignaledPresentFence` unless running N VSYNCs ahead.

    const FenceTimePtr& presentFenceForPreviousFrame() const;



    bool isFramePending() const { return mFramePending; }

    bool wouldBackpressureHwc() const { return mWouldBackpressureHwc; }

    bool didMissFrame() const { return mFrameMissed; }

    bool didMissHwcFrame() const { return mHwcFrameMissed && !mGpuFrameMissed; }

    FrameTime lastSignaledFrameTime() const { return mLastSignaledFrameTime; }
@@ -72,7 +68,7 @@ protected: 
    explicit FrameTarget(const std::string& displayLabel);

    ~FrameTarget() = default;



    bool wouldPresentEarly(Period minFramePeriod) const;

    bool wouldPresentEarly(Period vsyncPeriod, Period minFramePeriod) const;



    // Equivalent to `pastVsyncTime` unless running N VSYNCs ahead.

    TimePoint previousFrameVsyncTime(Period minFramePeriod) const {
@@ -81,8 +77,7 @@ protected: 


    void addFence(sp<Fence> presentFence, FenceTimePtr presentFenceTime,

                  TimePoint expectedPresentTime) {

        mFenceWithFenceTimes.next() = {std::move(presentFence), presentFenceTime,

                                       expectedPresentTime};

        mPresentFences.next() = {std::move(presentFence), presentFenceTime, expectedPresentTime};

    }



    VsyncId mVsyncId;
@@ -94,8 +89,9 @@ protected: 
    TracedOrdinal<bool> mFrameMissed;

    TracedOrdinal<bool> mHwcFrameMissed;

    TracedOrdinal<bool> mGpuFrameMissed;

    bool mWouldBackpressureHwc = false;



    struct FenceWithFenceTime {

    struct PresentFence {

        sp<Fence> fence = Fence::NO_FENCE;

        FenceTimePtr fenceTime = FenceTime::NO_FENCE;

        TimePoint expectedPresentTime = TimePoint();
@@ -106,9 +102,10 @@ protected: 
    // VSYNC of at least one previous frame has not yet passed. In other words, this is NOT the

    // `presentFenceForPreviousFrame` if running N VSYNCs ahead, but the one that should have been

    // signaled by now (unless that frame missed).

    FenceWithFenceTime presentFenceForPastVsync(Period minFramePeriod) const;

    std::array<FenceWithFenceTime, 2> mPresentFences;

    utils::RingBuffer<FenceWithFenceTime, 5> mFenceWithFenceTimes;

    std::pair<bool /* wouldBackpressure */, PresentFence> expectedSignaledPresentFence(

            Period vsyncPeriod, Period minFramePeriod) const;

    std::array<PresentFence, 2> mPresentFencesLegacy;

    utils::RingBuffer<PresentFence, 5> mPresentFences;



    FrameTime mLastSignaledFrameTime;
@@ -120,19 +117,6 @@ private: 
        static_assert(N > 1);

        return expectedFrameDuration() > (N - 1) * minFramePeriod;

    }



    FenceWithFenceTime pastVsyncTimePtr() const {

        FenceWithFenceTime pastFenceWithFenceTime;

        for (size_t i = 0; i < mFenceWithFenceTimes.size(); i++) {

            const auto& fenceWithFenceTime = mFenceWithFenceTimes[i];

            // TODO(b/354007767) Fix the below condition to avoid frame drop

            if (fenceWithFenceTime.expectedPresentTime > mFrameBeginTime) {

                return pastFenceWithFenceTime;

            }

            pastFenceWithFenceTime = fenceWithFenceTime;

        }

        return pastFenceWithFenceTime;

    }

};



// Computes a display's per-frame metrics about past/upcoming targeting of present deadlines.
@@ -155,7 +139,7 @@ public: 


    void beginFrame(const BeginFrameArgs&, const IVsyncSource&);



    std::optional<TimePoint> computeEarliestPresentTime(Period minFramePeriod,

    std::optional<TimePoint> computeEarliestPresentTime(Period vsyncPeriod, Period minFramePeriod,

                                                        Duration hwcMinWorkDuration);



    // TODO(b/241285191): Merge with FrameTargeter::endFrame.

services/surfaceflinger/Scheduler/src/FrameTargeter.cpp

+67 −34
Original line number Diff line number Diff line
@@ -18,8 +18,10 @@ 
#include <common/trace.h>

#include <scheduler/FrameTargeter.h>

#include <scheduler/IVsyncSource.h>

#include <utils/Log.h>



namespace android::scheduler {

using namespace std::chrono_literals;



FrameTarget::FrameTarget(const std::string& displayLabel)

      : mFramePending("PrevFramePending " + displayLabel, false),
@@ -27,32 +29,48 @@ FrameTarget::FrameTarget(const std::string& displayLabel) 
        mHwcFrameMissed("PrevHwcFrameMissed " + displayLabel, false),

        mGpuFrameMissed("PrevGpuFrameMissed " + displayLabel, false) {}



TimePoint FrameTarget::pastVsyncTime(Period minFramePeriod) const {

    // TODO(b/267315508): Generalize to N VSYNCs.

    const int shift = static_cast<int>(targetsVsyncsAhead<2>(minFramePeriod));

    return mExpectedPresentTime - Period::fromNs(minFramePeriod.ns() << shift);

std::pair<bool /* wouldBackpressure */, FrameTarget::PresentFence>

FrameTarget::expectedSignaledPresentFence(Period vsyncPeriod, Period minFramePeriod) const {

    if (!FlagManager::getInstance().allow_n_vsyncs_in_targeter()) {

        const size_t i = static_cast<size_t>(targetsVsyncsAhead<2>(minFramePeriod));

        return {true, mPresentFencesLegacy[i]};

    }



FrameTarget::FenceWithFenceTime FrameTarget::presentFenceForPastVsync(Period minFramePeriod) const {

    if (FlagManager::getInstance().allow_n_vsyncs_in_targeter()) {

        return pastVsyncTimePtr();

    bool wouldBackpressure = true;

    auto expectedPresentTime = mExpectedPresentTime;

    for (size_t i = mPresentFences.size(); i != 0; --i) {

        const auto& fence = mPresentFences[i - 1];



        if (fence.expectedPresentTime + minFramePeriod < expectedPresentTime - vsyncPeriod / 2) {

            wouldBackpressure = false;

        }

    const size_t i = static_cast<size_t>(targetsVsyncsAhead<2>(minFramePeriod));

    return mPresentFences[i];



        if (fence.expectedPresentTime <= mFrameBeginTime) {

            return {wouldBackpressure, fence};

        }



bool FrameTarget::wouldPresentEarly(Period minFramePeriod) const {

    // TODO(b/241285475): Since this is called during `composite`, the calls to `targetsVsyncsAhead`

    // should use `TimePoint::now()` in case of delays since `mFrameBeginTime`.

        expectedPresentTime = fence.expectedPresentTime;

    }

    return {wouldBackpressure, PresentFence{}};

}



    // TODO(b/267315508): Generalize to N VSYNCs.

    const bool allowNVsyncs = FlagManager::getInstance().allow_n_vsyncs_in_targeter();

    if (!allowNVsyncs && targetsVsyncsAhead<3>(minFramePeriod)) {

bool FrameTarget::wouldPresentEarly(Period vsyncPeriod, Period minFramePeriod) const {

    if (targetsVsyncsAhead<3>(minFramePeriod)) {

        return true;

    }



    const auto fence = presentFenceForPastVsync(minFramePeriod).fenceTime;

    return fence->isValid() && fence->getSignalTime() != Fence::SIGNAL_TIME_PENDING;

    const auto [wouldBackpressure, fence] =

            expectedSignaledPresentFence(vsyncPeriod, minFramePeriod);

    return wouldBackpressure && fence.fenceTime->isValid() &&

            fence.fenceTime->getSignalTime() != Fence::SIGNAL_TIME_PENDING;

}



const FenceTimePtr& FrameTarget::presentFenceForPreviousFrame() const {

    if (FlagManager::getInstance().allow_n_vsyncs_in_targeter()) {

        return mPresentFences.back().fenceTime;

    }



    return mPresentFencesLegacy.front().fenceTime;

}



void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& vsyncSource) {
@@ -86,27 +104,39 @@ void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& v 
    }



    if (!mSupportsExpectedPresentTime) {

        mEarliestPresentTime = computeEarliestPresentTime(minFramePeriod, args.hwcMinWorkDuration);

        mEarliestPresentTime =

                computeEarliestPresentTime(vsyncPeriod, minFramePeriod, args.hwcMinWorkDuration);

    }



    SFTRACE_FORMAT("%s %" PRId64 " vsyncIn %.2fms%s", __func__, ftl::to_underlying(args.vsyncId),

                   ticks<std::milli, float>(mExpectedPresentTime - TimePoint::now()),

                   mExpectedPresentTime == args.expectedVsyncTime ? "" : " (adjusted)");



    FenceWithFenceTime pastPresentFence = presentFenceForPastVsync(minFramePeriod);

    const auto [wouldBackpressure, fence] =

            expectedSignaledPresentFence(vsyncPeriod, minFramePeriod);



    // In cases where the present fence is about to fire, give it a small grace period instead of

    // giving up on the frame.

    //

    // TODO(b/280667110): The grace period should depend on `sfWorkDuration` and `vsyncPeriod` being

    // approximately equal, not whether backpressure propagation is enabled.

    const int graceTimeForPresentFenceMs = static_cast<int>(

            mBackpressureGpuComposition || !mCompositionCoverage.test(CompositionCoverage::Gpu));

    const int graceTimeForPresentFenceMs = [&] {

        const bool considerBackpressure =

                mBackpressureGpuComposition || !mCompositionCoverage.test(CompositionCoverage::Gpu);



        if (!FlagManager::getInstance().allow_n_vsyncs_in_targeter()) {

            return static_cast<int>(considerBackpressure);

        }



        if (!wouldBackpressure || !considerBackpressure) {

            return 0;

        }



        return static_cast<int>((std::abs(fence.expectedPresentTime.ns() - mFrameBeginTime.ns()) <=

                                 Duration(1ms).ns()));

    }();



    // Pending frames may trigger backpressure propagation.

    const auto& isFencePending = *isFencePendingFuncPtr;

    mFramePending = pastPresentFence.fenceTime != FenceTime::NO_FENCE &&

            isFencePending(pastPresentFence.fenceTime, graceTimeForPresentFenceMs);

    mFramePending = fence.fenceTime != FenceTime::NO_FENCE &&

            isFencePending(fence.fenceTime, graceTimeForPresentFenceMs);



    // A frame is missed if the prior frame is still pending. If no longer pending, then we still

    // count the frame as missed if the predicted present time was further in the past than when the
@@ -114,10 +144,10 @@ void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& v 
    // than a typical frame duration, but should not be so small that it reports reasonable drift as

    // a missed frame.

    mFrameMissed = mFramePending || [&] {

        const nsecs_t pastPresentTime = pastPresentFence.fenceTime->getSignalTime();

        const nsecs_t pastPresentTime = fence.fenceTime->getSignalTime();

        if (pastPresentTime < 0) return false;

        mLastSignaledFrameTime = {.signalTime = TimePoint::fromNs(pastPresentTime),

                                  .expectedPresentTime = pastPresentFence.expectedPresentTime};

                                  .expectedPresentTime = fence.expectedPresentTime};

        const nsecs_t frameMissedSlop = vsyncPeriod.ns() / 2;

        return lastScheduledPresentTime.ns() < pastPresentTime - frameMissedSlop;

    }();
@@ -128,11 +158,14 @@ void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& v 
    if (mFrameMissed) mFrameMissedCount++;

    if (mHwcFrameMissed) mHwcFrameMissedCount++;

    if (mGpuFrameMissed) mGpuFrameMissedCount++;



    mWouldBackpressureHwc = mFramePending && wouldBackpressure;

}



std::optional<TimePoint> FrameTargeter::computeEarliestPresentTime(Period minFramePeriod,

std::optional<TimePoint> FrameTargeter::computeEarliestPresentTime(Period vsyncPeriod,

                                                                   Period minFramePeriod,

                                                                   Duration hwcMinWorkDuration) {

    if (wouldPresentEarly(minFramePeriod)) {

    if (wouldPresentEarly(vsyncPeriod, minFramePeriod)) {

        return previousFrameVsyncTime(minFramePeriod) - hwcMinWorkDuration;

    }

    return {};
@@ -151,8 +184,8 @@ FenceTimePtr FrameTargeter::setPresentFence(sp<Fence> presentFence, FenceTimePtr 
    if (FlagManager::getInstance().allow_n_vsyncs_in_targeter()) {

        addFence(std::move(presentFence), presentFenceTime, mExpectedPresentTime);

    } else {

        mPresentFences[1] = mPresentFences[0];

        mPresentFences[0] = {std::move(presentFence), presentFenceTime, mExpectedPresentTime};

        mPresentFencesLegacy[1] = mPresentFencesLegacy[0];

        mPresentFencesLegacy[0] = {std::move(presentFence), presentFenceTime, mExpectedPresentTime};

    }

    return presentFenceTime;

}

services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp

+118 −131

File changed.

Preview size limit exceeded, changes collapsed.

services/surfaceflinger/SurfaceFlinger.cpp

+4 −1
Original line number Diff line number Diff line
@@ -2623,7 +2623,7 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, 
        }

    }



    if (pacesetterFrameTarget.isFramePending()) {

    if (pacesetterFrameTarget.wouldBackpressureHwc()) {

        if (mBackpressureGpuComposition || pacesetterFrameTarget.didMissHwcFrame()) {

            if (FlagManager::getInstance().vrr_config()) {

                mScheduler->getVsyncSchedule()->getTracker().onFrameMissed(
@@ -2921,6 +2921,9 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( 
        // Now that the current frame has been presented above, PowerAdvisor needs the present time

        // of the previous frame (whose fence is signaled by now) to determine how long the HWC had

        // waited on that fence to retire before presenting.

        // TODO(b/355238809) `presentFenceForPreviousFrame` might not always be signaled (e.g. on

        // devices

        //  where HWC does not block on the previous present fence). Revise this assumtion.

        const auto& previousPresentFence = pacesetterTarget.presentFenceForPreviousFrame();



        mPowerAdvisor->setSfPresentTiming(TimePoint::fromNs(previousPresentFence->getSignalTime()),

services/surfaceflinger/Utils/RingBuffer.h

+2 −0
Original line number Diff line number Diff line
@@ -43,8 +43,10 @@ public: 
    }



    T& front() { return (*this)[0]; }

    const T& front() const { return (*this)[0]; }



    T& back() { return (*this)[size() - 1]; }

    const T& back() const { return (*this)[size() - 1]; }



    T& operator[](size_t index) {

        return mBuffer[(static_cast<size_t>(mHead + 1) + index) % mCount];