Merge changes Iec0696d7,I19900d03

* changes:
  SF: Pull FpsRange to scheduler/Fps.h
  SF: Make thread safety macros zero-cost

6 files changed, 61 insertions, 60 deletions

diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index 3b9cfa6340..eeeaac1170 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -69,11 +69,6 @@ std::vector<Fps> constructKnownFrameRates(const DisplayModes& modes) {
 using AllRefreshRatesMapType = RefreshRateConfigs::AllRefreshRatesMapType;
 using RefreshRate = RefreshRateConfigs::RefreshRate;
 
-bool RefreshRate::inPolicy(Fps minRefreshRate, Fps maxRefreshRate) const {
-    using fps_approx_ops::operator<=;
-    return minRefreshRate <= getFps() && getFps() <= maxRefreshRate;
-}
-
 std::string RefreshRate::toString() const {
     return base::StringPrintf("{id=%d, hwcId=%d, fps=%.2f, width=%d, height=%d group=%d}",
                               getModeId().value(), mode->getHwcId(), getFps().getValue(),
@@ -84,7 +79,7 @@ std::string RefreshRateConfigs::Policy::toString() const {
     return base::StringPrintf("default mode ID: %d, allowGroupSwitching = %d"
                               ", primary range: %s, app request range: %s",
                               defaultMode.value(), allowGroupSwitching,
-                              primaryRange.toString().c_str(), appRequestRange.toString().c_str());
+                              to_string(primaryRange).c_str(), to_string(appRequestRange).c_str());
 }
 
 std::pair<nsecs_t, nsecs_t> RefreshRateConfigs::getDisplayFrames(nsecs_t layerPeriod,
@@ -396,8 +391,9 @@ auto RefreshRateConfigs::getBestRefreshRateLocked(const std::vector<LayerRequire
                 continue;
             }
 
-            bool inPrimaryRange = scores[i].refreshRate->inPolicy(policy->primaryRange.min,
-                                                                  policy->primaryRange.max);
+            const bool inPrimaryRange =
+                    policy->primaryRange.includes(scores[i].refreshRate->getFps());
+
             if ((primaryRangeIsSingleRate || !inPrimaryRange) &&
                 !(layer.focused &&
                   (layer.vote == LayerVoteType::ExplicitDefault ||
@@ -746,7 +742,7 @@ bool RefreshRateConfigs::isPolicyValidLocked(const Policy& policy) const {
         return false;
     }
     const RefreshRate& refreshRate = *iter->second;
-    if (!refreshRate.inPolicy(policy.primaryRange.min, policy.primaryRange.max)) {
+    if (!policy.primaryRange.includes(refreshRate.getFps())) {
         ALOGE("Default mode is not in the primary range.");
         return false;
     }
@@ -843,7 +839,7 @@ void RefreshRateConfigs::constructAvailableRefreshRates() {
     ALOGV("constructAvailableRefreshRates: %s ", policy->toString().c_str());
 
     auto filterRefreshRates =
-            [&](Fps min, Fps max, const char* listName,
+            [&](FpsRange range, const char* rangeName,
                 std::vector<const RefreshRate*>* outRefreshRates) REQUIRES(mLock) {
                 getSortedRefreshRateListLocked(
                         [&](const RefreshRate& refreshRate) REQUIRES(mLock) {
@@ -855,13 +851,13 @@ void RefreshRateConfigs::constructAvailableRefreshRates() {
                                     mode->getDpiY() == defaultMode->getDpiY() &&
                                     (policy->allowGroupSwitching ||
                                      mode->getGroup() == defaultMode->getGroup()) &&
-                                    refreshRate.inPolicy(min, max);
+                                    range.includes(mode->getFps());
                         },
                         outRefreshRates);
 
-                LOG_ALWAYS_FATAL_IF(outRefreshRates->empty(),
-                                    "No matching modes for %s range: min=%s max=%s", listName,
-                                    to_string(min).c_str(), to_string(max).c_str());
+                LOG_ALWAYS_FATAL_IF(outRefreshRates->empty(), "No matching modes for %s range %s",
+                                    rangeName, to_string(range).c_str());
+
                 auto stringifyRefreshRates = [&]() -> std::string {
                     std::string str;
                     for (auto refreshRate : *outRefreshRates) {
@@ -869,13 +865,11 @@ void RefreshRateConfigs::constructAvailableRefreshRates() {
                     }
                     return str;
                 };
-                ALOGV("%s refresh rates: %s", listName, stringifyRefreshRates().c_str());
+                ALOGV("%s refresh rates: %s", rangeName, stringifyRefreshRates().c_str());
             };
 
-    filterRefreshRates(policy->primaryRange.min, policy->primaryRange.max, "primary",
-                       &mPrimaryRefreshRates);
-    filterRefreshRates(policy->appRequestRange.min, policy->appRequestRange.max, "app request",
-                       &mAppRequestRefreshRates);
+    filterRefreshRates(policy->primaryRange, "primary", &mPrimaryRefreshRates);
+    filterRefreshRates(policy->appRequestRange, "app request", &mAppRequestRefreshRates);
 }
 
 Fps RefreshRateConfigs::findClosestKnownFrameRate(Fps frameRate) const {
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index ade1787c97..f5b97c27c6 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -22,7 +22,6 @@
 #include <type_traits>
 #include <utility>
 
-#include <android-base/stringprintf.h>
 #include <gui/DisplayEventReceiver.h>
 
 #include <scheduler/Fps.h>
@@ -101,21 +100,6 @@ public:
     using AllRefreshRatesMapType =
             std::unordered_map<DisplayModeId, std::unique_ptr<const RefreshRate>>;
 
-    struct FpsRange {
-        Fps min = Fps::fromValue(0.f);
-        Fps max = Fps::fromValue(std::numeric_limits<float>::max());
-
-        bool operator==(const FpsRange& other) const {
-            return isApproxEqual(min, other.min) && isApproxEqual(max, other.max);
-        }
-
-        bool operator!=(const FpsRange& other) const { return !(*this == other); }
-
-        std::string toString() const {
-            return base::StringPrintf("[%s %s]", to_string(min).c_str(), to_string(max).c_str());
-        }
-    };
-
     struct Policy {
     private:
         static constexpr int kAllowGroupSwitchingDefault = false;
@@ -140,24 +124,24 @@ public:
 
         Policy() = default;
 
-        Policy(DisplayModeId defaultMode, const FpsRange& range)
+        Policy(DisplayModeId defaultMode, FpsRange range)
               : Policy(defaultMode, kAllowGroupSwitchingDefault, range, range) {}
 
-        Policy(DisplayModeId defaultMode, bool allowGroupSwitching, const FpsRange& range)
+        Policy(DisplayModeId defaultMode, bool allowGroupSwitching, FpsRange range)
               : Policy(defaultMode, allowGroupSwitching, range, range) {}
 
-        Policy(DisplayModeId defaultMode, const FpsRange& primaryRange,
-               const FpsRange& appRequestRange)
+        Policy(DisplayModeId defaultMode, FpsRange primaryRange, FpsRange appRequestRange)
               : Policy(defaultMode, kAllowGroupSwitchingDefault, primaryRange, appRequestRange) {}
 
-        Policy(DisplayModeId defaultMode, bool allowGroupSwitching, const FpsRange& primaryRange,
-               const FpsRange& appRequestRange)
+        Policy(DisplayModeId defaultMode, bool allowGroupSwitching, FpsRange primaryRange,
+               FpsRange appRequestRange)
               : defaultMode(defaultMode),
                 allowGroupSwitching(allowGroupSwitching),
                 primaryRange(primaryRange),
                 appRequestRange(appRequestRange) {}
 
         bool operator==(const Policy& other) const {
+            using namespace fps_approx_ops;
             return defaultMode == other.defaultMode && primaryRange == other.primaryRange &&
                     appRequestRange == other.appRequestRange &&
                     allowGroupSwitching == other.allowGroupSwitching;
diff --git a/services/surfaceflinger/Scheduler/include/scheduler/Fps.h b/services/surfaceflinger/Scheduler/include/scheduler/Fps.h
index 639b3e5ed8..bd4f40989d 100644
--- a/services/surfaceflinger/Scheduler/include/scheduler/Fps.h
+++ b/services/surfaceflinger/Scheduler/include/scheduler/Fps.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <cmath>
+#include <limits>
 #include <ostream>
 #include <string>
 #include <type_traits>
@@ -60,6 +61,13 @@ private:
     nsecs_t mPeriod = 0;
 };
 
+struct FpsRange {
+    Fps min = Fps::fromValue(0.f);
+    Fps max = Fps::fromValue(std::numeric_limits<float>::max());
+
+    bool includes(Fps) const;
+};
+
 static_assert(std::is_trivially_copyable_v<Fps>);
 
 constexpr Fps operator""_Hz(unsigned long long frequency) {
@@ -111,8 +119,21 @@ inline bool operator>=(Fps lhs, Fps rhs) {
     return !isApproxLess(lhs, rhs);
 }
 
+inline bool operator==(FpsRange lhs, FpsRange rhs) {
+    return isApproxEqual(lhs.min, rhs.min) && isApproxEqual(lhs.max, rhs.max);
+}
+
+inline bool operator!=(FpsRange lhs, FpsRange rhs) {
+    return !(lhs == rhs);
+}
+
 } // namespace fps_approx_ops
 
+inline bool FpsRange::includes(Fps fps) const {
+    using fps_approx_ops::operator<=;
+    return min <= fps && fps <= max;
+}
+
 struct FpsApproxEqual {
     bool operator()(Fps lhs, Fps rhs) const { return isApproxEqual(lhs, rhs); }
 };
@@ -125,4 +146,9 @@ inline std::ostream& operator<<(std::ostream& stream, Fps fps) {
     return stream << to_string(fps);
 }
 
+inline std::string to_string(FpsRange range) {
+    const auto [min, max] = range;
+    return base::StringPrintf("[%s, %s]", to_string(min).c_str(), to_string(max).c_str());
+}
+
 } // namespace android
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 344c2cc7d4..59c07b6db1 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -147,15 +147,16 @@
 
 #define MAIN_THREAD ACQUIRE(mStateLock) RELEASE(mStateLock)
 
+// Note: The parentheses around `expr` are needed to deduce an lvalue or rvalue reference.
 #define ON_MAIN_THREAD(expr)                                       \
-    [&] {                                                          \
+    [&]() -> decltype(auto) {                                      \
         LOG_FATAL_IF(std::this_thread::get_id() != mMainThreadId); \
         UnnecessaryLock lock(mStateLock);                          \
         return (expr);                                             \
     }()
 
 #define MAIN_THREAD_GUARD(expr)                                    \
-    [&] {                                                          \
+    [&]() -> decltype(auto) {                                      \
         LOG_FATAL_IF(std::this_thread::get_id() != mMainThreadId); \
         MainThreadScopedGuard lock(SF_MAIN_THREAD);                \
         return (expr);                                             \
@@ -1345,9 +1346,8 @@ void SurfaceFlinger::disableExpensiveRendering() {
     auto future = mScheduler->schedule([=]() MAIN_THREAD {
         ATRACE_CALL();
         if (mPowerAdvisor.isUsingExpensiveRendering()) {
-            const auto& displays = ON_MAIN_THREAD(mDisplays);
-            for (const auto& [_, display] : displays) {
-                const static constexpr auto kDisable = false;
+            for (const auto& [_, display] : mDisplays) {
+                constexpr bool kDisable = false;
                 mPowerAdvisor.setExpensiveRenderingExpected(display->getId(), kDisable);
             }
         }
@@ -3236,9 +3236,7 @@ void SurfaceFlinger::persistDisplayBrightness(bool needsComposite) {
         return;
     }
 
-    const auto& displays = ON_MAIN_THREAD(mDisplays);
-
-    for (const auto& [_, display] : displays) {
+    for (const auto& [_, display] : ON_MAIN_THREAD(mDisplays)) {
         if (const auto brightness = display->getStagedBrightness(); brightness) {
             if (!needsComposite) {
                 const status_t error =
diff --git a/services/surfaceflinger/tests/unittests/FpsTest.cpp b/services/surfaceflinger/tests/unittests/FpsTest.cpp
index 88b74d2a22..2193c9dd76 100644
--- a/services/surfaceflinger/tests/unittests/FpsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FpsTest.cpp
@@ -68,4 +68,14 @@ TEST(FpsTest, getIntValue) {
     EXPECT_EQ(31, (30.5_Hz).getIntValue());
 }
 
+TEST(FpsTest, range) {
+    const auto fps = Fps::fromPeriodNsecs(16'666'665);
+
+    EXPECT_TRUE((FpsRange{60.000004_Hz, 60.000004_Hz}.includes(fps)));
+    EXPECT_TRUE((FpsRange{59_Hz, 60.1_Hz}.includes(fps)));
+    EXPECT_FALSE((FpsRange{75_Hz, 90_Hz}.includes(fps)));
+    EXPECT_FALSE((FpsRange{60.0011_Hz, 90_Hz}.includes(fps)));
+    EXPECT_FALSE((FpsRange{50_Hz, 59.998_Hz}.includes(fps)));
+}
+
 } // namespace android
diff --git a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
index 4efcc051bb..9143d61145 100644
--- a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
@@ -834,17 +834,6 @@ TEST_F(RefreshRateConfigsTest, twoModes_getBestRefreshRate_Explicit) {
     EXPECT_EQ(asRefreshRate(kMode90), configs.getBestRefreshRate(layers));
 }
 
-TEST_F(RefreshRateConfigsTest, testInPolicy) {
-    const auto refreshRate =
-            asRefreshRate(createDisplayMode(kModeId60, Fps::fromPeriodNsecs(16'666'665)));
-
-    EXPECT_TRUE(refreshRate.inPolicy(60.000004_Hz, 60.000004_Hz));
-    EXPECT_TRUE(refreshRate.inPolicy(59_Hz, 60.1_Hz));
-    EXPECT_FALSE(refreshRate.inPolicy(75_Hz, 90_Hz));
-    EXPECT_FALSE(refreshRate.inPolicy(60.0011_Hz, 90_Hz));
-    EXPECT_FALSE(refreshRate.inPolicy(50_Hz, 59.998_Hz));
-}
-
 TEST_F(RefreshRateConfigsTest, getBestRefreshRate_75HzContent) {
     TestableRefreshRateConfigs configs(kModes_60_90, kModeId60);