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

Commit d649463a authored by Leon Scroggins III's avatar Leon Scroggins III
Browse files

Only call onNewVsyncSchedule if the pacesetter changes 

When re-calculating the pacesetter display, we may end up with the same
display, and therefore the same VsyncSchedule. In that case, we don't
need to apply the new one.

Bug: 276367387
Test: atest libsurfaceflinger_unittest:SchedulerTest
Change-Id: I76750ffdba6c3d790a98b65aabbc8b13eab3b4ae
parent fb7ed56c

services/surfaceflinger/Scheduler/Scheduler.cpp

+24 −10
Original line number Diff line number Diff line
@@ -130,7 +130,7 @@ void Scheduler::registerDisplayInternal(PhysicalDisplayId displayId, 


        pacesetterVsyncSchedule = promotePacesetterDisplayLocked();

    }

    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));

    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));

}



void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) {
@@ -149,7 +149,7 @@ void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) { 


        pacesetterVsyncSchedule = promotePacesetterDisplayLocked();

    }

    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));

    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));

}



void Scheduler::run() {
@@ -693,16 +693,17 @@ void Scheduler::promotePacesetterDisplay(std::optional<PhysicalDisplayId> pacese 
        pacesetterVsyncSchedule = promotePacesetterDisplayLocked(pacesetterIdOpt);

    }



    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));

    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));

}



std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked(

        std::optional<PhysicalDisplayId> pacesetterIdOpt) {

    // TODO(b/241286431): Choose the pacesetter display.

    const auto oldPacesetterDisplayIdOpt = mPacesetterDisplayId;

    mPacesetterDisplayId = pacesetterIdOpt.value_or(mRefreshRateSelectors.begin()->first);

    ALOGI("Display %s is the pacesetter", to_string(*mPacesetterDisplayId).c_str());



    auto vsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);

    auto newVsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);

    if (const auto pacesetterPtr = pacesetterSelectorPtrLocked()) {

        pacesetterPtr->setIdleTimerCallbacks(

                {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
@@ -713,15 +714,28 @@ std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked( 


        pacesetterPtr->startIdleTimer();



        // Track the new period, which may have changed due to switching to a

        // new pacesetter or due to a hotplug event. In the former case, this

        // is important so that VSYNC modulation does not get stuck in the

        // initiated state if a transition started on the old pacesetter.

        const Fps refreshRate = pacesetterPtr->getActiveMode().modePtr->getFps();

        vsyncSchedule->startPeriodTransition(mSchedulerCallback, refreshRate.getPeriod(),

        newVsyncSchedule->startPeriodTransition(mSchedulerCallback, refreshRate.getPeriod(),

                                                true /* force */);

    }

    return vsyncSchedule;

    if (oldPacesetterDisplayIdOpt == mPacesetterDisplayId) {

        return nullptr;

    }

    return newVsyncSchedule;

}



void Scheduler::applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule> vsyncSchedule) {

    onNewVsyncSchedule(vsyncSchedule->getDispatch());

void Scheduler::applyNewVsyncScheduleIfNonNull(

        std::shared_ptr<VsyncSchedule> pacesetterSchedulePtr) {

    if (!pacesetterSchedulePtr) {

        // The pacesetter has not changed, so there is no new VsyncSchedule to

        // apply.

        return;

    }

    onNewVsyncSchedule(pacesetterSchedulePtr->getDispatch());

    std::vector<android::EventThread*> threads;

    {

        std::lock_guard<std::mutex> lock(mConnectionsLock);
@@ -731,7 +745,7 @@ void Scheduler::applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule> vsyncSchedu 
        }

    }

    for (auto* thread : threads) {

        thread->onNewVsyncSchedule(vsyncSchedule);

        thread->onNewVsyncSchedule(pacesetterSchedulePtr);

    }

}

services/surfaceflinger/Scheduler/Scheduler.h

+3 −1
Original line number Diff line number Diff line
@@ -329,10 +329,12 @@ private: 
    // MessageQueue and EventThread need to use the new pacesetter's

    // VsyncSchedule, and this must happen while mDisplayLock is *not* locked,

    // or else we may deadlock with EventThread.

    // Returns the new pacesetter's VsyncSchedule, or null if the pacesetter is

    // unchanged.

    std::shared_ptr<VsyncSchedule> promotePacesetterDisplayLocked(

            std::optional<PhysicalDisplayId> pacesetterIdOpt = std::nullopt)

            REQUIRES(kMainThreadContext, mDisplayLock);

    void applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule>) EXCLUDES(mDisplayLock);

    void applyNewVsyncScheduleIfNonNull(std::shared_ptr<VsyncSchedule>) EXCLUDES(mDisplayLock);



    // Blocks until the pacesetter's idle timer thread exits. `mDisplayLock` must not be locked by

    // the caller on the main thread to avoid deadlock, since the timer thread locks it before exit.

services/surfaceflinger/tests/unittests/SchedulerTest.cpp

+19 −0
Original line number Diff line number Diff line
@@ -384,4 +384,23 @@ TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) { 
    }

}



TEST_F(SchedulerTest, changingPacesetterChangesVsyncSchedule) {

    // Add a second display so we can change the pacesetter.

    mScheduler->registerDisplay(kDisplayId2,

                                std::make_shared<RefreshRateSelector>(kDisplay2Modes,

                                                                      kDisplay2Mode60->getId()));

    // Ensure that the pacesetter is the one we expect.

    mScheduler->setPacesetterDisplay(kDisplayId1);



    // Switching to the other will call onNewVsyncSchedule.

    EXPECT_CALL(*mEventThread, onNewVsyncSchedule(mScheduler->getVsyncSchedule(kDisplayId2)))

            .Times(1);

    mScheduler->setPacesetterDisplay(kDisplayId2);

}



TEST_F(SchedulerTest, promotingSamePacesetterDoesNotChangeVsyncSchedule) {

    EXPECT_CALL(*mEventThread, onNewVsyncSchedule(_)).Times(0);

    mScheduler->setPacesetterDisplay(kDisplayId1);

}



} // namespace android::scheduler