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

Commit 68a94093 authored by Marin Shalamanov's avatar Marin Shalamanov
Browse files

SF: Don't store config ids in RefreshRateStats 

Configs IDs can change during runtime, so we shouldn't store
them to calculate the refresh rate stats. Instead we store
only the current refresh rate. Additionally this CL removes
the dependency on RefreshRateConfigs.

Bug: 159590486
Test: atest RefreshRateStatsTest
Change-Id: Id0af0598fb1c72ea4c1328bdb372506f0436b8da
parent e8a663d7

services/surfaceflinger/Scheduler/RefreshRateStats.h

+15 −23
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ 


#include <numeric>



#include "Scheduler/RefreshRateConfigs.h"

#include "Fps.h"

#include "Scheduler/SchedulerUtils.h"

#include "TimeStats/TimeStats.h"
@@ -40,12 +40,10 @@ class RefreshRateStats { 
    static constexpr int64_t MS_PER_DAY = 24 * MS_PER_HOUR;



public:

    RefreshRateStats(const RefreshRateConfigs& refreshRateConfigs, TimeStats& timeStats,

                     HwcConfigIndexType currentConfigId,

    RefreshRateStats(TimeStats& timeStats, Fps currentRefreshRate,

                     android::hardware::graphics::composer::hal::PowerMode currentPowerMode)

          : mRefreshRateConfigs(refreshRateConfigs),

            mTimeStats(timeStats),

            mCurrentConfigMode(currentConfigId),

          : mTimeStats(timeStats),

            mCurrentRefreshRate(currentRefreshRate),

            mCurrentPowerMode(currentPowerMode) {}



    // Sets power mode.
@@ -59,12 +57,12 @@ public: 


    // Sets config mode. If the mode has changed, it records how much time was spent in the previous

    // mode.

    void setConfigMode(HwcConfigIndexType configId) {

        if (mCurrentConfigMode == configId) {

    void setRefreshRate(Fps currRefreshRate) {

        if (mCurrentRefreshRate.equalsWithMargin(currRefreshRate)) {

            return;

        }

        flushTime();

        mCurrentConfigMode = configId;

        mCurrentRefreshRate = currRefreshRate;

    }



    // Returns a map between human readable refresh rate and number of seconds the device spent in
@@ -79,10 +77,10 @@ public: 
        // Multiple configs may map to the same name, e.g. "60fps". Add the

        // times for such configs together.

        for (const auto& [configId, time] : mConfigModesTotalTime) {

            totalTime[mRefreshRateConfigs.getRefreshRateFromConfigId(configId).getName()] = 0;

            totalTime[to_string(configId)] = 0;

        }

        for (const auto& [configId, time] : mConfigModesTotalTime) {

            totalTime[mRefreshRateConfigs.getRefreshRateFromConfigId(configId).getName()] += time;

            totalTime[to_string(configId)] += time;

        }

        totalTime["ScreenOff"] = mScreenOffTime;

        return totalTime;
@@ -111,14 +109,11 @@ private: 
        uint32_t fps = 0;

        if (mCurrentPowerMode == android::hardware::graphics::composer::hal::PowerMode::ON) {

            // Normal power mode is counted under different config modes.

            if (mConfigModesTotalTime.find(mCurrentConfigMode) == mConfigModesTotalTime.end()) {

                mConfigModesTotalTime[mCurrentConfigMode] = 0;

            if (mConfigModesTotalTime.find(mCurrentRefreshRate) == mConfigModesTotalTime.end()) {

                mConfigModesTotalTime[mCurrentRefreshRate] = 0;

            }

            mConfigModesTotalTime[mCurrentConfigMode] += timeElapsedMs;

            fps = static_cast<uint32_t>(

                    std::round(mRefreshRateConfigs.getRefreshRateFromConfigId(mCurrentConfigMode)

                                       .getFps()

                                       .getValue()));

            mConfigModesTotalTime[mCurrentRefreshRate] += timeElapsedMs;

            fps = static_cast<uint32_t>(mCurrentRefreshRate.getIntValue());

        } else {

            mScreenOffTime += timeElapsedMs;

        }
@@ -136,16 +131,13 @@ private: 
                                  days, hours, mins, sec, secRemainderMs);

    }



    // Keeps information about refresh rate configs that device has.

    const RefreshRateConfigs& mRefreshRateConfigs;



    // Aggregate refresh rate statistics for telemetry.

    TimeStats& mTimeStats;



    HwcConfigIndexType mCurrentConfigMode;

    Fps mCurrentRefreshRate;

    android::hardware::graphics::composer::hal::PowerMode mCurrentPowerMode;



    std::unordered_map<HwcConfigIndexType /* configId */, int64_t /* duration in ms */>

    std::unordered_map<Fps, int64_t /* duration in ms */, std::hash<Fps>, Fps::EqualsInBuckets>

            mConfigModesTotalTime;

    int64_t mScreenOffTime = 0;

services/surfaceflinger/SurfaceFlinger.cpp

+6 −6
Original line number Diff line number Diff line
@@ -1071,11 +1071,12 @@ void SurfaceFlinger::setActiveConfigInternal() { 


    std::lock_guard<std::mutex> lock(mActiveConfigLock);

    mRefreshRateConfigs->setCurrentConfigId(mUpcomingActiveConfig.configId);

    mRefreshRateStats->setConfigMode(mUpcomingActiveConfig.configId);

    display->setActiveConfig(mUpcomingActiveConfig.configId);



    auto& refreshRate =

            mRefreshRateConfigs->getRefreshRateFromConfigId(mUpcomingActiveConfig.configId);

    mRefreshRateStats->setRefreshRate(refreshRate.getFps());



    if (refreshRate.getVsyncPeriod() != oldRefreshRate.getVsyncPeriod()) {

        mTimeStats->incrementRefreshRateSwitches();

    }
@@ -2868,10 +2869,10 @@ void SurfaceFlinger::initScheduler(PhysicalDisplayId primaryDisplayId) { 
            std::make_unique<scheduler::RefreshRateConfigs>(getHwComposer().getConfigs(

                                                                    primaryDisplayId),

                                                            currentConfig);

    const auto& currRefreshRate = mRefreshRateConfigs->getRefreshRateFromConfigId(currentConfig);

    mRefreshRateStats =

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

                                                          currentConfig, hal::PowerMode::OFF);

    mRefreshRateStats->setConfigMode(currentConfig);

            std::make_unique<scheduler::RefreshRateStats>(*mTimeStats, currRefreshRate.getFps(),

                                                          hal::PowerMode::OFF);



    mVsyncConfiguration = getFactory().createVsyncConfiguration(*mRefreshRateConfigs);

    mVsyncModulator.emplace(mVsyncConfiguration->getCurrentConfigs());
@@ -2879,8 +2880,7 @@ void SurfaceFlinger::initScheduler(PhysicalDisplayId primaryDisplayId) { 
    // start the EventThread

    mScheduler = getFactory().createScheduler(*mRefreshRateConfigs, *this);

    const auto configs = mVsyncConfiguration->getCurrentConfigs();

    const nsecs_t vsyncPeriod =

            mRefreshRateConfigs->getRefreshRateFromConfigId(currentConfig).getVsyncPeriod();

    const nsecs_t vsyncPeriod = currRefreshRate.getVsyncPeriod();

    mAppConnectionHandle =

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

                                         /*workDuration=*/configs.late.appWorkDuration,

services/surfaceflinger/tests/unittests/RefreshRateStatsTest.cpp

+17 −11
Original line number Diff line number Diff line
@@ -25,6 +25,8 @@ 
#include <log/log.h>

#include <thread>



#include "Scheduler/HwcStrongTypes.h"

#include "Scheduler/RefreshRateConfigs.h"

#include "Scheduler/RefreshRateStats.h"

#include "mock/DisplayHardware/MockDisplay.h"

#include "mock/MockTimeStats.h"
@@ -51,8 +53,9 @@ protected: 
    void init(const std::vector<std::shared_ptr<const HWC2::Display::Config>>& configs) {

        mRefreshRateConfigs =

                std::make_unique<RefreshRateConfigs>(configs, /*currentConfig=*/CONFIG_ID_0);

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

                                                               /*currentConfigId=*/CONFIG_ID_0,



        const auto currFps = mRefreshRateConfigs->getRefreshRateFromConfigId(CONFIG_ID_0).getFps();

        mRefreshRateStats = std::make_unique<RefreshRateStats>(mTimeStats, currFps,

                                                               /*currentPowerMode=*/PowerMode::OFF);

    }
@@ -81,7 +84,7 @@ RefreshRateStatsTest::~RefreshRateStatsTest() { 
std::shared_ptr<const HWC2::Display::Config> RefreshRateStatsTest::createConfig(

        HwcConfigIndexType configId, int32_t configGroup, int64_t vsyncPeriod) {

    return HWC2::Display::Config::Builder(mDisplay, configId.value())

            .setVsyncPeriod(int32_t(vsyncPeriod))

            .setVsyncPeriod(static_cast<int32_t>(vsyncPeriod))

            .setConfigGroup(configGroup)

            .build();

}
@@ -110,7 +113,8 @@ TEST_F(RefreshRateStatsTest, oneConfigTest) { 
    EXPECT_LT(screenOff, times["ScreenOff"]);

    EXPECT_EQ(0u, times.count("90.00fps"));



    mRefreshRateStats->setConfigMode(CONFIG_ID_0);

    const auto config0Fps = mRefreshRateConfigs->getRefreshRateFromConfigId(CONFIG_ID_0).getFps();

    mRefreshRateStats->setRefreshRate(config0Fps);

    mRefreshRateStats->setPowerMode(PowerMode::ON);

    screenOff = mRefreshRateStats->getTotalTimes()["ScreenOff"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));
@@ -126,7 +130,7 @@ TEST_F(RefreshRateStatsTest, oneConfigTest) { 
    EXPECT_LT(screenOff, times["ScreenOff"]);

    EXPECT_EQ(ninety, times["90.00fps"]);



    mRefreshRateStats->setConfigMode(CONFIG_ID_0);

    mRefreshRateStats->setRefreshRate(config0Fps);

    screenOff = mRefreshRateStats->getTotalTimes()["ScreenOff"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));

    times = mRefreshRateStats->getTotalTimes();
@@ -157,7 +161,9 @@ TEST_F(RefreshRateStatsTest, twoConfigsTest) { 
    times = mRefreshRateStats->getTotalTimes();

    EXPECT_LT(screenOff, times["ScreenOff"]);



    mRefreshRateStats->setConfigMode(CONFIG_ID_0);

    const auto config0Fps = mRefreshRateConfigs->getRefreshRateFromConfigId(CONFIG_ID_0).getFps();

    const auto config1Fps = mRefreshRateConfigs->getRefreshRateFromConfigId(CONFIG_ID_1).getFps();

    mRefreshRateStats->setRefreshRate(config0Fps);

    mRefreshRateStats->setPowerMode(PowerMode::ON);

    screenOff = mRefreshRateStats->getTotalTimes()["ScreenOff"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));
@@ -167,7 +173,7 @@ TEST_F(RefreshRateStatsTest, twoConfigsTest) { 
    EXPECT_LT(0, times["90.00fps"]);



    // When power mode is normal, time for configs updates.

    mRefreshRateStats->setConfigMode(CONFIG_ID_1);

    mRefreshRateStats->setRefreshRate(config1Fps);

    int ninety = mRefreshRateStats->getTotalTimes()["90.00fps"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));

    times = mRefreshRateStats->getTotalTimes();
@@ -176,7 +182,7 @@ TEST_F(RefreshRateStatsTest, twoConfigsTest) { 
    ASSERT_EQ(1u, times.count("60.00fps"));

    EXPECT_LT(0, times["60.00fps"]);



    mRefreshRateStats->setConfigMode(CONFIG_ID_0);

    mRefreshRateStats->setRefreshRate(config0Fps);

    int sixty = mRefreshRateStats->getTotalTimes()["60.00fps"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));

    times = mRefreshRateStats->getTotalTimes();
@@ -184,7 +190,7 @@ TEST_F(RefreshRateStatsTest, twoConfigsTest) { 
    EXPECT_LT(ninety, times["90.00fps"]);

    EXPECT_EQ(sixty, times["60.00fps"]);



    mRefreshRateStats->setConfigMode(CONFIG_ID_1);

    mRefreshRateStats->setRefreshRate(config1Fps);

    ninety = mRefreshRateStats->getTotalTimes()["90.00fps"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));

    times = mRefreshRateStats->getTotalTimes();
@@ -195,7 +201,7 @@ TEST_F(RefreshRateStatsTest, twoConfigsTest) { 
    // Because the power mode is not PowerMode::ON, switching the config

    // does not update refresh rates that come from the config.

    mRefreshRateStats->setPowerMode(PowerMode::DOZE);

    mRefreshRateStats->setConfigMode(CONFIG_ID_0);

    mRefreshRateStats->setRefreshRate(config0Fps);

    sixty = mRefreshRateStats->getTotalTimes()["60.00fps"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));

    times = mRefreshRateStats->getTotalTimes();
@@ -203,7 +209,7 @@ TEST_F(RefreshRateStatsTest, twoConfigsTest) { 
    EXPECT_EQ(ninety, times["90.00fps"]);

    EXPECT_EQ(sixty, times["60.00fps"]);



    mRefreshRateStats->setConfigMode(CONFIG_ID_1);

    mRefreshRateStats->setRefreshRate(config1Fps);

    screenOff = mRefreshRateStats->getTotalTimes()["ScreenOff"];

    std::this_thread::sleep_for(std::chrono::milliseconds(2));

    times = mRefreshRateStats->getTotalTimes();

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

+8 −6
Original line number Diff line number Diff line
@@ -223,11 +223,13 @@ public: 
                                         .build());

        }



        mFlinger->mRefreshRateConfigs = std::make_unique<

                scheduler::RefreshRateConfigs>(configs, /*currentConfig=*/HwcConfigIndexType(0));

        mFlinger->mRefreshRateStats = std::make_unique<

                scheduler::RefreshRateStats>(*mFlinger->mRefreshRateConfigs, *mFlinger->mTimeStats,

                                             /*currentConfig=*/HwcConfigIndexType(0),

        const auto currConfig = HwcConfigIndexType(0);

        mFlinger->mRefreshRateConfigs =

                std::make_unique<scheduler::RefreshRateConfigs>(configs, currConfig);

        const auto currFps =

                mFlinger->mRefreshRateConfigs->getRefreshRateFromConfigId(currConfig).getFps();

        mFlinger->mRefreshRateStats =

                std::make_unique<scheduler::RefreshRateStats>(*mFlinger->mTimeStats, currFps,

                                                              /*powerMode=*/hal::PowerMode::OFF);

        mFlinger->mVsyncConfiguration =

                mFactory.createVsyncConfiguration(*mFlinger->mRefreshRateConfigs);