Merge "Revert "SF: Flow DisplayModeRequest through mode set FSM"" into main

7 files changed, 159 insertions, 246 deletions

diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 7b0aad7316..799d62c19d 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -222,6 +222,7 @@ bool DisplayDevice::initiateModeChange(display::DisplayModeRequest&& desiredMode
                                        const hal::VsyncPeriodChangeConstraints& constraints,
                                        hal::VsyncPeriodChangeTimeline& outTimeline) {
     mPendingModeOpt = std::move(desiredMode);
+    mIsModeSetPending = true;
 
     const auto& mode = *mPendingModeOpt->mode.modePtr;
 
@@ -234,22 +235,9 @@ bool DisplayDevice::initiateModeChange(display::DisplayModeRequest&& desiredMode
     return true;
 }
 
-auto DisplayDevice::finalizeModeChange() -> ModeChange {
-    if (!mPendingModeOpt) return NoModeChange{"No pending mode"};
-
-    auto pendingMode = *std::exchange(mPendingModeOpt, std::nullopt);
-    auto& pendingModePtr = pendingMode.mode.modePtr;
-
-    if (!mRefreshRateSelector->displayModes().contains(pendingModePtr->getId())) {
-        return NoModeChange{"Unknown pending mode"};
-    }
-
-    if (getActiveMode().modePtr->getResolution() != pendingModePtr->getResolution()) {
-        return ResolutionChange{std::move(pendingMode)};
-    }
-
-    setActiveMode(pendingModePtr->getId(), pendingModePtr->getVsyncRate(), pendingMode.mode.fps);
-    return RefreshRateChange{std::move(pendingMode)};
+void DisplayDevice::finalizeModeChange(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) {
+    setActiveMode(modeId, vsyncRate, renderFps);
+    mIsModeSetPending = false;
 }
 
 nsecs_t DisplayDevice::getVsyncPeriodFromHWC() const {
@@ -587,14 +575,10 @@ auto DisplayDevice::getDesiredMode() const -> DisplayModeRequestOpt {
     return mDesiredModeOpt;
 }
 
-auto DisplayDevice::takeDesiredMode() -> DisplayModeRequestOpt {
-    DisplayModeRequestOpt desiredModeOpt;
-    {
-        std::scoped_lock lock(mDesiredModeLock);
-        std::swap(mDesiredModeOpt, desiredModeOpt);
-        mHasDesiredModeTrace = false;
-    }
-    return desiredModeOpt;
+void DisplayDevice::clearDesiredMode() {
+    std::scoped_lock lock(mDesiredModeLock);
+    mDesiredModeOpt.reset();
+    mHasDesiredModeTrace = false;
 }
 
 void DisplayDevice::adjustRefreshRate(Fps pacesetterDisplayRefreshRate) {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 4c09ffdf44..97b56a2f19 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -19,7 +19,6 @@
 #include <memory>
 #include <string>
 #include <unordered_map>
-#include <variant>
 
 #include <android-base/thread_annotations.h>
 #include <android/native_window.h>
@@ -196,11 +195,12 @@ public:
     using DisplayModeRequestOpt = ftl::Optional<display::DisplayModeRequest>;
 
     DisplayModeRequestOpt getDesiredMode() const EXCLUDES(mDesiredModeLock);
-    DisplayModeRequestOpt takeDesiredMode() EXCLUDES(mDesiredModeLock);
+    void clearDesiredMode() EXCLUDES(mDesiredModeLock);
 
-    bool isModeSetPending() const REQUIRES(kMainThreadContext) {
-        return mPendingModeOpt.has_value();
+    DisplayModeRequestOpt getPendingMode() const REQUIRES(kMainThreadContext) {
+        return mPendingModeOpt;
     }
+    bool isModeSetPending() const REQUIRES(kMainThreadContext) { return mIsModeSetPending; }
 
     scheduler::FrameRateMode getActiveMode() const REQUIRES(kMainThreadContext) {
         return mRefreshRateSelector->getActiveMode();
@@ -212,25 +212,8 @@ public:
                             hal::VsyncPeriodChangeTimeline& outTimeline)
             REQUIRES(kMainThreadContext);
 
-    struct NoModeChange {
-        const char* reason;
-    };
-
-    struct ResolutionChange {
-        display::DisplayModeRequest activeMode;
-    };
-
-    struct RefreshRateChange {
-        display::DisplayModeRequest activeMode;
-    };
-
-    using ModeChange = std::variant<NoModeChange, ResolutionChange, RefreshRateChange>;
-
-    // Clears the pending DisplayModeRequest, and returns the ModeChange that occurred. If it was a
-    // RefreshRateChange, the pending mode becomes the active mode. If it was a ResolutionChange,
-    // the caller is responsible for resizing the framebuffer to match the active resolution by
-    // recreating the DisplayDevice.
-    ModeChange finalizeModeChange() REQUIRES(kMainThreadContext);
+    void finalizeModeChange(DisplayModeId, Fps vsyncRate, Fps renderFps)
+            REQUIRES(kMainThreadContext);
 
     scheduler::RefreshRateSelector& refreshRateSelector() const { return *mRefreshRateSelector; }
 
@@ -267,8 +250,6 @@ public:
     void dump(utils::Dumper&) const;
 
 private:
-    friend class TestableSurfaceFlinger;
-
     template <size_t N>
     inline std::string concatId(const char (&str)[N]) const {
         return std::string(ftl::Concat(str, ' ', getId().value).str());
@@ -319,15 +300,12 @@ private:
     // This parameter is only used for hdr/sdr ratio overlay
     float mHdrSdrRatio = 1.0f;
 
-    // A DisplayModeRequest flows through three states: desired, pending, and active. Requests
-    // within a frame are merged into a single desired request. Unless cleared, the request is
-    // relayed to HWC on the next frame, and becomes pending. The mode becomes active once HWC
-    // signals the present fence to confirm the mode set.
     mutable std::mutex mDesiredModeLock;
     DisplayModeRequestOpt mDesiredModeOpt GUARDED_BY(mDesiredModeLock);
     TracedOrdinal<bool> mHasDesiredModeTrace GUARDED_BY(mDesiredModeLock);
 
     DisplayModeRequestOpt mPendingModeOpt GUARDED_BY(kMainThreadContext);
+    bool mIsModeSetPending GUARDED_BY(kMainThreadContext) = false;
 };
 
 struct DisplayDeviceState {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index b3692059e2..2503efcc07 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -59,7 +59,6 @@
 #include <ftl/concat.h>
 #include <ftl/fake_guard.h>
 #include <ftl/future.h>
-#include <ftl/match.h>
 #include <ftl/unit.h>
 #include <gui/AidlStatusUtil.h>
 #include <gui/BufferQueue.h>
@@ -1236,21 +1235,20 @@ status_t SurfaceFlinger::getDisplayStats(const sp<IBinder>& displayToken,
     return NO_ERROR;
 }
 
-void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode, bool force) {
-    const auto mode = desiredMode.mode;
-    const auto displayId = mode.modePtr->getPhysicalDisplayId();
-
+void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& request, bool force) {
+    const auto displayId = request.mode.modePtr->getPhysicalDisplayId();
     ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
 
     const auto display = getDisplayDeviceLocked(displayId);
     if (!display) {
-        ALOGW("%s: Unknown display %s", __func__, to_string(displayId).c_str());
+        ALOGW("%s: display is no longer valid", __func__);
         return;
     }
 
-    const bool emitEvent = desiredMode.emitEvent;
+    const auto mode = request.mode;
+    const bool emitEvent = request.emitEvent;
 
-    switch (display->setDesiredMode(std::move(desiredMode), force)) {
+    switch (display->setDesiredMode(std::move(request), force)) {
         case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch:
             // DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler.
             mScheduler->setRenderRate(displayId,
@@ -1346,55 +1344,61 @@ void SurfaceFlinger::finalizeDisplayModeChange(DisplayDevice& display) {
     const auto displayId = display.getPhysicalId();
     ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
 
-    ftl::match(
-            display.finalizeModeChange(),
-            [this, displayId](DisplayDevice::RefreshRateChange change) {
-                ftl::FakeGuard guard(mStateLock);
+    const auto pendingModeOpt = display.getPendingMode();
+    if (!pendingModeOpt) {
+        // There is no pending mode change. This can happen if the active
+        // display changed and the mode change happened on a different display.
+        return;
+    }
 
-                if (change.activeMode.emitEvent) {
-                    dispatchDisplayModeChangeEvent(displayId, change.activeMode.mode);
-                }
+    const auto& activeMode = pendingModeOpt->mode;
 
-                applyActiveMode(std::move(change.activeMode));
-            },
-            [&](DisplayDevice::ResolutionChange change) {
-                auto& state = mCurrentState.displays.editValueFor(display.getDisplayToken());
-                // Assign a new sequence ID to recreate the display and so its framebuffer.
-                state.sequenceId = DisplayDeviceState{}.sequenceId;
-                state.physical->activeMode = change.activeMode.mode.modePtr.get();
-
-                ftl::FakeGuard guard1(kMainThreadContext);
-                ftl::FakeGuard guard2(mStateLock);
-                processDisplayChangesLocked();
-
-                applyActiveMode(std::move(change.activeMode));
-            },
-            [](DisplayDevice::NoModeChange noChange) {
-                // TODO(b/255635821): Remove this case, as it should no longer happen.
-                ALOGE("A mode change was initiated but not finalized: %s", noChange.reason);
-            });
+    if (display.getActiveMode().modePtr->getResolution() != activeMode.modePtr->getResolution()) {
+        auto& state = mCurrentState.displays.editValueFor(display.getDisplayToken());
+        // We need to generate new sequenceId in order to recreate the display (and this
+        // way the framebuffer).
+        state.sequenceId = DisplayDeviceState{}.sequenceId;
+        state.physical->activeMode = activeMode.modePtr.get();
+        processDisplayChangesLocked();
+
+        // processDisplayChangesLocked will update all necessary components so we're done here.
+        return;
+    }
+
+    display.finalizeModeChange(activeMode.modePtr->getId(), activeMode.modePtr->getVsyncRate(),
+                               activeMode.fps);
+
+    if (displayId == mActiveDisplayId) {
+        mRefreshRateStats->setRefreshRate(activeMode.fps);
+        updatePhaseConfiguration(activeMode.fps);
+    }
+
+    if (pendingModeOpt->emitEvent) {
+        dispatchDisplayModeChangeEvent(displayId, activeMode);
+    }
 }
 
-void SurfaceFlinger::dropModeRequest(display::DisplayModeRequest&& request) {
-    if (request.mode.modePtr->getPhysicalDisplayId() == mActiveDisplayId) {
+void SurfaceFlinger::dropModeRequest(const sp<DisplayDevice>& display) {
+    display->clearDesiredMode();
+    if (display->getPhysicalId() == mActiveDisplayId) {
         // TODO(b/255635711): Check for pending mode changes on other displays.
         mScheduler->setModeChangePending(false);
     }
 }
 
-void SurfaceFlinger::applyActiveMode(display::DisplayModeRequest&& activeMode) {
-    auto activeModePtr = activeMode.mode.modePtr;
+void SurfaceFlinger::applyActiveMode(const sp<DisplayDevice>& display) {
+    const auto activeModeOpt = display->getDesiredMode();
+    auto activeModePtr = activeModeOpt->mode.modePtr;
     const auto displayId = activeModePtr->getPhysicalDisplayId();
-    const auto renderFps = activeMode.mode.fps;
+    const auto renderFps = activeModeOpt->mode.fps;
 
-    dropModeRequest(std::move(activeMode));
+    dropModeRequest(display);
 
     constexpr bool kAllowToEnable = true;
     mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable, std::move(activeModePtr).take());
     mScheduler->setRenderRate(displayId, renderFps);
 
     if (displayId == mActiveDisplayId) {
-        mRefreshRateStats->setRefreshRate(renderFps);
         updatePhaseConfiguration(renderFps);
     }
 }
@@ -1408,50 +1412,50 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
         const auto display = getDisplayDeviceLocked(id);
         if (!display) continue;
 
-        auto desiredModeOpt = display->takeDesiredMode();
+        auto desiredModeOpt = display->getDesiredMode();
         if (!desiredModeOpt) {
             continue;
         }
 
-        auto desiredMode = std::move(*desiredModeOpt);
-
         if (!shouldApplyRefreshRateSelectorPolicy(*display)) {
-            dropModeRequest(std::move(desiredMode));
+            dropModeRequest(display);
             continue;
         }
 
-        const auto desiredModeId = desiredMode.mode.modePtr->getId();
+        const auto desiredModeId = desiredModeOpt->mode.modePtr->getId();
         const auto displayModePtrOpt = physical.snapshot().displayModes().get(desiredModeId);
 
         if (!displayModePtrOpt) {
-            ALOGW("%s: Unknown mode %d for display %s", __func__, desiredModeId.value(),
-                  to_string(id).c_str());
-            dropModeRequest(std::move(desiredMode));
+            ALOGW("Desired display mode is no longer supported. Mode ID = %d",
+                  desiredModeId.value());
+            dropModeRequest(display);
             continue;
         }
 
-        if (display->getActiveMode() == desiredMode.mode) {
-            dropModeRequest(std::move(desiredMode));
+        ALOGV("%s changing active mode to %d(%s) for display %s", __func__, desiredModeId.value(),
+              to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(),
+              to_string(display->getId()).c_str());
+
+        if (display->getActiveMode() == desiredModeOpt->mode) {
+            applyActiveMode(display);
             continue;
         }
 
-        // The desired mode is different from the active mode. However, the allowed modes might have
-        // changed since setDesiredMode scheduled a mode transition.
-        if (!display->refreshRateSelector().isModeAllowed(desiredMode.mode)) {
-            dropModeRequest(std::move(desiredMode));
+        // Desired active mode was set, it is different than the mode currently in use, however
+        // allowed modes might have changed by the time we process the refresh.
+        // Make sure the desired mode is still allowed
+        if (!display->refreshRateSelector().isModeAllowed(desiredModeOpt->mode)) {
+            dropModeRequest(display);
             continue;
         }
 
-        ALOGV("Mode setting display %s to %d (%s)", to_string(id).c_str(), desiredModeId.value(),
-              to_string(displayModePtrOpt.value().get()->getVsyncRate()).c_str());
-
-        // TODO(b/142753666): Use constraints.
+        // TODO(b/142753666) use constrains
         hal::VsyncPeriodChangeConstraints constraints;
         constraints.desiredTimeNanos = systemTime();
         constraints.seamlessRequired = false;
         hal::VsyncPeriodChangeTimeline outTimeline;
 
-        if (!display->initiateModeChange(std::move(desiredMode), constraints, outTimeline)) {
+        if (!display->initiateModeChange(std::move(*desiredModeOpt), constraints, outTimeline)) {
             continue;
         }
 
@@ -1471,6 +1475,11 @@ void SurfaceFlinger::initiateDisplayModeChanges() {
     if (displayToUpdateImmediately) {
         const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately);
         finalizeDisplayModeChange(*display);
+
+        const auto desiredModeOpt = display->getDesiredMode();
+        if (desiredModeOpt && display->getActiveMode() == desiredModeOpt->mode) {
+            applyActiveMode(display);
+        }
     }
 }
 
@@ -7395,7 +7404,7 @@ void SurfaceFlinger::kernelTimerChanged(bool expired) {
     if (!updateOverlay) return;
 
     // Update the overlay on the main thread to avoid race conditions with
-    // RefreshRateSelector::getActiveMode.
+    // RefreshRateSelector::getActiveMode
     static_cast<void>(mScheduler->schedule([=, this] {
         const auto display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked());
         if (!display) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 4440c7b746..72003cd65e 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -727,9 +727,9 @@ private:
     void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext);
     void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext);
 
-    // TODO(b/241285191): Move to Scheduler.
-    void dropModeRequest(display::DisplayModeRequest&&) REQUIRES(mStateLock);
-    void applyActiveMode(display::DisplayModeRequest&&) REQUIRES(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);
 
     // Called on the main thread in response to setPowerMode()
     void setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode)
diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
index 220001b3b0..c463a9271b 100644
--- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
@@ -23,13 +23,10 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
-#define EXPECT_DISPLAY_MODE_REQUEST(expected, request)                              \
-    EXPECT_FRAME_RATE_MODE(expected.mode.modePtr, expected.mode.fps, request.mode); \
-    EXPECT_EQ(expected.emitEvent, request.emitEvent)
-
-#define EXPECT_DISPLAY_MODE_REQUEST_OPT(expected, requestOpt) \
-    ASSERT_TRUE(requestOpt);                                  \
-    EXPECT_DISPLAY_MODE_REQUEST(expected, (*requestOpt))
+#define EXPECT_DISPLAY_MODE_REQUEST(expected, requestOpt)                               \
+    ASSERT_TRUE(requestOpt);                                                            \
+    EXPECT_FRAME_RATE_MODE(expected.mode.modePtr, expected.mode.fps, requestOpt->mode); \
+    EXPECT_EQ(expected.emitEvent, requestOpt->emitEvent)
 
 namespace android {
 namespace {
@@ -40,7 +37,6 @@ using DisplayModeRequest = display::DisplayModeRequest;
 class InitiateModeChangeTest : public DisplayTransactionTest {
 public:
     using Action = DisplayDevice::DesiredModeAction;
-
     void SetUp() override {
         injectFakeBufferQueueFactory();
         injectFakeNativeWindowSurfaceFactory();
@@ -88,43 +84,36 @@ TEST_F(InitiateModeChangeTest, setDesiredModeToActiveMode) {
 TEST_F(InitiateModeChangeTest, setDesiredMode) {
     EXPECT_EQ(Action::InitiateDisplayModeSwitch,
               mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
-    EXPECT_DISPLAY_MODE_REQUEST_OPT(kDesiredMode90, mDisplay->getDesiredMode());
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode());
 
     EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120)));
-    EXPECT_DISPLAY_MODE_REQUEST_OPT(kDesiredMode120, mDisplay->getDesiredMode());
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode());
 }
 
-TEST_F(InitiateModeChangeTest, takeDesiredMode) {
+TEST_F(InitiateModeChangeTest, clearDesiredMode) {
     EXPECT_EQ(Action::InitiateDisplayModeSwitch,
               mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
-    EXPECT_EQ(kDesiredMode90, mDisplay->getDesiredMode());
+    EXPECT_TRUE(mDisplay->getDesiredMode());
 
-    EXPECT_EQ(kDesiredMode90, mDisplay->takeDesiredMode());
+    mDisplay->clearDesiredMode();
     EXPECT_FALSE(mDisplay->getDesiredMode());
 }
 
-TEST_F(InitiateModeChangeTest, initiateModeChange) FTL_FAKE_GUARD(kMainThreadContext) {
+TEST_F(InitiateModeChangeTest, initiateModeChange) REQUIRES(kMainThreadContext) {
     EXPECT_EQ(Action::InitiateDisplayModeSwitch,
               mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
-    EXPECT_DISPLAY_MODE_REQUEST_OPT(kDesiredMode90, mDisplay->getDesiredMode());
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode());
 
     const hal::VsyncPeriodChangeConstraints constraints{
             .desiredTimeNanos = systemTime(),
             .seamlessRequired = false,
     };
     hal::VsyncPeriodChangeTimeline timeline;
+    EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode());
 
-    auto desiredModeOpt = mDisplay->takeDesiredMode();
-    ASSERT_TRUE(desiredModeOpt);
+    mDisplay->clearDesiredMode();
     EXPECT_FALSE(mDisplay->getDesiredMode());
-
-    EXPECT_TRUE(mDisplay->initiateModeChange(std::move(*desiredModeOpt), constraints, timeline));
-
-    auto modeChange = mDisplay->finalizeModeChange();
-
-    auto* changePtr = std::get_if<DisplayDevice::RefreshRateChange>(&modeChange);
-    ASSERT_TRUE(changePtr);
-    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, changePtr->activeMode);
 }
 
 TEST_F(InitiateModeChangeTest, initiateRenderRateSwitch) {
@@ -136,47 +125,27 @@ TEST_F(InitiateModeChangeTest, initiateRenderRateSwitch) {
 TEST_F(InitiateModeChangeTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) {
     EXPECT_EQ(Action::InitiateDisplayModeSwitch,
               mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
-    EXPECT_DISPLAY_MODE_REQUEST_OPT(kDesiredMode90, mDisplay->getDesiredMode());
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode());
 
     const hal::VsyncPeriodChangeConstraints constraints{
             .desiredTimeNanos = systemTime(),
             .seamlessRequired = false,
     };
     hal::VsyncPeriodChangeTimeline timeline;
+    EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode());
 
-    auto desiredModeOpt = mDisplay->takeDesiredMode();
-    ASSERT_TRUE(desiredModeOpt);
-    EXPECT_FALSE(mDisplay->getDesiredMode());
-
-    EXPECT_TRUE(mDisplay->initiateModeChange(std::move(*desiredModeOpt), constraints, timeline));
-
-    auto modeChange = mDisplay->finalizeModeChange();
-    auto* changePtr = std::get_if<DisplayDevice::RefreshRateChange>(&modeChange);
-    ASSERT_TRUE(changePtr);
-    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, changePtr->activeMode);
-
-    // The latest request should override the desired mode.
-    EXPECT_EQ(Action::InitiateDisplayModeSwitch,
-              mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode60)));
     EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120)));
+    ASSERT_TRUE(mDisplay->getDesiredMode());
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode());
 
-    EXPECT_DISPLAY_MODE_REQUEST_OPT(kDesiredMode120, mDisplay->getDesiredMode());
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode());
 
-    // No pending mode change.
-    EXPECT_TRUE(
-            std::holds_alternative<DisplayDevice::NoModeChange>(mDisplay->finalizeModeChange()));
+    EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));
+    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getPendingMode());
 
-    desiredModeOpt = mDisplay->takeDesiredMode();
-    ASSERT_TRUE(desiredModeOpt);
+    mDisplay->clearDesiredMode();
     EXPECT_FALSE(mDisplay->getDesiredMode());
-
-    EXPECT_TRUE(mDisplay->initiateModeChange(std::move(*desiredModeOpt), constraints, timeline));
-
-    modeChange = mDisplay->finalizeModeChange();
-
-    changePtr = std::get_if<DisplayDevice::RefreshRateChange>(&modeChange);
-    ASSERT_TRUE(changePtr);
-    EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, changePtr->activeMode);
 }
 
 } // namespace
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 3291dc366a..8b16a8a480 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -179,7 +179,7 @@ TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequ
 
     Mock::VerifyAndClearExpectations(mComposer);
 
-    EXPECT_FALSE(mDisplay->getDesiredMode());
+    EXPECT_TRUE(mDisplay->getDesiredMode());
     EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
 
     // Verify that the next commit will complete the mode change and send
@@ -263,13 +263,11 @@ TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) {
 
     mFlinger.commit();
 
-    // The 120 Hz mode should be pending.
-    EXPECT_FALSE(mDisplay->getDesiredMode());
-    EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90);
+    ASSERT_TRUE(mDisplay->getDesiredMode());
+    EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120);
 
     mFlinger.commit();
 
-    // The 120 Hz mode should be active.
     EXPECT_FALSE(mDisplay->getDesiredMode());
     EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId120);
 }
