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

Commit 1746afd8 authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Stop forcing modeset on fold/unfold 

SF no longer needs to defer setDesiredDisplayModeSpecs for the inactive
display until it becomes active on fold/unfold, because it now supports
per-display modeset since I9da3a0be07f9fbb08f11485aa6ab9400259a4e09.

Remove SF::shouldApplyRefreshRateSelectorPolicy, which was used to skip
applying DM's policy while a display is powered off. Though it had also
been used to skip SF::requestDisplayModes in response to changes in the
Scheduler's policy, I4dd954bf9a943f181bd64950ff9edee863f53e99 suppressed
those calls in the Scheduler.

Bug: 255635711
Bug: 255635821
Flag: NONE (blocks P1 bug fix, and perf risk is limited to foldables)
Test: Watch refresh rate overlay for MRR CUJs plus folding/unfolding.
Test: DisplayModeSwitchingTest
Change-Id: Ia5afb75977249fc734bd78625d8ffd4c433a3a8d
parent 086507b2

services/surfaceflinger/SurfaceFlinger.cpp

+7 −48
Original line number Diff line number Diff line
@@ -1425,11 +1425,6 @@ void SurfaceFlinger::initiateDisplayModeChanges() { 
            continue;

        }



        if (!shouldApplyRefreshRateSelectorPolicy(*display)) {

            dropModeRequest(display);

            continue;

        }



        const auto desiredModeId = desiredModeOpt->mode.modePtr->getId();

        const auto displayModePtrOpt = physical.snapshot().displayModes().get(desiredModeId);
@@ -4228,12 +4223,6 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest 


        if (!display) continue;



        if (ftl::FakeGuard guard(kMainThreadContext);

            !shouldApplyRefreshRateSelectorPolicy(*display)) {

            ALOGV("%s(%s): Skipped applying policy", __func__, to_string(displayId).c_str());

            continue;

        }



        if (display->refreshRateSelector().isModeAllowed(request.mode)) {

            setDesiredMode(std::move(request));

        } else {
@@ -8562,35 +8551,11 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal( 
            break;

    }



    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) {

        PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector) {

    const scheduler::RefreshRateSelector::Policy currentPolicy = selector.getCurrentPolicy();

    ALOGV("Setting desired display mode specs: %s", currentPolicy.toString().c_str());
@@ -8621,7 +8586,7 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( 
        return INVALID_OPERATION;

    }



    setDesiredMode({std::move(preferredMode), .emitEvent = true, .force = force});

    setDesiredMode({std::move(preferredMode), .emitEvent = true});



    // Update the frameRateOverride list as the display render rate might have changed

    if (mScheduler->updateFrameRateOverrides(scheduler::GlobalSignals{}, preferredFps)) {
@@ -8979,13 +8944,8 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD 
                                                  const DisplayDevice& activeDisplay) {

    ATRACE_CALL();



    // For the first display activated during boot, there is no need to force setDesiredMode,

    // because DM is about to send its policy via setDesiredDisplayModeSpecs.

    bool forceApplyPolicy = false;



    if (inactiveDisplayPtr) {

        inactiveDisplayPtr->getCompositionDisplay()->setLayerCachingTexturePoolEnabled(false);

        forceApplyPolicy = true;

    }



    mActiveDisplayId = activeDisplay.getPhysicalId();
@@ -9002,12 +8962,11 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD 
    mActiveDisplayTransformHint = activeDisplay.getTransformHint();

    sActiveDisplayRotationFlags = ui::Transform::toRotationFlags(activeDisplay.getOrientation());



    // The policy of the new active/pacesetter display may have changed while it was inactive. In

    // that case, its preferred mode has not been propagated to HWC (via setDesiredMode). In either

    // case, the Scheduler's cachedModeChangedParams must be initialized to the newly active mode,

    // and the kernel idle timer of the newly active display must be toggled.

    applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay.refreshRateSelector(),

                                   forceApplyPolicy);

    // Whether or not the policy of the new active/pacesetter display changed while it was inactive

    // (in which case its preferred mode has already been propagated to HWC via setDesiredMode), the

    // Scheduler's cachedModeChangedParams must be initialized to the newly active mode, and the

    // kernel idle timer of the newly active display must be toggled.

    applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay.refreshRateSelector());

}



