Merge "Revert "Only call onNewVsyncSchedule if the pacesetter changes"" into udc-dev

3 files changed, 11 insertions, 46 deletions

diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 8ddcfa1ee6..3e12db61e7 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -130,7 +130,7 @@ void Scheduler::registerDisplayInternal(PhysicalDisplayId displayId,
 
         pacesetterVsyncSchedule = promotePacesetterDisplayLocked();
     }
-    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));
+    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
 }
 
 void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) {
@@ -149,7 +149,7 @@ void Scheduler::unregisterDisplay(PhysicalDisplayId displayId) {
 
         pacesetterVsyncSchedule = promotePacesetterDisplayLocked();
     }
-    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));
+    applyNewVsyncSchedule(std::move(pacesetterVsyncSchedule));
 }
 
 void Scheduler::run() {
@@ -693,17 +693,16 @@ void Scheduler::promotePacesetterDisplay(std::optional<PhysicalDisplayId> pacese
         pacesetterVsyncSchedule = promotePacesetterDisplayLocked(pacesetterIdOpt);
     }
 
-    applyNewVsyncScheduleIfNonNull(std::move(pacesetterVsyncSchedule));
+    applyNewVsyncSchedule(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 newVsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);
+    auto vsyncSchedule = getVsyncScheduleLocked(*mPacesetterDisplayId);
     if (const auto pacesetterPtr = pacesetterSelectorPtrLocked()) {
         pacesetterPtr->setIdleTimerCallbacks(
                 {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
@@ -714,28 +713,15 @@ std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked(
 
         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();
-        newVsyncSchedule->startPeriodTransition(mSchedulerCallback, refreshRate.getPeriod(),
-                                                true /* force */);
+        vsyncSchedule->startPeriodTransition(mSchedulerCallback, refreshRate.getPeriod(),
+                                             true /* force */);
     }
-    if (oldPacesetterDisplayIdOpt == mPacesetterDisplayId) {
-        return nullptr;
-    }
-    return newVsyncSchedule;
+    return vsyncSchedule;
 }
 
-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());
+void Scheduler::applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule> vsyncSchedule) {
+    onNewVsyncSchedule(vsyncSchedule->getDispatch());
     std::vector<android::EventThread*> threads;
     {
         std::lock_guard<std::mutex> lock(mConnectionsLock);
@@ -745,7 +731,7 @@ void Scheduler::applyNewVsyncScheduleIfNonNull(
         }
     }
     for (auto* thread : threads) {
-        thread->onNewVsyncSchedule(pacesetterSchedulePtr);
+        thread->onNewVsyncSchedule(vsyncSchedule);
     }
 }
 
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 4b2983b33d..1f6d378d5a 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -330,12 +330,10 @@ private:
     // 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 applyNewVsyncScheduleIfNonNull(std::shared_ptr<VsyncSchedule>) EXCLUDES(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.
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 0c43831455..dc76b4c90f 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -384,23 +384,4 @@ TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) {
     }
 }
 
-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