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

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

SF: VSyncReactor ensure period change confirmation 

(Code being corrected is flagged off, no production problems)

When a period change is happening, system turns on hwvsync and uses
the first vsync signal timed at the new vsync rate as confirmation
that the rate has indeed changed (and then things like modifying the
offsets occurs).

If a presentation fence was added to the VSyncReactor system before
the first hwvsync sample arrived, this would result in the period never
being confirmed, and would result in incorrect offsets and period
estimations, resulting in bad jank scores.

Test: 3 unit tests
Test: uibench run
Fixes: 147321414

Change-Id: Idbc87529e60333df5180de585dbba3525396562b
parent 842e9df6

services/surfaceflinger/Scheduler/VSyncReactor.cpp

+18 −6
Original line number Diff line number Diff line
@@ -175,10 +175,25 @@ nsecs_t VSyncReactor::expectedPresentTime() { 
    return mTracker->nextAnticipatedVSyncTimeFrom(mClock->now());

}



void VSyncReactor::startPeriodTransition(nsecs_t newPeriod) {

    mPeriodTransitioningTo = newPeriod;

    mMoreSamplesNeeded = true;

}



void VSyncReactor::endPeriodTransition() {

    mPeriodTransitioningTo.reset();

    mLastHwVsync.reset();

    mMoreSamplesNeeded = false;

}



void VSyncReactor::setPeriod(nsecs_t period) {

    std::lock_guard lk(mMutex);

    mLastHwVsync.reset();

    mPeriodTransitioningTo = period;

    if (period == getPeriod()) {

        endPeriodTransition();

    } else {

        startPeriodTransition(period);

    }

}



nsecs_t VSyncReactor::getPeriod() {
@@ -202,16 +217,13 @@ bool VSyncReactor::addResyncSample(nsecs_t timestamp, bool* periodFlushed) { 


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

    if (periodChangeDetected(timestamp)) {

        mMoreSamplesNeeded = false;

        *periodFlushed = true;



        mTracker->setPeriod(*mPeriodTransitioningTo);

        for (auto& entry : mCallbacks) {

            entry.second->setPeriod(*mPeriodTransitioningTo);

        }



        mPeriodTransitioningTo.reset();

        mLastHwVsync.reset();

        endPeriodTransition();

        *periodFlushed = true;

    } else if (mPeriodTransitioningTo) {

        mLastHwVsync = timestamp;

        mMoreSamplesNeeded = true;

services/surfaceflinger/Scheduler/VSyncReactor.h

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



private:

    void startPeriodTransition(nsecs_t newPeriod) REQUIRES(mMutex);

    void endPeriodTransition() REQUIRES(mMutex);

    bool periodChangeDetected(nsecs_t vsync_timestamp) REQUIRES(mMutex);



    std::unique_ptr<Clock> const mClock;

services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp

+40 −0
Original line number Diff line number Diff line
@@ -292,6 +292,46 @@ TEST_F(VSyncReactorTest, setPeriodCalledOnceConfirmedChange) { 
    EXPECT_TRUE(periodFlushed);

}



TEST_F(VSyncReactorTest, changingPeriodBackAbortsConfirmationProcess) {

    nsecs_t sampleTime = 0;

    nsecs_t const newPeriod = 5000;

    mReactor.setPeriod(newPeriod);

    bool periodFlushed = true;

    EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed));

    EXPECT_FALSE(periodFlushed);



    EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed));

    EXPECT_FALSE(periodFlushed);



    mReactor.setPeriod(period);

    EXPECT_FALSE(mReactor.addResyncSample(sampleTime += period, &periodFlushed));

    EXPECT_FALSE(periodFlushed);

}



TEST_F(VSyncReactorTest, changingToAThirdPeriodWillWaitForLastPeriod) {

    nsecs_t sampleTime = 0;

    nsecs_t const secondPeriod = 5000;

    nsecs_t const thirdPeriod = 2000;



    mReactor.setPeriod(secondPeriod);

    bool periodFlushed = true;

    EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed));

    EXPECT_FALSE(periodFlushed);

    EXPECT_TRUE(mReactor.addResyncSample(sampleTime += period, &periodFlushed));

    EXPECT_FALSE(periodFlushed);

    mReactor.setPeriod(thirdPeriod);

    EXPECT_TRUE(mReactor.addResyncSample(sampleTime += secondPeriod, &periodFlushed));

    EXPECT_FALSE(periodFlushed);

    EXPECT_FALSE(mReactor.addResyncSample(sampleTime += thirdPeriod, &periodFlushed));

    EXPECT_TRUE(periodFlushed);

}



TEST_F(VSyncReactorTest, presentFenceAdditionDoesNotInterruptConfirmationProcess) {

    nsecs_t const newPeriod = 5000;

    mReactor.setPeriod(newPeriod);

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

}



TEST_F(VSyncReactorTest, setPeriodCalledFirstTwoEventsNewPeriod) {

    nsecs_t const newPeriod = 5000;

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