commit 5c989f5c72c52cba1fc2264525fac2e58bdf9f94
author Dominik Laskowski <domlaskowski@google.com> Thu Apr 11 13:57:14 2024 -0400
committer Dominik Laskowski <domlaskowski@google.com> Wed Jun 12 15:06:21 2024 -0400
tree 93ec26158f341ba3c054685d8d134b3d8abc6c30
parent 9ad62b5e03be7d633dac9c5b9be1b81db45db00c

SF: Isolate modesetting in DisplayModeController

Move the per-display state machine for modesetting from DisplayDevice to
DMC. In lieu of mStateLock, protect display lookup from multiple threads
using a mutex internal to DMC, which fixes the following deadlock:

OneShotTimer::loop
    SF::requestDisplayModes
    mStateLock

SF::commit
    mStateLock
    SF::processDisplayChangesLocked (hotplug or resolution change)
    Scheduler::demotePacesetterDisplay
    OneShotTimer::stop

A notable change is that {initiate,finalize}DisplayModeChange(s) are no
longer called under mStateLock, thanks to DMC's granular, internal lock.
finalizeDisplayModeChange still locks mStateLock for resolution changes.

Add an ActiveModeListener to DMC and register a callback in SF to update
the refresh rate overlay, which still lives in DisplayDevice for now.

Fixes: 329450361
Bug: 241285876
Test: DisplayModeControllerTest
Test: libsurfaceflinger_unittest
Change-Id: I30ec756f134d2d67a70ac8797008dc792eac035e

services/surfaceflinger/SurfaceFlinger.h

diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1230717..ee541c4 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -737,12 +737,12 @@
     status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps,
                                        Fps maxFps);
 
-    void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext);
-    void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext);
+    void initiateDisplayModeChanges() REQUIRES(kMainThreadContext) EXCLUDES(mStateLock);
+    void finalizeDisplayModeChange(PhysicalDisplayId) REQUIRES(kMainThreadContext)
+            EXCLUDES(mStateLock);
 
-    // TODO(b/241285191): Replace DisplayDevice with DisplayModeRequest, and move to Scheduler.
-    void dropModeRequest(const sp<DisplayDevice>&) REQUIRES(mStateLock);
-    void applyActiveMode(const sp<DisplayDevice>&) REQUIRES(mStateLock);
+    void dropModeRequest(PhysicalDisplayId) REQUIRES(kMainThreadContext);
+    void applyActiveMode(PhysicalDisplayId) REQUIRES(kMainThreadContext);
 
     // Called on the main thread in response to setPowerMode()
     void setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode)
@@ -1098,8 +1098,7 @@
                                const DisplayDeviceState& drawingState)
             REQUIRES(mStateLock, kMainThreadContext);
 
-    void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&)
-            REQUIRES(mStateLock);
+    void dispatchDisplayModeChangeEvent(PhysicalDisplayId, const scheduler::FrameRateMode&);
 
     /*
      * VSYNC
@@ -1339,9 +1338,7 @@
     display::PhysicalDisplays mPhysicalDisplays GUARDED_BY(mStateLock);
 
     // The inner or outer display for foldables, assuming they have mutually exclusive power states.
-    // Atomic because writes from onActiveDisplayChangedLocked are not always under mStateLock, but
-    // reads from ISchedulerCallback::requestDisplayModes may happen concurrently.
-    std::atomic<PhysicalDisplayId> mActiveDisplayId GUARDED_BY(mStateLock);
+    std::atomic<PhysicalDisplayId> mActiveDisplayId;
 
     display::DisplayModeController mDisplayModeController;