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

Commit 3de6727d authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Fix jank after folding due to power sequence 

SF assumed that DM always powers off the active display before powering
on the other display. However, this is not the case when simultaneously
folding and pressing the power button to turn on the rear display.

SF then behaves as if all displays were off (e.g. synthetic VSYNC, low
thread priority), causing consistent jank until the next power or fold
event. Fix this by activating the other display if already powered on.

Fixes: 294230902
Test: No jank after simultaneously folding and hitting the power button.
Test: FoldableTest.promotesPacesetterOnConcurrentPowerOff
Change-Id: I5d0bdafc4002ce5ae86866f457132f8a0f178698
parent f88c330e

services/surfaceflinger/SurfaceFlinger.cpp

+29 −11
Original line number Diff line number Diff line
@@ -5522,6 +5522,9 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 
        // Turn off the display



        if (displayId == mActiveDisplayId) {

            if (const auto display = getActivatableDisplay()) {

                onActiveDisplayChangedLocked(activeDisplay.get(), *display);

            } else {

                if (setSchedFifo(false) != NO_ERROR) {

                    ALOGW("Failed to set SCHED_OTHER after powering off active display: %s",

                          strerror(errno));
@@ -5536,6 +5539,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: 
                    mScheduler->enableSyntheticVsync();

                }

            }

        }



        // Disable VSYNC before turning off the display.

        requestHardwareVsync(displayId, false);
@@ -7948,6 +7952,20 @@ void SurfaceFlinger::onActiveDisplaySizeChanged(const DisplayDevice& activeDispl 
    getRenderEngine().onActiveDisplaySizeChanged(activeDisplay.getSize());

}



sp<DisplayDevice> SurfaceFlinger::getActivatableDisplay() const {

    if (mPhysicalDisplays.size() == 1) return nullptr;



    // TODO(b/255635821): Choose the pacesetter display, considering both internal and external

    // displays. For now, pick the other internal display, assuming a dual-display foldable.

    return findDisplay([this](const DisplayDevice& display) REQUIRES(mStateLock) {

        const auto idOpt = PhysicalDisplayId::tryCast(display.getId());

        return idOpt && *idOpt != mActiveDisplayId && display.isPoweredOn() &&

                mPhysicalDisplays.get(*idOpt)

                        .transform(&PhysicalDisplay::isInternal)

                        .value_or(false);

    });

}



void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveDisplayPtr,

                                                  const DisplayDevice& activeDisplay) {

    ATRACE_CALL();

services/surfaceflinger/SurfaceFlinger.h

+5 −1
Original line number Diff line number Diff line
@@ -934,7 +934,8 @@ private: 
    template <typename Predicate>

    sp<DisplayDevice> findDisplay(Predicate p) const REQUIRES(mStateLock) {

        const auto it = std::find_if(mDisplays.begin(), mDisplays.end(),

                                     [&](const auto& pair) { return p(*pair.second); });

                                     [&](const auto& pair)

                                             REQUIRES(mStateLock) { return p(*pair.second); });



        return it == mDisplays.end() ? nullptr : it->second;

    }
@@ -1047,6 +1048,9 @@ private: 
    VirtualDisplayId acquireVirtualDisplay(ui::Size, ui::PixelFormat) REQUIRES(mStateLock);

    void releaseVirtualDisplay(VirtualDisplayId);



    // Returns a display other than `mActiveDisplayId` that can be activated, if any.

    sp<DisplayDevice> getActivatableDisplay() const REQUIRES(mStateLock, kMainThreadContext);



    void onActiveDisplayChangedLocked(const DisplayDevice* inactiveDisplayPtr,

                                      const DisplayDevice& activeDisplay)

            REQUIRES(mStateLock, kMainThreadContext);

services/surfaceflinger/tests/unittests/SurfaceFlinger_FoldableTest.cpp

+31 −1
Original line number Diff line number Diff line
@@ -55,13 +55,17 @@ struct FoldableTest : DisplayTransactionTest { 
    sp<DisplayDevice> mInnerDisplay, mOuterDisplay;

};



TEST_F(FoldableTest, foldUnfold) {

TEST_F(FoldableTest, promotesPacesetterOnBoot) {

    // When the device boots, the inner display should be the pacesetter.

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kInnerDisplayId);



    // ...and should still be after powering on.

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kInnerDisplayId);

}



TEST_F(FoldableTest, promotesPacesetterOnFoldUnfold) {

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);



    // The outer display should become the pacesetter after folding.

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::OFF);
@@ -72,6 +76,10 @@ TEST_F(FoldableTest, foldUnfold) { 
    mFlinger.setPowerModeInternal(mOuterDisplay, PowerMode::OFF);

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kInnerDisplayId);

}



TEST_F(FoldableTest, promotesPacesetterOnConcurrentPowerOn) {

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);



    // The inner display should stay the pacesetter if both are powered on.

    // TODO(b/255635821): The pacesetter should depend on the displays' refresh rates.
@@ -81,6 +89,28 @@ TEST_F(FoldableTest, foldUnfold) { 
    // The outer display should become the pacesetter if designated.

    mFlinger.scheduler()->setPacesetterDisplay(kOuterDisplayId);

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kOuterDisplayId);



    // The inner display should become the pacesetter if designated.

    mFlinger.scheduler()->setPacesetterDisplay(kInnerDisplayId);

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kInnerDisplayId);

}



TEST_F(FoldableTest, promotesPacesetterOnConcurrentPowerOff) {

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);

    mFlinger.setPowerModeInternal(mOuterDisplay, PowerMode::ON);



    // The outer display should become the pacesetter if the inner display powers off.

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::OFF);

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kOuterDisplayId);



    // The outer display should stay the pacesetter if both are powered on.

    // TODO(b/255635821): The pacesetter should depend on the displays' refresh rates.

    mFlinger.setPowerModeInternal(mInnerDisplay, PowerMode::ON);

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kOuterDisplayId);



    // The inner display should become the pacesetter if the outer display powers off.

    mFlinger.setPowerModeInternal(mOuterDisplay, PowerMode::OFF);

    ASSERT_EQ(mFlinger.scheduler()->pacesetterDisplayId(), kInnerDisplayId);

}



TEST_F(FoldableTest, doesNotRequestHardwareVsyncIfPoweredOff) {