Merge changes from topic "sf_157096772" into rvc-dev

* changes:
  SurfaceFlinger: clear LayerHistory on first touch
  Revert "SurfaceFlinger: more aggressive infrequent layer detection"
  Revert "SurfaceFlinger: tune MAX_FREQUENT_LAYER_PERIOD_NS for inactive layers"
  Revert "SurfaceFlinger: tune infrequent detection logic more"

services/surfaceflinger/Scheduler/LayerHistoryV2.cpp

@@ -62,13 +62,17 @@ void trace(const wp<Layer>& weak, LayerHistory::LayerVoteType type, int fps) {
     const auto layer = weak.promote();
     if (!layer) return;
 
-    const auto& name = layer->getName();
+    const auto makeTag = [layer](LayerHistory::LayerVoteType vote) {
-    const auto noVoteTag = "LFPS NoVote " + name;
+        return "LFPS " + RefreshRateConfigs::layerVoteTypeString(vote) + " " + layer->getName();
-    const auto heuristicVoteTag = "LFPS Heuristic " + name;
+    };
-    const auto explicitDefaultVoteTag = "LFPS ExplicitDefault" + name;
+
-    const auto explicitExactOrMultipleVoteTag = "LFPS ExplicitExactOrMultiple" + name;
+    const auto noVoteTag = makeTag(LayerHistory::LayerVoteType::NoVote);
-    const auto minVoteTag = "LFPS Min " + name;
+    const auto heuristicVoteTag = makeTag(LayerHistory::LayerVoteType::Heuristic);
-    const auto maxVoteTag = "LFPS Max " + name;
+    const auto explicitDefaultVoteTag = makeTag(LayerHistory::LayerVoteType::ExplicitDefault);
+    const auto explicitExactOrMultipleVoteTag =
+            makeTag(LayerHistory::LayerVoteType::ExplicitExactOrMultiple);
+    const auto minVoteTag = makeTag(LayerHistory::LayerVoteType::Min);
+    const auto maxVoteTag = makeTag(LayerHistory::LayerVoteType::Max);
 
     ATRACE_INT(noVoteTag.c_str(), type == LayerHistory::LayerVoteType::NoVote ? 1 : 0);
     ATRACE_INT(heuristicVoteTag.c_str(), type == LayerHistory::LayerVoteType::Heuristic ? fps : 0);
@@ -79,7 +83,7 @@ void trace(const wp<Layer>& weak, LayerHistory::LayerVoteType type, int fps) {
     ATRACE_INT(minVoteTag.c_str(), type == LayerHistory::LayerVoteType::Min ? 1 : 0);
     ATRACE_INT(maxVoteTag.c_str(), type == LayerHistory::LayerVoteType::Max ? 1 : 0);
 
-    ALOGD("%s: %s @ %d Hz", __FUNCTION__, name.c_str(), fps);
+    ALOGD("%s: %s @ %d Hz", __FUNCTION__, layer->getName().c_str(), fps);
 }
 } // namespace
 

services/surfaceflinger/Scheduler/LayerInfoV2.cpp

@@ -50,28 +50,42 @@ void LayerInfoV2::setLastPresentTime(nsecs_t lastPresentTime, nsecs_t now,
     }
 }
 
