SF: Dedupe functions to apply Scheduler::Policy
The only functional difference is the early out when there is no change
in timer state, which now applies to LayerRequirement state as well.
Bug: 185535769
Test: libsurfaceflinger_unittest
Change-Id: Ia7234847e4f44d78dbf3920d77f38f3bfa40e5a2
diff --git a/services/surfaceflinger/Scheduler/LayerHistory.cpp b/services/surfaceflinger/Scheduler/LayerHistory.cpp
index 0efc28b..5f64efa 100644
--- a/services/surfaceflinger/Scheduler/LayerHistory.cpp
+++ b/services/surfaceflinger/Scheduler/LayerHistory.cpp
@@ -83,8 +83,7 @@
void LayerHistory::registerLayer(Layer* layer, LayerVoteType type) {
std::lock_guard lock(mLock);
- LOG_ALWAYS_FATAL_IF(findLayer(layer->getSequence()).first !=
- LayerHistory::layerStatus::NotFound,
+ LOG_ALWAYS_FATAL_IF(findLayer(layer->getSequence()).first != LayerStatus::NotFound,
"%s already registered", layer->getName().c_str());
auto info = std::make_unique<LayerInfo>(layer->getName(), layer->getOwnerUid(), type);
@@ -108,9 +107,9 @@
auto id = layer->getSequence();
auto [found, layerPair] = findLayer(id);
- if (found == LayerHistory::layerStatus::NotFound) {
+ if (found == LayerStatus::NotFound) {
// Offscreen layer
- ALOGV("LayerHistory::record: %s not registered", layer->getName().c_str());
+ ALOGV("%s: %s not registered", __func__, layer->getName().c_str());
return;
}
@@ -126,16 +125,15 @@
info->setLastPresentTime(presentTime, now, updateType, mModeChangePending, layerProps);
// Activate layer if inactive.
- if (found == LayerHistory::layerStatus::LayerInInactiveMap) {
+ if (found == LayerStatus::LayerInInactiveMap) {
mActiveLayerInfos.insert(
{id, std::make_pair(layerPair->first, std::move(layerPair->second))});
mInactiveLayerInfos.erase(id);
}
}
-LayerHistory::Summary LayerHistory::summarize(const RefreshRateConfigs& refreshRateConfigs,
- nsecs_t now) {
- LayerHistory::Summary summary;
+auto LayerHistory::summarize(const RefreshRateConfigs& configs, nsecs_t now) -> Summary {
+ Summary summary;
std::lock_guard lock(mLock);
@@ -148,9 +146,9 @@
ALOGV("%s has priority: %d %s focused", info->getName().c_str(), frameRateSelectionPriority,
layerFocused ? "" : "not");
- const auto vote = info->getRefreshRateVote(refreshRateConfigs, now);
+ const auto vote = info->getRefreshRateVote(configs, now);
// Skip NoVote layer as those don't have any requirements
- if (vote.type == LayerHistory::LayerVoteType::NoVote) {
+ if (vote.type == LayerVoteType::NoVote) {
continue;
}
@@ -187,7 +185,7 @@
it = mInactiveLayerInfos.erase(it);
} else {
if (CC_UNLIKELY(mTraceEnabled)) {
- trace(*info, LayerHistory::LayerVoteType::NoVote, 0);
+ trace(*info, LayerVoteType::NoVote, 0);
}
info->onLayerInactive(now);
it++;
@@ -224,7 +222,7 @@
it++;
} else {
if (CC_UNLIKELY(mTraceEnabled)) {
- trace(*info, LayerHistory::LayerVoteType::NoVote, 0);
+ trace(*info, LayerVoteType::NoVote, 0);
}
info->onLayerInactive(now);
// move this to the inactive map
@@ -251,37 +249,23 @@
float LayerHistory::getLayerFramerate(nsecs_t now, int32_t id) const {
std::lock_guard lock(mLock);
auto [found, layerPair] = findLayer(id);
- if (found != LayerHistory::layerStatus::NotFound) {
+ if (found != LayerStatus::NotFound) {
return layerPair->second->getFps(now).getValue();
}
return 0.f;
}
-std::pair<LayerHistory::layerStatus, LayerHistory::LayerPair*> LayerHistory::findLayer(int32_t id) {
+auto LayerHistory::findLayer(int32_t id) -> std::pair<LayerStatus, LayerPair*> {
// the layer could be in either the active or inactive map, try both
auto it = mActiveLayerInfos.find(id);
if (it != mActiveLayerInfos.end()) {
- return std::make_pair(LayerHistory::layerStatus::LayerInActiveMap, &(it->second));
+ return {LayerStatus::LayerInActiveMap, &(it->second)};
}
it = mInactiveLayerInfos.find(id);
if (it != mInactiveLayerInfos.end()) {
- return std::make_pair(LayerHistory::layerStatus::LayerInInactiveMap, &(it->second));
+ return {LayerStatus::LayerInInactiveMap, &(it->second)};
}
- return std::make_pair(LayerHistory::layerStatus::NotFound, nullptr);
-}
-
-std::pair<LayerHistory::layerStatus, const LayerHistory::LayerPair*> LayerHistory::findLayer(
- int32_t id) const {
- // the layer could be in either the active or inactive map, try both
- auto it = mActiveLayerInfos.find(id);
- if (it != mActiveLayerInfos.end()) {
- return std::make_pair(LayerHistory::layerStatus::LayerInActiveMap, &(it->second));
- }
- it = mInactiveLayerInfos.find(id);
- if (it != mInactiveLayerInfos.end()) {
- return std::make_pair(LayerHistory::layerStatus::LayerInInactiveMap, &(it->second));
- }
- return std::make_pair(LayerHistory::layerStatus::NotFound, nullptr);
+ return {LayerStatus::NotFound, nullptr};
}
} // namespace android::scheduler
diff --git a/services/surfaceflinger/Scheduler/LayerHistory.h b/services/surfaceflinger/Scheduler/LayerHistory.h
index cc55700..7b6096f 100644
--- a/services/surfaceflinger/Scheduler/LayerHistory.h
+++ b/services/surfaceflinger/Scheduler/LayerHistory.h
@@ -89,7 +89,7 @@
// worst case time complexity is O(2 * inactive + active)
void partitionLayers(nsecs_t now) REQUIRES(mLock);
- enum class layerStatus {
+ enum class LayerStatus {
NotFound,
LayerInActiveMap,
LayerInInactiveMap,
@@ -97,9 +97,11 @@
// looks up a layer by sequence id in both layerInfo maps.
// The first element indicates if and where the item was found
- std::pair<layerStatus, LayerHistory::LayerPair*> findLayer(int32_t id) REQUIRES(mLock);
- std::pair<layerStatus, const LayerHistory::LayerPair*> findLayer(int32_t id) const
- REQUIRES(mLock);
+ std::pair<LayerStatus, LayerPair*> findLayer(int32_t id) REQUIRES(mLock);
+
+ std::pair<LayerStatus, const LayerPair*> findLayer(int32_t id) const REQUIRES(mLock) {
+ return const_cast<LayerHistory*>(this)->findLayer(id);
+ }
mutable std::mutex mLock;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index de27bd1..2690716 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -531,50 +531,13 @@
}
void Scheduler::chooseRefreshRateForContent() {
- {
- std::scoped_lock lock(mRefreshRateConfigsLock);
- if (!mRefreshRateConfigs->canSwitch()) return;
- }
+ const auto configs = holdRefreshRateConfigs();
+ if (!configs->canSwitch()) return;
ATRACE_CALL();
- DisplayModePtr newMode;
- GlobalSignals consideredSignals;
-
- bool frameRateChanged;
- bool frameRateOverridesChanged;
-
- const auto refreshRateConfigs = holdRefreshRateConfigs();
- LayerHistory::Summary summary = mLayerHistory.summarize(*refreshRateConfigs, systemTime());
- {
- std::lock_guard<std::mutex> lock(mPolicyLock);
- mPolicy.contentRequirements = std::move(summary);
-
- std::tie(newMode, consideredSignals) = chooseDisplayMode();
- frameRateOverridesChanged = updateFrameRateOverrides(consideredSignals, newMode->getFps());
-
- if (mPolicy.mode == newMode) {
- // We don't need to change the display mode, but we might need to send an event
- // about a mode change, since it was suppressed due to a previous idleConsidered
- if (!consideredSignals.idle) {
- dispatchCachedReportedMode();
- }
- frameRateChanged = false;
- } else {
- mPolicy.mode = newMode;
- frameRateChanged = true;
- }
- }
- if (frameRateChanged) {
- const auto newRefreshRate = refreshRateConfigs->getRefreshRateFromModeId(newMode->getId());
-
- mSchedulerCallback.changeRefreshRate(newRefreshRate,
- consideredSignals.idle ? DisplayModeEvent::None
- : DisplayModeEvent::Changed);
- }
- if (frameRateOverridesChanged) {
- mSchedulerCallback.triggerOnFrameRateOverridesChanged();
- }
+ LayerHistory::Summary summary = mLayerHistory.summarize(*configs, systemTime());
+ applyPolicy(&Policy::contentRequirements, std::move(summary));
}
void Scheduler::resetIdleTimer() {
@@ -636,7 +599,7 @@
}
void Scheduler::idleTimerCallback(TimerState state) {
- handleTimerStateChanged(&mPolicy.idleTimer, state);
+ applyPolicy(&Policy::idleTimer, state);
ATRACE_INT("ExpiredIdleTimer", static_cast<int>(state));
}
@@ -646,14 +609,14 @@
// Clear layer history to get fresh FPS detection.
// NOTE: Instead of checking all the layers, we should be checking the layer
// that is currently on top. b/142507166 will give us this capability.
- if (handleTimerStateChanged(&mPolicy.touch, touch)) {
+ if (applyPolicy(&Policy::touch, touch).touch) {
mLayerHistory.clear();
}
ATRACE_INT("TouchState", static_cast<int>(touch));
}
void Scheduler::displayPowerTimerCallback(TimerState state) {
- handleTimerStateChanged(&mPolicy.displayPowerTimer, state);
+ applyPolicy(&Policy::displayPowerTimer, state);
ATRACE_INT("ExpiredDisplayPowerTimer", static_cast<int>(state));
}
@@ -695,8 +658,8 @@
return false;
}
-template <class T>
-bool Scheduler::handleTimerStateChanged(T* currentState, T newState) {
+template <typename S, typename T>
+auto Scheduler::applyPolicy(S Policy::*statePtr, T&& newState) -> GlobalSignals {
DisplayModePtr newMode;
GlobalSignals consideredSignals;
@@ -706,15 +669,17 @@
const auto refreshRateConfigs = holdRefreshRateConfigs();
{
std::lock_guard<std::mutex> lock(mPolicyLock);
- if (*currentState == newState) {
- return false;
- }
- *currentState = newState;
+
+ auto& currentState = mPolicy.*statePtr;
+ if (currentState == newState) return {};
+ currentState = std::forward<T>(newState);
+
std::tie(newMode, consideredSignals) = chooseDisplayMode();
frameRateOverridesChanged = updateFrameRateOverrides(consideredSignals, newMode->getFps());
+
if (mPolicy.mode == newMode) {
// We don't need to change the display mode, but we might need to send an event
- // about a mode change, since it was suppressed due to a previous idleConsidered
+ // about a mode change, since it was suppressed if previously considered idle.
if (!consideredSignals.idle) {
dispatchCachedReportedMode();
}
@@ -733,7 +698,7 @@
if (frameRateOverridesChanged) {
mSchedulerCallback.triggerOnFrameRateOverridesChanged();
}
- return consideredSignals.touch;
+ return consideredSignals;
}
auto Scheduler::chooseDisplayMode() -> std::pair<DisplayModePtr, GlobalSignals> {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 468c4cc..9c32b1f 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -263,14 +263,17 @@
void touchTimerCallback(TimerState);
void displayPowerTimerCallback(TimerState);
- // handles various timer features to change the refresh rate.
- template <class T>
- bool handleTimerStateChanged(T* currentState, T newState);
-
void setVsyncPeriod(nsecs_t period);
using GlobalSignals = RefreshRateConfigs::GlobalSignals;
+ struct Policy;
+
+ // Sets the S state of the policy to the T value under mPolicyLock, and chooses a display mode
+ // that fulfills the new policy if the state changed. Returns the signals that were considered.
+ template <typename S, typename T>
+ GlobalSignals applyPolicy(S Policy::*, T&&) EXCLUDES(mPolicyLock);
+
// Returns the display mode that fulfills the policy, and the signals that were considered.
std::pair<DisplayModePtr, GlobalSignals> chooseDisplayMode() REQUIRES(mPolicyLock);
@@ -323,7 +326,7 @@
mutable std::mutex mPolicyLock;
- struct {
+ struct Policy {
// Policy for choosing the display mode.
LayerHistory::Summary contentRequirements;
TimerState idleTimer = TimerState::Reset;
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
index cdb2240..e108bea 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryTest.cpp
@@ -97,7 +97,7 @@
void setDefaultLayerVote(Layer* layer,
LayerHistory::LayerVoteType vote) NO_THREAD_SAFETY_ANALYSIS {
auto [found, layerPair] = history().findLayer(layer->getSequence());
- if (found != LayerHistory::layerStatus::NotFound) {
+ if (found != LayerHistory::LayerStatus::NotFound) {
layerPair->second->setDefaultLayerVote(vote);
}
}
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index f48abb7..a992a91 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -228,7 +228,8 @@
mScheduler->setRefreshRateConfigs(
std::make_shared<RefreshRateConfigs>(DisplayModes{mode60, mode120}, mode60->getId()));
- sp<MockLayer> layer = sp<MockLayer>::make(mFlinger.flinger());
+ const sp<MockLayer> layer = sp<MockLayer>::make(mFlinger.flinger());
+ EXPECT_CALL(*layer, isVisible()).WillOnce(Return(true));
mScheduler->recordLayerHistory(layer.get(), 0, LayerHistory::LayerUpdateType::Buffer);
@@ -240,6 +241,10 @@
EXPECT_CALL(mSchedulerCallback, changeRefreshRate(Is120Hz(), _)).Times(1);
mScheduler->chooseRefreshRateForContent();
+
+ // No-op if layer requirements have not changed.
+ EXPECT_CALL(mSchedulerCallback, changeRefreshRate(_, _)).Times(0);
+ mScheduler->chooseRefreshRateForContent();
}
} // namespace android::scheduler