Merge "SF: Fix mode setting for UDFPS with disabled AOD" into udc-qpr-dev

3 files changed, 82 insertions, 9 deletions

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 74d4e69a29..77ae03d04b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1380,8 +1380,7 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
             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 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest
 
         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 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal(
             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 427e6bbcd5..47ada25d26 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -710,6 +710,9 @@ private:
             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 3738381dac..9ef3e9e825 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -468,6 +468,37 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
 }
 
 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 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
 
     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 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
 
     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