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

Commit f77025c8 authored by Kevin DuBois's avatar Kevin DuBois
Browse files

SF: VSyncReactor change offsets at correct time 

The VRR/MRR timing loop (currently flagged off) was changing
offsets incorrectly on the first hwvsync signal after initiating
a rate change. With HWC2.3 and prior, the correct strategy for
DispSync to employ is to enable hwvsync, and await the first observed
signal at the new rate. This patch makes the new system apply the
updated offsets at the correct time.

Fixes: b/146455831
Test: 2 new, 3 fixed unit tests
Test: boot on coral with integrated patches, fiddle with rate changes.

Change-Id: Iafae2e5112a5015441405055159538feb6c23a4b
parent 69a2660a

services/surfaceflinger/Scheduler/VSyncReactor.cpp

+35 −15
Original line number Diff line number Diff line
@@ -154,7 +154,7 @@ bool VSyncReactor::addPresentFence(const std::shared_ptr<FenceTime>& fence) { 
        mTracker->addVsyncTimestamp(signalTime);

    }



    return false; // TODO(b/144707443): add policy for turning on HWVsync.

    return mMoreSamplesNeeded;

}



void VSyncReactor::setIgnorePresentFences(bool ignoration) {
@@ -176,14 +176,9 @@ nsecs_t VSyncReactor::expectedPresentTime() { 
}



void VSyncReactor::setPeriod(nsecs_t period) {

    mTracker->setPeriod(period);

    {

        std::lock_guard<std::mutex> lk(mMutex);

        mPeriodChangeInProgress = true;

        for (auto& entry : mCallbacks) {

            entry.second->setPeriod(period);

        }

    }

    std::lock_guard lk(mMutex);

    mLastHwVsync.reset();

    mPeriodTransitioningTo = period;

}



nsecs_t VSyncReactor::getPeriod() {
@@ -194,15 +189,40 @@ void VSyncReactor::beginResync() {} 


void VSyncReactor::endResync() {}



bool VSyncReactor::periodChangeDetected(nsecs_t vsync_timestamp) {

    if (!mLastHwVsync || !mPeriodTransitioningTo) {

        return false;

    }

    auto const distance = vsync_timestamp - *mLastHwVsync;

    return std::abs(distance - *mPeriodTransitioningTo) < std::abs(distance - getPeriod());

}



bool VSyncReactor::addResyncSample(nsecs_t timestamp, bool* periodFlushed) {

    assert(periodFlushed);

    mTracker->addVsyncTimestamp(timestamp);

    {



    std::lock_guard<std::mutex> lk(mMutex);

        *periodFlushed = mPeriodChangeInProgress;

        mPeriodChangeInProgress = false;

    if (periodChangeDetected(timestamp)) {

        mMoreSamplesNeeded = false;

        *periodFlushed = true;



        mTracker->setPeriod(*mPeriodTransitioningTo);

        for (auto& entry : mCallbacks) {

            entry.second->setPeriod(*mPeriodTransitioningTo);

        }

    return false;



        mPeriodTransitioningTo.reset();

        mLastHwVsync.reset();

    } else if (mPeriodTransitioningTo) {

        mLastHwVsync = timestamp;

        mMoreSamplesNeeded = true;

        *periodFlushed = false;

    } else {

        mMoreSamplesNeeded = false;

        *periodFlushed = false;

    }



    mTracker->addVsyncTimestamp(timestamp);

    return mMoreSamplesNeeded;

}



status_t VSyncReactor::addEventListener(const char* name, nsecs_t phase,

services/surfaceflinger/Scheduler/VSyncReactor.h

+7 −1
Original line number Diff line number Diff line
@@ -61,6 +61,8 @@ public: 
    void reset() final;



private:

    bool periodChangeDetected(nsecs_t vsync_timestamp) REQUIRES(mMutex);



    std::unique_ptr<Clock> const mClock;

    std::unique_ptr<VSyncTracker> const mTracker;

    std::unique_ptr<VSyncDispatch> const mDispatch;
@@ -69,7 +71,11 @@ private: 
    std::mutex mMutex;

    bool mIgnorePresentFences GUARDED_BY(mMutex) = false;

    std::vector<std::shared_ptr<FenceTime>> mUnfiredFences GUARDED_BY(mMutex);

    bool mPeriodChangeInProgress GUARDED_BY(mMutex) = false;



    bool mMoreSamplesNeeded GUARDED_BY(mMutex) = false;

    std::optional<nsecs_t> mPeriodTransitioningTo GUARDED_BY(mMutex);

    std::optional<nsecs_t> mLastHwVsync GUARDED_BY(mMutex);



    std::unordered_map<DispSync::Callback*, std::unique_ptr<CallbackRepeater>> mCallbacks

            GUARDED_BY(mMutex);

};

services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp

+77 −13
Original line number Diff line number Diff line
@@ -159,7 +159,6 @@ protected: 
    static constexpr nsecs_t mPhase = 3000;

    static constexpr nsecs_t mAnotherPhase = 5200;

    static constexpr nsecs_t period = 10000;

    static constexpr nsecs_t mAnotherPeriod = 23333;

    static constexpr nsecs_t mFakeCbTime = 2093;

    static constexpr nsecs_t mFakeNow = 2214;

    static constexpr const char mName[] = "callbacky";
@@ -274,10 +273,38 @@ TEST_F(VSyncReactorTest, getPeriod) { 
    EXPECT_THAT(mReactor.getPeriod(), Eq(fakePeriod));

}



TEST_F(VSyncReactorTest, setPeriod) {

    nsecs_t const fakePeriod = 4098;

    EXPECT_CALL(*mMockTracker, setPeriod(fakePeriod));

    mReactor.setPeriod(fakePeriod);

TEST_F(VSyncReactorTest, setPeriodCalledOnceConfirmedChange) {

    nsecs_t const newPeriod = 5000;

    EXPECT_CALL(*mMockTracker, setPeriod(_)).Times(0);

    mReactor.setPeriod(newPeriod);



    bool periodFlushed = true;

    EXPECT_TRUE(mReactor.addResyncSample(10000, &periodFlushed));

    EXPECT_FALSE(periodFlushed);



    EXPECT_TRUE(mReactor.addResyncSample(20000, &periodFlushed));

    EXPECT_FALSE(periodFlushed);



    Mock::VerifyAndClearExpectations(mMockTracker.get());

    EXPECT_CALL(*mMockTracker, setPeriod(newPeriod)).Times(1);



    EXPECT_FALSE(mReactor.addResyncSample(25000, &periodFlushed));

    EXPECT_TRUE(periodFlushed);

}



TEST_F(VSyncReactorTest, setPeriodCalledFirstTwoEventsNewPeriod) {

    nsecs_t const newPeriod = 5000;

    EXPECT_CALL(*mMockTracker, setPeriod(_)).Times(0);

    mReactor.setPeriod(newPeriod);



    bool periodFlushed = true;

    EXPECT_TRUE(mReactor.addResyncSample(5000, &periodFlushed));

    EXPECT_FALSE(periodFlushed);

    Mock::VerifyAndClearExpectations(mMockTracker.get());



    EXPECT_CALL(*mMockTracker, setPeriod(newPeriod)).Times(1);

    EXPECT_FALSE(mReactor.addResyncSample(10000, &periodFlushed));

    EXPECT_TRUE(periodFlushed);

}



TEST_F(VSyncReactorTest, addResyncSampleTypical) {
@@ -291,12 +318,43 @@ TEST_F(VSyncReactorTest, addResyncSampleTypical) { 


TEST_F(VSyncReactorTest, addResyncSamplePeriodChanges) {

    bool periodFlushed = false;

    nsecs_t const fakeTimestamp = 4398;

    nsecs_t const newPeriod = 3490;

    EXPECT_CALL(*mMockTracker, addVsyncTimestamp(fakeTimestamp));

    nsecs_t const newPeriod = 4000;



    mReactor.setPeriod(newPeriod);

    EXPECT_FALSE(mReactor.addResyncSample(fakeTimestamp, &periodFlushed));



    auto time = 0;

    auto constexpr numTimestampSubmissions = 10;

    for (auto i = 0; i < numTimestampSubmissions; i++) {

        time += period;

        EXPECT_TRUE(mReactor.addResyncSample(time, &periodFlushed));

        EXPECT_FALSE(periodFlushed);

    }



    time += newPeriod;

    EXPECT_FALSE(mReactor.addResyncSample(time, &periodFlushed));

    EXPECT_TRUE(periodFlushed);



    for (auto i = 0; i < numTimestampSubmissions; i++) {

        time += newPeriod;

        EXPECT_FALSE(mReactor.addResyncSample(time, &periodFlushed));

        EXPECT_FALSE(periodFlushed);

    }

}



TEST_F(VSyncReactorTest, addPresentFenceWhileAwaitingPeriodConfirmationRequestsHwVsync) {

    auto time = 0;

    bool periodFlushed = false;

    nsecs_t const newPeriod = 4000;

    mReactor.setPeriod(newPeriod);



    time += period;

    mReactor.addResyncSample(time, &periodFlushed);

    EXPECT_TRUE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));



    time += newPeriod;

    mReactor.addResyncSample(time, &periodFlushed);



    EXPECT_FALSE(mReactor.addPresentFence(generateSignalledFenceWithTime(0)));

}



static nsecs_t computeWorkload(nsecs_t period, nsecs_t phase) {
@@ -399,20 +457,26 @@ TEST_F(VSyncReactorTest, addEventListenerChangePeriod) { 
    mReactor.addEventListener(mName, mAnotherPhase, &outerCb, lastCallbackTime);

}



TEST_F(VSyncReactorTest, changingPeriodChangesOfsetsOnNextCb) {

TEST_F(VSyncReactorTest, changingPeriodChangesOffsetsOnNextCb) {

    static constexpr nsecs_t anotherPeriod = 23333;

    Sequence seq;

    EXPECT_CALL(*mMockDispatch, registerCallback(_, std::string(mName)))

            .InSequence(seq)

            .WillOnce(Return(mFakeToken));

    EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mPhase), mFakeNow))

            .InSequence(seq);

    EXPECT_CALL(*mMockTracker, setPeriod(mAnotherPeriod));

    EXPECT_CALL(*mMockTracker, setPeriod(anotherPeriod));

    EXPECT_CALL(*mMockDispatch,

                schedule(mFakeToken, computeWorkload(mAnotherPeriod, mPhase), mFakeNow))

                schedule(mFakeToken, computeWorkload(anotherPeriod, mPhase), mFakeNow))

            .InSequence(seq);



    mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);

    mReactor.setPeriod(mAnotherPeriod);



    bool periodFlushed = false;

    mReactor.setPeriod(anotherPeriod);

    EXPECT_TRUE(mReactor.addResyncSample(anotherPeriod, &periodFlushed));

    EXPECT_FALSE(mReactor.addResyncSample(anotherPeriod * 2, &periodFlushed));



    mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);

}