SF: Add unit test for mode switching
Test: atest DisplayModeSwitchingTest
Bug: 190982486
Change-Id: Ie12ab8de5d93b8a3adbb818bdda9003f4c6bbed7
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 3dc6d8b..af635f9 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -71,6 +71,7 @@
"MessageQueueTest.cpp",
"SurfaceFlinger_CreateDisplayTest.cpp",
"SurfaceFlinger_DestroyDisplayTest.cpp",
+ "SurfaceFlinger_DisplayModeSwitching.cpp",
"SurfaceFlinger_DisplayTransactionCommitTest.cpp",
"SurfaceFlinger_GetDisplayNativePrimariesTest.cpp",
"SurfaceFlinger_HotplugTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
new file mode 100644
index 0000000..025edc2
--- /dev/null
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -0,0 +1,295 @@
+/*
+ * 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.
+ */
+
+#include "mock/MockEventThread.h"
+#undef LOG_TAG
+#define LOG_TAG "LibSurfaceFlingerUnittests"
+
+#include "DisplayTransactionTestHelpers.h"
+
+#include "Fps.h"
+
+namespace android {
+namespace {
+
+using android::hardware::graphics::composer::V2_4::Error;
+using android::hardware::graphics::composer::V2_4::VsyncPeriodChangeTimeline;
+
+class DisplayModeSwitchingTest : public DisplayTransactionTest {
+public:
+ 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,
+ kDisplayMode90DifferentResolution})
+ .setActiveMode(kDisplayModeId60)
+ .inject();
+
+ setupScheduler();
+
+ // isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy
+ // will call setActiveConfig instead of setActiveConfigWithConstraints.
+ ON_CALL(*mComposer, isVsyncPeriodSwitchSupported()).WillByDefault(Return(true));
+ }
+
+protected:
+ void setupScheduler();
+ void testChangeRefreshRate(bool isDisplayActive, bool isRefreshRequired);
+
+ sp<DisplayDevice> mDisplay;
+ mock::EventThread* mAppEventThread;
+
+ const DisplayModeId kDisplayModeId60 = DisplayModeId(0);
+ const DisplayModePtr kDisplayMode60 =
+ DisplayMode::Builder(hal::HWConfigId(kDisplayModeId60.value()))
+ .setId(kDisplayModeId60)
+ .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())
+ .setVsyncPeriod((60_Hz).getPeriodNsecs())
+ .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((90_Hz).getPeriodNsecs())
+ .setGroup(1)
+ .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((120_Hz).getPeriodNsecs())
+ .setGroup(2)
+ .setHeight(1000)
+ .setWidth(1000)
+ .build();
+
+ const DisplayModeId kDisplayModeId90DifferentResolution = DisplayModeId(3);
+ const DisplayModePtr kDisplayMode90DifferentResolution =
+ DisplayMode::Builder(hal::HWConfigId(kDisplayModeId90DifferentResolution.value()))
+ .setId(kDisplayModeId90DifferentResolution)
+ .setPhysicalDisplayId(PrimaryDisplayVariant::DISPLAY_ID::get())
+ .setVsyncPeriod((90_Hz).getPeriodNsecs())
+ .setGroup(3)
+ .setHeight(2000)
+ .setWidth(2000)
+ .build();
+};
+
+void DisplayModeSwitchingTest::setupScheduler() {
+ auto eventThread = std::make_unique<mock::EventThread>();
+ mAppEventThread = eventThread.get();
+ auto sfEventThread = std::make_unique<mock::EventThread>();
+
+ EXPECT_CALL(*eventThread, registerDisplayEventConnection(_));
+ EXPECT_CALL(*eventThread, createEventConnection(_, _))
+ .WillOnce(Return(new EventThreadConnection(eventThread.get(), /*callingUid=*/0,
+ ResyncCallback())));
+
+ EXPECT_CALL(*sfEventThread, registerDisplayEventConnection(_));
+ EXPECT_CALL(*sfEventThread, createEventConnection(_, _))
+ .WillOnce(Return(new EventThreadConnection(sfEventThread.get(), /*callingUid=*/0,
+ ResyncCallback())));
+
+ auto vsyncController = std::make_unique<mock::VsyncController>();
+ auto vsyncTracker = std::make_unique<mock::VSyncTracker>();
+
+ EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));
+ EXPECT_CALL(*vsyncTracker, currentPeriod())
+ .WillRepeatedly(
+ Return(TestableSurfaceFlinger::FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));
+ EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));
+ mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),
+ std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr,
+ /*hasMultipleModes*/ true);
+}
+
+TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRequired) {
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId60);
+
+ mFlinger.onActiveDisplayChanged(mDisplay);
+
+ mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
+ kDisplayModeId90.value(), false, 0.f, 120.f, 0.f, 120.f);
+
+ ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kDisplayModeId90);
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId60);
+
+ // Verify that next commit will call setActiveConfigWithConstraints in HWC
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
+ hal::HWConfigId(kDisplayModeId90.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ Mock::VerifyAndClearExpectations(mComposer);
+ ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId60);
+
+ // Verify that the next commit will complete the mode change and send
+ // a onModeChanged event to the framework.
+
+ EXPECT_CALL(*mAppEventThread, onModeChanged(kDisplayMode90));
+ mFlinger.commit();
+ Mock::VerifyAndClearExpectations(mAppEventThread);
+
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId90);
+}
+
+TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithoutRefreshRequired) {
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+
+ mFlinger.onActiveDisplayChanged(mDisplay);
+
+ mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
+ kDisplayModeId90.value(), true, 0.f, 120.f, 0.f, 120.f);
+
+ ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kDisplayModeId90);
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId60);
+
+ // Verify that next commit will call setActiveConfigWithConstraints in HWC
+ // and complete the mode change.
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
+ hal::HWConfigId(kDisplayModeId90.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ EXPECT_CALL(*mAppEventThread, onModeChanged(kDisplayMode90));
+
+ mFlinger.commit();
+
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId90);
+}
+
+TEST_F(DisplayModeSwitchingTest, twoConsecutiveSetDesiredDisplayModeSpecs) {
+ // Test that if we call setDesiredDisplayModeSpecs while a previous mode change
+ // is still being processed the later call will be respected.
+
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId60);
+
+ mFlinger.onActiveDisplayChanged(mDisplay);
+
+ mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
+ kDisplayModeId90.value(), false, 0.f, 120.f, 0.f, 120.f);
+
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
+ hal::HWConfigId(kDisplayModeId90.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
+ kDisplayModeId120.value(), false, 0.f, 180.f, 0.f, 180.f);
+
+ ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kDisplayModeId120);
+
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
+ hal::HWConfigId(kDisplayModeId120.value()), _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ mFlinger.commit();
+
+ ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kDisplayModeId120);
+
+ mFlinger.commit();
+
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId120);
+}
+
+TEST_F(DisplayModeSwitchingTest, changeResolution_OnActiveDisplay_WithoutRefreshRequired) {
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId60);
+
+ mFlinger.onActiveDisplayChanged(mDisplay);
+
+ mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
+ kDisplayModeId90DifferentResolution.value(), false, 0.f,
+ 120.f, 0.f, 120.f);
+
+ ASSERT_TRUE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getDesiredActiveMode()->mode->getId(), kDisplayModeId90DifferentResolution);
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId60);
+
+ // Verify that next commit will call setActiveConfigWithConstraints in HWC
+ // and complete the mode change.
+ const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};
+ EXPECT_CALL(*mComposer,
+ setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
+ hal::HWConfigId(
+ kDisplayModeId90DifferentResolution.value()),
+ _, _))
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+
+ EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplay->getPhysicalId(), 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.
+ EXPECT_CALL(*mConsumer, setDefaultBufferSize(2000, 2000)).WillOnce(Return(NO_ERROR));
+ EXPECT_CALL(*mConsumer, consumerConnect(_, false)).WillOnce(Return(NO_ERROR));
+ EXPECT_CALL(*mComposer, setClientTargetSlotCount(_)).WillOnce(Return(hal::Error::NONE));
+
+ // Create a new native surface to be used by the recreated display.
+ mNativeWindowSurface = nullptr;
+ 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);
+
+ ASSERT_FALSE(mDisplay->getDesiredActiveMode().has_value());
+ ASSERT_EQ(mDisplay->getActiveMode()->getId(), kDisplayModeId90DifferentResolution);
+}
+
+} // namespace
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 4c5789e..6ce72a8 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -24,6 +24,7 @@
#include <compositionengine/impl/OutputLayerCompositionState.h>
#include <compositionengine/mock/DisplaySurface.h>
#include <gui/ScreenCaptureResults.h>
+#include <algorithm>
#include "BufferQueueLayer.h"
#include "BufferStateLayer.h"
@@ -305,6 +306,11 @@
return mFlinger->destroyDisplay(displayToken);
}
+ auto getDisplay(const sp<IBinder>& displayToken) {
+ Mutex::Autolock lock(mFlinger->mStateLock);
+ return mFlinger->getDisplayDeviceLocked(displayToken);
+ }
+
void enableHalVirtualDisplays(bool enable) { mFlinger->enableHalVirtualDisplays(enable); }
auto setupNewDisplayDeviceInternal(
@@ -393,6 +399,21 @@
return SurfaceFlinger::calculateMaxAcquiredBufferCount(refreshRate, presentLatency);
}
+ auto setDesiredDisplayModeSpecs(const sp<IBinder>& displayToken, ui::DisplayModeId defaultMode,
+ bool allowGroupSwitching, float primaryRefreshRateMin,
+ float primaryRefreshRateMax, float appRequestRefreshRateMin,
+ float appRequestRefreshRateMax) {
+ return mFlinger->setDesiredDisplayModeSpecs(displayToken, defaultMode, allowGroupSwitching,
+ primaryRefreshRateMin, primaryRefreshRateMax,
+ appRequestRefreshRateMin,
+ appRequestRefreshRateMax);
+ }
+
+ void onActiveDisplayChanged(const sp<DisplayDevice>& activeDisplay) {
+ Mutex::Autolock lock(mFlinger->mStateLock);
+ mFlinger->onActiveDisplayChangedLocked(activeDisplay);
+ }
+
/* ------------------------------------------------------------------------
* Read-only access to private data to assert post-conditions.
*/
@@ -480,7 +501,7 @@
static constexpr hal::HWDisplayId DEFAULT_HWC_DISPLAY_ID = 1000;
static constexpr int32_t DEFAULT_WIDTH = 1920;
static constexpr int32_t DEFAULT_HEIGHT = 1280;
- static constexpr int32_t DEFAULT_VSYNC_PERIOD = 16'666'666;
+ static constexpr int32_t DEFAULT_VSYNC_PERIOD = 16'666'667;
static constexpr int32_t DEFAULT_CONFIG_GROUP = 7;
static constexpr int32_t DEFAULT_DPI = 320;
static constexpr hal::HWConfigId DEFAULT_ACTIVE_CONFIG = 0;
@@ -634,10 +655,10 @@
mCreationArgs.connectionType = connectionType;
mCreationArgs.isPrimary = isPrimary;
- mActiveModeId = DisplayModeId(0);
+ mCreationArgs.activeModeId = DisplayModeId(0);
DisplayModePtr activeMode =
DisplayMode::Builder(FakeHwcDisplayInjector::DEFAULT_ACTIVE_CONFIG)
- .setId(mActiveModeId)
+ .setId(mCreationArgs.activeModeId)
.setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))
.setWidth(FakeHwcDisplayInjector::DEFAULT_WIDTH)
.setHeight(FakeHwcDisplayInjector::DEFAULT_HEIGHT)
@@ -673,7 +694,7 @@
auto& mutableDisplayDevice() { return mFlinger.mutableDisplays()[mDisplayToken]; }
auto& setActiveMode(DisplayModeId mode) {
- mActiveModeId = mode;
+ mCreationArgs.activeModeId = mode;
return *this;
}
@@ -728,14 +749,29 @@
const auto physicalId = PhysicalDisplayId::tryCast(*displayId);
LOG_ALWAYS_FATAL_IF(!physicalId);
LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);
- state.physical = {.id = *physicalId, .type = *type, .hwcDisplayId = *mHwcDisplayId};
+
+ const DisplayModePtr activeModePtr =
+ *std::find_if(mCreationArgs.supportedModes.begin(),
+ mCreationArgs.supportedModes.end(), [&](DisplayModePtr mode) {
+ return mode->getId() == mCreationArgs.activeModeId;
+ });
+ state.physical = {.id = *physicalId,
+ .type = *type,
+ .hwcDisplayId = *mHwcDisplayId,
+ .deviceProductInfo = {},
+ .supportedModes = mCreationArgs.supportedModes,
+ .activeMode = activeModePtr};
}
state.isSecure = mCreationArgs.isSecure;
+ mCreationArgs.refreshRateConfigs =
+ std::make_shared<scheduler::RefreshRateConfigs>(mCreationArgs.supportedModes,
+ mCreationArgs.activeModeId);
+
sp<DisplayDevice> device = new DisplayDevice(mCreationArgs);
if (!device->isVirtual()) {
- device->setActiveMode(mActiveModeId);
+ device->setActiveMode(mCreationArgs.activeModeId);
}
mFlinger.mutableDisplays().emplace(mDisplayToken, device);
mFlinger.mutableCurrentState().displays.add(mDisplayToken, state);
@@ -753,7 +789,6 @@
sp<BBinder> mDisplayToken = new BBinder();
DisplayDeviceCreationArgs mCreationArgs;
const std::optional<hal::HWDisplayId> mHwcDisplayId;
- DisplayModeId mActiveModeId;
};
private: