commit 59db956b74a09712b3289316c40eae19c4cf2279
author Dominik Laskowski <domlaskowski@google.com> Thu Oct 27 16:18:53 2022 -0400
committer Dominik Laskowski <domlaskowski@google.com> Fri Nov 04 14:07:55 2022 -0400
tree 780fe826f284bd4a39dd1957de92388430267dc6
parent 8ecd08e78a1474285c38a38cf6db4426c5738dba

SF: Split Scheduler::setRefreshRateSelector

Add helper functions to bind/unbind the idle timer.

Bug: 255635821
Test: Build (-Wthread-safety)
Change-Id: I68cd1274e2b0591652a259b7f60d0a370883e512

services/surfaceflinger/Scheduler/Scheduler.cpp

diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 0e1b775..6e34a68 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -94,36 +94,24 @@
     }
 }
 
-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
-        // as the callback can attempt to acquire the lock before stopIdleTimer can finish
-        // the execution. It's safe to FakeGuard as main thread is the only thread that
-        // writes to the mRefreshRateSelector.
-        ftl::FakeGuard guard(mRefreshRateSelectorLock);
-        if (mRefreshRateSelector) {
-            mRefreshRateSelector->stopIdleTimer();
-            mRefreshRateSelector->clearIdleTimerCallbacks();
-        }
+void Scheduler::setRefreshRateSelector(RefreshRateSelectorPtr newSelectorPtr) {
+    // No need to lock for reads on kMainThreadContext.
+    if (const auto& selectorPtr = FTL_FAKE_GUARD(mRefreshRateSelectorLock, mRefreshRateSelector)) {
+        unbindIdleTimer(*selectorPtr);
     }
+
     {
-        // Clear state that depends on the current instance.
+        // Clear state that depends on the current RefreshRateSelector.
         std::scoped_lock lock(mPolicyLock);
         mPolicy = {};
     }
 
     std::scoped_lock lock(mRefreshRateSelectorLock);
-    mRefreshRateSelector = std::move(selectorPtr);
-    if (!mRefreshRateSelector) return;
+    mRefreshRateSelector = std::move(newSelectorPtr);
 
-    mRefreshRateSelector->setIdleTimerCallbacks(
-            {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
-                          .onExpired = [this] { idleTimerCallback(TimerState::Expired); }},
-             .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); },
-                        .onExpired = [this] { kernelIdleTimerCallback(TimerState::Expired); }}});
-
-    mRefreshRateSelector->startIdleTimer();
+    if (mRefreshRateSelector) {
+        bindIdleTimer(*mRefreshRateSelector);
+    }
 }
 
 void Scheduler::registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr) {
@@ -546,6 +534,21 @@
     mLayerHistory.clear();
 }
 
+void Scheduler::bindIdleTimer(RefreshRateSelector& selector) {
+    selector.setIdleTimerCallbacks(
+            {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
+                          .onExpired = [this] { idleTimerCallback(TimerState::Expired); }},
+             .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); },
+                        .onExpired = [this] { kernelIdleTimerCallback(TimerState::Expired); }}});
+
+    selector.startIdleTimer();
+}
+
+void Scheduler::unbindIdleTimer(RefreshRateSelector& selector) {
+    selector.stopIdleTimer();
+    selector.clearIdleTimerCallbacks();
+}
+
 void Scheduler::kernelIdleTimerCallback(TimerState state) {
     ATRACE_INT("ExpiredKernelIdleTimer", static_cast<int>(state));