commit b5a094b997f4db0b2f21ea6b2be6fa8bc6ba2f57
author Dominik Laskowski <domlaskowski@google.com> Thu Oct 27 12:00:12 2022 -0400
committer Dominik Laskowski <domlaskowski@google.com> Wed Nov 02 13:05:03 2022 -0400
tree 60b3372f2e515333878ca16b27b4414a482c0eb1
parent 3681e5a1cb88ea9762daedb4770c03b9858802f0

SF: Avoid registering DisplayDevice with Scheduler

The Scheduler should only care about the RefreshRateSelector part, and
should not needlessly extend the compositionengine::Display's lifetime
until the DisplayDevice is unregistered.

Make Scheduler::registerDisplay infallible, such that SurfaceFlinger::
processDisplayChanged does not need to unregister before registering.

Bug: 241285191
Test: libsurfaceflinger_unittest
Change-Id: I12b3855167e98f48ae368d39264edcb456efb293

services/surfaceflinger/Scheduler/Scheduler.cpp

diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 499cee6..0e1b775 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -94,7 +94,7 @@
     }
 }
 
-void Scheduler::setRefreshRateSelector(std::shared_ptr<RefreshRateSelector> selectorPtr) {
+void Scheduler::setRefreshRateSelector(RefreshRateSelectorPtr selectorPtr) {
     // The current RefreshRateSelector instance may outlive this call, so unbind its idle timer.
     {
         // mRefreshRateSelectorLock is not locked here to avoid the deadlock
@@ -126,13 +126,12 @@
     mRefreshRateSelector->startIdleTimer();
 }
 
-void Scheduler::registerDisplay(sp<const DisplayDevice> display) {
-    if (display->isPrimary()) {
-        mLeaderDisplayId = display->getPhysicalId();
+void Scheduler::registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr) {
+    if (!mLeaderDisplayId) {
+        mLeaderDisplayId = displayId;
     }
 
-    const bool ok = mDisplays.try_emplace(display->getPhysicalId(), std::move(display)).second;
-    ALOGE_IF(!ok, "%s: Duplicate display", __func__);
+    mRefreshRateSelectors.emplace_or_replace(displayId, std::move(selectorPtr));
 }
 
 void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) {
@@ -140,7 +139,7 @@
         mLeaderDisplayId.reset();
     }
 
-    mDisplays.erase(displayId);
+    mRefreshRateSelectors.erase(displayId);
 }
 
 void Scheduler::run() {
@@ -711,10 +710,9 @@
 
     const auto globalSignals = makeGlobalSignals();
 
-    for (const auto& [id, display] : mDisplays) {
+    for (const auto& [id, selectorPtr] : mRefreshRateSelectors) {
         auto rankedRefreshRates =
-                display->holdRefreshRateSelector()
-                        ->getRankedRefreshRates(mPolicy.contentRequirements, globalSignals);
+                selectorPtr->getRankedRefreshRates(mPolicy.contentRequirements, globalSignals);
 
         for (const auto& [modePtr, score] : rankedRefreshRates.ranking) {
             const auto [it, inserted] = refreshRateTallies.try_emplace(modePtr->getFps(), score);
@@ -733,7 +731,7 @@
 
     // Find the first refresh rate common to all displays.
     while (maxScoreIt != refreshRateTallies.cend() &&
-           maxScoreIt->second.displayCount != mDisplays.size()) {
+           maxScoreIt->second.displayCount != mRefreshRateSelectors.size()) {
         ++maxScoreIt;
     }
 
@@ -742,7 +740,8 @@
         for (auto it = maxScoreIt + 1; it != refreshRateTallies.cend(); ++it) {
             const auto [fps, tally] = *it;
 
-            if (tally.displayCount == mDisplays.size() && tally.score > maxScoreIt->second.score) {
+            if (tally.displayCount == mRefreshRateSelectors.size() &&
+                tally.score > maxScoreIt->second.score) {
                 maxScoreIt = it;
             }
         }