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

Commit 894a6d48 authored by Ady Abraham's avatar Ady Abraham
Browse files

SF: pass a render rate to VsyncTracker instead of a divisor 

passing a divisor to VsyncTracker might result in the wrong frame
rate when the vsync period is changing. The Divisor was calculated in
VsyncReactor which calculates it based on the new period, and passed
to VsyncTracker which might still be learning the new period, hence
using the old period.

Bug: 267780202
Test: SF unit tests
Change-Id: Ibb632d4ae6621a6ac0c0123e78f4c4d75699bd9e
parent f34a813b

services/surfaceflinger/Scheduler/Scheduler.cpp

+1 −5
Original line number Diff line number Diff line
@@ -418,11 +418,7 @@ void Scheduler::setRenderRate(Fps renderFrameRate) { 
    ALOGV("%s %s (%s)", __func__, to_string(mode.fps).c_str(),

          to_string(mode.modePtr->getFps()).c_str());



    const auto divisor = RefreshRateSelector::getFrameRateDivisor(mode.modePtr->getFps(), mode.fps);

    LOG_ALWAYS_FATAL_IF(divisor == 0, "%s <> %s -- not divisors", to_string(mode.fps).c_str(),

                        to_string(mode.fps).c_str());



    mVsyncSchedule->getTracker().setDivisor(static_cast<unsigned>(divisor));

    mVsyncSchedule->getTracker().setRenderRate(renderFrameRate);

}



void Scheduler::resync() {

services/surfaceflinger/Scheduler/VSyncPredictor.cpp

+19 −6
Original line number Diff line number Diff line
@@ -272,13 +272,26 @@ nsecs_t VSyncPredictor::nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const { 
    // update the mLastVsyncSequence for reference point

    mLastVsyncSequence = getVsyncSequenceLocked(timePoint);



    const auto mod = mLastVsyncSequence->seq % mDivisor;

    if (mod == 0) {

    const auto renderRatePhase = [&]() REQUIRES(mMutex) -> int {

        if (!mRenderRate) return 0;



        const auto divisor =

                RefreshRateSelector::getFrameRateDivisor(Fps::fromPeriodNsecs(mIdealPeriod),

                                                         *mRenderRate);

        if (divisor <= 1) return 0;



        const int mod = mLastVsyncSequence->seq % divisor;

        if (mod == 0) return 0;



        return divisor - mod;

    }();



    if (renderRatePhase == 0) {

        return mLastVsyncSequence->vsyncTime;

    }



    auto const [slope, intercept] = getVSyncPredictionModelLocked();

    const auto approximateNextVsync = mLastVsyncSequence->vsyncTime + slope * (mDivisor - mod);

    const auto approximateNextVsync = mLastVsyncSequence->vsyncTime + slope * renderRatePhase;

    return nextAnticipatedVSyncTimeFromLocked(approximateNextVsync - slope / 2);

}
@@ -317,10 +330,10 @@ bool VSyncPredictor::isVSyncInPhaseLocked(nsecs_t timePoint, unsigned divisor) c 
    return vsyncSequence.seq % divisor == 0;

}



void VSyncPredictor::setDivisor(unsigned divisor) {

    ALOGV("%s: %d", __func__, divisor);

void VSyncPredictor::setRenderRate(Fps fps) {

    ALOGV("%s: %s", __func__, to_string(fps).c_str());

    std::lock_guard lock(mMutex);

    mDivisor = divisor;

    mRenderRate = fps;

}



VSyncPredictor::Model VSyncPredictor::getVSyncPredictionModel() const {

services/surfaceflinger/Scheduler/VSyncPredictor.h

+2 −2
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ public: 


    bool isVSyncInPhase(nsecs_t timePoint, Fps frameRate) const final EXCLUDES(mMutex);



    void setDivisor(unsigned divisor) final EXCLUDES(mMutex);

    void setRenderRate(Fps) final EXCLUDES(mMutex);



    void dump(std::string& result) const final EXCLUDES(mMutex);
@@ -106,7 +106,7 @@ private: 
    size_t mLastTimestampIndex GUARDED_BY(mMutex) = 0;

    std::vector<nsecs_t> mTimestamps GUARDED_BY(mMutex);



    unsigned mDivisor GUARDED_BY(mMutex) = 1;

    std::optional<Fps> mRenderRate GUARDED_BY(mMutex);



    mutable std::optional<VsyncSequence> mLastVsyncSequence GUARDED_BY(mMutex);

};

services/surfaceflinger/Scheduler/VSyncTracker.h

+5 −4
Original line number Diff line number Diff line
@@ -80,15 +80,16 @@ public: 
    virtual bool isVSyncInPhase(nsecs_t timePoint, Fps frameRate) const = 0;



    /*

     * Sets a divisor on the rate (which is a multiplier of the period).

     * Sets a render rate on the tracker. If the render rate is not a divisor

     * of the period, the render rate is ignored until the period changes.

     * The tracker will continue to track the vsync timeline and expect it

     * to match the current period, however, nextAnticipatedVSyncTimeFrom will

     * return vsyncs according to the divisor set. Setting a divisor is useful

     * return vsyncs according to the render rate set. Setting a render rate is useful

     * when a display is running at 120Hz but the render frame rate is 60Hz.

     *

     * \param [in] divisor   The rate divisor the tracker should operate at.

     * \param [in] Fps   The render rate the tracker should operate at.

     */

    virtual void setDivisor(unsigned divisor) = 0;

    virtual void setRenderRate(Fps) = 0;



    virtual void dump(std::string& result) const = 0;

services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h

+1 −1
Original line number Diff line number Diff line
@@ -100,7 +100,7 @@ public: 
        return true;

    }



    void setDivisor(unsigned) override {}

    void setRenderRate(Fps) override {}



    nsecs_t nextVSyncTime(nsecs_t timePoint) const {

        if (timePoint % mPeriod == 0) {