commit b972419619256ea07ebdb35b737946faf5c21bdc
author Dominik Laskowski <domlaskowski@google.com> Fri Jul 12 15:55:28 2024 -0400
committer Dominik Laskowski <domlaskowski@google.com> Tue Jul 16 12:50:42 2024 -0400
tree 5744e5a9aa701c3e5b95f73de6027c6047f6242e
parent 794127db5b35dafbf91779741c0a9eee6db6872a

SF: Do not toggle idle timer on display hotplug

As a short-term measure to avoid a deadlock during display hotplug, skip
stopping and starting the pacesetter's idle timer on demotion/promotion.
This assumes that hotplug results in demoting/promoting the same display
as pacesetter, currently always the active internal display.

Fixes: 329450361
Flag: com.android.graphics.surfaceflinger.flags.connected_display
Test: Manual (foldable, connected display)
Change-Id: Ieeb026666c8abdd14e0d4690a624fb60306b1bc1

services/surfaceflinger/Scheduler/Scheduler.h

diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 4dba6fc..94583db 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -377,8 +377,17 @@
     void resyncAllToHardwareVsync(bool allowToEnable) EXCLUDES(mDisplayLock);
     void setVsyncConfig(const VsyncConfig&, Period vsyncPeriod);
 
-    void promotePacesetterDisplay(PhysicalDisplayId pacesetterId) REQUIRES(kMainThreadContext)
-            EXCLUDES(mDisplayLock);
+    // TODO: b/241286431 - Remove this option, which assumes that the pacesetter does not change
+    // when a (secondary) display is registered or unregistered. In the short term, this avoids
+    // a deadlock where the main thread joins with the timer thread as the timer thread waits to
+    // lock a mutex held by the main thread.
+    struct PromotionParams {
+        // Whether to stop and start the idle timer. Ignored unless connected_display flag is set.
+        bool toggleIdleTimer;
+    };
+
+    void promotePacesetterDisplay(PhysicalDisplayId pacesetterId, PromotionParams)
+            REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock);
 
     // Changes to the displays (e.g. registering and unregistering) must be made
     // while mDisplayLock is locked, and the new pacesetter then must be promoted while
@@ -386,13 +395,16 @@
     // MessageQueue and EventThread need to use the new pacesetter's
     // VsyncSchedule, and this must happen while mDisplayLock is *not* locked,
     // or else we may deadlock with EventThread.
-    std::shared_ptr<VsyncSchedule> promotePacesetterDisplayLocked(PhysicalDisplayId pacesetterId)
+    std::shared_ptr<VsyncSchedule> promotePacesetterDisplayLocked(PhysicalDisplayId pacesetterId,
+                                                                  PromotionParams)
             REQUIRES(kMainThreadContext, mDisplayLock);
     void applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule>) EXCLUDES(mDisplayLock);
 
-    // Blocks until the pacesetter's idle timer thread exits. `mDisplayLock` must not be locked by
-    // the caller on the main thread to avoid deadlock, since the timer thread locks it before exit.
-    void demotePacesetterDisplay() REQUIRES(kMainThreadContext) EXCLUDES(mDisplayLock, mPolicyLock);
+    // If toggleIdleTimer is true, the calling thread blocks until the pacesetter's idle timer
+    // thread exits, in which case mDisplayLock must not be locked by the caller to avoid deadlock,
+    // since the timer thread locks it before exit.
+    void demotePacesetterDisplay(PromotionParams) REQUIRES(kMainThreadContext)
+            EXCLUDES(mDisplayLock, mPolicyLock);
 
     void registerDisplayInternal(PhysicalDisplayId, RefreshRateSelectorPtr, VsyncSchedulePtr,
                                  PhysicalDisplayId activeDisplayId) REQUIRES(kMainThreadContext)