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)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:b972419619256ea07ebdb35b737946faf5c21bdc)
Merged-In: Ieeb026666c8abdd14e0d4690a624fb60306b1bc1
Change-Id: Ieeb026666c8abdd14e0d4690a624fb60306b1bc1
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 9ea3f35..5ec7e48 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -82,7 +82,7 @@
mTouchTimer.reset();
// Stop idle timer and clear callbacks, as the RefreshRateSelector may outlive the Scheduler.
- demotePacesetterDisplay();
+ demotePacesetterDisplay({.toggleIdleTimer = true});
}
void Scheduler::initVsync(frametimeline::TokenManager& tokenManager,
@@ -118,9 +118,10 @@
}
void Scheduler::setPacesetterDisplay(PhysicalDisplayId pacesetterId) {
- demotePacesetterDisplay();
+ constexpr PromotionParams kPromotionParams = {.toggleIdleTimer = true};
- promotePacesetterDisplay(pacesetterId);
+ demotePacesetterDisplay(kPromotionParams);
+ promotePacesetterDisplay(pacesetterId, kPromotionParams);
}
void Scheduler::registerDisplay(PhysicalDisplayId displayId, RefreshRateSelectorPtr selectorPtr,
@@ -139,16 +140,22 @@
RefreshRateSelectorPtr selectorPtr,
VsyncSchedulePtr schedulePtr,
PhysicalDisplayId activeDisplayId) {
- demotePacesetterDisplay();
+ const bool isPrimary = (ftl::FakeGuard(mDisplayLock), !mPacesetterDisplayId);
- auto [pacesetterVsyncSchedule, isNew] = [&]() FTL_FAKE_GUARD(kMainThreadContext) {
+ // Start the idle timer for the first registered (i.e. primary) display.
+ const PromotionParams promotionParams = {.toggleIdleTimer = isPrimary};
+
+ demotePacesetterDisplay(promotionParams);
+
+ auto [pacesetterVsyncSchedule, isNew] = [&]() REQUIRES(kMainThreadContext) {
std::scoped_lock lock(mDisplayLock);
const bool isNew = mDisplays
.emplace_or_replace(displayId, displayId, std::move(selectorPtr),
std::move(schedulePtr), mFeatures)
.second;
- return std::make_pair(promotePacesetterDisplayLocked(activeDisplayId), isNew);
+ return std::make_pair(promotePacesetterDisplayLocked(activeDisplayId, promotionParams),
+ isNew);
}();
applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
@@ -166,7 +173,8 @@
dispatchHotplug(displayId, Hotplug::Disconnected);
- demotePacesetterDisplay();
+ constexpr PromotionParams kPromotionParams = {.toggleIdleTimer = false};
+ demotePacesetterDisplay(kPromotionParams);
std::shared_ptr<VsyncSchedule> pacesetterVsyncSchedule;
{
@@ -178,7 +186,7 @@
// headless virtual display.)
LOG_ALWAYS_FATAL_IF(mDisplays.empty(), "Cannot unregister all displays!");
- pacesetterVsyncSchedule = promotePacesetterDisplayLocked(activeDisplayId);
+ pacesetterVsyncSchedule = promotePacesetterDisplayLocked(activeDisplayId, kPromotionParams);
}
applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
}
@@ -917,19 +925,18 @@
return mFrameRateOverrideMappings.updateFrameRateOverridesByContent(frameRateOverrides);
}
-void Scheduler::promotePacesetterDisplay(PhysicalDisplayId pacesetterId) {
+void Scheduler::promotePacesetterDisplay(PhysicalDisplayId pacesetterId, PromotionParams params) {
std::shared_ptr<VsyncSchedule> pacesetterVsyncSchedule;
-
{
std::scoped_lock lock(mDisplayLock);
- pacesetterVsyncSchedule = promotePacesetterDisplayLocked(pacesetterId);
+ pacesetterVsyncSchedule = promotePacesetterDisplayLocked(pacesetterId, params);
}
applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
}
std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked(
- PhysicalDisplayId pacesetterId) {
+ PhysicalDisplayId pacesetterId, PromotionParams params) {
// TODO: b/241286431 - Choose the pacesetter among mDisplays.
mPacesetterDisplayId = pacesetterId;
ALOGI("Display %s is the pacesetter", to_string(pacesetterId).c_str());
@@ -938,15 +945,18 @@
if (const auto pacesetterOpt = pacesetterDisplayLocked()) {
const Display& pacesetter = *pacesetterOpt;
- pacesetter.selectorPtr->setIdleTimerCallbacks(
- {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
- .onExpired = [this] { idleTimerCallback(TimerState::Expired); }},
- .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); },
- .onExpired = [this] { kernelIdleTimerCallback(TimerState::Expired); }},
- .vrr = {.onReset = [this] { mSchedulerCallback.vrrDisplayIdle(false); },
- .onExpired = [this] { mSchedulerCallback.vrrDisplayIdle(true); }}});
+ if (!FlagManager::getInstance().connected_display() || params.toggleIdleTimer) {
+ pacesetter.selectorPtr->setIdleTimerCallbacks(
+ {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
+ .onExpired = [this] { idleTimerCallback(TimerState::Expired); }},
+ .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); },
+ .onExpired =
+ [this] { kernelIdleTimerCallback(TimerState::Expired); }},
+ .vrr = {.onReset = [this] { mSchedulerCallback.vrrDisplayIdle(false); },
+ .onExpired = [this] { mSchedulerCallback.vrrDisplayIdle(true); }}});
- pacesetter.selectorPtr->startIdleTimer();
+ pacesetter.selectorPtr->startIdleTimer();
+ }
newVsyncSchedulePtr = pacesetter.schedulePtr;
@@ -966,11 +976,14 @@
}
}
-void Scheduler::demotePacesetterDisplay() {
- // No need to lock for reads on kMainThreadContext.
- if (const auto pacesetterPtr = FTL_FAKE_GUARD(mDisplayLock, pacesetterSelectorPtrLocked())) {
- pacesetterPtr->stopIdleTimer();
- pacesetterPtr->clearIdleTimerCallbacks();
+void Scheduler::demotePacesetterDisplay(PromotionParams params) {
+ if (!FlagManager::getInstance().connected_display() || params.toggleIdleTimer) {
+ // No need to lock for reads on kMainThreadContext.
+ if (const auto pacesetterPtr =
+ FTL_FAKE_GUARD(mDisplayLock, pacesetterSelectorPtrLocked())) {
+ pacesetterPtr->stopIdleTimer();
+ pacesetterPtr->clearIdleTimerCallbacks();
+ }
}
// Clear state that depends on the pacesetter's RefreshRateSelector.
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)