Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ca96eff4 authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Fix mode setting for UDFPS with disabled AOD 

When AOD is off, DM sends its policy to SF while the display is powered
off. SF defers applying the policy of powered off displays, assuming it
will be applied when the display becomes the active display upon power-
on, as a special case for foldables. However, in the case of UDFPS with
disabled AOD, there is no change in active display, so SF did not apply
the policy to switch to 120 Hz on the lockscreen.

Tighten the condition for skipping mode set to only affect foldables in
non-concurrent mode.

Fixes: 300189378
Test: 120 Hz for UDFPS when Smooth Display and AOD are disabled.
Test: 60/120 Hz still works on fold/unfold and concurrent mode.
Test: DisplayModeSwitchingTest.powerOffDuringModeSet
Change-Id: Iedd25f8caed69c1dd304ee9f6341dba6bcdc87d7
parent 93a12837

services/surfaceflinger/SurfaceFlinger.cpp

+25 −9
Original line number Diff line number Diff line
@@ -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();

services/surfaceflinger/SurfaceFlinger.h

+3 −0
Original line number Diff line number Diff line
@@ -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&,

services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp

+54 −0
Original line number Diff line number Diff line
@@ -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