SF: Isolate modesetting in DisplayModeController
Move the per-display state machine for modesetting from DisplayDevice to
DMC. In lieu of mStateLock, protect display lookup from multiple threads
using a mutex internal to DMC, which fixes the following deadlock:
OneShotTimer::loop
SF::requestDisplayModes
mStateLock
SF::commit
mStateLock
SF::processDisplayChangesLocked (hotplug or resolution change)
Scheduler::demotePacesetterDisplay
OneShotTimer::stop
A notable change is that {initiate,finalize}DisplayModeChange(s) are no
longer called under mStateLock, thanks to DMC's granular, internal lock.
finalizeDisplayModeChange still locks mStateLock for resolution changes.
Add an ActiveModeListener to DMC and register a callback in SF to update
the refresh rate overlay, which still lives in DisplayDevice for now.
Fixes: 329450361
Bug: 241285876
Test: DisplayModeControllerTest
Test: libsurfaceflinger_unittest
Change-Id: I30ec756f134d2d67a70ac8797008dc792eac035e
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 5145e11..98d5754 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -70,9 +70,9 @@
"DisplayIdGeneratorTest.cpp",
"DisplayTransactionTest.cpp",
"DisplayDevice_GetBestColorModeTest.cpp",
- "DisplayDevice_InitiateModeChange.cpp",
"DisplayDevice_SetDisplayBrightnessTest.cpp",
"DisplayDevice_SetProjectionTest.cpp",
+ "DisplayModeControllerTest.cpp",
"EventThreadTest.cpp",
"FlagManagerTest.cpp",
"FpsReporterTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
deleted file mode 100644
index c463a92..0000000
--- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
+++ /dev/null
@@ -1,152 +0,0 @@
-/*
- * Copyright 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#undef LOG_TAG
-#define LOG_TAG "LibSurfaceFlingerUnittests"
-
-#include "DisplayTransactionTestHelpers.h"
-#include "mock/MockFrameRateMode.h"
-
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-
-#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 {
-
-using FakeDisplayDeviceInjector = TestableSurfaceFlinger::FakeDisplayDeviceInjector;
-using DisplayModeRequest = display::DisplayModeRequest;
-
-class InitiateModeChangeTest : public DisplayTransactionTest {
-public:
- using Action = DisplayDevice::DesiredModeAction;
- void SetUp() override {
- injectFakeBufferQueueFactory();
- injectFakeNativeWindowSurfaceFactory();
-
- PrimaryDisplayVariant::setupHwcHotplugCallExpectations(this);
- PrimaryDisplayVariant::setupFramebufferConsumerBufferQueueCallExpectations(this);
- PrimaryDisplayVariant::setupFramebufferProducerBufferQueueCallExpectations(this);
- PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this);
- PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this);
-
- mFlinger.onComposerHalHotplugEvent(PrimaryDisplayVariant::HWC_DISPLAY_ID,
- DisplayHotplugEvent::CONNECTED);
- mFlinger.configureAndCommit();
-
- mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)
- .setDisplayModes(makeModes(kMode60, kMode90, kMode120), kModeId60)
- .inject();
- }
-
-protected:
- sp<DisplayDevice> mDisplay;
-
- static constexpr DisplayModeId kModeId60{0};
- static constexpr DisplayModeId kModeId90{1};
- static constexpr DisplayModeId kModeId120{2};
-
- static inline const ftl::NonNull<DisplayModePtr> kMode60 =
- ftl::as_non_null(createDisplayMode(kModeId60, 60_Hz));
- static inline const ftl::NonNull<DisplayModePtr> kMode90 =
- ftl::as_non_null(createDisplayMode(kModeId90, 90_Hz));
- static inline const ftl::NonNull<DisplayModePtr> kMode120 =
- ftl::as_non_null(createDisplayMode(kModeId120, 120_Hz));
-
- static inline const DisplayModeRequest kDesiredMode30{{30_Hz, kMode60}, .emitEvent = false};
- static inline const DisplayModeRequest kDesiredMode60{{60_Hz, kMode60}, .emitEvent = true};
- static inline const DisplayModeRequest kDesiredMode90{{90_Hz, kMode90}, .emitEvent = false};
- static inline const DisplayModeRequest kDesiredMode120{{120_Hz, kMode120}, .emitEvent = true};
-};
-
-TEST_F(InitiateModeChangeTest, setDesiredModeToActiveMode) {
- EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode60)));
- EXPECT_FALSE(mDisplay->getDesiredMode());
-}
-
-TEST_F(InitiateModeChangeTest, setDesiredMode) {
- EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
- EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getDesiredMode());
-
- EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120)));
- EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode());
-}
-
-TEST_F(InitiateModeChangeTest, clearDesiredMode) {
- EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
- EXPECT_TRUE(mDisplay->getDesiredMode());
-
- mDisplay->clearDesiredMode();
- EXPECT_FALSE(mDisplay->getDesiredMode());
-}
-
-TEST_F(InitiateModeChangeTest, initiateModeChange) REQUIRES(kMainThreadContext) {
- EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
- 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());
-
- mDisplay->clearDesiredMode();
- EXPECT_FALSE(mDisplay->getDesiredMode());
-}
-
-TEST_F(InitiateModeChangeTest, initiateRenderRateSwitch) {
- EXPECT_EQ(Action::InitiateRenderRateSwitch,
- mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode30)));
- EXPECT_FALSE(mDisplay->getDesiredMode());
-}
-
-TEST_F(InitiateModeChangeTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) {
- EXPECT_EQ(Action::InitiateDisplayModeSwitch,
- mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode90)));
- 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());
-
- EXPECT_EQ(Action::None, mDisplay->setDesiredMode(DisplayModeRequest(kDesiredMode120)));
- ASSERT_TRUE(mDisplay->getDesiredMode());
- EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getDesiredMode());
-
- EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDisplay->getPendingMode());
-
- EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));
- EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDisplay->getPendingMode());
-
- mDisplay->clearDesiredMode();
- EXPECT_FALSE(mDisplay->getDesiredMode());
-}
-
-} // namespace
-} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp
new file mode 100644
index 0000000..d971150
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp
@@ -0,0 +1,234 @@
+/*
+ * Copyright 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef LOG_TAG
+#define LOG_TAG "LibSurfaceFlingerUnittests"
+
+#include "Display/DisplayModeController.h"
+#include "Display/DisplaySnapshot.h"
+#include "DisplayHardware/HWComposer.h"
+#include "DisplayIdentificationTestHelpers.h"
+#include "FpsOps.h"
+#include "mock/DisplayHardware/MockComposer.h"
+#include "mock/DisplayHardware/MockDisplayMode.h"
+#include "mock/MockFrameRateMode.h"
+
+#include <ftl/fake_guard.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#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::display {
+namespace {
+
+namespace hal = android::hardware::graphics::composer::hal;
+
+using testing::_;
+using testing::DoAll;
+using testing::Return;
+using testing::SetArgPointee;
+
+class DisplayModeControllerTest : public testing::Test {
+public:
+ using Action = DisplayModeController::DesiredModeAction;
+
+ void SetUp() override {
+ mDmc.setHwComposer(mComposer.get());
+ mDmc.setActiveModeListener(
+ [this](PhysicalDisplayId displayId, Fps vsyncRate, Fps renderFps) {
+ mActiveModeListener.Call(displayId, vsyncRate, renderFps);
+ });
+
+ constexpr uint8_t kPort = 111;
+ EXPECT_CALL(*mComposerHal, getDisplayIdentificationData(kHwcDisplayId, _, _))
+ .WillOnce(DoAll(SetArgPointee<1>(kPort), SetArgPointee<2>(getInternalEdid()),
+ Return(hal::Error::NONE)));
+
+ EXPECT_CALL(*mComposerHal, setClientTargetSlotCount(kHwcDisplayId));
+ EXPECT_CALL(*mComposerHal,
+ setVsyncEnabled(kHwcDisplayId, hal::IComposerClient::Vsync::DISABLE));
+ EXPECT_CALL(*mComposerHal, onHotplugConnect(kHwcDisplayId));
+
+ const auto infoOpt = mComposer->onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
+ ASSERT_TRUE(infoOpt);
+
+ mDisplayId = infoOpt->id;
+ mDisplaySnapshotOpt.emplace(mDisplayId, ui::DisplayConnectionType::Internal,
+ makeModes(kMode60, kMode90, kMode120), ui::ColorModes{},
+ std::nullopt);
+
+ ftl::FakeGuard guard(kMainThreadContext);
+ mDmc.registerDisplay(*mDisplaySnapshotOpt, kModeId60,
+ scheduler::RefreshRateSelector::Config{});
+ }
+
+protected:
+ hal::VsyncPeriodChangeConstraints expectModeSet(const DisplayModeRequest& request,
+ hal::VsyncPeriodChangeTimeline& timeline,
+ bool subsequent = false) {
+ EXPECT_CALL(*mComposerHal,
+ isSupported(Hwc2::Composer::OptionalFeature::RefreshRateSwitching))
+ .WillOnce(Return(true));
+
+ if (!subsequent) {
+ EXPECT_CALL(*mComposerHal, getDisplayConnectionType(kHwcDisplayId, _))
+ .WillOnce(DoAll(SetArgPointee<1>(
+ hal::IComposerClient::DisplayConnectionType::INTERNAL),
+ Return(hal::V2_4::Error::NONE)));
+ }
+
+ const hal::VsyncPeriodChangeConstraints constraints{
+ .desiredTimeNanos = systemTime(),
+ .seamlessRequired = false,
+ };
+
+ const hal::HWConfigId hwcModeId = request.mode.modePtr->getHwcId();
+
+ EXPECT_CALL(*mComposerHal,
+ setActiveConfigWithConstraints(kHwcDisplayId, hwcModeId, constraints, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(hal::V2_4::Error::NONE)));
+
+ return constraints;
+ }
+
+ static constexpr hal::HWDisplayId kHwcDisplayId = 1234;
+
+ Hwc2::mock::Composer* mComposerHal = new testing::StrictMock<Hwc2::mock::Composer>();
+ const std::unique_ptr<HWComposer> mComposer{
+ std::make_unique<impl::HWComposer>(std::unique_ptr<Hwc2::Composer>(mComposerHal))};
+
+ testing::MockFunction<void(PhysicalDisplayId, Fps, Fps)> mActiveModeListener;
+
+ DisplayModeController mDmc;
+
+ PhysicalDisplayId mDisplayId;
+ std::optional<DisplaySnapshot> mDisplaySnapshotOpt;
+
+ static constexpr DisplayModeId kModeId60{0};
+ static constexpr DisplayModeId kModeId90{1};
+ static constexpr DisplayModeId kModeId120{2};
+
+ static inline const ftl::NonNull<DisplayModePtr> kMode60 =
+ ftl::as_non_null(mock::createDisplayMode(kModeId60, 60_Hz));
+ static inline const ftl::NonNull<DisplayModePtr> kMode90 =
+ ftl::as_non_null(mock::createDisplayMode(kModeId90, 90_Hz));
+ static inline const ftl::NonNull<DisplayModePtr> kMode120 =
+ ftl::as_non_null(mock::createDisplayMode(kModeId120, 120_Hz));
+
+ static inline const DisplayModeRequest kDesiredMode30{{30_Hz, kMode60}, .emitEvent = false};
+ static inline const DisplayModeRequest kDesiredMode60{{60_Hz, kMode60}, .emitEvent = true};
+ static inline const DisplayModeRequest kDesiredMode90{{90_Hz, kMode90}, .emitEvent = false};
+ static inline const DisplayModeRequest kDesiredMode120{{120_Hz, kMode120}, .emitEvent = true};
+};
+
+TEST_F(DisplayModeControllerTest, setDesiredModeToActiveMode) {
+ EXPECT_CALL(mActiveModeListener, Call(_, _, _)).Times(0);
+
+ EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode60)));
+ EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId));
+}
+
+TEST_F(DisplayModeControllerTest, setDesiredMode) {
+ // Called because setDesiredMode resets the render rate to the active refresh rate.
+ EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1);
+
+ EXPECT_EQ(Action::InitiateDisplayModeSwitch,
+ mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90)));
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId));
+
+ // No action since a mode switch has already been initiated.
+ EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode120)));
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getDesiredMode(mDisplayId));
+}
+
+TEST_F(DisplayModeControllerTest, clearDesiredMode) {
+ // Called because setDesiredMode resets the render rate to the active refresh rate.
+ EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1);
+
+ EXPECT_EQ(Action::InitiateDisplayModeSwitch,
+ mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90)));
+ EXPECT_TRUE(mDmc.getDesiredMode(mDisplayId));
+
+ mDmc.clearDesiredMode(mDisplayId);
+ EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId));
+}
+
+TEST_F(DisplayModeControllerTest, initiateModeChange) REQUIRES(kMainThreadContext) {
+ // Called because setDesiredMode resets the render rate to the active refresh rate.
+ EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1);
+
+ EXPECT_EQ(Action::InitiateDisplayModeSwitch,
+ mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90)));
+
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId));
+ auto modeRequest = kDesiredMode90;
+
+ hal::VsyncPeriodChangeTimeline timeline;
+ const auto constraints = expectModeSet(modeRequest, timeline);
+
+ EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId));
+
+ mDmc.clearDesiredMode(mDisplayId);
+ EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId));
+}
+
+TEST_F(DisplayModeControllerTest, initiateRenderRateSwitch) {
+ EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 30_Hz)).Times(1);
+
+ EXPECT_EQ(Action::InitiateRenderRateSwitch,
+ mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode30)));
+ EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId));
+}
+
+TEST_F(DisplayModeControllerTest, initiateDisplayModeSwitch) FTL_FAKE_GUARD(kMainThreadContext) {
+ // Called because setDesiredMode resets the render rate to the active refresh rate.
+ EXPECT_CALL(mActiveModeListener, Call(mDisplayId, 60_Hz, 60_Hz)).Times(1);
+
+ EXPECT_EQ(Action::InitiateDisplayModeSwitch,
+ mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode90)));
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getDesiredMode(mDisplayId));
+ auto modeRequest = kDesiredMode90;
+
+ hal::VsyncPeriodChangeTimeline timeline;
+ auto constraints = expectModeSet(modeRequest, timeline);
+
+ EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId));
+
+ // No action since a mode switch has already been initiated.
+ EXPECT_EQ(Action::None, mDmc.setDesiredMode(mDisplayId, DisplayModeRequest(kDesiredMode120)));
+
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode90, mDmc.getPendingMode(mDisplayId));
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getDesiredMode(mDisplayId));
+ modeRequest = kDesiredMode120;
+
+ constexpr bool kSubsequent = true;
+ constraints = expectModeSet(modeRequest, timeline, kSubsequent);
+
+ EXPECT_TRUE(mDmc.initiateModeChange(mDisplayId, std::move(modeRequest), constraints, timeline));
+ EXPECT_DISPLAY_MODE_REQUEST(kDesiredMode120, mDmc.getPendingMode(mDisplayId));
+
+ mDmc.clearDesiredMode(mDisplayId);
+ EXPECT_FALSE(mDmc.getDesiredMode(mDisplayId));
+}
+
+} // namespace
+} // namespace android::display
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 078b4fe..0c3e875 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -76,6 +76,7 @@
mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)
.setRefreshRateSelector(std::move(selectorPtr))
.inject(std::move(vsyncController), std::move(vsyncTracker));
+ mDisplayId = mDisplay->getPhysicalId();
// isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy
// will call setActiveConfig instead of setActiveConfigWithConstraints.
@@ -112,7 +113,11 @@
protected:
void setupScheduler(std::shared_ptr<scheduler::RefreshRateSelector>);
+ auto& dmc() { return mFlinger.mutableDisplayModeController(); }
+
sp<DisplayDevice> mDisplay, mOuterDisplay;
+ PhysicalDisplayId mDisplayId;
+
mock::EventThread* mAppEventThread;
static constexpr DisplayModeId kModeId60{0};
@@ -167,17 +172,17 @@
TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithRefreshRequired) {
ftl::FakeGuard guard(kMainThreadContext);
- EXPECT_FALSE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60);
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
mock::createDisplayModeSpecs(kModeId90, false, 0, 120));
- ASSERT_TRUE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90);
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ ASSERT_TRUE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90);
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60);
// Verify that next commit will call setActiveConfigWithConstraints in HWC
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
@@ -187,8 +192,8 @@
Mock::VerifyAndClearExpectations(mComposer);
- EXPECT_TRUE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_TRUE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60);
// Verify that the next commit will complete the mode change and send
// a onModeChanged event to the framework.
@@ -198,23 +203,23 @@
mFlinger.commit();
Mock::VerifyAndClearExpectations(mAppEventThread);
- EXPECT_FALSE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90);
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90);
}
TEST_F(DisplayModeSwitchingTest, changeRefreshRateOnActiveDisplayWithoutRefreshRequired) {
ftl::FakeGuard guard(kMainThreadContext);
- EXPECT_FALSE(mDisplay->getDesiredMode());
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
mock::createDisplayModeSpecs(kModeId90, true, 0, 120));
- ASSERT_TRUE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90);
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ ASSERT_TRUE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90);
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60);
// Verify that next commit will call setActiveConfigWithConstraints in HWC
// and complete the mode change.
@@ -226,8 +231,8 @@
mFlinger.commit();
- EXPECT_FALSE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90);
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90);
}
TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) {
@@ -236,8 +241,8 @@
// Test that if we call setDesiredDisplayModeSpecs while a previous mode change
// is still being processed the later call will be respected.
- EXPECT_FALSE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60);
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
@@ -252,46 +257,45 @@
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
mock::createDisplayModeSpecs(kModeId120, false, 0, 180));
- ASSERT_TRUE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120);
+ ASSERT_TRUE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId120);
EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId120);
mFlinger.commit();
- ASSERT_TRUE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120);
+ ASSERT_TRUE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId120);
mFlinger.commit();
- EXPECT_FALSE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId120);
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId120);
}
TEST_F(DisplayModeSwitchingTest, changeResolutionOnActiveDisplayWithoutRefreshRequired) {
ftl::FakeGuard guard(kMainThreadContext);
- EXPECT_FALSE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60);
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
mock::createDisplayModeSpecs(kModeId90_4K, false, 0, 120));
- ASSERT_TRUE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90_4K);
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId60);
+ ASSERT_TRUE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getDesiredMode(mDisplayId)->mode.modePtr->getId(), kModeId90_4K);
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId60);
// Verify that next commit will call setActiveConfigWithConstraints in HWC
// and complete the mode change.
const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};
EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId90_4K);
- EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplay->getPhysicalId(), true));
+ EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplayId, true));
- // Misc expecations. We don't need to enforce these method calls, but since the helper methods
- // already set expectations we should add new ones here, otherwise the test will fail.
+ // Override expectations set up by PrimaryDisplayVariant.
EXPECT_CALL(*mConsumer,
setDefaultBufferSize(static_cast<uint32_t>(kResolution4K.getWidth()),
static_cast<uint32_t>(kResolution4K.getHeight())))
@@ -304,31 +308,28 @@
injectFakeNativeWindowSurfaceFactory();
PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this);
- const auto displayToken = mDisplay->getDisplayToken().promote();
-
mFlinger.commit();
- // The DisplayDevice will be destroyed and recreated,
- // so we need to update with the new instance.
- mDisplay = mFlinger.getDisplay(displayToken);
-
- EXPECT_FALSE(mDisplay->getDesiredMode());
- EXPECT_EQ(mDisplay->getActiveMode().modePtr->getId(), kModeId90_4K);
+ EXPECT_FALSE(dmc().getDesiredMode(mDisplayId));
+ EXPECT_EQ(dmc().getActiveMode(mDisplayId).modePtr->getId(), kModeId90_4K);
}
MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") {
- if (!arg->getDesiredMode()) {
+ const auto displayId = arg->getPhysicalId();
+ auto& dmc = flinger->mutableDisplayModeController();
+
+ if (!dmc.getDesiredMode(displayId)) {
*result_listener << "No desired mode";
return false;
}
- if (arg->getDesiredMode()->mode.modePtr->getId() != modeId) {
+ if (dmc.getDesiredMode(displayId)->mode.modePtr->getId() != modeId) {
*result_listener << "Unexpected desired mode " << ftl::to_underlying(modeId);
return false;
}
// VsyncModulator should react to mode switches on the pacesetter display.
- if (arg->getPhysicalId() == flinger->scheduler()->pacesetterDisplayId() &&
+ if (displayId == flinger->scheduler()->pacesetterDisplayId() &&
!flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) {
*result_listener << "VsyncModulator did not shift to early phase";
return false;
@@ -337,8 +338,10 @@
return true;
}
-MATCHER_P(ModeSettledTo, modeId, "") {
- if (const auto desiredOpt = arg->getDesiredMode()) {
+MATCHER_P2(ModeSettledTo, dmc, modeId, "") {
+ const auto displayId = arg->getPhysicalId();
+
+ if (const auto desiredOpt = dmc->getDesiredMode(displayId)) {
*result_listener << "Unsettled desired mode "
<< ftl::to_underlying(desiredOpt->mode.modePtr->getId());
return false;
@@ -346,7 +349,7 @@
ftl::FakeGuard guard(kMainThreadContext);
- if (arg->getActiveMode().modePtr->getId() != modeId) {
+ if (dmc->getActiveMode(displayId).modePtr->getId() != modeId) {
*result_listener << "Settled to unexpected active mode " << ftl::to_underlying(modeId);
return false;
}
@@ -367,14 +370,14 @@
EXPECT_TRUE(innerDisplay->isPoweredOn());
EXPECT_FALSE(outerDisplay->isPoweredOn());
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF);
mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON);
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
@@ -400,14 +403,14 @@
mFlinger.commit();
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60));
mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF);
mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
@@ -420,12 +423,12 @@
mFlinger.commit();
EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60));
mFlinger.commit();
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60));
}
TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
@@ -440,14 +443,14 @@
EXPECT_TRUE(innerDisplay->isPoweredOn());
EXPECT_FALSE(outerDisplay->isPoweredOn());
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON);
mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
@@ -473,13 +476,13 @@
mFlinger.commit();
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60));
}
TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) {
EXPECT_TRUE(mDisplay->isPoweredOn());
- EXPECT_THAT(mDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(mDisplay, ModeSettledTo(&dmc(), kModeId60));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
@@ -502,7 +505,7 @@
mFlinger.commit();
- EXPECT_THAT(mDisplay, ModeSettledTo(kModeId90));
+ EXPECT_THAT(mDisplay, ModeSettledTo(&dmc(), kModeId90));
}
TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
@@ -518,14 +521,14 @@
EXPECT_TRUE(innerDisplay->isPoweredOn());
EXPECT_FALSE(outerDisplay->isPoweredOn());
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON);
mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId60));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
@@ -555,8 +558,8 @@
mFlinger.commit();
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId60));
mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF);
mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON);
@@ -570,13 +573,13 @@
mFlinger.commit();
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90));
EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId120));
mFlinger.commit();
- EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
- EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
+ EXPECT_THAT(innerDisplay, ModeSettledTo(&dmc(), kModeId90));
+ EXPECT_THAT(outerDisplay, ModeSettledTo(&dmc(), kModeId120));
}
} // namespace
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
index 1bae5ff..933d03d 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
@@ -232,7 +232,9 @@
// Invocation
DisplayDeviceState state;
- if constexpr (constexpr auto connectionType = Case::Display::CONNECTION_TYPE::value) {
+
+ constexpr auto kConnectionTypeOpt = Case::Display::CONNECTION_TYPE::value;
+ if constexpr (kConnectionTypeOpt) {
const auto displayId = PhysicalDisplayId::tryCast(Case::Display::DISPLAY_ID::get());
ASSERT_TRUE(displayId);
const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;
@@ -257,7 +259,7 @@
const auto it = mFlinger.mutablePhysicalDisplays()
.emplace_or_replace(*displayId, displayToken, *displayId,
- *connectionType, makeModes(activeMode),
+ *kConnectionTypeOpt, makeModes(activeMode),
std::move(colorModes), std::nullopt)
.first;
@@ -291,9 +293,12 @@
EXPECT_EQ(Case::Display::DISPLAY_FLAGS & DisplayDevice::eReceivesInput,
device->receivesInput());
- if constexpr (Case::Display::CONNECTION_TYPE::value) {
+ if constexpr (kConnectionTypeOpt) {
ftl::FakeGuard guard(kMainThreadContext);
- EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID, device->getActiveMode().modePtr->getHwcId());
+ EXPECT_EQ(Case::Display::HWC_ACTIVE_CONFIG_ID,
+ mFlinger.mutableDisplayModeController()
+ .getActiveMode(device->getPhysicalId())
+ .modePtr->getHwcId());
}
}
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 8c72a7d..007383b 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -191,6 +191,8 @@
void setupComposer(std::unique_ptr<Hwc2::Composer> composer) {
mFlinger->mCompositionEngine->setHwComposer(
std::make_unique<impl::HWComposer>(std::move(composer)));
+ mFlinger->mDisplayModeController.setHwComposer(
+ &mFlinger->mCompositionEngine->getHwComposer());
}
void setupPowerAdvisor(std::unique_ptr<Hwc2::PowerAdvisor> powerAdvisor) {
@@ -1055,7 +1057,6 @@
auto& modes = mDisplayModes;
auto& activeModeId = mActiveModeId;
- std::optional<Fps> refreshRateOpt;
DisplayDeviceState state;
state.isSecure = mCreationArgs.isSecure;
@@ -1093,7 +1094,9 @@
const auto activeModeOpt = modes.get(activeModeId);
LOG_ALWAYS_FATAL_IF(!activeModeOpt);
- refreshRateOpt = activeModeOpt->get()->getPeakFps();
+
+ // Save a copy for use after `modes` is consumed.
+ const Fps refreshRate = activeModeOpt->get()->getPeakFps();
state.physical = {.id = *physicalId,
.hwcDisplayId = *mHwcDisplayId,
@@ -1109,6 +1112,9 @@
.registerDisplay(*physicalId, it->second.snapshot(),
mCreationArgs.refreshRateSelector);
+ mFlinger.mutableDisplayModeController().setActiveMode(*physicalId, activeModeId,
+ refreshRate, refreshRate);
+
if (mFlinger.scheduler() && mSchedulerRegistration) {
mFlinger.scheduler()->registerDisplay(*physicalId,
mCreationArgs.refreshRateSelector,
@@ -1120,10 +1126,6 @@
sp<DisplayDevice> display = sp<DisplayDevice>::make(mCreationArgs);
mFlinger.mutableDisplays().emplace_or_replace(mDisplayToken, display);
- if (refreshRateOpt) {
- display->setActiveMode(activeModeId, *refreshRateOpt, *refreshRateOpt);
- }
-
mFlinger.mutableCurrentState().displays.add(mDisplayToken, state);
mFlinger.mutableDrawingState().displays.add(mDisplayToken, state);