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

Commit 16589056 authored by Marin Shalamanov's avatar Marin Shalamanov Committed by Android (Google) Code Review
Browse files

Merge changes Ie12ab8de,I4be91274 

* changes:
  SF: Add unit test for mode switching
  SF: Complete mode change immediately if refresh is not required
parents 2ebf7ed1 c7e9f82d

services/surfaceflinger/SurfaceFlinger.cpp

+22 −2
Original line number Diff line number Diff line
@@ -1222,6 +1222,8 @@ void SurfaceFlinger::desiredActiveModeChangeDone(const sp<DisplayDevice>& displa 
void SurfaceFlinger::setActiveModeInHwcIfNeeded() {

    ATRACE_CALL();



    std::optional<PhysicalDisplayId> displayToUpdateImmediately;



    for (const auto& iter : mDisplays) {

        const auto& display = iter.second;

        if (!display || !display->isInternal()) {
@@ -1286,8 +1288,26 @@ void SurfaceFlinger::setActiveModeInHwcIfNeeded() { 
        }

        mScheduler->onNewVsyncPeriodChangeTimeline(outTimeline);



        // Scheduler will submit an empty frame to HWC if needed.

        if (outTimeline.refreshRequired) {

            // Scheduler will submit an empty frame to HWC.

            mSetActiveModePending = true;

        } else {

            // Updating the internal state should be done outside the loop,

            // because it can recreate a DisplayDevice and modify mDisplays

            // which will invalidate the iterator.

            displayToUpdateImmediately = display->getPhysicalId();

        }

    }



    if (displayToUpdateImmediately) {

        updateInternalStateWithChangedMode();



        const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately);

        const auto desiredActiveMode = display->getDesiredActiveMode();

        if (desiredActiveMode &&

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

            desiredActiveModeChangeDone(display);

        }

    }

}

services/surfaceflinger/SurfaceFlinger.h

+1 −2
Original line number Diff line number Diff line
@@ -661,8 +661,7 @@ private: 
    // Sets the desired active mode bit. It obtains the lock, and sets mDesiredActiveMode.

    void setDesiredActiveMode(const ActiveModeInfo& info) REQUIRES(mStateLock);

    status_t setActiveModeFromBackdoor(const sp<IBinder>& displayToken, int id);

    // Once HWC has returned the present fence, this sets the active mode and a new refresh

    // rate in SF.

    // Sets the active mode and a new refresh rate in SF.

    void updateInternalStateWithChangedMode() REQUIRES(mStateLock);

    // Calls to setActiveMode on the main thread if there is a pending mode change

    // that needs to be applied.

services/surfaceflinger/tests/unittests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -71,6 +71,7 @@ cc_test { 
        "MessageQueueTest.cpp",

        "SurfaceFlinger_CreateDisplayTest.cpp",

        "SurfaceFlinger_DestroyDisplayTest.cpp",

        "SurfaceFlinger_DisplayModeSwitching.cpp",

        "SurfaceFlinger_DisplayTransactionCommitTest.cpp",

        "SurfaceFlinger_GetDisplayNativePrimariesTest.cpp",

        "SurfaceFlinger_HotplugTest.cpp",

services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp

0 → 100644
+295 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2021 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#include "mock/MockEventThread.h"

#undef LOG_TAG

#define LOG_TAG "LibSurfaceFlingerUnittests"



#include "DisplayTransactionTestHelpers.h"



#include "Fps.h"



namespace android {

namespace {



using android::hardware::graphics::composer::V2_4::Error;

using android::hardware::graphics::composer::V2_4::VsyncPeriodChangeTimeline;



class DisplayModeSwitchingTest : public DisplayTransactionTest {

public:

    void SetUp() override {

        injectFakeBufferQueueFactory();

        injectFakeNativeWindowSurfaceFactory();



        PrimaryDisplayVariant::setupHwcHotplugCallExpectations(this);

        PrimaryDisplayVariant::setupFramebufferConsumerBufferQueueCallExpectations(this);

        PrimaryDisplayVariant::setupFramebufferProducerBufferQueueCallExpectations(this);

        PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this);

        PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this);



        mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED);



        mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)

                           .setSupportedModes({kDisplayMode60, kDisplayMode90, kDisplayMode120,

                                               kDisplayMode90DifferentResolution})

                           .setActiveMode(kDisplayModeId60)

                           .inject();



        setupScheduler();



        // isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy

        // will call setActiveConfig instead of setActiveConfigWithConstraints.

        ON_CALL(*mComposer, isVsyncPeriodSwitchSupported()).WillByDefault(Return(true));

    }



protected:

    void setupScheduler();

    void testChangeRefreshRate(bool isDisplayActive, bool isRefreshRequired);



    sp<DisplayDevice> mDisplay;

    mock::EventThread* mAppEventThread;



    const DisplayModeId kDisplayModeId60 = DisplayModeId(0);

    const DisplayModePtr kDisplayMode60 =

            DisplayMode::Builder(hal::HWConfigId(kDisplayModeId60.value()))

                    .setId(kDisplayModeId60)

                    .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())

                    .setVsyncPeriod((60_Hz).getPeriodNsecs())

                    .setGroup(0)

                    .setHeight(1000)

                    .setWidth(1000)

                    .build();



    const DisplayModeId kDisplayModeId90 = DisplayModeId(1);

    const DisplayModePtr kDisplayMode90 =

            DisplayMode::Builder(hal::HWConfigId(kDisplayModeId90.value()))

                    .setId(kDisplayModeId90)

                    .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())

                    .setVsyncPeriod((90_Hz).getPeriodNsecs())

                    .setGroup(1)

                    .setHeight(1000)

                    .setWidth(1000)

                    .build();



    const DisplayModeId kDisplayModeId120 = DisplayModeId(2);

    const DisplayModePtr kDisplayMode120 =

            DisplayMode::Builder(hal::HWConfigId(kDisplayModeId120.value()))

                    .setId(kDisplayModeId120)

                    .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())

                    .setVsyncPeriod((120_Hz).getPeriodNsecs())

                    .setGroup(2)

                    .setHeight(1000)

                    .setWidth(1000)

                    .build();



    const DisplayModeId kDisplayModeId90DifferentResolution = DisplayModeId(3);

    const DisplayModePtr kDisplayMode90DifferentResolution =

            DisplayMode::Builder(hal::HWConfigId(kDisplayModeId90DifferentResolution.value()))

                    .setId(kDisplayModeId90DifferentResolution)

                    .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())

                    .setVsyncPeriod((90_Hz).getPeriodNsecs())

                    .setGroup(3)

                    .setHeight(2000)

                    .setWidth(2000)

                    .build();

};



