Revert "SF: Flow DisplayModeRequest through mode set FSM"
This reverts commit aaab4c3b4f012f48bca428b9db1cd954a6fbc8e9.
Reason for revert: b/317378302
Change-Id: I516f52c41d27d9ea3eca969a3f786d2115521dac
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 671382c..950b05e 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -222,6 +222,7 @@
const hal::VsyncPeriodChangeConstraints& constraints,
hal::VsyncPeriodChangeTimeline& outTimeline) {
mPendingModeOpt = std::move(desiredMode);
+ mIsModeSetPending = true;
const auto& mode = *mPendingModeOpt->mode.modePtr;
@@ -234,22 +235,9 @@
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 {
@@ -583,14 +571,10 @@
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 3090477..ac390cb 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>
@@ -195,11 +194,12 @@
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();
@@ -211,25 +211,8 @@
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; }
@@ -266,8 +249,6 @@
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());
@@ -318,15 +299,12 @@
// 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 7230547..2769e5d 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>
@@ -1230,21 +1229,20 @@
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,
@@ -1340,55 +1338,61 @@
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();
+ 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();
- ftl::FakeGuard guard1(kMainThreadContext);
- ftl::FakeGuard guard2(mStateLock);
- processDisplayChangesLocked();
+ // processDisplayChangesLocked will update all necessary components so we're done here.
+ return;
+ }
- 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);
- });
+ 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);
}
}
@@ -1402,50 +1406,50 @@
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;
}
@@ -1465,6 +1469,11 @@
if (displayToUpdateImmediately) {
const auto display = getDisplayDeviceLocked(*displayToUpdateImmediately);
finalizeDisplayModeChange(*display);
+
+ const auto desiredModeOpt = display->getDesiredMode();
+ if (desiredModeOpt && display->getActiveMode() == desiredModeOpt->mode) {
+ applyActiveMode(display);
+ }
}
}
@@ -7382,7 +7391,7 @@
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 d2db685..6b44401 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -725,9 +725,9 @@
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 220001b..c463a92 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 @@
class InitiateModeChangeTest : public DisplayTransactionTest {
public:
using Action = DisplayDevice::DesiredModeAction;
-
void SetUp() override {
injectFakeBufferQueueFactory();
injectFakeNativeWindowSurfaceFactory();
@@ -88,43 +84,36 @@
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, 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 1af4a7a..7ad97a2 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -179,7 +179,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 47d64d3..22cb24b 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -484,10 +484,6 @@
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 {