status_t SurfaceFlinger::addWindowInfosListener(const sp<IWindowInfosListener>& windowInfosListener,

services/surfaceflinger/SurfaceFlinger.h

+1 −5
Original line number Diff line number Diff line
@@ -754,13 +754,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&,

                                            bool force = false)

                                            const scheduler::RefreshRateSelector&)

            REQUIRES(mStateLock, kMainThreadContext);



    void commitTransactionsLegacy() EXCLUDES(mStateLock) REQUIRES(kMainThreadContext);

services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp

+41 −37
Original line number Diff line number Diff line
@@ -327,7 +327,9 @@ MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") { 
        return false;

    }



    if (!flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) {

    // VsyncModulator should react to mode switches on the pacesetter display.

    if (arg->getPhysicalId() == flinger->scheduler()->pacesetterDisplayId() &&

        !flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) {

        *result_listener << "VsyncModulator did not shift to early phase";

        return false;

    }
@@ -368,8 +370,8 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { 
    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));



    // Only the inner display is powered on.

    mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);

    mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF);

    mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON);



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
@@ -385,40 +387,44 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { 
                                                                               0.f, 120.f)));



    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));

    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));



    const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};

    EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);

    EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);



    mFlinger.commit();



    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));

    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));



    mFlinger.commit();



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));



    innerDisplay->setPowerMode(hal::PowerMode::OFF);

    outerDisplay->setPowerMode(hal::PowerMode::ON);

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));



    // Only the outer display is powered on.

    mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay);

    mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF);

    mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));

    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));



    EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);

    EXPECT_EQ(NO_ERROR,

              mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),

                                                  mock::createDisplayModeSpecs(kModeId60, false,

                                                                               0.f, 120.f)));



    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));

    EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId60);



    mFlinger.commit();



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));

    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));

    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));



    mFlinger.commit();



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));

    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));

}
@@ -437,10 +443,8 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { 
    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));



    outerDisplay->setPowerMode(hal::PowerMode::ON);



    // Both displays are powered on.

    mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);

    mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON);

    mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
@@ -485,7 +489,7 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { 
    EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90));



    // Power off the display before the mode has been set.

    mDisplay->setPowerMode(hal::PowerMode::OFF);

    mFlinger.setPowerModeInternal(mDisplay, hal::PowerMode::OFF);



    const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};

    EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);
@@ -517,10 +521,8 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { 
    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));



    outerDisplay->setPowerMode(hal::PowerMode::ON);



    // Both displays are powered on.

    mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);

    mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON);

    mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
@@ -539,40 +541,42 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { 
    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));



    // Power off the outer display before the mode has been set.

    outerDisplay->setPowerMode(hal::PowerMode::OFF);

    mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF);



    const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};

    EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);

    EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);



    mFlinger.commit();



    // Powering off the inactive display should abort the mode set.

    // Powering off the inactive display should not abort the mode set.

    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));

    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));



    mFlinger.commit();



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));



    innerDisplay->setPowerMode(hal::PowerMode::OFF);

    outerDisplay->setPowerMode(hal::PowerMode::ON);

    mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF);

    mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);



    // Only the outer display is powered on.

    mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay);

    EXPECT_EQ(NO_ERROR,

              mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),

                                                  mock::createDisplayModeSpecs(kModeId120, false,

                                                                               0.f, 120.f)));



    EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);

    EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId120);



    mFlinger.commit();



    // The mode set should resume once the display becomes active.

    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));

    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));

    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId120));



    mFlinger.commit();



    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));

    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));

}



} // namespace