Only call onNewVsyncSchedule if the pacesetter changes

When re-calculating the pacesetter display, we may end up with the same
display, and therefore the same VsyncSchedule. In that case, we don't
need to apply the new one.

Bug: 276367387
Test: atest libsurfaceflinger_unittest:SchedulerTest
Change-Id: I76750ffdba6c3d790a98b65aabbc8b13eab3b4ae
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 3e12db6..8ddcfa1 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -130,7 +130,7 @@
 
         pacesetterVsyncSchedule = promotePacesetterDisplayLocked();
     }
-    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
+    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));
 }
 
 void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) {
@@ -149,7 +149,7 @@
 
         pacesetterVsyncSchedule = promotePacesetterDisplayLocked();
     }
-    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
+    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));
 }
 
 void Scheduler::run() {
@@ -693,16 +693,17 @@
         pacesetterVsyncSchedule = promotePacesetterDisplayLocked(pacesetterIdOpt);
     }
 
-    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
+    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));
 }
 
 std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked(
         std::optional<PhysicalDisplayId> pacesetterIdOpt) {
     // TODO(b/241286431): Choose the pacesetter display.
+    const auto oldPacesetterDisplayIdOpt = mPacesetterDisplayId;
     mPacesetterDisplayId = pacesetterIdOpt.value_or(mRefreshRateSelectors.begin()->first);
     ALOGI("Display %s is the pacesetter", to_string(*mPacesetterDisplayId).c_str());
 
-    auto vsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);
+    auto newVsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);
     if (const auto pacesetterPtr = pacesetterSelectorPtrLocked()) {
         pacesetterPtr->setIdleTimerCallbacks(
                 {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
@@ -713,15 +714,28 @@
 
         pacesetterPtr->startIdleTimer();
 
+        // Track the new period, which may have changed due to switching to a
+        // new pacesetter or due to a hotplug event. In the former case, this
+        // is important so that VSYNC modulation does not get stuck in the
+        // initiated state if a transition started on the old pacesetter.
         const Fps refreshRate = pacesetterPtr->getActiveMode().modePtr->getFps();
-        vsyncSchedule->startPeriodTransition(mSchedulerCallback, refreshRate.getPeriod(),
-                                             true /* force */);
+        newVsyncSchedule->startPeriodTransition(mSchedulerCallback, refreshRate.getPeriod(),
+                                                true /* force */);
     }
-    return vsyncSchedule;
+    if (oldPacesetterDisplayIdOpt == mPacesetterDisplayId) {
+        return nullptr;
+    }
+    return newVsyncSchedule;
 }
 
-void Scheduler::applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule> vsyncSchedule) {
-    onNewVsyncSchedule(vsyncSchedule->getDispatch());
+void Scheduler::applyNewVsyncScheduleIfNonNull(
+        std::shared_ptr<VsyncSchedule> pacesetterSchedulePtr) {
+    if (!pacesetterSchedulePtr) {
+        // The pacesetter has not changed, so there is no new VsyncSchedule to
+        // apply.
+        return;
+    }
+    onNewVsyncSchedule(pacesetterSchedulePtr->getDispatch());
     std::vector<android::EventThread*> threads;
     {
         std::lock_guard<std::mutex> lock(mConnectionsLock);
@@ -731,7 +745,7 @@
         }
     }
     for (auto* thread : threads) {
-        thread->onNewVsyncSchedule(vsyncSchedule);
+        thread->onNewVsyncSchedule(pacesetterSchedulePtr);
     }
 }
 
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 3423652..720a1cb 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -329,10 +329,12 @@
     // 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.
+    // Returns the new pacesetter's VsyncSchedule, or null if the pacesetter is
+    // unchanged.
     std::shared_ptr<VsyncSchedule> promotePacesetterDisplayLocked(
             std::optional<PhysicalDisplayId> pacesetterIdOpt = std::nullopt)
             REQUIRES(kMainThreadContext, mDisplayLock);
-    void applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule>) EXCLUDES(mDisplayLock);
+    void applyNewVsyncScheduleIfNonNull(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.
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index dc76b4c..0c43831 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -384,4 +384,23 @@
     }
 }
 
+TEST_F(SchedulerTest, changingPacesetterChangesVsyncSchedule) {
+    // Add a second display so we can change the pacesetter.
+    mScheduler->registerDisplay(kDisplayId2,
+                                std::make_shared<RefreshRateSelector>(kDisplay2Modes,
+                                                                      kDisplay2Mode60->getId()));
+    // Ensure that the pacesetter is the one we expect.
+    mScheduler->setPacesetterDisplay(kDisplayId1);
+
+    // Switching to the other will call onNewVsyncSchedule.
+    EXPECT_CALL(*mEventThread, onNewVsyncSchedule(mScheduler->getVsyncSchedule(kDisplayId2)))
+            .Times(1);
+    mScheduler->setPacesetterDisplay(kDisplayId2);
+}
+
+TEST_F(SchedulerTest, promotingSamePacesetterDoesNotChangeVsyncSchedule) {
+    EXPECT_CALL(*mEventThread, onNewVsyncSchedule(_)).Times(0);
+    mScheduler->setPacesetterDisplay(kDisplayId1);
+}
+
 } // namespace android::scheduler