SF: Fix pacesetter promotion for folded mirroring
The pacesetter display is demoted/promoted in response to a hotplug. On
foldables, the promoted display was hard-coded to the inner display, so
a hotplug while folded would incorrectly use that powered-off display as
the pacesetter, causing system-wide jank until the next fold/unfold.
Fixes: 347248313
Flag: EXEMPT bugfix
Test: Connect and disconnect external display while folded.
Test: Fold and unfold while external display is connected.
Test: Pacesetter is still correct on folded/unfolded boot.
Change-Id: Id5cb29c3cbaa8ed455a15d8be3a32e79a470cce5
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 60681a2..26e11e5 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -123,19 +123,22 @@
promotePacesetterDisplay(pacesetterIdOpt);
}
-void Scheduler::registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr) {
+void Scheduler::registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr,
+ PhysicalDisplayId activeDisplayId) {
auto schedulePtr =
std::make_shared<VsyncSchedule>(selectorPtr->getActiveMode().modePtr, mFeatures,
[this](PhysicalDisplayId id, bool enable) {
onHardwareVsyncRequest(id, enable);
});
- registerDisplayInternal(displayId, std::move(selectorPtr), std::move(schedulePtr));
+ registerDisplayInternal(displayId, std::move(selectorPtr), std::move(schedulePtr),
+ activeDisplayId);
}
void Scheduler::registerDisplayInternal(PhysicalDisplayId displayId,
RefreshRateSelectorPtr selectorPtr,
- VsyncSchedulePtr schedulePtr) {
+ VsyncSchedulePtr schedulePtr,
+ PhysicalDisplayId activeDisplayId) {
demotePacesetterDisplay();
auto [pacesetterVsyncSchedule, isNew] = [&]() FTL_FAKE_GUARD(kMainThreadContext) {
@@ -145,7 +148,7 @@
std::move(schedulePtr), mFeatures)
.second;
- return std::make_pair(promotePacesetterDisplayLocked(), isNew);
+ return std::make_pair(promotePacesetterDisplayLocked(activeDisplayId), isNew);
}();
applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
@@ -158,7 +161,9 @@
dispatchHotplug(displayId, Hotplug::Connected);
}
-void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) {
+void Scheduler::unregisterDisplay(PhysicalDisplayId displayId, PhysicalDisplayId activeDisplayId) {
+ LOG_ALWAYS_FATAL_IF(displayId == activeDisplayId, "Cannot unregister the active display!");
+
dispatchHotplug(displayId, Hotplug::Disconnected);
demotePacesetterDisplay();
@@ -173,7 +178,7 @@
// headless virtual display.)
LOG_ALWAYS_FATAL_IF(mDisplays.empty(), "Cannot unregister all displays!");
- pacesetterVsyncSchedule = promotePacesetterDisplayLocked();
+ pacesetterVsyncSchedule = promotePacesetterDisplayLocked(activeDisplayId);
}
applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
}
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index ccaa05f..1a4aa79 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -101,9 +101,16 @@
using ConstVsyncSchedulePtr = std::shared_ptr<const VsyncSchedule>;
using VsyncSchedulePtr = std::shared_ptr<VsyncSchedule>;
- void registerDisplay(PhysicalDisplayId, RefreshRateSelectorPtr) REQUIRES(kMainThreadContext)
+ // After registration/unregistration, `activeDisplayId` is promoted to pacesetter. Note that the
+ // active display is never unregistered, since hotplug disconnect never happens for activatable
+ // displays, i.e. a foldable's internal displays or otherwise the (internal or external) primary
+ // display.
+ // TODO: b/255635821 - Remove active display parameters.
+ void registerDisplay(PhysicalDisplayId, RefreshRateSelectorPtr,
+ PhysicalDisplayId activeDisplayId) REQUIRES(kMainThreadContext)
EXCLUDES(mDisplayLock);
- void unregisterDisplay(PhysicalDisplayId) REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock);
+ void unregisterDisplay(PhysicalDisplayId, PhysicalDisplayId activeDisplayId)
+ REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock);
void run();
@@ -390,8 +397,9 @@
// the caller on the main thread to avoid deadlock, since the timer thread locks it before exit.
void demotePacesetterDisplay() REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock, mPolicyLock);
- void registerDisplayInternal(PhysicalDisplayId, RefreshRateSelectorPtr, VsyncSchedulePtr)
- REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock);
+ void registerDisplayInternal(PhysicalDisplayId, RefreshRateSelectorPtr, VsyncSchedulePtr,
+ PhysicalDisplayId activeDisplayId) REQUIRES(kMainThreadContext)
+ EXCLUDES(mDisplayLock);
struct Policy;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 596ec12..5fa6d26 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3855,7 +3855,8 @@
ftl::FakeGuard guard(kMainThreadContext);
// For hotplug reconnect, renew the registration since display modes have been reloaded.
- mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector());
+ mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector(),
+ mActiveDisplayId);
}
if (display->isVirtual()) {
@@ -3894,7 +3895,7 @@
if (display->isVirtual()) {
releaseVirtualDisplay(display->getVirtualId());
} else {
- mScheduler->unregisterDisplay(display->getPhysicalId());
+ mScheduler->unregisterDisplay(display->getPhysicalId(), mActiveDisplayId);
}
}
@@ -4506,7 +4507,8 @@
getFactory(), activeRefreshRate, *mTimeStats);
// The pacesetter must be registered before EventThread creation below.
- mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector());
+ mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector(),
+ mActiveDisplayId);
if (FlagManager::getInstance().vrr_config()) {
mScheduler->setRenderRate(display->getPhysicalId(), activeMode.fps,
/*applyImmediately*/ true);
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 4fb0690..fc54a8b 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -343,12 +343,15 @@
}
TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) {
+ constexpr PhysicalDisplayId kActiveDisplayId = kDisplayId1;
mScheduler->registerDisplay(kDisplayId1,
std::make_shared<RefreshRateSelector>(kDisplay1Modes,
- kDisplay1Mode60->getId()));
+ kDisplay1Mode60->getId()),
+ kActiveDisplayId);
mScheduler->registerDisplay(kDisplayId2,
std::make_shared<RefreshRateSelector>(kDisplay2Modes,
- kDisplay2Mode60->getId()));
+ kDisplay2Mode60->getId()),
+ kActiveDisplayId);
mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON);
mScheduler->setDisplayPowerMode(kDisplayId2, hal::PowerMode::ON);
@@ -411,10 +414,10 @@
{
// The kDisplayId3 does not support 120Hz, The pacesetter display rate is chosen to be 120
// Hz. In this case only the display kDisplayId3 choose 60Hz as it does not support 120Hz.
- mScheduler
- ->registerDisplay(kDisplayId3,
- std::make_shared<RefreshRateSelector>(kDisplay3Modes,
- kDisplay3Mode60->getId()));
+ mScheduler->registerDisplay(kDisplayId3,
+ std::make_shared<RefreshRateSelector>(kDisplay3Modes,
+ kDisplay3Mode60->getId()),
+ kActiveDisplayId);
mScheduler->setDisplayPowerMode(kDisplayId3, hal::PowerMode::ON);
const GlobalSignals globalSignals = {.touch = true};
@@ -457,12 +460,15 @@
}
TEST_F(SchedulerTest, onFrameSignalMultipleDisplays) {
+ constexpr PhysicalDisplayId kActiveDisplayId = kDisplayId1;
mScheduler->registerDisplay(kDisplayId1,
std::make_shared<RefreshRateSelector>(kDisplay1Modes,
- kDisplay1Mode60->getId()));
+ kDisplay1Mode60->getId()),
+ kActiveDisplayId);
mScheduler->registerDisplay(kDisplayId2,
std::make_shared<RefreshRateSelector>(kDisplay2Modes,
- kDisplay2Mode60->getId()));
+ kDisplay2Mode60->getId()),
+ kActiveDisplayId);
using VsyncIds = std::vector<std::pair<PhysicalDisplayId, VsyncId>>;
@@ -585,7 +591,8 @@
mFlinger.getTimeStats(),
mSchedulerCallback};
- scheduler.registerDisplay(kMode->getPhysicalDisplayId(), vrrSelectorPtr, vrrTracker);
+ scheduler.registerDisplay(kMode->getPhysicalDisplayId(), vrrSelectorPtr, std::nullopt,
+ vrrTracker);
vrrSelectorPtr->setActiveMode(kMode->getId(), frameRate);
scheduler.setRenderRate(kMode->getPhysicalDisplayId(), frameRate, /*applyImmediately*/ false);
vrrTracker->addVsyncTimestamp(0);
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index 198a5de..f063809 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -53,7 +53,7 @@
factory, selectorPtr->getActiveMode().fps, timeStats) {
const auto displayId = selectorPtr->getActiveMode().modePtr->getPhysicalDisplayId();
registerDisplay(displayId, std::move(selectorPtr), std::move(controller),
- std::move(tracker));
+ std::move(tracker), displayId);
ON_CALL(*this, postMessage).WillByDefault([](sp<MessageHandler>&& handler) {
// Execute task to prevent broken promise exception on destruction.
@@ -85,14 +85,16 @@
void registerDisplay(
PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr,
+ std::optional<PhysicalDisplayId> activeDisplayIdOpt = {},
std::shared_ptr<VSyncTracker> vsyncTracker = std::make_shared<mock::VSyncTracker>()) {
registerDisplay(displayId, std::move(selectorPtr),
- std::make_unique<mock::VsyncController>(), vsyncTracker);
+ std::make_unique<mock::VsyncController>(), vsyncTracker,
+ activeDisplayIdOpt.value_or(displayId));
}
void registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr,
std::unique_ptr<VsyncController> controller,
- std::shared_ptr<VSyncTracker> tracker) {
+ std::shared_ptr<VSyncTracker> tracker, PhysicalDisplayId activeDisplayId) {
ftl::FakeGuard guard(kMainThreadContext);
Scheduler::registerDisplayInternal(displayId, std::move(selectorPtr),
std::shared_ptr<VsyncSchedule>(
@@ -101,16 +103,12 @@
mock::VSyncDispatch>(),
std::move(controller),
mockRequestHardwareVsync
- .AsStdFunction())));
+ .AsStdFunction())),
+ activeDisplayId);
}
testing::MockFunction<void(PhysicalDisplayId, bool)> mockRequestHardwareVsync;
- void unregisterDisplay(PhysicalDisplayId displayId) {
- ftl::FakeGuard guard(kMainThreadContext);
- Scheduler::unregisterDisplay(displayId);
- }
-
void setDisplayPowerMode(PhysicalDisplayId displayId, hal::PowerMode powerMode) {
ftl::FakeGuard guard(kMainThreadContext);
Scheduler::setDisplayPowerMode(displayId, powerMode);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 007383b..4197cbd 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -1118,8 +1118,8 @@
if (mFlinger.scheduler() && mSchedulerRegistration) {
mFlinger.scheduler()->registerDisplay(*physicalId,
mCreationArgs.refreshRateSelector,
- std::move(controller),
- std::move(tracker));
+ std::move(controller), std::move(tracker),
+ mFlinger.mutableActiveDisplayId());
}
}