SF: Query display modes only once during display creation / update
Currently HWComposer::getActiveMode / getModes is called multiple
times when creating or updating a display. This is unnecessary
and risky because the getActiveMode may return a different value
if a hotplug event has ocured in between.
In this CL we call getActiveMode and getModes only once and propagate
the values using DisplayDeviceState.
Bug: 159590486
Test: presubmit
Change-Id: I97418163426a0ae19f79bc2912a7d725c953ad01
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index 9ec2d16..1664301 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -136,7 +136,7 @@
DisplayTransactionTest();
};
-constexpr int32_t DEFAULT_REFRESH_RATE = 16'666'667;
+constexpr int32_t DEFAULT_VSYNC_PERIOD = 16'666'667;
constexpr int32_t DEFAULT_DPI = 320;
constexpr int DEFAULT_VIRTUAL_DISPLAY_SURFACE_FORMAT = HAL_PIXEL_FORMAT_RGB_565;
@@ -436,7 +436,7 @@
getDisplayAttribute(HWC_DISPLAY_ID, HWC_ACTIVE_CONFIG_ID,
IComposerClient::Attribute::VSYNC_PERIOD, _))
.WillRepeatedly(
- DoAll(SetArgPointee<3>(DEFAULT_REFRESH_RATE), Return(Error::NONE)));
+ DoAll(SetArgPointee<3>(DEFAULT_VSYNC_PERIOD), Return(Error::NONE)));
EXPECT_CALL(*test->mComposer,
getDisplayAttribute(HWC_DISPLAY_ID, HWC_ACTIVE_CONFIG_ID,
IComposerClient::Attribute::DPI_X, _))
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp
index 7a9403b..ef8b149 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp
@@ -86,16 +86,16 @@
// The display refresh period should be set in the orientedDisplaySpaceRect tracker.
FrameStats stats;
mFlinger.getAnimFrameTracker().getStats(&stats);
- EXPECT_EQ(DEFAULT_REFRESH_RATE, stats.refreshPeriodNano);
+ EXPECT_EQ(DEFAULT_VSYNC_PERIOD, stats.refreshPeriodNano);
// The display transaction needed flag should be set.
EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded));
// The compositor timing should be set to default values
const auto& compositorTiming = mFlinger.getCompositorTiming();
- EXPECT_EQ(-DEFAULT_REFRESH_RATE, compositorTiming.deadline);
- EXPECT_EQ(DEFAULT_REFRESH_RATE, compositorTiming.interval);
- EXPECT_EQ(DEFAULT_REFRESH_RATE, compositorTiming.presentLatency);
+ EXPECT_EQ(-DEFAULT_VSYNC_PERIOD, compositorTiming.deadline);
+ EXPECT_EQ(DEFAULT_VSYNC_PERIOD, compositorTiming.interval);
+ EXPECT_EQ(DEFAULT_VSYNC_PERIOD, compositorTiming.presentLatency);
}
} // namespace
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
index 2117628..6502420 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
@@ -105,7 +105,7 @@
struct DispSyncIsSupportedVariant {
static void setupResetModelCallExpectations(DisplayTransactionTest* test) {
- EXPECT_CALL(*test->mVsyncController, startPeriodTransition(DEFAULT_REFRESH_RATE)).Times(1);
+ EXPECT_CALL(*test->mVsyncController, startPeriodTransition(DEFAULT_VSYNC_PERIOD)).Times(1);
EXPECT_CALL(*test->mVSyncTracker, resetModel()).Times(1);
}
};
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
index 9a95ab4..2b5cb77 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
@@ -17,6 +17,8 @@
#undef LOG_TAG
#define LOG_TAG "LibSurfaceFlingerUnittests"
+#include "DisplayHardware/DisplayMode.h"
+
#include "DisplayTransactionTestHelpers.h"
namespace android {
@@ -232,13 +234,26 @@
// Invocation
DisplayDeviceState state;
- if (const auto connectionType = Case::Display::CONNECTION_TYPE::value) {
+ if constexpr (constexpr auto connectionType = Case::Display::CONNECTION_TYPE::value) {
const auto displayId = PhysicalDisplayId::tryCast(Case::Display::DISPLAY_ID::get());
ASSERT_TRUE(displayId);
const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;
ASSERT_TRUE(hwcDisplayId);
mFlinger.getHwComposer().allocatePhysicalDisplay(*hwcDisplayId, *displayId);
- state.physical = {.id = *displayId, .type = *connectionType, .hwcDisplayId = *hwcDisplayId};
+ DisplayModePtr activeMode = DisplayMode::Builder(Case::Display::HWC_ACTIVE_CONFIG_ID)
+ .setWidth(Case::Display::WIDTH)
+ .setHeight(Case::Display::HEIGHT)
+ .setVsyncPeriod(DEFAULT_VSYNC_PERIOD)
+ .setDpiX(DEFAULT_DPI)
+ .setDpiY(DEFAULT_DPI)
+ .setConfigGroup(0)
+ .build();
+ DisplayModes modes{activeMode};
+ state.physical = {.id = *displayId,
+ .type = *connectionType,
+ .hwcDisplayId = *hwcDisplayId,
+ .supportedModes = modes,
+ .activeMode = activeMode};
}
state.isSecure = static_cast<bool>(Case::Display::SECURE);