SF: Move desired active mode management to DisplayDevice
SF would set the desired active mode on the specific DisplayDevice the
mode was requested and use it to swicth the active display mode.
Change-Id: Ib4f01ec63b2087ab2a81490051d1e029afc71854
Test: SF unit tests
Test: refresh rate switching is working on device with
more than one display
Bug: 187539899
Change-Id: Ie0e5ffe364f81775c2101ffd0cf2c596051947fa
Merged-In: Ie0e5ffe364f81775c2101ffd0cf2c596051947fa
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index b5086fa..9e704c3 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -51,6 +51,7 @@
"DisplayIdGeneratorTest.cpp",
"DisplayTransactionTest.cpp",
"DisplayDevice_GetBestColorModeTest.cpp",
+ "DisplayDevice_InitiateModeChange.cpp",
"DisplayDevice_SetProjectionTest.cpp",
"EventThreadTest.cpp",
"FpsReporterTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
new file mode 100644
index 0000000..d4cfbbb
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
@@ -0,0 +1,173 @@
+/*
+ * 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 <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace android {
+namespace {
+
+using FakeDisplayDeviceInjector = TestableSurfaceFlinger::FakeDisplayDeviceInjector;
+
+class InitiateModeChangeTest : public DisplayTransactionTest {
+public:
+ using Event = scheduler::RefreshRateConfigEvent;
+
+ void SetUp() override {
+ injectFakeBufferQueueFactory();
+ injectFakeNativeWindowSurfaceFactory();
+
+ PrimaryDisplayVariant::setupHwcHotplugCallExpectations(this);
+ PrimaryDisplayVariant::setupFramebufferConsumerBufferQueueCallExpectations(this);
+ PrimaryDisplayVariant::setupFramebufferProducerBufferQueueCallExpectations(this);
+ PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this);
+ PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this);
+
+ mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED);
+
+ mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)
+ .setSupportedModes({kDisplayMode60, kDisplayMode90, kDisplayMode120})
+ .setActiveMode(kDisplayModeId60)
+ .inject();
+ }
+
+protected:
+ sp<DisplayDevice> mDisplay;
+
+ const DisplayModeId kDisplayModeId60 = DisplayModeId(0);
+ const DisplayModePtr kDisplayMode60 =
+ DisplayMode::Builder(hal::HWConfigId(kDisplayModeId60.value()))
+ .setId(kDisplayModeId60)
+ .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())
+ .setVsyncPeriod(int32_t(16'666'667))
+ .setGroup(0)
+ .setHeight(1000)
+ .setWidth(1000)
+ .build();
+
+ const DisplayModeId kDisplayModeId90 = DisplayModeId(1);
+ const DisplayModePtr kDisplayMode90 =
+ DisplayMode::Builder(hal::HWConfigId(kDisplayModeId90.value()))
+ .setId(kDisplayModeId90)
+ .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())
+ .setVsyncPeriod(int32_t(11'111'111))
+ .setGroup(0)
+ .setHeight(1000)
+ .setWidth(1000)
+ .build();
+
+ const DisplayModeId kDisplayModeId120 = DisplayModeId(2);
+ const DisplayModePtr kDisplayMode120 =
+ DisplayMode::Builder(hal::HWConfigId(kDisplayModeId120.value()))
+ .setId(kDisplayModeId120)
+ .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())
+ .setVsyncPeriod(int32_t(8'333'333))
+ .setGroup(0)
+ .setHeight(1000)
+ .setWidth(1000)
+ .build();
+};
+
+TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setCurrentMode) {
+ EXPECT_FALSE(mDisplay->setDesiredActiveMode({kDisplayMode60, Event::None}));
+ EXPECT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+}
+
+TEST_F(InitiateModeChangeTest, setDesiredActiveMode_setNewMode) {
+ EXPECT_TRUE(mDisplay->setDesiredActiveMode({kDisplayMode90, Event::None}));
+ ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
+ EXPECT_EQ(kDisplayMode90, mDisplay->getDesiredActiveMode()->mode);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+
+ // Setting another mode should be cached but return false
+ EXPECT_FALSE(mDisplay->setDesiredActiveMode({kDisplayMode120, Event::None}));
+ ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
+ EXPECT_EQ(kDisplayMode120, mDisplay->getDesiredActiveMode()->mode);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+}
+
+TEST_F(InitiateModeChangeTest, clearDesiredActiveModeState) {
+ EXPECT_TRUE(mDisplay->setDesiredActiveMode({kDisplayMode90, Event::None}));
+ ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
+
+ mDisplay->clearDesiredActiveModeState();
+ ASSERT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+}
+
+TEST_F(InitiateModeChangeTest, initiateModeChange) NO_THREAD_SAFETY_ANALYSIS {
+ EXPECT_TRUE(mDisplay->setDesiredActiveMode({kDisplayMode90, Event::None}));
+ ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
+ EXPECT_EQ(kDisplayMode90, mDisplay->getDesiredActiveMode()->mode);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+
+ hal::VsyncPeriodChangeConstraints constraints{
+ .desiredTimeNanos = systemTime(),
+ .seamlessRequired = false,
+ };
+ hal::VsyncPeriodChangeTimeline timeline;
+ EXPECT_EQ(OK,
+ mDisplay->initiateModeChange(*mDisplay->getDesiredActiveMode(), constraints,
+ &timeline));
+ EXPECT_EQ(kDisplayMode90, mDisplay->getUpcomingActiveMode().mode);
+ EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+
+ mDisplay->clearDesiredActiveModeState();
+ ASSERT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+}
+
+TEST_F(InitiateModeChangeTest, getUpcomingActiveMode_desiredActiveModeChanged)
+NO_THREAD_SAFETY_ANALYSIS {
+ EXPECT_TRUE(mDisplay->setDesiredActiveMode({kDisplayMode90, Event::None}));
+ ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
+ EXPECT_EQ(kDisplayMode90, mDisplay->getDesiredActiveMode()->mode);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+
+ hal::VsyncPeriodChangeConstraints constraints{
+ .desiredTimeNanos = systemTime(),
+ .seamlessRequired = false,
+ };
+ hal::VsyncPeriodChangeTimeline timeline;
+ EXPECT_EQ(OK,
+ mDisplay->initiateModeChange(*mDisplay->getDesiredActiveMode(), constraints,
+ &timeline));
+ EXPECT_EQ(kDisplayMode90, mDisplay->getUpcomingActiveMode().mode);
+ EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+
+ EXPECT_FALSE(mDisplay->setDesiredActiveMode({kDisplayMode120, Event::None}));
+ ASSERT_NE(std::nullopt, mDisplay->getDesiredActiveMode());
+ EXPECT_EQ(kDisplayMode120, mDisplay->getDesiredActiveMode()->mode);
+ EXPECT_EQ(Event::None, mDisplay->getDesiredActiveMode()->event);
+
+ EXPECT_EQ(kDisplayMode90, mDisplay->getUpcomingActiveMode().mode);
+ EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+
+ EXPECT_EQ(OK,
+ mDisplay->initiateModeChange(*mDisplay->getDesiredActiveMode(), constraints,
+ &timeline));
+ EXPECT_EQ(kDisplayMode120, mDisplay->getUpcomingActiveMode().mode);
+ EXPECT_EQ(Event::None, mDisplay->getUpcomingActiveMode().event);
+
+ mDisplay->clearDesiredActiveModeState();
+ ASSERT_EQ(std::nullopt, mDisplay->getDesiredActiveMode());
+}
+
+} // namespace
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
index b4a1481..4ff7592 100644
--- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
+++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
@@ -543,17 +543,34 @@
}
TEST_F(EventThreadTest, postConfigChangedPrimary) {
- mThread->onModeChanged(INTERNAL_DISPLAY_ID, DisplayModeId(7), 16666666);
+ const auto mode = DisplayMode::Builder(hal::HWConfigId(0))
+ .setPhysicalDisplayId(INTERNAL_DISPLAY_ID)
+ .setId(DisplayModeId(7))
+ .setVsyncPeriod(16666666)
+ .build();
+
+ mThread->onModeChanged(mode);
expectConfigChangedEventReceivedByConnection(INTERNAL_DISPLAY_ID, 7, 16666666);
}
TEST_F(EventThreadTest, postConfigChangedExternal) {
- mThread->onModeChanged(EXTERNAL_DISPLAY_ID, DisplayModeId(5), 16666666);
+ const auto mode = DisplayMode::Builder(hal::HWConfigId(0))
+ .setPhysicalDisplayId(EXTERNAL_DISPLAY_ID)
+ .setId(DisplayModeId(5))
+ .setVsyncPeriod(16666666)
+ .build();
+
+ mThread->onModeChanged(mode);
expectConfigChangedEventReceivedByConnection(EXTERNAL_DISPLAY_ID, 5, 16666666);
}
TEST_F(EventThreadTest, postConfigChangedPrimary64bit) {
- mThread->onModeChanged(DISPLAY_ID_64BIT, DisplayModeId(7), 16666666);
+ const auto mode = DisplayMode::Builder(hal::HWConfigId(0))
+ .setPhysicalDisplayId(DISPLAY_ID_64BIT)
+ .setId(DisplayModeId(7))
+ .setVsyncPeriod(16666666)
+ .build();
+ mThread->onModeChanged(mode);
expectConfigChangedEventReceivedByConnection(DISPLAY_ID_64BIT, 7, 16666666);
}
@@ -562,7 +579,13 @@
sp<MockEventThreadConnection> suppressConnection =
createConnection(suppressConnectionEventRecorder);
- mThread->onModeChanged(INTERNAL_DISPLAY_ID, DisplayModeId(9), 16666666);
+ const auto mode = DisplayMode::Builder(hal::HWConfigId(0))
+ .setPhysicalDisplayId(INTERNAL_DISPLAY_ID)
+ .setId(DisplayModeId(9))
+ .setVsyncPeriod(16666666)
+ .build();
+
+ mThread->onModeChanged(mode);
expectConfigChangedEventReceivedByConnection(INTERNAL_DISPLAY_ID, 9, 16666666);
auto args = suppressConnectionEventRecorder.waitForCall();
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index b6dfd7a..5713c2f 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -199,20 +199,21 @@
// onModeChange is called.
mScheduler->clearOptionalFieldsInFeatures();
EXPECT_NO_FATAL_FAILURE(mScheduler->dispatchCachedReportedMode());
- EXPECT_CALL(*mEventThread, onModeChanged(_, _, _)).Times(0);
+ EXPECT_CALL(*mEventThread, onModeChanged(_)).Times(0);
}
TEST_F(SchedulerTest, onNonPrimaryDisplayModeChanged_invalidParameters) {
- DisplayModeId modeId = DisplayModeId(111);
- nsecs_t vsyncPeriod = 111111;
+ const auto mode = DisplayMode::Builder(hal::HWConfigId(0))
+ .setId(DisplayModeId(111))
+ .setPhysicalDisplayId(PHYSICAL_DISPLAY_ID)
+ .setVsyncPeriod(111111)
+ .build();
// If the handle is incorrect, the function should return before
// onModeChange is called.
Scheduler::ConnectionHandle invalidHandle = {.id = 123};
- EXPECT_NO_FATAL_FAILURE(mScheduler->onNonPrimaryDisplayModeChanged(invalidHandle,
- PHYSICAL_DISPLAY_ID, modeId,
- vsyncPeriod));
- EXPECT_CALL(*mEventThread, onModeChanged(_, _, _)).Times(0);
+ EXPECT_NO_FATAL_FAILURE(mScheduler->onNonPrimaryDisplayModeChanged(invalidHandle, mode));
+ EXPECT_CALL(*mEventThread, onModeChanged(_)).Times(0);
}
TEST_F(SchedulerTest, calculateMaxAcquiredBufferCount) {
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index 3b129d5..a99dabe 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -100,9 +100,8 @@
mFeatures.cachedModeChangedParams.reset();
}
- void onNonPrimaryDisplayModeChanged(ConnectionHandle handle, PhysicalDisplayId displayId,
- DisplayModeId modeId, nsecs_t vsyncPeriod) {
- return Scheduler::onNonPrimaryDisplayModeChanged(handle, displayId, modeId, vsyncPeriod);
+ void onNonPrimaryDisplayModeChanged(ConnectionHandle handle, DisplayModePtr mode) {
+ return Scheduler::onNonPrimaryDisplayModeChanged(handle, mode);
}
~TestableScheduler() {
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index f5d3577..393387b 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -724,7 +724,7 @@
return *this;
}
- sp<DisplayDevice> inject() {
+ sp<DisplayDevice> inject() NO_THREAD_SAFETY_ANALYSIS {
const auto displayId = mCreationArgs.compositionDisplay->getDisplayId();
DisplayDeviceState state;
diff --git a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
index 485b4ac..d25973e 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
@@ -33,7 +33,7 @@
MOCK_METHOD0(onScreenReleased, void());
MOCK_METHOD0(onScreenAcquired, void());
MOCK_METHOD2(onHotplugReceived, void(PhysicalDisplayId, bool));
- MOCK_METHOD3(onModeChanged, void(PhysicalDisplayId, DisplayModeId, nsecs_t));
+ MOCK_METHOD1(onModeChanged, void(DisplayModePtr));
MOCK_METHOD2(onFrameRateOverridesChanged,
void(PhysicalDisplayId, std::vector<FrameRateOverride>));
MOCK_CONST_METHOD1(dump, void(std::string&));