@@ -326,7 +324,7 @@ TEST_F(DisplayModeSwitchingTest, changeResolutionOnActiveDisplayWithoutRefreshRe
     EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90_4K);
 }
 
-MATCHER_P2(HasDesiredMode, flinger, modeId, "") {
+MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") {
     if (!arg->getDesiredMode()) {
         *result_listener << "No desired mode";
         return false;
@@ -345,33 +343,12 @@ MATCHER_P2(HasDesiredMode, flinger, modeId, "") {
     return true;
 }
 
-MATCHER_P(HasPendingMode, modeId, "") {
-    const auto pendingOpt = TestableSurfaceFlinger::getPendingMode(arg);
-
-    if (!pendingOpt) {
-        *result_listener << "No pending mode";
-        return false;
-    }
-
-    if (pendingOpt->mode.modePtr->getId() != modeId) {
-        *result_listener << "Unexpected pending mode " << modeId;
-        return false;
-    }
-
-    return true;
-}
-
-MATCHER_P(HasActiveMode, modeId, "") {
+MATCHER_P(ModeSettledTo, modeId, "") {
     if (const auto desiredOpt = arg->getDesiredMode()) {
         *result_listener << "Unsettled desired mode " << desiredOpt->mode.modePtr->getId();
         return false;
     }
 
-    if (const auto pendingOpt = TestableSurfaceFlinger::getPendingMode(arg)) {
-        *result_listener << "Unsettled pending mode " << pendingOpt->mode.modePtr->getId();
-        return false;
-    }
-
     ftl::FakeGuard guard(kMainThreadContext);
 
     if (arg->getActiveMode().modePtr->getId() != modeId) {
@@ -388,14 +365,14 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
     EXPECT_TRUE(innerDisplay->isPoweredOn());
     EXPECT_FALSE(outerDisplay->isPoweredOn());
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId60));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     // Only the inner display is powered on.
     mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId60));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     EXPECT_EQ(NO_ERROR,
               mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
@@ -407,8 +384,8 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
                                                   mock::createDisplayModeSpecs(kModeId60.value(),
                                                                                false, 0.f, 120.f)));
 
