SF: use frame rate override even when the physical range is single
When the physical range is a single rate (e.g. 120Hz), render rates
where discarded if they could be run at a lower physical rate
(e.g. 30fps of 60Hz). This CL enables these frame rates to be able to
save power by running on a slower rate.
Bug: 296079213
Test: play 30fps video while the device policy has a
single physical refresh rate
Test: atest libsurfaceflinger_unittest
Change-Id: I4d6d6ddffd6004022d02329148709204ff9d57db
Merged-In: I4d6d6ddffd6004022d02329148709204ff9d57db
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
index c44e22e..6b7d7df 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
@@ -148,8 +148,8 @@
} // namespace
auto RefreshRateSelector::createFrameRateModes(
- std::function<bool(const DisplayMode&)>&& filterModes, const FpsRange& renderRange) const
- -> std::vector<FrameRateMode> {
+ const Policy& policy, std::function<bool(const DisplayMode&)>&& filterModes,
+ const FpsRange& renderRange) const -> std::vector<FrameRateMode> {
struct Key {
Fps fps;
int32_t group;
@@ -202,11 +202,25 @@
ALOGV("%s: including %s (%s)", __func__, to_string(fps).c_str(),
to_string(mode->getFps()).c_str());
} else {
- // We might need to update the map as we found a lower refresh rate
- if (isStrictlyLess(mode->getFps(), existingIter->second->second->getFps())) {
+ // If the primary physical range is a single rate, prefer to stay in that rate
+ // even if there is a lower physical refresh rate available. This would cause more
+ // cases to stay within the primary physical range
+ const Fps existingModeFps = existingIter->second->second->getFps();
+ const bool existingModeIsPrimaryRange = policy.primaryRangeIsSingleRate() &&
+ policy.primaryRanges.physical.includes(existingModeFps);
+ const bool newModeIsPrimaryRange = policy.primaryRangeIsSingleRate() &&
+ policy.primaryRanges.physical.includes(mode->getFps());
+ if (newModeIsPrimaryRange == existingModeIsPrimaryRange) {
+ // We might need to update the map as we found a lower refresh rate
+ if (isStrictlyLess(mode->getFps(), existingModeFps)) {
+ existingIter->second = it;
+ ALOGV("%s: changing %s (%s) as we found a lower physical rate", __func__,
+ to_string(fps).c_str(), to_string(mode->getFps()).c_str());
+ }
+ } else if (newModeIsPrimaryRange) {
existingIter->second = it;
- ALOGV("%s: changing %s (%s)", __func__, to_string(fps).c_str(),
- to_string(mode->getFps()).c_str());
+ ALOGV("%s: changing %s (%s) to stay in the primary range", __func__,
+ to_string(fps).c_str(), to_string(mode->getFps()).c_str());
}
}
}
@@ -500,10 +514,8 @@
// If the primary range consists of a single refresh rate then we can only
// move out the of range if layers explicitly request a different refresh
// rate.
- const bool primaryRangeIsSingleRate =
- isApproxEqual(policy->primaryRanges.physical.min, policy->primaryRanges.physical.max);
-
- if (!signals.touch && signals.idle && !(primaryRangeIsSingleRate && hasExplicitVoteLayers)) {
+ if (!signals.touch && signals.idle &&
+ !(policy->primaryRangeIsSingleRate() && hasExplicitVoteLayers)) {
ALOGV("Idle");
const auto ranking = rankFrameRates(activeMode.getGroup(), RefreshRateOrder::Ascending);
ATRACE_FORMAT_INSTANT("%s (Idle)", to_string(ranking.front().frameRateMode.fps).c_str());
@@ -577,8 +589,11 @@
continue;
}
- const bool inPrimaryRange = policy->primaryRanges.render.includes(fps);
- if ((primaryRangeIsSingleRate || !inPrimaryRange) &&
+ const bool inPrimaryPhysicalRange =
+ policy->primaryRanges.physical.includes(modePtr->getFps());
+ const bool inPrimaryRenderRange = policy->primaryRanges.render.includes(fps);
+ if (((policy->primaryRangeIsSingleRate() && !inPrimaryPhysicalRange) ||
+ !inPrimaryRenderRange) &&
!(layer.focused &&
(layer.vote == LayerVoteType::ExplicitDefault ||
layer.vote == LayerVoteType::ExplicitExact))) {
@@ -689,7 +704,7 @@
return score.overallScore == 0;
});
- if (primaryRangeIsSingleRate) {
+ if (policy->primaryRangeIsSingleRate()) {
// If we never scored any layers, then choose the rate from the primary
// range instead of picking a random score from the app range.
if (noLayerScore) {
@@ -1234,14 +1249,14 @@
(supportsFrameRateOverride() || ranges.render.includes(mode.getFps()));
};
- auto frameRateModes = createFrameRateModes(filterModes, ranges.render);
+ auto frameRateModes = createFrameRateModes(*policy, filterModes, ranges.render);
if (frameRateModes.empty()) {
ALOGW("No matching frame rate modes for %s range. policy: %s", rangeName,
policy->toString().c_str());
// TODO(b/292105422): Ideally DisplayManager should not send render ranges smaller than
// the min supported. See b/292047939.
// For not we just ignore the render ranges.
- frameRateModes = createFrameRateModes(filterModes, {});
+ frameRateModes = createFrameRateModes(*policy, filterModes, {});
}
LOG_ALWAYS_FATAL_IF(frameRateModes.empty(),
"No matching frame rate modes for %s range even after ignoring the "
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.h b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
index 7af8d03..b25919e 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
@@ -101,6 +101,11 @@
}
bool operator!=(const Policy& other) const { return !(*this == other); }
+
+ bool primaryRangeIsSingleRate() const {
+ return isApproxEqual(primaryRanges.physical.min, primaryRanges.physical.max);
+ }
+
std::string toString() const;
};
@@ -468,8 +473,8 @@
}
std::vector<FrameRateMode> createFrameRateModes(
- std::function<bool(const DisplayMode&)>&& filterModes, const FpsRange&) const
- REQUIRES(mLock);
+ const Policy&, std::function<bool(const DisplayMode&)>&& filterModes,
+ const FpsRange&) const REQUIRES(mLock);
// The display modes of the active display. The DisplayModeIterators below are pointers into
// this container, so must be invalidated whenever the DisplayModes change. The Policy below
diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
index 60ad7a3..2d87ddd 100644
--- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
@@ -78,7 +78,7 @@
mDisplay->setDesiredActiveMode(
{scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, mDisplay->getDesiredActiveMode()->modeOpt);
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
// Setting another mode should be cached but return None
@@ -86,7 +86,7 @@
mDisplay->setDesiredActiveMode(
{scheduler::FrameRateMode{120_Hz, kMode120}, Event::None}));
ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, mDisplay->getDesiredActiveMode()->modeOpt);
+ EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
}
@@ -105,7 +105,7 @@
mDisplay->setDesiredActiveMode(
{scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, mDisplay->getDesiredActiveMode()->modeOpt);
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
hal::VsyncPeriodChangeConstraints constraints{
@@ -136,7 +136,7 @@
mDisplay->setDesiredActiveMode(
{scheduler::FrameRateMode{90_Hz, kMode90}, Event::None}));
ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, mDisplay->getDesiredActiveMode()->modeOpt);
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
hal::VsyncPeriodChangeConstraints constraints{
@@ -154,7 +154,7 @@
mDisplay->setDesiredActiveMode(
{scheduler::FrameRateMode{120_Hz, kMode120}, Event::None}));
ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
- EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, mDisplay->getDesiredActiveMode()->modeOpt);
+ EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getDesiredActiveMode()->modeOpt);
EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getUpcomingActiveMode().modeOpt);
diff --git a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp
index aaf55fb..0397b99 100644
--- a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp
@@ -3125,5 +3125,69 @@
{DisplayModeId(kModeId60), kLowerThanMin, kLowerThanMin}));
}
+// b/296079213
+TEST_P(RefreshRateSelectorTest, frameRateOverrideInBlockingZone60_120) {
+ auto selector = createSelector(kModes_60_120, kModeId120);
+
+ const FpsRange only120 = {120_Hz, 120_Hz};
+ const FpsRange allRange = {0_Hz, 120_Hz};
+ EXPECT_EQ(SetPolicyResult::Changed,
+ selector.setDisplayManagerPolicy(
+ {kModeId120, {only120, allRange}, {allRange, allRange}}));
+
+ std::vector<LayerRequirement> layers = {{.weight = 1.f}};
+ layers[0].name = "30Hz ExplicitExactOrMultiple";
+ layers[0].desiredRefreshRate = 30_Hz;
+ layers[0].vote = LayerVoteType::ExplicitExactOrMultiple;
+
+ if (GetParam() != Config::FrameRateOverride::Enabled) {
+ EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz,
+ selector.getBestScoredFrameRate(layers).frameRateMode);
+ } else {
+ EXPECT_FRAME_RATE_MODE(kMode120, 30_Hz,
+ selector.getBestScoredFrameRate(layers).frameRateMode);
+ }
+}
+
+TEST_P(RefreshRateSelectorTest, frameRateOverrideInBlockingZone60_90) {
+ auto selector = createSelector(kModes_60_90, kModeId90);
+
+ const FpsRange only90 = {90_Hz, 90_Hz};
+ const FpsRange allRange = {0_Hz, 90_Hz};
+ EXPECT_EQ(SetPolicyResult::Changed,
+ selector.setDisplayManagerPolicy(
+ {kModeId90, {only90, allRange}, {allRange, allRange}}));
+
+ std::vector<LayerRequirement> layers = {{.weight = 1.f}};
+ layers[0].name = "30Hz ExplicitExactOrMultiple";
+ layers[0].desiredRefreshRate = 30_Hz;
+ layers[0].vote = LayerVoteType::ExplicitExactOrMultiple;
+
+ if (GetParam() != Config::FrameRateOverride::Enabled) {
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz,
+ selector.getBestScoredFrameRate(layers).frameRateMode);
+ } else {
+ EXPECT_FRAME_RATE_MODE(kMode90, 30_Hz,
+ selector.getBestScoredFrameRate(layers).frameRateMode);
+ }
+}
+
+TEST_P(RefreshRateSelectorTest, frameRateOverrideInBlockingZone60_90_NonDivisor) {
+ auto selector = createSelector(kModes_60_90, kModeId90);
+
+ const FpsRange only90 = {90_Hz, 90_Hz};
+ const FpsRange allRange = {0_Hz, 90_Hz};
+ EXPECT_EQ(SetPolicyResult::Changed,
+ selector.setDisplayManagerPolicy(
+ {kModeId90, {only90, allRange}, {allRange, allRange}}));
+
+ std::vector<LayerRequirement> layers = {{.weight = 1.f}};
+ layers[0].name = "60Hz ExplicitExactOrMultiple";
+ layers[0].desiredRefreshRate = 60_Hz;
+ layers[0].vote = LayerVoteType::ExplicitExactOrMultiple;
+
+ EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, selector.getBestScoredFrameRate(layers).frameRateMode);
+}
+
} // namespace
} // namespace android::scheduler
diff --git a/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h b/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h
index ef9cd9b..4cfdd58 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockFrameRateMode.h
@@ -19,5 +19,7 @@
#include <scheduler/FrameRateMode.h>
// Use a C style macro to keep the line numbers printed in gtest
-#define EXPECT_FRAME_RATE_MODE(modePtr, fps, mode) \
- EXPECT_EQ((scheduler::FrameRateMode{(fps), (modePtr)}), (mode))
+#define EXPECT_FRAME_RATE_MODE(_modePtr, _fps, _mode) \
+ EXPECT_EQ((scheduler::FrameRateMode{(_fps), (_modePtr)}), (_mode)) \
+ << "Expected " << (_fps) << " (" << (_modePtr)->getFps() << ") but was " \
+ << (_mode).fps << " (" << (_mode).modePtr->getFps() << ")"