-bool LayerInfoV2::isFrequent(nsecs_t now) {
+bool LayerInfoV2::isFrameTimeValid(const FrameTimeData& frameTime) const {
-    mLastReportedIsFrequent = [&] {
+    return frameTime.queueTime >= std::chrono::duration_cast<std::chrono::nanoseconds>(
-        for (auto it = mFrameTimes.crbegin(); it != mFrameTimes.crend(); ++it) {
+                                          mFrameTimeValidSince.time_since_epoch())
-            if (now - it->queueTime >= MAX_FREQUENT_LAYER_PERIOD_NS.count()) {
+                                          .count();
-                ALOGV("%s infrequent (last frame is %.2fms ago)", mName.c_str(),
+}
-                      (now - mFrameTimes.back().queueTime) / 1e6f);
+
+bool LayerInfoV2::isFrequent(nsecs_t now) const {
+    // Find the first valid frame time
+    auto it = mFrameTimes.begin();
+    for (; it != mFrameTimes.end(); ++it) {
+        if (isFrameTimeValid(*it)) {
+            break;
+        }
+    }
+
+    // If we know nothing about this layer we consider it as frequent as it might be the start
+    // of an animation.
+    if (std::distance(it, mFrameTimes.end()) < FREQUENT_LAYER_WINDOW_SIZE) {
+        return true;
+    }
+
+    // Find the first active frame
+    for (; it != mFrameTimes.end(); ++it) {
+        if (it->queueTime >= getActiveLayerThreshold(now)) {
+            break;
+        }
+    }
+
+    const auto numFrames = std::distance(it, mFrameTimes.end());
+    if (numFrames < FREQUENT_LAYER_WINDOW_SIZE) {
         return false;
     }
 
-            const auto numFrames = std::distance(mFrameTimes.crbegin(), it + 1);
+    // Layer is considered frequent if the average frame rate is higher than the threshold
-            if (numFrames >= FREQUENT_LAYER_WINDOW_SIZE) {
+    const auto totalTime = mFrameTimes.back().queueTime - it->queueTime;
-                ALOGV("%s frequent (burst of %zu frames)", mName.c_str(), numFrames);
+    return (1e9f * (numFrames - 1)) / totalTime >= MIN_FPS_FOR_FREQUENT_LAYER;
-                return true;
-            }
-        }
-
-        ALOGV("%s %sfrequent (not enough frames %zu)", mName.c_str(),
-              mLastReportedIsFrequent ? "" : "in", mFrameTimes.size());
-        return mLastReportedIsFrequent;
-    }();
-
-    return mLastReportedIsFrequent;
 }
 
 bool LayerInfoV2::hasEnoughDataForHeuristic() const {
@@ -80,6 +94,10 @@ bool LayerInfoV2::hasEnoughDataForHeuristic() const {
         return false;
     }
 
+    if (!isFrameTimeValid(mFrameTimes.front())) {
+        return false;
+    }
+
     if (mFrameTimes.size() < HISTORY_SIZE &&
         mFrameTimes.back().queueTime - mFrameTimes.front().queueTime < HISTORY_TIME.count()) {
         return false;
@@ -190,7 +208,7 @@ std::pair<LayerHistory::LayerVoteType, float> LayerInfoV2::getRefreshRate(nsecs_
         return {LayerHistory::LayerVoteType::Heuristic, refreshRate.value()};
     }
 
-    ALOGV("%s Max (can't resolve refresh rate", mName.c_str());
+    ALOGV("%s Max (can't resolve refresh rate)", mName.c_str());
     return {LayerHistory::LayerVoteType::Max, 0};
 }
 

services/surfaceflinger/Scheduler/LayerInfoV2.h

@@ -47,7 +47,9 @@ class LayerInfoV2 {
     // is within a threshold. If a layer is infrequent, its average refresh rate is disregarded in
     // favor of a low refresh rate.
     static constexpr size_t FREQUENT_LAYER_WINDOW_SIZE = 3;
-    static constexpr std::chrono::nanoseconds MAX_FREQUENT_LAYER_PERIOD_NS = 150ms;
+    static constexpr float MIN_FPS_FOR_FREQUENT_LAYER = 10.0f;
+    static constexpr auto MAX_FREQUENT_LAYER_PERIOD_NS =
+            std::chrono::nanoseconds(static_cast<nsecs_t>(1e9f / MIN_FPS_FOR_FREQUENT_LAYER)) + 1ms;
 
     friend class LayerHistoryTestV2;
 
@@ -82,7 +84,11 @@ public:
     nsecs_t getLastUpdatedTime() const { return mLastUpdatedTime; }
 
     void clearHistory() {
-        mFrameTimes.clear();
+        // Mark mFrameTimeValidSince to now to ignore all previous frame times.
+        // We are not deleting the old frame to keep track of whether we should treat the first
+        // buffer as Max as we don't know anything about this layer or Min as this layer is
+        // posting infrequent updates.
+        mFrameTimeValidSince = std::chrono::steady_clock::now();
         mLastReportedRefreshRate = 0.0f;
     }
 
@@ -94,11 +100,12 @@ private:
         bool pendingConfigChange;
     };
 
-    bool isFrequent(nsecs_t now);
+    bool isFrequent(nsecs_t now) const;
     bool hasEnoughDataForHeuristic() const;
     std::optional<float> calculateRefreshRateIfPossible();
     std::pair<nsecs_t, bool> calculateAverageFrameTime() const;
     bool isRefreshRateStable(nsecs_t averageFrameTime, bool missingPresentTime) const;
+    bool isFrameTimeValid(const FrameTimeData&) const;
 
     const std::string mName;
 
@@ -110,13 +117,6 @@ private:
 
     float mLastReportedRefreshRate = 0.0f;
 
-    // Used to determine whether a layer should be considered frequent or
-    // not when we don't have enough frames. This member will not be cleared
-    // as part of clearHistory() to remember whether this layer was frequent
-    // or not before we processed touch boost (or anything else that would
-    // clear layer history).
-    bool mLastReportedIsFrequent = true;
-
     // Holds information about the layer vote
     struct {
         LayerHistory::LayerVoteType type;
@@ -124,6 +124,8 @@ private:
     } mLayerVote;
 
     std::deque<FrameTimeData> mFrameTimes;
+    std::chrono::time_point<std::chrono::steady_clock> mFrameTimeValidSince =
+            std::chrono::steady_clock::now();
     static constexpr size_t HISTORY_SIZE = 90;
     static constexpr std::chrono::nanoseconds HISTORY_TIME = 1s;
 };

services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp

@@ -31,6 +31,23 @@ namespace android::scheduler {
 using AllRefreshRatesMapType = RefreshRateConfigs::AllRefreshRatesMapType;
 using RefreshRate = RefreshRateConfigs::RefreshRate;
 
+std::string RefreshRateConfigs::layerVoteTypeString(LayerVoteType vote) {
+    switch (vote) {
+        case LayerVoteType::NoVote:
+            return "NoVote";
+        case LayerVoteType::Min:
+            return "Min";
+        case LayerVoteType::Max:
+            return "Max";
+        case LayerVoteType::Heuristic:
+            return "Heuristic";
+        case LayerVoteType::ExplicitDefault:
+            return "ExplicitDefault";
+        case LayerVoteType::ExplicitExactOrMultiple:
+            return "ExplicitExactOrMultiple";
+    }
+}
+
 const RefreshRate& RefreshRateConfigs::getRefreshRateForContent(
         const std::vector<LayerRequirement>& layers) const {
     std::lock_guard lock(mLock);
@@ -146,6 +163,7 @@ const RefreshRate& RefreshRateConfigs::getBestRefreshRate(
     const bool primaryRangeIsSingleRate = policy->primaryRange.min == policy->primaryRange.max;
 
     if (!touchActive && idle && !(primaryRangeIsSingleRate && hasExplicitVoteLayers)) {
+        ALOGV("Idle - choose %s", getMinRefreshRateByPolicyLocked().getName().c_str());
         return getMinRefreshRateByPolicyLocked();
     }
 
@@ -168,7 +186,8 @@ const RefreshRate& RefreshRateConfigs::getBestRefreshRate(
     }
 
     for (const auto& layer : layers) {
-        ALOGV("Calculating score for %s (type: %d)", layer.name.c_str(), layer.vote);
+        ALOGV("Calculating score for %s (%s, weight %.2f)", layer.name.c_str(),
+              layerVoteTypeString(layer.vote).c_str(), layer.weight);
         if (layer.vote == LayerVoteType::NoVote || layer.vote == LayerVoteType::Min) {
             continue;
         }
@@ -254,10 +273,8 @@ const RefreshRate& RefreshRateConfigs::getBestRefreshRate(
                     return 1.0f / iter;
                 }();
                 ALOGV("%s (%s, weight %.2f) %.2fHz gives %s score of %.2f", layer.name.c_str(),
-                      layer.vote == LayerVoteType::ExplicitExactOrMultiple
+                      layerVoteTypeString(layer.vote).c_str(), weight, 1e9f / layerPeriod,
-                              ? "ExplicitExactOrMultiple"
+                      scores[i].first->name.c_str(), layerScore);
-                              : "Heuristic",
-                      weight, 1e9f / layerPeriod, scores[i].first->name.c_str(), layerScore);
                 scores[i].second += weight * layerScore;
                 continue;
             }
@@ -276,6 +293,8 @@ const RefreshRate& RefreshRateConfigs::getBestRefreshRate(
         // range instead of picking a random score from the app range.
         if (std::all_of(scores.begin(), scores.end(),
                         [](std::pair<const RefreshRate*, float> p) { return p.second == 0; })) {
+            ALOGV("layers not scored - choose %s",
+                  getMaxRefreshRateByPolicyLocked().getName().c_str());
             return getMaxRefreshRateByPolicyLocked();
         } else {
             return *bestRefreshRate;

services/surfaceflinger/Scheduler/RefreshRateConfigs.h

@@ -255,6 +255,9 @@ public:
     // Stores the current configId the device operates at
     void setCurrentConfigId(HwcConfigIndexType configId) EXCLUDES(mLock);
 
+    // Returns a string that represents the layer vote type
+    static std::string layerVoteTypeString(LayerVoteType vote);
+
     RefreshRateConfigs(const std::vector<std::shared_ptr<const HWC2::Display::Config>>& configs,
                        HwcConfigIndexType currentConfigId);
 

services/surfaceflinger/tests/unittests/LayerHistoryTestV2.cpp

@@ -103,13 +103,12 @@ TEST_F(LayerHistoryTestV2, oneLayer) {
     EXPECT_TRUE(history().summarize(time).empty());
     EXPECT_EQ(0, activeLayerCount());
 
-    // The first few updates are considered frequent
+    // Max returned if active layers have insufficient history.
     for (int i = 0; i < PRESENT_TIME_HISTORY_SIZE - 1; i++) {
         history().record(layer.get(), 0, time);
         ASSERT_EQ(1, history().summarize(time).size());
         EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
         EXPECT_EQ(1, activeLayerCount());
-        EXPECT_EQ(1, frequentLayerCount(time));
     }
 
     // Max is returned since we have enough history but there is no timestamp votes.
@@ -118,7 +117,6 @@ TEST_F(LayerHistoryTestV2, oneLayer) {
         ASSERT_EQ(1, history().summarize(time).size());
         EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
         EXPECT_EQ(1, activeLayerCount());
-        EXPECT_EQ(1, frequentLayerCount(time));
     }
 }
 
@@ -135,7 +133,7 @@ TEST_F(LayerHistoryTestV2, oneInvisibleLayer) {
     history().record(layer.get(), 0, time);
     auto summary = history().summarize(time);
     ASSERT_EQ(1, history().summarize(time).size());
-    // Layer is still considered active so we expect to get Max
+    // Layer is still considered inactive so we expect to get Min
     EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
     EXPECT_EQ(1, activeLayerCount());
 
@@ -466,15 +464,28 @@ TEST_F(LayerHistoryTestV2, inactiveLayers) {
 
     nsecs_t time = systemTime();
 
-    // The first few updates are considered frequent
+    // the very first updates makes the layer frequent
-    for (int i = 0; i < PRESENT_TIME_HISTORY_SIZE - 1; i++) {
+    for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
-        history().record(layer.get(), 0, time);
+        history().record(layer.get(), time, time);
+        time += MAX_FREQUENT_LAYER_PERIOD_NS.count();
+
+        EXPECT_EQ(1, layerCount());
         ASSERT_EQ(1, history().summarize(time).size());
         EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
         EXPECT_EQ(1, activeLayerCount());
         EXPECT_EQ(1, frequentLayerCount(time));
     }
 
+    // the next update with the MAX_FREQUENT_LAYER_PERIOD_NS will get us to infrequent
+    history().record(layer.get(), time, time);
+    time += MAX_FREQUENT_LAYER_PERIOD_NS.count();
+
+    EXPECT_EQ(1, layerCount());
+    ASSERT_EQ(1, history().summarize(time).size());
+    EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
+    EXPECT_EQ(1, activeLayerCount());
+    EXPECT_EQ(0, frequentLayerCount(time));
+
     // advance the time for the previous frame to be inactive
     time += MAX_ACTIVE_LAYER_PERIOD_NS.count();
 
@@ -499,36 +510,6 @@ TEST_F(LayerHistoryTestV2, inactiveLayers) {
     EXPECT_EQ(LayerHistory::LayerVoteType::Max, history().summarize(time)[0].vote);
     EXPECT_EQ(1, activeLayerCount());
     EXPECT_EQ(1, frequentLayerCount(time));
-
-    // advance the time for the previous frame to be inactive
-    time += MAX_ACTIVE_LAYER_PERIOD_NS.count();
-
-    // Now event if we post a quick few frame we should stay infrequent
-    for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
-        history().record(layer.get(), time, time);
-        time += HI_FPS_PERIOD;
-
-        EXPECT_EQ(1, layerCount());
-        ASSERT_EQ(1, history().summarize(time).size());
-        EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
-        EXPECT_EQ(1, activeLayerCount());
-        EXPECT_EQ(0, frequentLayerCount(time));
-    }
-
-    // clear the history
-    history().clear();
-
-    // Now event if we post a quick few frame we should stay infrequent
-    for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE - 1; i++) {
-        history().record(layer.get(), time, time);
-        time += HI_FPS_PERIOD;
-
-        EXPECT_EQ(1, layerCount());
-        ASSERT_EQ(1, history().summarize(time).size());
-        EXPECT_EQ(LayerHistory::LayerVoteType::Min, history().summarize(time)[0].vote);
-        EXPECT_EQ(1, activeLayerCount());
-        EXPECT_EQ(0, frequentLayerCount(time));
-    }
 }
 
 TEST_F(LayerHistoryTestV2, invisibleExplicitLayer) {
@@ -547,11 +528,9 @@ TEST_F(LayerHistoryTestV2, invisibleExplicitLayer) {
 
     nsecs_t time = systemTime();
 
-    // Post a few buffers to the layers to make them active
+    // Post a buffer to the layers to make them active
-    for (int i = 0; i < FREQUENT_LAYER_WINDOW_SIZE; i++) {
     history().record(explicitVisiblelayer.get(), time, time);
     history().record(explicitInvisiblelayer.get(), time, time);
-    }
 
     EXPECT_EQ(2, layerCount());
     ASSERT_EQ(1, history().summarize(time).size());