-    EXPECT_THAT(innerDisplay, HasDesiredMode(&mFlinger, kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
     EXPECT_CALL(*mComposer,
@@ -418,13 +395,13 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasPendingMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     innerDisplay->setPowerMode(hal::PowerMode::OFF);
     outerDisplay->setPowerMode(hal::PowerMode::ON);
@@ -432,8 +409,8 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
     // Only the outer display is powered on.
     mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay);
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasDesiredMode(&mFlinger, kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
 
     EXPECT_CALL(*mComposer,
                 setActiveConfigWithConstraints(kOuterDisplayHwcId,
@@ -442,13 +419,13 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasPendingMode(kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
 }
 
 TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
@@ -457,16 +434,16 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
     EXPECT_TRUE(innerDisplay->isPoweredOn());
     EXPECT_FALSE(outerDisplay->isPoweredOn());
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId60));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     outerDisplay->setPowerMode(hal::PowerMode::ON);
 
     // Both displays are powered on.
     mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId60));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     EXPECT_EQ(NO_ERROR,
               mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
@@ -478,8 +455,8 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
                                                   mock::createDisplayModeSpecs(kModeId60.value(),
                                                                                false, 0.f, 120.f)));
 
-    EXPECT_THAT(innerDisplay, HasDesiredMode(&mFlinger, kModeId90));
-    EXPECT_THAT(outerDisplay, HasDesiredMode(&mFlinger, kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
 
     const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
     EXPECT_CALL(*mComposer,
@@ -494,25 +471,25 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasPendingMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasPendingMode(kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
 }
 
 TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
     EXPECT_TRUE(mDisplay->isPoweredOn());
-    EXPECT_THAT(mDisplay, HasActiveMode(kModeId60));
+    EXPECT_THAT(mDisplay, ModeSettledTo(kModeId60));
 
     EXPECT_EQ(NO_ERROR,
               mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
                                                   mock::createDisplayModeSpecs(kModeId90.value(),
                                                                                false, 0.f, 120.f)));
 
-    EXPECT_THAT(mDisplay, HasDesiredMode(&mFlinger, kModeId90));
+    EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
 
     // Power off the display before the mode has been set.
     mDisplay->setPowerMode(hal::PowerMode::OFF);
@@ -527,11 +504,11 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
 
     // Powering off should not abort the mode set.
     EXPECT_FALSE(mDisplay->isPoweredOn());
-    EXPECT_THAT(mDisplay, HasPendingMode(kModeId90));
+    EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
 
     mFlinger.commit();
 
-    EXPECT_THAT(mDisplay, HasActiveMode(kModeId90));
+    EXPECT_THAT(mDisplay, ModeSettledTo(kModeId90));
 }
 
 TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
@@ -540,16 +517,16 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
     EXPECT_TRUE(innerDisplay->isPoweredOn());
     EXPECT_FALSE(outerDisplay->isPoweredOn());
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId60));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     outerDisplay->setPowerMode(hal::PowerMode::ON);
 
     // Both displays are powered on.
     mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay);
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId60));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     EXPECT_EQ(NO_ERROR,
               mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
@@ -561,8 +538,8 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
                                                   mock::createDisplayModeSpecs(kModeId60.value(),
                                                                                false, 0.f, 120.f)));
 
