Merge "SF: Fix mode setting for UDFPS with disabled AOD" into udc-qpr-dev
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 74d4e69..77ae03d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1380,8 +1380,7 @@
continue;
}
- if (!display->isPoweredOn()) {
- // Display is no longer powered on, so abort the mode change.
+ if (!shouldApplyRefreshRateSelectorPolicy(*display)) {
clearDesiredActiveModeState(display);
continue;
}
@@ -3885,8 +3884,9 @@
if (!display) continue;
- if (!display->isPoweredOn()) {
- ALOGV("%s(%s): Display is powered off", __func__, to_string(displayId).c_str());
+ if (ftl::FakeGuard guard(kMainThreadContext);
+ !shouldApplyRefreshRateSelectorPolicy(*display)) {
+ ALOGV("%s(%s): Skipped applying policy", __func__, to_string(displayId).c_str());
continue;
}
@@ -7645,17 +7645,33 @@
break;
}
- // TODO(b/255635711): Apply the policy once the display is powered on, which is currently only
- // done for the internal display that becomes active on fold/unfold. For now, assume that DM
- // always powers on the secondary (internal or external) display before setting its policy.
- if (!display->isPoweredOn()) {
- ALOGV("%s(%s): Display is powered off", __func__, to_string(displayId).c_str());
+ if (!shouldApplyRefreshRateSelectorPolicy(*display)) {
+ ALOGV("%s(%s): Skipped applying policy", __func__, to_string(displayId).c_str());
return NO_ERROR;
}
return applyRefreshRateSelectorPolicy(displayId, selector);
}
+bool SurfaceFlinger::shouldApplyRefreshRateSelectorPolicy(const DisplayDevice& display) const {
+ if (display.isPoweredOn() || mPhysicalDisplays.size() == 1) return true;
+
+ LOG_ALWAYS_FATAL_IF(display.isVirtual());
+ const auto displayId = display.getPhysicalId();
+
+ // The display is powered off, and this is a multi-display device. If the display is the
+ // inactive internal display of a dual-display foldable, then the policy will be applied
+ // when it becomes active upon powering on.
+ //
+ // TODO(b/255635711): Remove this function (i.e. returning `false` as a special case) once
+ // concurrent mode setting across multiple (potentially powered off) displays is supported.
+ //
+ return displayId == mActiveDisplayId ||
+ !mPhysicalDisplays.get(displayId)
+ .transform(&PhysicalDisplay::isInternal)
+ .value_or(false);
+}
+
status_t SurfaceFlinger::applyRefreshRateSelectorPolicy(
PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector, bool force) {
const scheduler::RefreshRateSelector::Policy currentPolicy = selector.getCurrentPolicy();
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 427e6bb..47ada25 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -710,6 +710,9 @@
const sp<DisplayDevice>&, const scheduler::RefreshRateSelector::PolicyVariant&)
EXCLUDES(mStateLock) REQUIRES(kMainThreadContext);
+ bool shouldApplyRefreshRateSelectorPolicy(const DisplayDevice&) const
+ REQUIRES(mStateLock, kMainThreadContext);
+
// TODO(b/241285191): Look up RefreshRateSelector on Scheduler to remove redundant parameter.
status_t applyRefreshRateSelectorPolicy(PhysicalDisplayId,
const scheduler::RefreshRateSelector&,
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 3738381..9ef3e9e 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -468,6 +468,37 @@
}
TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
+ EXPECT_TRUE(mDisplay->isPoweredOn());
+ EXPECT_THAT(mDisplay, ModeSettledTo(kModeId60));
+
+ EXPECT_EQ(NO_ERROR,
+ mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
+ mock::createDisplayModeSpecs(kModeId90.value(),
+ false, 0.f, 120.f)));
+
+ EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+
+ // Power off the display before the mode has been set.
+ mDisplay->setPowerMode(hal::PowerMode::OFF);
+
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(kInnerDisplayHwcId,
+ hal::HWConfigId(kModeId90.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ // Powering off should not abort the mode set.
+ EXPECT_FALSE(mDisplay->isPoweredOn());
+ EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+
+ mFlinger.commit();
+
+ EXPECT_THAT(mDisplay, ModeSettledTo(kModeId90));
+}
+
+TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -508,6 +539,7 @@
mFlinger.commit();
+ // Powering off the inactive display should abort the mode set.
EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
@@ -515,6 +547,28 @@
EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+
+ innerDisplay->setPowerMode(hal::PowerMode::OFF);
+ outerDisplay->setPowerMode(hal::PowerMode::ON);
+
+ // Only the outer display is powered on.
+ mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay);
+
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(kOuterDisplayHwcId,
+ hal::HWConfigId(kModeId60.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ // The mode set should resume once the display becomes active.
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
+
+ mFlinger.commit();
+
+ EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
}
} // namespace