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

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

SF: avoid dispatching close vsyncs 

When the vsync period changes, the next vsync callback might be targeting
the same vsync event, that now has a newer time based on the current
period. This means that the client might be woken up for 2 frames within
the same vsync. This change tries to avoid that by making sure that the
vsync callbacks are at least a vsync period apart minus a safe distance.

Test: new unit test
Bug: 235566681
Change-Id: Ifb9b3f8b726976452d5131c8b758d1d5ca0e3639
parent fdff5cde

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

+24 −9
Original line number Diff line number Diff line
@@ -100,14 +100,8 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim 
        return getExpectedCallbackTime(nextVsyncTime, timing);

    }



    bool const alreadyDispatchedForVsync = mLastDispatchTime &&

            ((*mLastDispatchTime + mMinVsyncDistance) >= nextVsyncTime &&

             (*mLastDispatchTime - mMinVsyncDistance) <= nextVsyncTime);

    if (alreadyDispatchedForVsync) {

        nextVsyncTime =

                tracker.nextAnticipatedVSyncTimeFrom(*mLastDispatchTime + mMinVsyncDistance);

    nextVsyncTime = adjustVsyncIfNeeded(tracker, nextVsyncTime);

    nextWakeupTime = nextVsyncTime - timing.workDuration - timing.readyDuration;

    }



    auto const nextReadyTime = nextVsyncTime - timing.readyDuration;

    mScheduleTiming = timing;
@@ -123,6 +117,25 @@ bool VSyncDispatchTimerQueueEntry::hasPendingWorkloadUpdate() const { 
    return mWorkloadUpdateInfo.has_value();

}



nsecs_t VSyncDispatchTimerQueueEntry::adjustVsyncIfNeeded(VSyncTracker& tracker,

                                                          nsecs_t nextVsyncTime) const {

    bool const alreadyDispatchedForVsync = mLastDispatchTime &&

            ((*mLastDispatchTime + mMinVsyncDistance) >= nextVsyncTime &&

             (*mLastDispatchTime - mMinVsyncDistance) <= nextVsyncTime);

    const nsecs_t currentPeriod = tracker.currentPeriod();

    bool const nextVsyncTooClose = mLastDispatchTime &&

            (nextVsyncTime - *mLastDispatchTime + mMinVsyncDistance) <= currentPeriod;

    if (alreadyDispatchedForVsync) {

        return tracker.nextAnticipatedVSyncTimeFrom(*mLastDispatchTime + mMinVsyncDistance);

    }



    if (nextVsyncTooClose) {

        return tracker.nextAnticipatedVSyncTimeFrom(*mLastDispatchTime + currentPeriod);

    }



    return nextVsyncTime;

}



void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) {

    if (!mArmedInfo && !mWorkloadUpdateInfo) {

        return;
@@ -136,7 +149,9 @@ void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) { 
    const auto earliestReadyBy = now + mScheduleTiming.workDuration + mScheduleTiming.readyDuration;

    const auto earliestVsync = std::max(earliestReadyBy, mScheduleTiming.earliestVsync);



    const auto nextVsyncTime = tracker.nextAnticipatedVSyncTimeFrom(earliestVsync);

    const auto nextVsyncTime =

            adjustVsyncIfNeeded(tracker, /*nextVsyncTime*/

                                tracker.nextAnticipatedVSyncTimeFrom(earliestVsync));

    const auto nextReadyTime = nextVsyncTime - mScheduleTiming.readyDuration;

    const auto nextWakeupTime = nextReadyTime - mScheduleTiming.workDuration;

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h

+2 −0
Original line number Diff line number Diff line
@@ -84,6 +84,8 @@ public: 
    void dump(std::string& result) const;



private:

    nsecs_t adjustVsyncIfNeeded(VSyncTracker& tracker, nsecs_t nextVsyncTime) const;



    const std::string mName;

    const VSyncDispatch::Callback mCallback;

services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp

+41 −0
Original line number Diff line number Diff line
@@ -44,6 +44,8 @@ public: 
        ON_CALL(*this, nextAnticipatedVSyncTimeFrom(_))

                .WillByDefault(Invoke(this, &MockVSyncTracker::nextVSyncTime));

        ON_CALL(*this, addVsyncTimestamp(_)).WillByDefault(Return(true));

        ON_CALL(*this, currentPeriod())

                .WillByDefault(Invoke(this, &MockVSyncTracker::getCurrentPeriod));

    }



    MOCK_METHOD1(addVsyncTimestamp, bool(nsecs_t));
@@ -62,6 +64,8 @@ public: 
        return (timePoint - (timePoint % mPeriod) + mPeriod);

    }



    nsecs_t getCurrentPeriod() const { return mPeriod; }



protected:

    nsecs_t const mPeriod;

};
@@ -393,6 +397,43 @@ TEST_F(VSyncDispatchTimerQueueTest, basicTwoAlarmSetting) { 
    EXPECT_THAT(cb1.mCalls[0], Eq(1063));

}



TEST_F(VSyncDispatchTimerQueueTest, noCloseCallbacksAfterPeriodChange) {

    EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(_))

            .Times(4)

            .WillOnce(Return(1000))

            .WillOnce(Return(2000))

            .WillOnce(Return(2500))

            .WillOnce(Return(4000));



    Sequence seq;

    EXPECT_CALL(mMockClock, alarmAt(_, 900)).InSequence(seq);

    EXPECT_CALL(mMockClock, alarmAt(_, 1900)).InSequence(seq);

    EXPECT_CALL(mMockClock, alarmAt(_, 3900)).InSequence(seq);



    CountingCallback cb(mDispatch);



    mDispatch.schedule(cb, {.workDuration = 100, .readyDuration = 0, .earliestVsync = 0});



    advanceToNextCallback();



    ASSERT_THAT(cb.mCalls.size(), Eq(1));

    EXPECT_THAT(cb.mCalls[0], Eq(1000));



    mDispatch.schedule(cb, {.workDuration = 100, .readyDuration = 0, .earliestVsync = 1000});



    advanceToNextCallback();



    ASSERT_THAT(cb.mCalls.size(), Eq(2));

    EXPECT_THAT(cb.mCalls[1], Eq(2000));



    mDispatch.schedule(cb, {.workDuration = 100, .readyDuration = 0, .earliestVsync = 2000});



    advanceToNextCallback();



    ASSERT_THAT(cb.mCalls.size(), Eq(3));

    EXPECT_THAT(cb.mCalls[2], Eq(4000));

}



TEST_F(VSyncDispatchTimerQueueTest, rearmsFaroutTimeoutWhenCancellingCloseOne) {

    EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(_))

            .Times(4)