SF: Refactor setter for DM and override policy
Move DisplayDevice::setRefreshRatePolicy to RefreshRateConfigs, and
merge the repetitive set{DisplayManager,Override}Policy helpers.
Improve type correctness of the parameters and return value.
Bug: 241285191
Test: libsurfaceflinger_unittest
Change-Id: Iee87316e5702282b828bc3f28cd7d30041030ed5
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index fb50588..c10b817 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -28,6 +28,7 @@
#include <android-base/stringprintf.h>
#include <ftl/enum.h>
#include <ftl/fake_guard.h>
+#include <ftl/match.h>
#include <utils/Trace.h>
#include "../SurfaceFlingerProperties.h"
@@ -117,6 +118,20 @@
return false;
}
+std::string toString(const RefreshRateConfigs::PolicyVariant& policy) {
+ using namespace std::string_literals;
+
+ return ftl::match(
+ policy,
+ [](const RefreshRateConfigs::DisplayManagerPolicy& policy) {
+ return "DisplayManagerPolicy"s + policy.toString();
+ },
+ [](const RefreshRateConfigs::OverridePolicy& policy) {
+ return "OverridePolicy"s + policy.toString();
+ },
+ [](RefreshRateConfigs::NoOverridePolicy) { return "NoOverridePolicy"s; });
+}
+
} // namespace
struct RefreshRateConfigs::RefreshRateScoreComparator {
@@ -874,35 +889,60 @@
policy.appRequestRange.max >= policy.primaryRange.max;
}
-status_t RefreshRateConfigs::setDisplayManagerPolicy(const Policy& policy) {
- std::lock_guard lock(mLock);
- if (!isPolicyValidLocked(policy)) {
- ALOGE("Invalid refresh rate policy: %s", policy.toString().c_str());
- return BAD_VALUE;
- }
- mGetRankedRefreshRatesCache.reset();
- Policy previousPolicy = *getCurrentPolicyLocked();
- mDisplayManagerPolicy = policy;
- if (*getCurrentPolicyLocked() == previousPolicy) {
- return CURRENT_POLICY_UNCHANGED;
- }
- constructAvailableRefreshRates();
- return NO_ERROR;
-}
+auto RefreshRateConfigs::setPolicy(const PolicyVariant& policy) -> SetPolicyResult {
+ Policy oldPolicy;
+ {
+ std::lock_guard lock(mLock);
+ oldPolicy = *getCurrentPolicyLocked();
-status_t RefreshRateConfigs::setOverridePolicy(const std::optional<Policy>& policy) {
- std::lock_guard lock(mLock);
- if (policy && !isPolicyValidLocked(*policy)) {
- return BAD_VALUE;
+ const bool valid = ftl::match(
+ policy,
+ [this](const auto& policy) {
+ ftl::FakeGuard guard(mLock);
+ if (!isPolicyValidLocked(policy)) {
+ ALOGE("Invalid policy: %s", policy.toString().c_str());
+ return false;
+ }
+
+ using T = std::decay_t<decltype(policy)>;
+
+ if constexpr (std::is_same_v<T, DisplayManagerPolicy>) {
+ mDisplayManagerPolicy = policy;
+ } else {
+ static_assert(std::is_same_v<T, OverridePolicy>);
+ mOverridePolicy = policy;
+ }
+ return true;
+ },
+ [this](NoOverridePolicy) {
+ ftl::FakeGuard guard(mLock);
+ mOverridePolicy.reset();
+ return true;
+ });
+
+ if (!valid) {
+ return SetPolicyResult::Invalid;
+ }
+
+ mGetRankedRefreshRatesCache.reset();
+
+ if (*getCurrentPolicyLocked() == oldPolicy) {
+ return SetPolicyResult::Unchanged;
+ }
+ constructAvailableRefreshRates();
}
- mGetRankedRefreshRatesCache.reset();
- Policy previousPolicy = *getCurrentPolicyLocked();
- mOverridePolicy = policy;
- if (*getCurrentPolicyLocked() == previousPolicy) {
- return CURRENT_POLICY_UNCHANGED;
- }
- constructAvailableRefreshRates();
- return NO_ERROR;
+
+ const auto displayId = getActiveMode().getPhysicalDisplayId();
+ const unsigned numModeChanges = std::exchange(mNumModeSwitchesInPolicy, 0u);
+
+ ALOGI("Display %s policy changed\n"
+ "Previous: %s\n"
+ "Current: %s\n"
+ "%u mode changes were performed under the previous policy",
+ to_string(displayId).c_str(), oldPolicy.toString().c_str(), toString(policy).c_str(),
+ numModeChanges);
+
+ return SetPolicyResult::Changed;
}
const RefreshRateConfigs::Policy* RefreshRateConfigs::getCurrentPolicyLocked() const {
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index 8b89104..2c2e34a 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -21,6 +21,7 @@
#include <optional>
#include <type_traits>
#include <utility>
+#include <variant>
#include <gui/DisplayEventReceiver.h>
@@ -67,8 +68,7 @@
static constexpr nsecs_t MARGIN_FOR_PERIOD_CALCULATION =
std::chrono::nanoseconds(800us).count();
- struct Policy {
- private:
+ class Policy {
static constexpr int kAllowGroupSwitchingDefault = false;
public:
@@ -118,23 +118,28 @@
std::string toString() const;
};
- // Return code set*Policy() to indicate the current policy is unchanged.
- static constexpr int CURRENT_POLICY_UNCHANGED = 1;
+ enum class SetPolicyResult { Invalid, Unchanged, Changed };
// We maintain the display manager policy and the override policy separately. The override
// policy is used by CTS tests to get a consistent device state for testing. While the override
// policy is set, it takes precedence over the display manager policy. Once the override policy
// is cleared, we revert to using the display manager policy.
+ struct DisplayManagerPolicy : Policy {
+ using Policy::Policy;
+ };
- // Sets the display manager policy to choose refresh rates. The return value will be:
- // - A negative value if the policy is invalid or another error occurred.
- // - NO_ERROR if the policy was successfully updated, and the current policy is different from
- // what it was before the call.
- // - CURRENT_POLICY_UNCHANGED if the policy was successfully updated, but the current policy
- // is the same as it was before the call.
- status_t setDisplayManagerPolicy(const Policy& policy) EXCLUDES(mLock);
- // Sets the override policy. See setDisplayManagerPolicy() for the meaning of the return value.
- status_t setOverridePolicy(const std::optional<Policy>& policy) EXCLUDES(mLock);
+ struct OverridePolicy : Policy {
+ using Policy::Policy;
+ };
+
+ struct NoOverridePolicy {};
+
+ using PolicyVariant = std::variant<DisplayManagerPolicy, OverridePolicy, NoOverridePolicy>;
+
+ SetPolicyResult setPolicy(const PolicyVariant&) EXCLUDES(mLock) REQUIRES(kMainThreadContext);
+
+ void onModeChangeInitiated() REQUIRES(kMainThreadContext) { mNumModeSwitchesInPolicy++; }
+
// Gets the current policy, which will be the override policy if active, and the display manager
// policy otherwise.
Policy getCurrentPolicy() const EXCLUDES(mLock);
@@ -418,6 +423,8 @@
Policy mDisplayManagerPolicy GUARDED_BY(mLock);
std::optional<Policy> mOverridePolicy GUARDED_BY(mLock);
+ unsigned mNumModeSwitchesInPolicy GUARDED_BY(kMainThreadContext) = 0;
+
mutable std::mutex mLock;
// A sorted list of known frame rates that a Heuristic layer will choose