commit 22488f69b442064261f8764e64e3705fd2e22cb3
author Dominik Laskowski <domlaskowski@google.com> Thu Mar 28 09:53:04 2019 -0700
committer Dominik Laskowski <domlaskowski@google.com> Mon Apr 08 16:24:41 2019 +0000
tree 7f6b0699b2ade60c7328515b5fbb3742b4dd09e6
parent 3c9aec6db94841a5b16075923d22b38423dfa667

SF: Remove per-display state in scheduler

This CL removes per-display RefreshRateConfigs and AllowedDisplayConfigs
to avoid bugs in the untested multi-display code path of the scheduler,
adds checks to prevent crashes if the internal display is removed, and
cleans up related code by:

1) Replacing AllowedDisplayConfigs with a simple set.
2) Making setAllowedDisplayConfigs consistent with setPowerMode.
3) Removing unnecessary locking and allocation.

Bug: 129433906
Test: Boot with single/multiple display(s)
Change-Id: I3f59e9bdeaceb2cf48b4b9b71cd27f1d6a574680
(cherry picked from commit 645365116b7c77204aaffbb88f9407549445396c)

services/surfaceflinger/SurfaceFlinger.h

diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 7e8e836..7abcfee 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -46,7 +46,6 @@
 #include <utils/Trace.h>
 #include <utils/threads.h>
 
-#include "AllowedDisplayConfigs.h"
 #include "DisplayDevice.h"
 #include "DisplayHardware/HWC2.h"
 #include "DisplayHardware/PowerAdvisor.h"
@@ -74,6 +73,7 @@
 #include <thread>
 #include <type_traits>
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
 
 using namespace android::surfaceflinger;
@@ -407,12 +407,7 @@
             float frameScale, bool childrenOnly) override;
     status_t getDisplayStats(const sp<IBinder>& displayToken, DisplayStatInfo* stats) override;
     status_t getDisplayConfigs(const sp<IBinder>& displayToken,
-                               Vector<DisplayInfo>* configs) override {
-        Mutex::Autolock _l(mStateLock);
-        return getDisplayConfigsLocked(displayToken, configs);
-    }
-    status_t getDisplayConfigsLocked(const sp<IBinder>& displayToken, Vector<DisplayInfo>* configs)
-            REQUIRES(mStateLock);
+                               Vector<DisplayInfo>* configs) override;
     int getActiveConfig(const sp<IBinder>& displayToken) override;
     status_t getDisplayColorModes(const sp<IBinder>& displayToken,
                                   Vector<ui::ColorMode>* configs) override;
@@ -491,20 +486,10 @@
     struct ActiveConfigInfo {
         RefreshRateType type;
         int configId;
-        sp<IBinder> displayToken;
         Scheduler::ConfigEvent event;
 
         bool operator!=(const ActiveConfigInfo& other) const {
-            if (type != other.type) {
-                return true;
-            }
-            if (configId != other.configId) {
-                return true;
-            }
-            if (displayToken != other.displayToken) {
-                return true;
-            }
-            return (event != other.event);
+            return type != other.type || configId != other.configId || event != other.event;
         }
     };
 
@@ -519,14 +504,14 @@
     // desired config was set, HWC needs to update the panel on the next refresh, and when
     // we receive the fence back, we know that the process was complete. It returns whether
     // we need to wait for the next invalidate
-    bool performSetActiveConfig();
+    bool performSetActiveConfig() REQUIRES(mStateLock);
     // called on the main thread in response to setPowerMode()
     void setPowerModeInternal(const sp<DisplayDevice>& display, int mode) REQUIRES(mStateLock);
 
     // called on the main thread in response to setAllowedDisplayConfigs()
-    void setAllowedDisplayConfigsInternal(
-            const sp<IBinder>& displayToken,
-            std::unique_ptr<const AllowedDisplayConfigs>&& allowedConfigs) REQUIRES(mStateLock);
+    void setAllowedDisplayConfigsInternal(const sp<DisplayDevice>& display,
+                                          const std::vector<int32_t>& allowedConfigs)
+            REQUIRES(mStateLock);
 
     // Returns whether the transaction actually modified any state
     bool handleMessageTransaction();
@@ -805,7 +790,7 @@
     // the desired refresh rate.
     void setRefreshRateTo(RefreshRateType, Scheduler::ConfigEvent event) REQUIRES(mStateLock);
 
-    bool isConfigAllowed(const DisplayId& displayId, int32_t config);
+    bool isDisplayConfigAllowed(int32_t configId) REQUIRES(mStateLock);
 
     /*
      * Display identification
@@ -1111,14 +1096,13 @@
     std::unique_ptr<Scheduler> mScheduler;
     sp<Scheduler::ConnectionHandle> mAppConnectionHandle;
     sp<Scheduler::ConnectionHandle> mSfConnectionHandle;
-    std::unique_ptr<scheduler::RefreshRateStats> mRefreshRateStats;
 
-    std::unordered_map<DisplayId, std::shared_ptr<scheduler::RefreshRateConfigs>>
-            mRefreshRateConfigs;
+    scheduler::RefreshRateConfigs mRefreshRateConfigs;
+    scheduler::RefreshRateStats mRefreshRateStats{mRefreshRateConfigs, *mTimeStats};
 
-    std::mutex mAllowedConfigsLock;
-    std::unordered_map<DisplayId, std::unique_ptr<const AllowedDisplayConfigs>> mAllowedConfigs
-            GUARDED_BY(mAllowedConfigsLock);
+    // All configs are allowed if the set is empty.
+    using DisplayConfigs = std::unordered_set<int32_t>;
+    DisplayConfigs mAllowedDisplayConfigs GUARDED_BY(mStateLock);
 
     std::mutex mActiveConfigLock;
     // This bit is set once we start setting the config. We read from this bit during the