SF: Generalize immediate mode set to multi-display
When initiating a mode set, HWC may request an immediate mode change, in
which case SF finalizes the change on the same (rather than next) frame.
However, this would only be honored for at most one display, and further
requests would be dropped.
This was a historical hack: The loop used to iterate over mDisplays, and
finalizeDisplayModeChange may recreate/invalidate the DisplayDevice. Now
that the loop is over the immutable mPhysicalDisplays, the DisplayDevice
can be safely recreated within the loop.
Note that the only known user of this code path is ATV, so multi-display
devices should not be affected by this change.
Bug: 255635711
Flag: EXEMPT refactor
Test: DisplayModeSwitchingTest (WithoutRefreshRequired cases)
Change-Id: I842e39c2c5509de99b80fa9561ecb13b66f98c40
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5b40acf..bba27b2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1481,8 +1481,6 @@
void SurfaceFlinger::initiateDisplayModeChanges() {
ATRACE_CALL();
- std::optional<PhysicalDisplayId> displayToUpdateImmediately;
-
for (const auto& [displayId, physical] : FTL_FAKE_GUARD(mStateLock, mPhysicalDisplays)) {
auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId);
if (!desiredModeOpt) {
@@ -1536,21 +1534,14 @@
if (outTimeline.refreshRequired) {
scheduleComposite(FrameHint::kNone);
} else {
- // TODO(b/255635711): Remove `displayToUpdateImmediately` to `finalizeDisplayModeChange`
- // for all displays. This was only needed when the loop iterated over `mDisplays` rather
- // than `mPhysicalDisplays`.
- displayToUpdateImmediately = displayId;
- }
- }
+ // HWC has requested to apply the mode change immediately rather than on the next frame.
+ finalizeDisplayModeChange(displayId);
- if (displayToUpdateImmediately) {
- const auto displayId = *displayToUpdateImmediately;
- finalizeDisplayModeChange(displayId);
-
- const auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId);
- if (desiredModeOpt &&
- mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) {
- applyActiveMode(displayId);
+ const auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId);
+ if (desiredModeOpt &&
+ mDisplayModeController.getActiveMode(displayId) == desiredModeOpt->mode) {
+ applyActiveMode(displayId);
+ }
}
}
}
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 6af965c..4b0a7c3 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -127,9 +127,9 @@
static constexpr HWDisplayId kInnerDisplayHwcId = PrimaryDisplayVariant::HWC_DISPLAY_ID;
static constexpr HWDisplayId kOuterDisplayHwcId = kInnerDisplayHwcId + 1;
- auto injectOuterDisplay() {
- constexpr PhysicalDisplayId kOuterDisplayId = PhysicalDisplayId::fromPort(254u);
+ static constexpr PhysicalDisplayId kOuterDisplayId = PhysicalDisplayId::fromPort(254u);
+ auto injectOuterDisplay() {
// For the inner display, this is handled by setupHwcHotplugCallExpectations.
EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _))
.WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL),
@@ -268,6 +268,38 @@
EXPECT_THAT(mDisplay, ModeSettledTo(&dmc(), kModeId90));
}
+TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnTwoDisplaysWithoutRefreshRequired) {
+ const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId90, 120_Hz,
+ true)));
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId60, 60_Hz,
+ true)));
+
+ EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
+
+ // Verify that next commit will call setActiveConfigWithConstraints in HWC
+ // and complete the mode change.
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};
+ EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);
+ EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);
+
+ EXPECT_CALL(*mAppEventThread, onModeChanged(_)).Times(2);
+
+ mFlinger.commit();
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60));
+}
+
TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) {
// Test that if we call setDesiredDisplayModeSpecs while a previous mode change
// is still being processed the later call will be respected.