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
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index af9fc70..6eea7f1 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -510,8 +510,11 @@
std::scoped_lock lock(mDisplayLock);
ftl::FakeGuard guard(kMainThreadContext);
- for (const auto& [id, _] : mDisplays) {
- resyncToHardwareVsyncLocked(id, allowToEnable);
+ for (const auto& [id, display] : mDisplays) {
+ if (display.powerMode != hal::PowerMode::OFF ||
+ !FlagManager::getInstance().multithreaded_present()) {
+ resyncToHardwareVsyncLocked(id, allowToEnable);
+ }
}
}
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 4c6539e..e515895 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -608,6 +608,85 @@
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,
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index b036e99..2a1b88e 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -115,6 +115,15 @@
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; }