SF: Clean up DisplayDevice::initiateModeChange
Trace the pending mode after successfully setting it on HWC.
Remove unused status_t return value, and redundant warning log. Disallow
nullptr for out parameter.
Bug: 255635711
Test: presubmit
Change-Id: I381e5528d6336c300a80abcebccdd76369126c04
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 50e94bf..7fdf9e7 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -217,9 +217,9 @@
updateRefreshRateOverlayRate(vsyncRate, renderFps);
}
-status_t DisplayDevice::initiateModeChange(const ActiveModeInfo& info,
- const hal::VsyncPeriodChangeConstraints& constraints,
- hal::VsyncPeriodChangeTimeline* outTimeline) {
+bool DisplayDevice::initiateModeChange(const ActiveModeInfo& info,
+ const hal::VsyncPeriodChangeConstraints& constraints,
+ hal::VsyncPeriodChangeTimeline& outTimeline) {
if (!info.modeOpt || info.modeOpt->modePtr->getPhysicalDisplayId() != getPhysicalId()) {
ALOGE("Trying to initiate a mode change to invalid mode %s on display %s",
info.modeOpt ? std::to_string(info.modeOpt->modePtr->getId().value()).c_str()
@@ -230,11 +230,15 @@
mPendingMode = info;
mIsModeSetPending = true;
- const auto& pendingMode = *info.modeOpt->modePtr;
- ATRACE_INT(mPendingModeFpsTrace.c_str(), pendingMode.getVsyncRate().getIntValue());
+ const auto& mode = *info.modeOpt->modePtr;
- return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), pendingMode.getHwcId(),
- constraints, outTimeline);
+ if (mHwComposer.setActiveModeWithConstraints(getPhysicalId(), mode.getHwcId(), constraints,
+ &outTimeline) != OK) {
+ return false;
+ }
+
+ ATRACE_INT(mPendingModeFpsTrace.c_str(), mode.getVsyncRate().getIntValue());
+ return true;
}
void DisplayDevice::finalizeModeChange(DisplayModeId modeId, Fps vsyncRate, Fps renderFps) {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index c70c1df..a061fca 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -223,9 +223,8 @@
void setActiveMode(DisplayModeId, Fps vsyncRate, Fps renderFps);
- status_t initiateModeChange(const ActiveModeInfo&,
- const hal::VsyncPeriodChangeConstraints& constraints,
- hal::VsyncPeriodChangeTimeline* outTimeline)
+ bool initiateModeChange(const ActiveModeInfo&, const hal::VsyncPeriodChangeConstraints&,
+ hal::VsyncPeriodChangeTimeline& outTimeline)
REQUIRES(kMainThreadContext);
void finalizeModeChange(DisplayModeId, Fps vsyncRate, Fps renderFps)
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 04088ec..11d48f6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1397,11 +1397,7 @@
constraints.seamlessRequired = false;
hal::VsyncPeriodChangeTimeline outTimeline;
- const auto status = display->initiateModeChange(*desiredModeOpt, constraints, &outTimeline);
- if (status != NO_ERROR) {
- // initiateModeChange may fail if a hotplug event is just about
- // to be sent. We just log the error in this case.
- ALOGW("initiateModeChange failed: %d", status);
+ if (!display->initiateModeChange(*desiredModeOpt, constraints, outTimeline)) {
continue;
}
diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
index 8da641c..3873b0c 100644
--- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
@@ -103,13 +103,12 @@
EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredMode()->modeOpt);
EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);
- hal::VsyncPeriodChangeConstraints constraints{
+ const hal::VsyncPeriodChangeConstraints constraints{
.desiredTimeNanos = systemTime(),
.seamlessRequired = false,
};
hal::VsyncPeriodChangeTimeline timeline;
- EXPECT_EQ(OK,
- mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));
+ EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));
EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);
EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
@@ -130,13 +129,12 @@
EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getDesiredMode()->modeOpt);
EXPECT_EQ(Event::None, mDisplay->getDesiredMode()->event);
- hal::VsyncPeriodChangeConstraints constraints{
+ const hal::VsyncPeriodChangeConstraints constraints{
.desiredTimeNanos = systemTime(),
.seamlessRequired = false,
};
hal::VsyncPeriodChangeTimeline timeline;
- EXPECT_EQ(OK,
- mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));
+ EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));
EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);
EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
@@ -149,8 +147,7 @@
EXPECT_FRAME_RATE_MODE(kMode90, 90_Hz, *mDisplay->getPendingMode().modeOpt);
EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);
- EXPECT_EQ(OK,
- mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, &timeline));
+ EXPECT_TRUE(mDisplay->initiateModeChange(*mDisplay->getDesiredMode(), constraints, timeline));
EXPECT_FRAME_RATE_MODE(kMode120, 120_Hz, *mDisplay->getPendingMode().modeOpt);
EXPECT_EQ(Event::None, mDisplay->getPendingMode().event);