-    EXPECT_THAT(innerDisplay, HasDesiredMode(&mFlinger, kModeId90));
-    EXPECT_THAT(outerDisplay, HasDesiredMode(&mFlinger, kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
 
     // Power off the outer display before the mode has been set.
     outerDisplay->setPowerMode(hal::PowerMode::OFF);
@@ -576,13 +553,13 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
     mFlinger.commit();
 
     // Powering off the inactive display should abort the mode set.
-    EXPECT_THAT(innerDisplay, HasPendingMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId120));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
 
     innerDisplay->setPowerMode(hal::PowerMode::OFF);
     outerDisplay->setPowerMode(hal::PowerMode::ON);
@@ -598,13 +575,13 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
     mFlinger.commit();
 
     // The mode set should resume once the display becomes active.
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasPendingMode(kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
 
     mFlinger.commit();
 
-    EXPECT_THAT(innerDisplay, HasActiveMode(kModeId90));
-    EXPECT_THAT(outerDisplay, HasActiveMode(kModeId60));
+    EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+    EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
 }
 
 } // namespace
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 16815be1b3..27030d1027 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -484,10 +484,6 @@ public:
         return mFlinger->setDisplayBrightness(display, brightness);
     }
 
-    static const auto& getPendingMode(const sp<DisplayDevice>& display) {
-        return display->mPendingModeOpt;
-    }
-
     // Allow reading display state without locking, as if called on the SF main thread.
     auto setPowerModeInternal(const sp<DisplayDevice>& display,
                               hal::PowerMode mode) NO_THREAD_SAFETY_ANALYSIS {