void DisplayModeSwitchingTest::setupScheduler() {

    auto eventThread = std::make_unique<mock::EventThread>();

    mAppEventThread = eventThread.get();

    auto sfEventThread = std::make_unique<mock::EventThread>();



    EXPECT_CALL(*eventThread, registerDisplayEventConnection(_));

    EXPECT_CALL(*eventThread, createEventConnection(_, _))

            .WillOnce(Return(new EventThreadConnection(eventThread.get(), /*callingUid=*/0,

                                                       ResyncCallback())));



    EXPECT_CALL(*sfEventThread, registerDisplayEventConnection(_));

    EXPECT_CALL(*sfEventThread, createEventConnection(_, _))

            .WillOnce(Return(new EventThreadConnection(sfEventThread.get(), /*callingUid=*/0,

                                                       ResyncCallback())));



    auto vsyncController = std::make_unique<mock::VsyncController>();

    auto vsyncTracker = std::make_unique<mock::VSyncTracker>();



    EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));

    EXPECT_CALL(*vsyncTracker, currentPeriod())

            .WillRepeatedly(

                    Return(TestableSurfaceFlinger::FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));

    EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));

    mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),

                            std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr,

                            /*hasMultipleModes*/ true);

}



TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRequired) {

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

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



    mFlinger.onActiveDisplayChanged(mDisplay);



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

                                        kDisplayModeId90.value(), false, 0.f, 120.f, 0.f, 120.f);



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

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

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



    // Verify that next commit will call setActiveConfigWithConstraints in HWC

    const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};

    EXPECT_CALL(*mComposer,

                setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,

                                               hal::HWConfigId(kDisplayModeId90.value()), _, _))

            .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));



    mFlinger.commit();



    Mock::VerifyAndClearExpectations(mComposer);

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

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



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

    // a onModeChanged event to the framework.



    EXPECT_CALL(*mAppEventThread, onModeChanged(kDisplayMode90));

    mFlinger.commit();

    Mock::VerifyAndClearExpectations(mAppEventThread);



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

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

}



TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefreshRequired) {

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



    mFlinger.onActiveDisplayChanged(mDisplay);



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

                                        kDisplayModeId90.value(), true, 0.f, 120.f, 0.f, 120.f);



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

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

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



    // Verify that next commit will call setActiveConfigWithConstraints in HWC

    // and complete the mode change.

    const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};

    EXPECT_CALL(*mComposer,

                setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,

                                               hal::HWConfigId(kDisplayModeId90.value()), _, _))

            .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));



    EXPECT_CALL(*mAppEventThread, onModeChanged(kDisplayMode90));



    mFlinger.commit();



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

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

}



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.



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

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



    mFlinger.onActiveDisplayChanged(mDisplay);



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

                                        kDisplayModeId90.value(), false, 0.f, 120.f, 0.f, 120.f);



    const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};

    EXPECT_CALL(*mComposer,

                setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,

                                               hal::HWConfigId(kDisplayModeId90.value()), _, _))

            .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));



    mFlinger.commit();



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

                                        kDisplayModeId120.value(), false, 0.f, 180.f, 0.f, 180.f);



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

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



    EXPECT_CALL(*mComposer,

                setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,

                                               hal::HWConfigId(kDisplayModeId120.value()), _, _))

            .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));



    mFlinger.commit();



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

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



    mFlinger.commit();



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

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

}



TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefreshRequired) {

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

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



    mFlinger.onActiveDisplayChanged(mDisplay);



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

                                        kDisplayModeId90DifferentResolution.value(), false, 0.f,

                                        120.f, 0.f, 120.f);



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

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

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



    // Verify that next commit will call setActiveConfigWithConstraints in HWC

    // and complete the mode change.

    const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};

    EXPECT_CALL(*mComposer,

                setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,

                                               hal::HWConfigId(

                                                       kDisplayModeId90DifferentResolution.value()),

                                               _, _))

            .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));



    EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplay->getPhysicalId(), true));



    // Misc expecations. We don't need to enforce these method calls, but since the helper methods

    // already set expectations we should add new ones here, otherwise the test will fail.

    EXPECT_CALL(*mConsumer, setDefaultBufferSize(2000, 2000)).WillOnce(Return(NO_ERROR));

    EXPECT_CALL(*mConsumer, consumerConnect(_, false)).WillOnce(Return(NO_ERROR));

    EXPECT_CALL(*mComposer, setClientTargetSlotCount(_)).WillOnce(Return(hal::Error::NONE));



    // Create a new native surface to be used by the recreated display.

    mNativeWindowSurface = nullptr;

    injectFakeNativeWindowSurfaceFactory();

    PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this);



    const auto displayToken = mDisplay->getDisplayToken().promote();



    mFlinger.commit();



    // The DisplayDevice will be destroyed and recreated,

    // so we need to update with the new instance.

    mDisplay = mFlinger.getDisplay(displayToken);



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

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

}



} // namespace

} // namespace android

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+42 −7
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ 
#include <compositionengine/impl/OutputLayerCompositionState.h>

#include <compositionengine/mock/DisplaySurface.h>

#include <gui/ScreenCaptureResults.h>

#include <algorithm>



#include "BufferQueueLayer.h"

#include "BufferStateLayer.h"
@@ -305,6 +306,11 @@ public: 
        return mFlinger->destroyDisplay(displayToken);

    }



    auto getDisplay(const sp<IBinder>& displayToken) {

        Mutex::Autolock lock(mFlinger->mStateLock);

        return mFlinger->getDisplayDeviceLocked(displayToken);

    }



    void enableHalVirtualDisplays(bool enable) { mFlinger->enableHalVirtualDisplays(enable); }



    auto setupNewDisplayDeviceInternal(
@@ -393,6 +399,21 @@ public: 
        return SurfaceFlinger::calculateMaxAcquiredBufferCount(refreshRate, presentLatency);

    }



    auto setDesiredDisplayModeSpecs(const sp<IBinder>& displayToken, ui::DisplayModeId defaultMode,

                                    bool allowGroupSwitching, float primaryRefreshRateMin,

                                    float primaryRefreshRateMax, float appRequestRefreshRateMin,

                                    float appRequestRefreshRateMax) {

        return mFlinger->setDesiredDisplayModeSpecs(displayToken, defaultMode, allowGroupSwitching,

                                                    primaryRefreshRateMin, primaryRefreshRateMax,

                                                    appRequestRefreshRateMin,

                                                    appRequestRefreshRateMax);

    }



    void onActiveDisplayChanged(const sp<DisplayDevice>& activeDisplay) {

        Mutex::Autolock lock(mFlinger->mStateLock);

        mFlinger->onActiveDisplayChangedLocked(activeDisplay);

    }



    /* ------------------------------------------------------------------------

     * Read-only access to private data to assert post-conditions.

     */
