SurfaceFlinger: fix refresh rate override when changing refresh rate
If an app's frame rate is same as the display refresh rate, keep it
on the frame rate override list. The reason for this is if an app
is added to the list after the display refresh rate changed, the few
first frames would be running the the display refresh rate and not at
the override one.
Test: atest FrameRateOverrideHostTest
Test: run TouchLatency while changing refresh rates and collect systrace
Bug: 182544360
Change-Id: I66472a1230affae9336520083f0950ae252ede02
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index 91e8043..a5cf797 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -499,11 +499,7 @@
// Now that we scored all the refresh rates we need to pick the one that got the highest
// score.
const RefreshRate* bestRefreshRate = getBestRefreshRate(scores.begin(), scores.end());
-
- // If the nest refresh rate is the current one, we don't have an override
- if (!bestRefreshRate->getFps().equalsWithMargin(displayFrameRate)) {
- frameRateOverrides.emplace(uid, bestRefreshRate->getFps());
- }
+ frameRateOverrides.emplace(uid, bestRefreshRate->getFps());
}
return frameRateOverrides;
@@ -839,11 +835,6 @@
return static_cast<int>(numPeriodsRounded);
}
-int RefreshRateConfigs::getRefreshRateDivider(Fps frameRate) const {
- std::lock_guard lock(mLock);
- return getFrameRateDivider(mCurrentRefreshRate->getFps(), frameRate);
-}
-
void RefreshRateConfigs::dump(std::string& result) const {
std::lock_guard lock(mLock);
base::StringAppendF(&result, "DesiredDisplayModeSpecs (DisplayManager): %s\n\n",
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index 2bc22b4..ee89149 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -324,8 +324,10 @@
bool supportsFrameRateOverride() const { return mSupportsFrameRateOverride; }
- // Returns a divider for the current refresh rate
- int getRefreshRateDivider(Fps frameRate) const EXCLUDES(mLock);
+ // Return the display refresh rate divider to match the layer
+ // frame rate, or 0 if the display refresh rate is not a multiple of the
+ // layer refresh rate.
+ static int getFrameRateDivider(Fps displayFrameRate, Fps layerFrameRate);
using UidToFrameRateOverride = std::map<uid_t, Fps>;
// Returns the frame rate override for each uid.
@@ -373,11 +375,6 @@
const Policy* getCurrentPolicyLocked() const REQUIRES(mLock);
bool isPolicyValidLocked(const Policy& policy) const REQUIRES(mLock);
- // Return the display refresh rate divider to match the layer
- // frame rate, or 0 if the display refresh rate is not a multiple of the
- // layer refresh rate.
- static int getFrameRateDivider(Fps displayFrameRate, Fps layerFrameRate);
-
// calculates a score for a layer. Used to determine the display refresh rate
// and the frame rate override for certains applications.
float calculateLayerScoreLocked(const LayerRequirement&, const RefreshRate&,
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 9813270..4edbdd2 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -235,12 +235,7 @@
return true;
}
- const auto divider = mRefreshRateConfigs.getRefreshRateDivider(*frameRate);
- if (divider <= 1) {
- return true;
- }
-
- return mVsyncSchedule.tracker->isVSyncInPhase(expectedVsyncTimestamp, divider);
+ return mVsyncSchedule.tracker->isVSyncInPhase(expectedVsyncTimestamp, *frameRate);
}
impl::EventThread::ThrottleVsyncCallback Scheduler::makeThrottleVsyncCallback() const {
@@ -354,6 +349,10 @@
std::lock_guard<std::mutex> lock(mFeatureStateLock);
// Cache the last reported modes for primary display.
mFeatures.cachedModeChangedParams = {handle, displayId, modeId, vsyncPeriod};
+
+ // Invalidate content based refresh rate selection so it could be calculated
+ // again for the new refresh rate.
+ mFeatures.contentRequirements.clear();
}
onNonPrimaryDisplayModeChanged(handle, displayId, modeId, vsyncPeriod);
}
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index 7cca206..028f7a6 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -30,6 +30,7 @@
#include <algorithm>
#include <chrono>
#include <sstream>
+#include "RefreshRateConfigs.h"
#undef LOG_TAG
#define LOG_TAG "VSyncPredictor"
@@ -225,13 +226,14 @@
}
/*
- * Returns whether a given vsync timestamp is in phase with a vsync divider.
- * For example, if the vsync timestamps are (16,32,48,64):
- * isVSyncInPhase(16, 2) = true
- * isVSyncInPhase(32, 2) = false
- * isVSyncInPhase(48, 2) = true
+ * Returns whether a given vsync timestamp is in phase with a frame rate.
+ * If the frame rate is not a divider of the refresh rate, it is always considered in phase.
+ * For example, if the vsync timestamps are (16.6,33.3,50.0,66.6):
+ * isVSyncInPhase(16.6, 30) = true
+ * isVSyncInPhase(33.3, 30) = false
+ * isVSyncInPhase(50.0, 30) = true
*/
-bool VSyncPredictor::isVSyncInPhase(nsecs_t timePoint, int divider) const {
+bool VSyncPredictor::isVSyncInPhase(nsecs_t timePoint, Fps frameRate) const {
struct VsyncError {
nsecs_t vsyncTimestamp;
float error;
@@ -239,11 +241,13 @@
bool operator<(const VsyncError& other) const { return error < other.error; }
};
+ std::lock_guard lock(mMutex);
+ const auto divider =
+ RefreshRateConfigs::getFrameRateDivider(Fps::fromPeriodNsecs(mIdealPeriod), frameRate);
if (divider <= 1 || timePoint == 0) {
return true;
}
- std::lock_guard lock(mMutex);
const nsecs_t period = mRateMap[mIdealPeriod].slope;
const nsecs_t justBeforeTimePoint = timePoint - period / 2;
const nsecs_t dividedPeriod = mIdealPeriod / divider;
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index 381cf81..40e6944 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -64,7 +64,7 @@
VSyncPredictor::Model getVSyncPredictionModel() const EXCLUDES(mMutex);
- bool isVSyncInPhase(nsecs_t timePoint, int divider) const final EXCLUDES(mMutex);
+ bool isVSyncInPhase(nsecs_t timePoint, Fps frameRate) const final EXCLUDES(mMutex);
void dump(std::string& result) const final EXCLUDES(mMutex);
diff --git a/services/surfaceflinger/Scheduler/VSyncTracker.h b/services/surfaceflinger/Scheduler/VSyncTracker.h
index 2cd9b3d..95750ad 100644
--- a/services/surfaceflinger/Scheduler/VSyncTracker.h
+++ b/services/surfaceflinger/Scheduler/VSyncTracker.h
@@ -17,6 +17,7 @@
#pragma once
#include <utils/Timers.h>
+#include "Fps.h"
#include "VSyncDispatch.h"
namespace android::scheduler {
@@ -69,12 +70,12 @@
virtual bool needsMoreSamples() const = 0;
/*
- * Checks if a vsync timestamp is in phase for a given divider.
+ * Checks if a vsync timestamp is in phase for a frame rate
*
* \param [in] timePoint A vsync timestamp
- * \param [in] divider The divider to check for
+ * \param [in] frameRate The frame rate to check for
*/
- virtual bool isVSyncInPhase(nsecs_t timePoint, int divider) const = 0;
+ virtual bool isVSyncInPhase(nsecs_t timePoint, Fps frameRate) const = 0;
virtual void dump(std::string& result) const = 0;
diff --git a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
index 06f2036..15bd0e1 100644
--- a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
@@ -1621,29 +1621,35 @@
EXPECT_EQ(KernelIdleTimerAction::TurnOff, refreshRateConfigs->getIdleTimerAction());
}
-TEST_F(RefreshRateConfigsTest, getRefreshRateDivider) {
+TEST_F(RefreshRateConfigsTest, getFrameRateDivider) {
auto refreshRateConfigs =
std::make_unique<RefreshRateConfigs>(m30_60_72_90_120Device,
/*currentConfigId=*/HWC_CONFIG_ID_30);
const auto frameRate = Fps(30.f);
- EXPECT_EQ(1, refreshRateConfigs->getRefreshRateDivider(frameRate));
+ Fps displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps();
+ EXPECT_EQ(1, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, frameRate));
refreshRateConfigs->setCurrentModeId(HWC_CONFIG_ID_60);
- EXPECT_EQ(2, refreshRateConfigs->getRefreshRateDivider(frameRate));
+ displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps();
+ EXPECT_EQ(2, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, frameRate));
refreshRateConfigs->setCurrentModeId(HWC_CONFIG_ID_72);
- EXPECT_EQ(0, refreshRateConfigs->getRefreshRateDivider(frameRate));
+ displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps();
+ EXPECT_EQ(0, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, frameRate));
refreshRateConfigs->setCurrentModeId(HWC_CONFIG_ID_90);
- EXPECT_EQ(3, refreshRateConfigs->getRefreshRateDivider(frameRate));
+ displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps();
+ EXPECT_EQ(3, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, frameRate));
refreshRateConfigs->setCurrentModeId(HWC_CONFIG_ID_120);
- EXPECT_EQ(4, refreshRateConfigs->getRefreshRateDivider(frameRate));
+ displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps();
+ EXPECT_EQ(4, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, frameRate));
refreshRateConfigs->setCurrentModeId(HWC_CONFIG_ID_90);
- EXPECT_EQ(4, refreshRateConfigs->getRefreshRateDivider(Fps(22.5f)));
- EXPECT_EQ(4, refreshRateConfigs->getRefreshRateDivider(Fps(22.6f)));
+ displayRefreshRate = refreshRateConfigs->getCurrentRefreshRate().getFps();
+ EXPECT_EQ(4, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, Fps(22.5f)));
+ EXPECT_EQ(4, RefreshRateConfigs::getFrameRateDivider(displayRefreshRate, Fps(22.6f)));
}
TEST_F(RefreshRateConfigsTest, getFrameRateOverrides_noLayers) {
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index 0af5f30..42b1993 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -52,7 +52,7 @@
void setPeriod(nsecs_t) final {}
void resetModel() final {}
bool needsMoreSamples() const final { return false; }
- bool isVSyncInPhase(nsecs_t, int) const final { return false; }
+ bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
void dump(std::string&) const final {}
private:
@@ -89,7 +89,7 @@
void setPeriod(nsecs_t) final {}
void resetModel() final {}
bool needsMoreSamples() const final { return false; }
- bool isVSyncInPhase(nsecs_t, int) const final { return false; }
+ bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
void dump(std::string&) const final {}
private:
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 00cf574..b64cce9 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -49,7 +49,7 @@
MOCK_METHOD1(setPeriod, void(nsecs_t));
MOCK_METHOD0(resetModel, void());
MOCK_CONST_METHOD0(needsMoreSamples, bool());
- MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, int));
+ MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, Fps));
MOCK_CONST_METHOD1(dump, void(std::string&));
nsecs_t nextVSyncTime(nsecs_t timePoint) const {
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index a4ddbf4..2a658dd 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -469,7 +469,9 @@
for (int divider = 1; divider < maxDivider; divider++) {
for (int i = 0; i < maxPeriods; i++) {
const bool expectedInPhase = (i % divider) == 0;
- EXPECT_THAT(expectedInPhase, tracker.isVSyncInPhase(mNow + i * mPeriod - bias, divider))
+ EXPECT_THAT(expectedInPhase,
+ tracker.isVSyncInPhase(mNow + i * mPeriod - bias,
+ Fps::fromPeriodNsecs(divider * mPeriod)))
<< "vsync at " << mNow + (i + 1) * mPeriod - bias << " is "
<< (expectedInPhase ? "not " : "") << "in phase for divider " << divider;
}
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index b9651ea..5826a9b 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -46,7 +46,7 @@
MOCK_METHOD1(setPeriod, void(nsecs_t));
MOCK_METHOD0(resetModel, void());
MOCK_CONST_METHOD0(needsMoreSamples, bool());
- MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, int));
+ MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, Fps));
MOCK_CONST_METHOD1(dump, void(std::string&));
};
diff --git a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
index de98025..5b0c1f3 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
@@ -33,7 +33,7 @@
MOCK_METHOD1(setPeriod, void(nsecs_t));
MOCK_METHOD0(resetModel, void());
MOCK_CONST_METHOD0(needsMoreSamples, bool());
- MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, int));
+ MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, Fps));
MOCK_CONST_METHOD1(dump, void(std::string&));
};