diff options
5 files changed, 110 insertions, 24 deletions
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp index c44e22e9d0..6b7d7df5a3 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp +++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp @@ -148,8 +148,8 @@ std::string toString(const RefreshRateSelector::PolicyVariant& policy) { } // 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 @@ auto RefreshRateSelector::createFrameRateModes( 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 @@ auto RefreshRateSelector::getRankedFrameRatesLocked(const std::vector<LayerRequi // 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 @@ auto RefreshRateSelector::getRankedFrameRatesLocked(const std::vector<LayerRequi 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 @@ auto RefreshRateSelector::getRankedFrameRatesLocked(const std::vector<LayerRequi 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 @@ void RefreshRateSelector::constructAvailableRefreshRates() { (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 7af8d0397f..b25919e7a0 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h +++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h @@ -101,6 +101,11 @@ public: } 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 @@ private: } 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 60ad7a3a03..2d87ddd488 100644 --- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp @@ -78,7 +78,7 @@ TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setNewMode) { 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 @@ TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setNewMode) { 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 @@ TEST_F(InitiateModeChangeTest, initiateModeChange) NO_THREAD_SAFETY_ANALYSIS { 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 @@ NO_THREAD_SAFETY_ANALYSIS { 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 @@ NO_THREAD_SAFETY_ANALYSIS { 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 aaf55fbeae..0397b9936f 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp @@ -3125,5 +3125,69 @@ TEST_P(RefreshRateSelectorTest, frameRateIsLowerThanMinSupported) { {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 ef9cd9bc43..4cfdd58c70 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() << ")" |