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

Commit 792ea802 authored by Leon Scroggins III's avatar Leon Scroggins III
Browse files

Do not startPeriodTransition for OFF displays 

Inspired by discussion in Ida666af9a3a9b2e940c1f861ce3b765f67738f1f.
resyncAllToHardwareVsync calls resyncToHardwareVsyncLocked for *all*
physical displays, even if they are turned off. This can be seen in
dumpsys - for example, on a two-display device on which only one display
has turned on since boot, mPeriodConfirmationInProgress may be set to
1 (true) for the display that is off.

Correct this by checking Scheduler's record of the display's PowerMode.
Skip resyncToHardwareVsyncLocked if the display is OFF. (This matches
Scheduler::onHardwareVsyncRequest's decision of whether to call the
mSchedulerCallback's requestHardwareVsync.) This prevents attempting to
start a period transition, as well as making hardware VSYNCs allowed.

Bug: 255601557
Test: adb shell dumpsys SurfaceFlinger --vsync
Test: atest libsurfaceflinger_unittest:SchedulerTest
Change-Id: I88e7451622c259056d26748c64fcaa0bdcb836b8
parent 4cd9cb2c

services/surfaceflinger/Scheduler/Scheduler.cpp

+5 −2
Original line number Diff line number Diff line
@@ -510,10 +510,13 @@ void Scheduler::resyncAllToHardwareVsync(bool allowToEnable) { 
    std::scoped_lock lock(mDisplayLock);

    ftl::FakeGuard guard(kMainThreadContext);



    for (const auto& [id, _] : mDisplays) {

    for (const auto& [id, display] : mDisplays) {

        if (display.powerMode != hal::PowerMode::OFF ||

            !FlagManager::getInstance().multithreaded_present()) {

            resyncToHardwareVsyncLocked(id, allowToEnable);

        }

    }

}



void Scheduler::resyncToHardwareVsyncLocked(PhysicalDisplayId id, bool allowToEnable,

                                            DisplayModePtr modePtr) {

services/surfaceflinger/tests/unittests/SchedulerTest.cpp

+79 −0
Original line number Diff line number Diff line
@@ -608,6 +608,85 @@ TEST_F(SchedulerTest, nextFrameIntervalTest) { 
                                             TimePoint::fromNs(4500)));

}



TEST_F(SchedulerTest, resyncAllToHardwareVsync) FTL_FAKE_GUARD(kMainThreadContext) {

    // resyncAllToHardwareVsync will result in requesting hardware VSYNC on both displays, since

    // they are both on.

    EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId1, true)).Times(1);

    EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId2, true)).Times(1);



    mScheduler->registerDisplay(kDisplayId2,

                                std::make_shared<RefreshRateSelector>(kDisplay2Modes,

                                                                      kDisplay2Mode60->getId()));

    mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON);

    mScheduler->setDisplayPowerMode(kDisplayId2, hal::PowerMode::ON);



    static constexpr bool kDisallow = true;

    mScheduler->disableHardwareVsync(kDisplayId1, kDisallow);

    mScheduler->disableHardwareVsync(kDisplayId2, kDisallow);



    static constexpr bool kAllowToEnable = true;

    mScheduler->resyncAllToHardwareVsync(kAllowToEnable);

}



TEST_F(SchedulerTest, resyncAllDoNotAllow) FTL_FAKE_GUARD(kMainThreadContext) {

    // Without setting allowToEnable to true, resyncAllToHardwareVsync does not

    // result in requesting hardware VSYNC.

    EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId1, _)).Times(0);



    mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON);



    static constexpr bool kDisallow = true;

    mScheduler->disableHardwareVsync(kDisplayId1, kDisallow);



    static constexpr bool kAllowToEnable = false;

    mScheduler->resyncAllToHardwareVsync(kAllowToEnable);

}



TEST_F(SchedulerTest, resyncAllSkipsOffDisplays) FTL_FAKE_GUARD(kMainThreadContext) {

    SET_FLAG_FOR_TEST(flags::multithreaded_present, true);



    // resyncAllToHardwareVsync will result in requesting hardware VSYNC on display 1, which is on,

    // but not on display 2, which is off.

    EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId1, true)).Times(1);

    EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId2, _)).Times(0);



    mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON);



    mScheduler->registerDisplay(kDisplayId2,

                                std::make_shared<RefreshRateSelector>(kDisplay2Modes,

                                                                      kDisplay2Mode60->getId()));

    ASSERT_EQ(hal::PowerMode::OFF, mScheduler->getDisplayPowerMode(kDisplayId2));



    static constexpr bool kDisallow = true;

    mScheduler->disableHardwareVsync(kDisplayId1, kDisallow);

    mScheduler->disableHardwareVsync(kDisplayId2, kDisallow);



    static constexpr bool kAllowToEnable = true;

    mScheduler->resyncAllToHardwareVsync(kAllowToEnable);

}



TEST_F(SchedulerTest, resyncAllLegacyAppliesToOffDisplays) FTL_FAKE_GUARD(kMainThreadContext) {

    SET_FLAG_FOR_TEST(flags::multithreaded_present, false);



    // In the legacy code, prior to the flag, resync applied to OFF displays.

    EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId1, true)).Times(1);

    EXPECT_CALL(mScheduler->mockRequestHardwareVsync, Call(kDisplayId2, true)).Times(1);



    mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON);



    mScheduler->registerDisplay(kDisplayId2,

                                std::make_shared<RefreshRateSelector>(kDisplay2Modes,

                                                                      kDisplay2Mode60->getId()));

    ASSERT_EQ(hal::PowerMode::OFF, mScheduler->getDisplayPowerMode(kDisplayId2));



    static constexpr bool kDisallow = true;

    mScheduler->disableHardwareVsync(kDisplayId1, kDisallow);

    mScheduler->disableHardwareVsync(kDisplayId2, kDisallow);



    static constexpr bool kAllowToEnable = true;

    mScheduler->resyncAllToHardwareVsync(kAllowToEnable);

}



class AttachedChoreographerTest : public SchedulerTest {

protected:

    void frameRateTestScenario(Fps layerFps, int8_t frameRateCompatibility, Fps displayFps,

services/surfaceflinger/tests/unittests/TestableScheduler.h

+9 −0
Original line number Diff line number Diff line
@@ -115,6 +115,15 @@ public: 
        Scheduler::setPacesetterDisplay(displayId);

    }



    std::optional<hal::PowerMode> getDisplayPowerMode(PhysicalDisplayId id) {

        ftl::FakeGuard guard1(kMainThreadContext);

        ftl::FakeGuard guard2(mDisplayLock);

        return mDisplays.get(id).transform(

                [](const Display& display) { return display.powerMode; });

    }



    using Scheduler::resyncAllToHardwareVsync;



    auto& mutableAppConnectionHandle() { return mAppConnectionHandle; }

    auto& mutableLayerHistory() { return mLayerHistory; }

    auto& mutableAttachedChoreographers() { return mAttachedChoreographers; }