@@ -480,7 +501,7 @@ public: 
        static constexpr hal::HWDisplayId DEFAULT_HWC_DISPLAY_ID = 1000;

        static constexpr int32_t DEFAULT_WIDTH = 1920;

        static constexpr int32_t DEFAULT_HEIGHT = 1280;

        static constexpr int32_t DEFAULT_VSYNC_PERIOD = 16'666'666;

        static constexpr int32_t DEFAULT_VSYNC_PERIOD = 16'666'667;

        static constexpr int32_t DEFAULT_CONFIG_GROUP = 7;

        static constexpr int32_t DEFAULT_DPI = 320;

        static constexpr hal::HWConfigId DEFAULT_ACTIVE_CONFIG = 0;
@@ -634,10 +655,10 @@ public: 
            mCreationArgs.connectionType = connectionType;

            mCreationArgs.isPrimary = isPrimary;



            mActiveModeId = DisplayModeId(0);

            mCreationArgs.activeModeId = DisplayModeId(0);

            DisplayModePtr activeMode =

                    DisplayMode::Builder(FakeHwcDisplayInjector::DEFAULT_ACTIVE_CONFIG)

                            .setId(mActiveModeId)

                            .setId(mCreationArgs.activeModeId)

                            .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))

                            .setWidth(FakeHwcDisplayInjector::DEFAULT_WIDTH)

                            .setHeight(FakeHwcDisplayInjector::DEFAULT_HEIGHT)
@@ -673,7 +694,7 @@ public: 
        auto& mutableDisplayDevice() { return mFlinger.mutableDisplays()[mDisplayToken]; }



        auto& setActiveMode(DisplayModeId mode) {

            mActiveModeId = mode;

            mCreationArgs.activeModeId = mode;

            return *this;

        }
@@ -728,14 +749,29 @@ public: 
                const auto physicalId = PhysicalDisplayId::tryCast(*displayId);

                LOG_ALWAYS_FATAL_IF(!physicalId);

                LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);

                state.physical = {.id = *physicalId, .type = *type, .hwcDisplayId = *mHwcDisplayId};



                const DisplayModePtr activeModePtr =

                        *std::find_if(mCreationArgs.supportedModes.begin(),

                                      mCreationArgs.supportedModes.end(), [&](DisplayModePtr mode) {

                                          return mode->getId() == mCreationArgs.activeModeId;

                                      });

                state.physical = {.id = *physicalId,

                                  .type = *type,

                                  .hwcDisplayId = *mHwcDisplayId,

                                  .deviceProductInfo = {},

                                  .supportedModes = mCreationArgs.supportedModes,

                                  .activeMode = activeModePtr};

            }



            state.isSecure = mCreationArgs.isSecure;



            mCreationArgs.refreshRateConfigs =

                    std::make_shared<scheduler::RefreshRateConfigs>(mCreationArgs.supportedModes,

                                                                    mCreationArgs.activeModeId);



            sp<DisplayDevice> device = new DisplayDevice(mCreationArgs);

            if (!device->isVirtual()) {

                device->setActiveMode(mActiveModeId);

                device->setActiveMode(mCreationArgs.activeModeId);

            }

            mFlinger.mutableDisplays().emplace(mDisplayToken, device);

            mFlinger.mutableCurrentState().displays.add(mDisplayToken, state);
@@ -753,7 +789,6 @@ public: 
        sp<BBinder> mDisplayToken = new BBinder();

        DisplayDeviceCreationArgs mCreationArgs;

        const std::optional<hal::HWDisplayId> mHwcDisplayId;

        DisplayModeId mActiveModeId;

    };



private: