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

Commit 4b32e4bc authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Clean up DisplayDevice::initiateModeChange 

Trace the pending mode after successfully setting it on HWC.

Remove unused status_t return value, and redundant warning log. Disallow
nullptr for out parameter.

Bug: 255635711
Test: presubmit
Change-Id: I381e5528d6336c300a80abcebccdd76369126c04
parent 6165155e

services/surfaceflinger/DisplayDevice.cpp

+11 −7
Original line number Diff line number Diff line
@@ -217,9 +217,9 @@ void DisplayDevice::setActiveMode(DisplayModeId modeId, Fps vsyncRate, Fps rende 
    updateRefreshRateOverlayRate(vsyncRate, renderFps);

}



status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info,

bool DisplayDevice::initiateModeChange(const ActiveModeInfo& info,

                                       const hal::VsyncPeriodChangeConstraints& constraints,

                                           hal::VsyncPeriodChangeTimeline* outTimeline) {

                                       hal::VsyncPeriodChangeTimeline& outTimeline) {

    if (!info.modeOpt || info.modeOpt->modePtr->getPhysicalDisplayId() != getPhysicalId()) {

        ALOGE("Trying to initiate a mode change to invalid mode %s on display %s",

              info.modeOpt ? std::to_string(info.modeOpt->modePtr->getId().value()).c_str()
@@ -230,11 +230,15 @@ status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info, 
    mPendingMode = info;

    mIsModeSetPending = true;



    const auto& pendingMode = *info.modeOpt->modePtr;

    ATRACE_INT(mPendingModeFpsTrace.c_str(), pendingMode.getVsyncRate().getIntValue());

    const auto& mode = *info.modeOpt->modePtr;



    return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), pendingMode.getHwcId(),

                                                    constraints, outTimeline);

    if (mHwComposer.setActiveModeWithConstraints(getPhysicalId(), mode.getHwcId(), constraints,

                                                 &outTimeline) != OK) {

        return false;

    }



    ATRACE_INT(mPendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue());

    return true;

}



void DisplayDevice::finalizeModeChange(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) {

services/surfaceflinger/DisplayDevice.h

+2 −3
Original line number Diff line number Diff line
@@ -223,9 +223,8 @@ public: 


    void setActiveMode(DisplayModeId, Fps vsyncRate, Fps renderFps);



    status_t initiateModeChange(const ActiveModeInfo&,

                                const hal::VsyncPeriodChangeConstraints& constraints,

                                hal::VsyncPeriodChangeTimeline* outTimeline)

    bool initiateModeChange(const ActiveModeInfo&, const hal::VsyncPeriodChangeConstraints&,

                            hal::VsyncPeriodChangeTimeline& outTimeline)

            REQUIRES(kMainThreadContext);



    void finalizeModeChange(DisplayModeId, Fps vsyncRate, Fps renderFps)

services/surfaceflinger/SurfaceFlinger.cpp

+1 −5
Original line number Diff line number Diff line
@@ -1397,11 +1397,7 @@ void SurfaceFlinger::initiateDisplayModeChanges() { 
        constraints.seamlessRequired = false;

        hal::VsyncPeriodChangeTimeline outTimeline;



        const auto status = display->initiateModeChange(*desiredModeOpt, constraints, &outTimeline);

        if (status != NO_ERROR) {

            // initiateModeChange may fail if a hotplug event is just about

            // to be sent. We just log the error in this case.

            ALOGW("initiateModeChange failed: %d", status);

        if (!display->initiateModeChange(*desiredModeOpt, constraints, outTimeline)) {

            continue;

        }

services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp

+5 −8
Original line number Diff line number Diff line
@@ -103,13 +103,12 @@ TEST_F(InitiateModeChangeTest, initiateModeChange) REQUIRES(kMainThreadContext) 
    EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredMode()->modeOpt);

    EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);



    hal::VsyncPeriodChangeConstraints constraints{

    const hal::VsyncPeriodChangeConstraints constraints{

            .desiredTimeNanos = systemTime(),

            .seamlessRequired = false,

    };

    hal::VsyncPeriodChangeTimeline timeline;

    EXPECT_EQ(OK,

              mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));

    EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));

    EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);

    EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
@@ -130,13 +129,12 @@ TEST_F(InitiateModeChangeTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainTh 
    EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredMode()->modeOpt);

    EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);



    hal::VsyncPeriodChangeConstraints constraints{

    const hal::VsyncPeriodChangeConstraints constraints{

            .desiredTimeNanos = systemTime(),

            .seamlessRequired = false,

    };

    hal::VsyncPeriodChangeTimeline timeline;

    EXPECT_EQ(OK,

              mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));

    EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));

    EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);

    EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
@@ -149,8 +147,7 @@ TEST_F(InitiateModeChangeTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainTh 
    EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);

    EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);



    EXPECT_EQ(OK,

              mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));

    EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));

    EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getPendingMode().modeOpt);

    EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);