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

Commit 92900f81 authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

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
parent 6429b0b3

services/surfaceflinger/SurfaceFlinger.cpp

+7 −16
Original line number Diff line number Diff line
@@ -1481,8 +1481,6 @@ void SurfaceFlinger::applyActiveMode(PhysicalDisplayId displayId) { 
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,15 +1534,7 @@ void SurfaceFlinger::initiateDisplayModeChanges() { 
        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;

        }

    }



    if (displayToUpdateImmediately) {

        const auto displayId = *displayToUpdateImmediately;

            // HWC has requested to apply the mode change immediately rather than on the next frame.

            finalizeDisplayModeChange(displayId);



            const auto desiredModeOpt = mDisplayModeController.getDesiredMode(displayId);
@@ -1554,6 +1544,7 @@ void SurfaceFlinger::initiateDisplayModeChanges() { 
            }

        }

    }

}



void SurfaceFlinger::disableExpensiveRendering() {

    const char* const whence = __func__;

services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp

+34 −2
Original line number Diff line number Diff line
@@ -127,9 +127,9 @@ public: 
    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 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateWithoutRefreshRequired) { 
    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.