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

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

SF: use the new work duration when avoiding a vsync skip 

When flags::dont_skip_on_early() is set, the scheduler will avoid
skipping a vsync and instead try to present it even if the work
duration is longer than the time to the next vsync. This CL fixes
a bug where we used the old work duration instead of the new one,
leading to starting the nex frame too late (and missing it).

Bug: 273702768
Bug: 307912181
Test: presubmit (including unit test fixes)
Change-Id: I990af3b56a2f345dd192b17f09673a8710a742fc
parent b06d4130

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

+11 −12
Original line number Diff line number Diff line
@@ -38,19 +38,16 @@ using base::StringAppendF; 


namespace {



nsecs_t getExpectedCallbackTime(nsecs_t now, nsecs_t nextVsyncTime,

nsecs_t getExpectedCallbackTime(nsecs_t nextVsyncTime,

                                const VSyncDispatch::ScheduleTiming& timing) {

    const auto expectedCallbackTime = nextVsyncTime - timing.readyDuration - timing.workDuration;

    const auto baseTime =

            FlagManager::getInstance().dont_skip_on_early() ? now : expectedCallbackTime;

    return std::max(baseTime, expectedCallbackTime);

    return nextVsyncTime - timing.readyDuration - timing.workDuration;

}



nsecs_t getExpectedCallbackTime(VSyncTracker& tracker, nsecs_t now,

                                const VSyncDispatch::ScheduleTiming& timing) {

    const auto nextVsyncTime = tracker.nextAnticipatedVSyncTimeFrom(

            std::max(timing.earliestVsync, now + timing.workDuration + timing.readyDuration));

    return getExpectedCallbackTime(now, nextVsyncTime, timing);

    return getExpectedCallbackTime(nextVsyncTime, timing);

}



} // namespace
@@ -106,21 +103,23 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim 
            mArmedInfo && ((nextWakeupTime > (mArmedInfo->mActualWakeupTime + mMinVsyncDistance)));

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

        if (wouldSkipAVsyncTarget || wouldSkipAWakeup) {

            return getExpectedCallbackTime(now, mArmedInfo->mActualVsyncTime, timing);

            nextVsyncTime = mArmedInfo->mActualVsyncTime;

        } else {

            nextVsyncTime = adjustVsyncIfNeeded(tracker, nextVsyncTime);

        }

        nextWakeupTime = std::max(now, nextVsyncTime - timing.workDuration - timing.readyDuration);

    } else {

        if (wouldSkipAVsyncTarget && wouldSkipAWakeup) {

            return getExpectedCallbackTime(now, nextVsyncTime, timing);

        }

            return getExpectedCallbackTime(nextVsyncTime, timing);

        }



        nextVsyncTime = adjustVsyncIfNeeded(tracker, nextVsyncTime);

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

    }



    auto const nextReadyTime = nextVsyncTime - timing.readyDuration;

    mScheduleTiming = timing;

    mArmedInfo = {nextWakeupTime, nextVsyncTime, nextReadyTime};

    return getExpectedCallbackTime(now, nextVsyncTime, timing);

    return nextWakeupTime;

}



void VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate(VSyncDispatch::ScheduleTiming timing) {

services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp

+16 −4
Original line number Diff line number Diff line
@@ -783,7 +783,9 @@ TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipASchedule 
TEST_F(VSyncDispatchTimerQueueTest, movesCallbackBackwardsAndSkipAScheduledTargetVSync) {

    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);



    EXPECT_CALL(mMockClock, alarmAt(_, 500));

    Sequence seq;

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

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

    CountingCallback cb(mDispatch);

    auto result =

            mDispatch->schedule(cb,
@@ -873,7 +875,9 @@ TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState) 
TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesAffectSchedulingState) {

    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);



    EXPECT_CALL(mMockClock, alarmAt(_, 600));

    Sequence seq;

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

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



    CountingCallback cb(mDispatch);

    auto result =
@@ -1119,6 +1123,7 @@ TEST_F(VSyncDispatchTimerQueueTest, doesNotUpdatesVsyncTimeForCloseWakeupTime) { 


    Sequence seq;

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

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



    CountingCallback cb(mDispatch);
@@ -1132,7 +1137,7 @@ TEST_F(VSyncDispatchTimerQueueTest, doesNotUpdatesVsyncTimeForCloseWakeupTime) { 
    ASSERT_THAT(cb.mCalls.size(), Eq(1));

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

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

    EXPECT_THAT(cb.mWakeupTime[0], Eq(600));

    EXPECT_THAT(cb.mWakeupTime[0], Eq(0));

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

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

}
@@ -1161,7 +1166,9 @@ TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) { 
TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) {

    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);



    EXPECT_CALL(mMockClock, alarmAt(_, 500));

    Sequence seq;

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

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

    CountingCallback cb(mDispatch);

    auto result =

            mDispatch->schedule(cb,
@@ -1177,6 +1184,11 @@ TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) { 


    advanceToNextCallback();

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

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

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

    EXPECT_THAT(cb.mWakeupTime[0], Eq(300));

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

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

}



class VSyncDispatchTimerQueueEntryTest : public testing::Test {