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

Commit 39d25347 authored by Leon Scroggins's avatar Leon Scroggins
Browse files

Revert "Only call onNewVsyncSchedule if the pacesetter changes" 

This reverts commit d649463a.

Reason for revert: Speculative fix for a regression in SF performance.

Bug: 278987666
Test: APC
Change-Id: I5bf393a2eef4dc7254ccf931a0fab3c67eb315ad
parent d649463a

services/surfaceflinger/Scheduler/Scheduler.cpp

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


        pacesetterVsyncSchedule = promotePacesetterDisplayLocked();

    }

    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));

    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));

}



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


        pacesetterVsyncSchedule = promotePacesetterDisplayLocked();

    }

    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));

    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));

}



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

    }



    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));

    applyNewVsyncSchedule(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 newVsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);

    auto vsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);

    if (const auto pacesetterPtr = pacesetterSelectorPtrLocked()) {

        pacesetterPtr->setIdleTimerCallbacks(

                {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
@@ -714,28 +713,15 @@ 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();

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

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

                                             true /* force */);

    }

    if (oldPacesetterDisplayIdOpt == mPacesetterDisplayId) {

        return nullptr;

    }

    return newVsyncSchedule;

    return vsyncSchedule;

}



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());

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

    onNewVsyncSchedule(vsyncSchedule->getDispatch());

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

    {

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

    }

    for (auto* thread : threads) {

        thread->onNewVsyncSchedule(pacesetterSchedulePtr);

        thread->onNewVsyncSchedule(vsyncSchedule);

    }

}

services/surfaceflinger/Scheduler/Scheduler.h

+1 −3
Original line number Diff line number Diff line
@@ -329,12 +329,10 @@ 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 applyNewVsyncScheduleIfNonNull(std::shared_ptr<VsyncSchedule>) EXCLUDES(mDisplayLock);

    void applyNewVsyncSchedule(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

+0 −19
Original line number Diff line number Diff line
@@ -384,23 +384,4 @@ 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