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

Commit 02bb207a authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Deduplicate state for active display mode 

RefreshRateConfigs tracks the active mode, so remove the duplicate (and
not thread-safe) state in DisplayDevice.

Bug: 241285191
Test: Build (-Wthread-safety)
Change-Id: I6b551cc68da3916a797a28085be984667fa1901e
parent 926bb129

services/surfaceflinger/DisplayDevice.cpp

+9 −14
Original line number Diff line number Diff line
@@ -192,17 +192,16 @@ bool DisplayDevice::isPoweredOn() const { 
}



void DisplayDevice::setActiveMode(DisplayModeId modeId, const display::DisplaySnapshot& snapshot) {

    const auto modeOpt = snapshot.displayModes().get(modeId);

    LOG_ALWAYS_FATAL_IF(!modeOpt, "Unknown mode");

    const auto fpsOpt = snapshot.displayModes().get(modeId).transform(

            [](const DisplayModePtr& mode) { return mode->getFps(); });



    mActiveMode = modeOpt->get();

    const Fps fps = mActiveMode->getFps();

    LOG_ALWAYS_FATAL_IF(!fpsOpt, "Unknown mode");

    const Fps fps = *fpsOpt;



    ATRACE_INT(mActiveModeFPSTrace.c_str(), fps.getIntValue());



    if (mRefreshRateConfigs) {

    mRefreshRateConfigs->setActiveModeId(modeId);

    }



    if (mRefreshRateOverlay) {

        mRefreshRateOverlay->changeRefreshRate(fps);

    }
@@ -224,10 +223,6 @@ status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info, 
                                                    constraints, outTimeline);

}



const DisplayModePtr& DisplayDevice::getActiveMode() const {

    return mActiveMode;

}



nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const {

    const auto physicalId = getPhysicalId();

    if (!mHwComposer.isConnected(physicalId)) {
@@ -240,7 +235,7 @@ nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const { 
        return vsyncPeriod;

    }



    return getActiveMode()->getFps().getPeriodNsecs();

    return refreshRateConfigs().getActiveModePtr()->getVsyncPeriod();

}



nsecs_t DisplayDevice::getRefreshTimestamp() const {
@@ -451,7 +446,7 @@ void DisplayDevice::enableRefreshRateOverlay(bool enable, bool showSpinnner) { 
    mRefreshRateOverlay = std::make_unique<RefreshRateOverlay>(fpsRange, showSpinnner);

    mRefreshRateOverlay->setLayerStack(getLayerStack());

    mRefreshRateOverlay->setViewport(getSize());

    mRefreshRateOverlay->changeRefreshRate(getActiveMode()->getFps());

    mRefreshRateOverlay->changeRefreshRate(getActiveMode().getFps());

}



bool DisplayDevice::onKernelTimerChanged(std::optional<DisplayModeId> desiredModeId,
@@ -492,7 +487,7 @@ bool DisplayDevice::setDesiredActiveMode(const ActiveModeInfo& info) { 
    }



    // Check if we are already at the desired mode

    if (getActiveMode()->getId() == info.mode->getId()) {

    if (refreshRateConfigs().getActiveModePtr()->getId() == info.mode->getId()) {

        return false;

    }

services/surfaceflinger/DisplayDevice.h

+8 −6
Original line number Diff line number Diff line
@@ -189,8 +189,6 @@ public: 
    /* ------------------------------------------------------------------------

     * Display mode management.

     */

    const DisplayModePtr& getActiveMode() const;



    struct ActiveModeInfo {

        DisplayModePtr mode;

        scheduler::DisplayModeEvent event = scheduler::DisplayModeEvent::None;
@@ -207,6 +205,10 @@ public: 
        return mUpcomingActiveMode;

    }



    const DisplayMode& getActiveMode() const REQUIRES(kMainThreadContext) {

        return mRefreshRateConfigs->getActiveMode();

    }



    // Precondition: DisplaySnapshot must contain a mode with DisplayModeId.

    void setActiveMode(DisplayModeId, const display::DisplaySnapshot&) REQUIRES(kMainThreadContext);
@@ -226,7 +228,7 @@ public: 
    }



    // Enables an overlay to be displayed with the current refresh rate

    void enableRefreshRateOverlay(bool enable, bool showSpinner);

    void enableRefreshRateOverlay(bool enable, bool showSpinner) REQUIRES(kMainThreadContext);

    bool isRefreshRateOverlayEnabled() const { return mRefreshRateOverlay != nullptr; }

    bool onKernelTimerChanged(std::optional<DisplayModeId>, bool timerExpired);

    void animateRefreshRateOverlay();
@@ -265,10 +267,10 @@ private: 


    static ui::Transform::RotationFlags sPrimaryDisplayRotationFlags;



    // allow initial power mode as null.

    // Allow nullopt as initial power mode.

    std::optional<hardware::graphics::composer::hal::PowerMode> mPowerMode;

    DisplayModePtr mActiveMode;

    std::optional<float> mStagedBrightness = std::nullopt;



    std::optional<float> mStagedBrightness;

    float mBrightness = -1.f;



    std::atomic<nsecs_t> mLastHwVsync = 0;

services/surfaceflinger/SurfaceFlinger.cpp

+35 −34
Original line number Diff line number Diff line
@@ -680,7 +680,7 @@ void SurfaceFlinger::bootFinished() { 


    sp<IBinder> input(defaultServiceManager()->getService(String16("inputflinger")));



    static_cast<void>(mScheduler->schedule([=] {

    static_cast<void>(mScheduler->schedule([=]() FTL_FAKE_GUARD(kMainThreadContext) {

        if (input == nullptr) {

            ALOGE("Failed to link to input service");

        } else {
@@ -708,8 +708,9 @@ void SurfaceFlinger::bootFinished() { 


        mBootStage = BootStage::FINISHED;



        if (property_get_bool("sf.debug.show_refresh_rate_overlay", false)) {

            FTL_FAKE_GUARD(mStateLock, enableRefreshRateOverlay(true));

        if (base::GetBoolProperty("sf.debug.show_refresh_rate_overlay"s, false)) {

            ftl::FakeGuard guard(mStateLock);

            enableRefreshRateOverlay(true);

        }

    }));

}
@@ -1059,7 +1060,7 @@ status_t SurfaceFlinger::getDynamicDisplayInfo(const sp<IBinder>& displayToken, 


    const PhysicalDisplayId displayId = snapshot.displayId();



    info->activeDisplayModeId = display->getActiveMode()->getId().value();

    info->activeDisplayModeId = display->refreshRateConfigs().getActiveModePtr()->getId().value();

    info->activeColorMode = display->getCompositionDisplay()->getState().colorMode;

    info->supportedColorModes = getDisplayColorModes(displayId);

    info->hdrCapabilities = display->getHdrCapabilities();
@@ -1183,7 +1184,7 @@ void SurfaceFlinger::updateInternalStateWithChangedMode() { 
        return;

    }



    if (display->getActiveMode()->getResolution() != upcomingModeInfo.mode->getResolution()) {

    if (display->getActiveMode().getResolution() != upcomingModeInfo.mode->getResolution()) {

        auto& state = mCurrentState.displays.editValueFor(display->getDisplayToken());

        // We need to generate new sequenceId in order to recreate the display (and this

        // way the framebuffer).
@@ -1269,7 +1270,7 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() { 
        ALOGV("%s changing active mode to %d(%s) for display %s", __func__, desiredModeId.value(),

              to_string(*refreshRateOpt).c_str(), to_string(display->getId()).c_str());



        if (display->getActiveMode()->getId() == desiredModeId) {

        if (display->getActiveMode().getId() == desiredModeId) {

            // we are already in the requested mode, there is nothing left to do

            desiredActiveModeChangeDone(display);

            continue;
@@ -1318,7 +1319,7 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() { 
        const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately);

        const auto desiredActiveMode = display->getDesiredActiveMode();

        if (desiredActiveMode &&

            display->getActiveMode()->getId() == desiredActiveMode->mode->getId()) {

            display->getActiveMode().getId() == desiredActiveMode->mode->getId()) {

            desiredActiveModeChangeDone(display);

        }

    }
@@ -2107,7 +2108,7 @@ bool SurfaceFlinger::commit(TimePoint frameTime, VsyncId vsyncId, TimePoint expe 
            activeDisplay->getPowerMode() == hal::PowerMode::ON;

    if (mPowerHintSessionEnabled) {

        const auto& display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked()).get();

        const Period vsyncPeriod = Period::fromNs(display->getActiveMode()->getVsyncPeriod());

        const Period vsyncPeriod = Period::fromNs(display->getActiveMode().getVsyncPeriod());

        mPowerAdvisor->setCommitStart(frameTime);

        mPowerAdvisor->setExpectedPresentTime(mExpectedPresentTime);
@@ -3084,7 +3085,7 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, 


void SurfaceFlinger::updateInternalDisplayVsyncLocked(const sp<DisplayDevice>& activeDisplay) {

    mVsyncConfiguration->reset();

    const Fps refreshRate = activeDisplay->refreshRateConfigs().getActiveMode().getFps();

    const Fps refreshRate = activeDisplay->getActiveMode().getFps();

    updatePhaseConfiguration(refreshRate);

    mRefreshRateStats->setRefreshRate(refreshRate);

}
@@ -3394,11 +3395,13 @@ void SurfaceFlinger::triggerOnFrameRateOverridesChanged() { 
void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) {

    LOG_ALWAYS_FATAL_IF(mScheduler);



    const auto currRefreshRate = display->getActiveMode()->getFps();

    mRefreshRateStats = std::make_unique<scheduler::RefreshRateStats>(*mTimeStats, currRefreshRate,

    const auto activeModePtr = display->refreshRateConfigs().getActiveModePtr();

    const Fps activeRefreshRate = activeModePtr->getFps();

    mRefreshRateStats =

            std::make_unique<scheduler::RefreshRateStats>(*mTimeStats, activeRefreshRate,

                                                          hal::PowerMode::OFF);



    mVsyncConfiguration = getFactory().createVsyncConfiguration(currRefreshRate);

    mVsyncConfiguration = getFactory().createVsyncConfiguration(activeRefreshRate);

    mVsyncModulator = sp<VsyncModulator>::make(mVsyncConfiguration->getCurrentConfigs());



    using Feature = scheduler::Feature;
@@ -3431,7 +3434,7 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { 
    mScheduler->startTimers();



    const auto configs = mVsyncConfiguration->getCurrentConfigs();

    const nsecs_t vsyncPeriod = currRefreshRate.getPeriodNsecs();

    const nsecs_t vsyncPeriod = activeRefreshRate.getPeriodNsecs();

    mAppConnectionHandle =

            mScheduler->createConnection("app", mFrameTimeline->getTokenManager(),

                                         /*workDuration=*/configs.late.appWorkDuration,
@@ -3459,7 +3462,7 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { 
    // This is a bit hacky, but this avoids a back-pointer into the main SF

    // classes from EventThread, and there should be no run-time binder cost

    // anyway since there are no connected apps at this point.

    mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, display->getActiveMode());

    mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, activeModePtr);

}



void SurfaceFlinger::updatePhaseConfiguration(const Fps& refreshRate) {
@@ -5348,10 +5351,10 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co 


    if (const auto display = getDefaultDisplayDeviceLocked()) {

        std::string fps, xDpi, yDpi;

        if (const auto activeMode = display->getActiveMode()) {

            fps = to_string(activeMode->getFps());

        if (const auto activeModePtr = display->refreshRateConfigs().getActiveModePtr()) {

            fps = to_string(activeModePtr->getFps());



            const auto dpi = activeMode->getDpi();

            const auto dpi = activeModePtr->getDpi();

            xDpi = base::StringPrintf("%.2f", dpi.x);

            yDpi = base::StringPrintf("%.2f", dpi.y);

        } else {
@@ -5851,17 +5854,15 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r 
                return NO_ERROR;

            }

            case 1034: {

                auto future = mScheduler->schedule([&] {

                auto future = mScheduler->schedule(

                        [&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) {

                            switch (n = data.readInt32()) {

                                case 0:

                                case 1:

                            FTL_FAKE_GUARD(mStateLock,

                                           enableRefreshRateOverlay(static_cast<bool>(n)));

                                    enableRefreshRateOverlay(static_cast<bool>(n));

                                    break;

                        default: {

                            reply->writeBool(

                                    FTL_FAKE_GUARD(mStateLock, isRefreshRateOverlayEnabled()));

                        }

                                default:

                                    reply->writeBool(isRefreshRateOverlayEnabled());

                            }

                        });
@@ -6799,12 +6800,12 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal( 


    // TODO(b/140204874): Leave the event in until we do proper testing with all apps that might

    // be depending in this callback.

    const auto activeMode = display->getActiveMode();

    const auto activeModePtr = display->refreshRateConfigs().getActiveModePtr();

    if (isDisplayActiveLocked(display)) {

        mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, activeMode);

        mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, activeModePtr);

        toggleKernelIdleTimer();

    } else {

        mScheduler->onNonPrimaryDisplayModeChanged(mAppConnectionHandle, activeMode);

        mScheduler->onNonPrimaryDisplayModeChanged(mAppConnectionHandle, activeModePtr);

    }



    auto preferredModeOpt =

services/surfaceflinger/SurfaceFlinger.h

+1 −1
Original line number Diff line number Diff line
@@ -1340,7 +1340,7 @@ private: 


    std::unique_ptr<Hwc2::PowerAdvisor> mPowerAdvisor;



    void enableRefreshRateOverlay(bool enable) REQUIRES(mStateLock);

    void enableRefreshRateOverlay(bool enable) REQUIRES(mStateLock, kMainThreadContext);



    // Flag used to set override desired display mode from backdoor

    bool mDebugDisplayModeSetByBackdoor = false;

services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp

+20 −11
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 


#include "DisplayTransactionTestHelpers.h"



#include <ftl/fake_guard.h>

#include <scheduler/Fps.h>



namespace android {
@@ -111,8 +112,10 @@ void DisplayModeSwitchingTest::setupScheduler( 
}



TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRequired) {

    ftl::FakeGuard guard(kMainThreadContext);



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60);



    mFlinger.onActiveDisplayChanged(mDisplay);
@@ -121,7 +124,7 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRe 


    ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kModeId90);

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60);



    // Verify that next commit will call setActiveConfigWithConstraints in HWC

    const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
@@ -134,7 +137,7 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRe 


    Mock::VerifyAndClearExpectations(mComposer);

    ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60);



    // Verify that the next commit will complete the mode change and send

    // a onModeChanged event to the framework.
@@ -144,10 +147,12 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRe 
    Mock::VerifyAndClearExpectations(mAppEventThread);



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId90);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId90);

}



TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefreshRequired) {

    ftl::FakeGuard guard(kMainThreadContext);



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());



    mFlinger.onActiveDisplayChanged(mDisplay);
@@ -157,7 +162,7 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefres 


    ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kModeId90);

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60);



    // Verify that next commit will call setActiveConfigWithConstraints in HWC

    // and complete the mode change.
@@ -172,15 +177,17 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefres 
    mFlinger.commit();



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId90);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId90);

}



TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) {

    ftl::FakeGuard guard(kMainThreadContext);



    // Test that if we call setDesiredDisplayModeSpecs while a previous mode change

    // is still being processed the later call will be respected.



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60);



    mFlinger.onActiveDisplayChanged(mDisplay);
@@ -214,12 +221,14 @@ TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) { 
    mFlinger.commit();



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId120);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId120);

}



TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefreshRequired) {

    ftl::FakeGuard guard(kMainThreadContext);



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60);



    mFlinger.onActiveDisplayChanged(mDisplay);
@@ -228,7 +237,7 @@ TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefresh 


    ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kModeId90_4K);

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId60);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId60);



    // Verify that next commit will call setActiveConfigWithConstraints in HWC

    // and complete the mode change.
@@ -263,7 +272,7 @@ TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefresh 
    mDisplay = mFlinger.getDisplay(displayToken);



    ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());

    ASSERT_EQ(mDisplay->getActiveMode()->getId(), kModeId90_4K);

    ASSERT_EQ(mDisplay->getActiveMode().getId(), kModeId90_4K);

}



} // namespace