SF: Add DisplayDevice::initiateModeChange
This CL creates DisplayDevice::initiateModeChange which calls
HWC::setActiveConfigWithConstraints. SF should call DisplayDevice
instead of directly calling HWComposer.
This is a step towards removing the cached display modes from
the HWComposer class.
Test: presubmit
Bug: 159590486
Change-Id: Id05d2eacbbb6ed335fe205231b85e8af9a8ccd91
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 4d36f0c..3133e90 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -96,7 +96,7 @@
MOCK_CONST_METHOD1(isVsyncPeriodSwitchSupported, bool(PhysicalDisplayId));
MOCK_CONST_METHOD1(getDisplayVsyncPeriod, nsecs_t(PhysicalDisplayId));
MOCK_METHOD4(setActiveModeWithConstraints,
- status_t(PhysicalDisplayId, DisplayModeId,
+ status_t(PhysicalDisplayId, hal::HWConfigId,
const hal::VsyncPeriodChangeConstraints&,
hal::VsyncPeriodChangeTimeline*));
MOCK_METHOD2(setAutoLowLatencyMode, status_t(PhysicalDisplayId, bool));
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index e3f15eb..b4a3ed1 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -51,12 +51,16 @@
ui::Transform::RotationFlags DisplayDevice::sPrimaryDisplayRotationFlags = ui::Transform::ROT_0;
DisplayDeviceCreationArgs::DisplayDeviceCreationArgs(
- const sp<SurfaceFlinger>& flinger, const wp<IBinder>& displayToken,
+ const sp<SurfaceFlinger>& flinger, HWComposer& hwComposer, const wp<IBinder>& displayToken,
std::shared_ptr<compositionengine::Display> compositionDisplay)
- : flinger(flinger), displayToken(displayToken), compositionDisplay(compositionDisplay) {}
+ : flinger(flinger),
+ hwComposer(hwComposer),
+ displayToken(displayToken),
+ compositionDisplay(compositionDisplay) {}
DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args)
: mFlinger(args.flinger),
+ mHwComposer(args.hwComposer),
mDisplayToken(args.displayToken),
mSequenceId(args.sequenceId),
mConnectionType(args.connectionType),
@@ -146,6 +150,19 @@
mActiveModeId = id;
}
+status_t DisplayDevice::initiateModeChange(DisplayModeId modeId,
+ const hal::VsyncPeriodChangeConstraints& constraints,
+ hal::VsyncPeriodChangeTimeline* outTimeline) const {
+ const auto mode = getMode(modeId);
+ if (!mode) {
+ ALOGE("Trying to initiate a mode change to invalid mode %s on display %s",
+ std::to_string(modeId.value()).c_str(), to_string(getId()).c_str());
+ return BAD_VALUE;
+ }
+ return mHwComposer.setActiveModeWithConstraints(getPhysicalId(), mode->getHwcId(), constraints,
+ outTimeline);
+}
+
const DisplayModePtr& DisplayDevice::getActiveMode() const {
return mSupportedModes[mActiveModeId.value()];
}
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 3acdb16..85f2f3b 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -34,6 +34,7 @@
#include <ui/HdrCapabilities.h>
#include <ui/Region.h>
#include <ui/Transform.h>
+#include <utils/Errors.h>
#include <utils/Mutex.h>
#include <utils/RefBase.h>
#include <utils/Timers.h>
@@ -161,6 +162,9 @@
*/
const DisplayModePtr& getActiveMode() const;
void setActiveMode(DisplayModeId);
+ status_t initiateModeChange(DisplayModeId modeId,
+ const hal::VsyncPeriodChangeConstraints& constraints,
+ hal::VsyncPeriodChangeTimeline* outTimeline) const;
// Return the immutable list of supported display modes. The HWC may report different modes
// after a hotplug reconnect event, in which case the DisplayDevice object will be recreated.
@@ -184,6 +188,7 @@
private:
const sp<SurfaceFlinger> mFlinger;
+ HWComposer& mHwComposer;
const wp<IBinder> mDisplayToken;
const int32_t mSequenceId;
const std::optional<DisplayConnectionType> mConnectionType;
@@ -240,9 +245,11 @@
struct DisplayDeviceCreationArgs {
// We use a constructor to ensure some of the values are set, without
// assuming a default value.
- DisplayDeviceCreationArgs(const sp<SurfaceFlinger>&, const wp<IBinder>& displayToken,
+ DisplayDeviceCreationArgs(const sp<SurfaceFlinger>&, HWComposer& hwComposer,
+ const wp<IBinder>& displayToken,
std::shared_ptr<compositionengine::Display>);
const sp<SurfaceFlinger> flinger;
+ HWComposer& hwComposer;
const wp<IBinder> displayToken;
const std::shared_ptr<compositionengine::Display> compositionDisplay;
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 6350144..46dc54e 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -674,20 +674,14 @@
}
status_t HWComposer::setActiveModeWithConstraints(
- PhysicalDisplayId displayId, DisplayModeId modeId,
+ PhysicalDisplayId displayId, hal::HWConfigId hwcModeId,
const hal::VsyncPeriodChangeConstraints& constraints,
hal::VsyncPeriodChangeTimeline* outTimeline) {
RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
- auto& displayData = mDisplayData[displayId];
- if (modeId.value() >= displayData.modes.size()) {
- LOG_DISPLAY_ERROR(displayId, ("Invalid mode " + std::to_string(modeId.value())).c_str());
- return BAD_INDEX;
- }
-
- const auto hwcConfigId = displayData.modes[modeId.value()]->getHwcId();
- auto error = displayData.hwcDisplay->setActiveConfigWithConstraints(hwcConfigId, constraints,
- outTimeline);
+ auto error = mDisplayData[displayId].hwcDisplay->setActiveConfigWithConstraints(hwcModeId,
+ constraints,
+ outTimeline);
RETURN_IF_HWC_ERROR(error, displayId, UNKNOWN_ERROR);
return NO_ERROR;
}
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 59c3699..1ffe276 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -198,7 +198,7 @@
virtual DisplayConnectionType getDisplayConnectionType(PhysicalDisplayId) const = 0;
virtual bool isVsyncPeriodSwitchSupported(PhysicalDisplayId) const = 0;
virtual nsecs_t getDisplayVsyncPeriod(PhysicalDisplayId) const = 0;
- virtual status_t setActiveModeWithConstraints(PhysicalDisplayId, DisplayModeId,
+ virtual status_t setActiveModeWithConstraints(PhysicalDisplayId, hal::HWConfigId,
const hal::VsyncPeriodChangeConstraints&,
hal::VsyncPeriodChangeTimeline* outTimeline) = 0;
virtual status_t setAutoLowLatencyMode(PhysicalDisplayId, bool on) = 0;
@@ -329,7 +329,7 @@
DisplayConnectionType getDisplayConnectionType(PhysicalDisplayId) const override;
bool isVsyncPeriodSwitchSupported(PhysicalDisplayId) const override;
nsecs_t getDisplayVsyncPeriod(PhysicalDisplayId displayId) const override;
- status_t setActiveModeWithConstraints(PhysicalDisplayId, DisplayModeId,
+ status_t setActiveModeWithConstraints(PhysicalDisplayId, hal::HWConfigId,
const hal::VsyncPeriodChangeConstraints&,
hal::VsyncPeriodChangeTimeline* outTimeline) override;
status_t setAutoLowLatencyMode(PhysicalDisplayId, bool) override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 894f352..3a9f7de 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1173,7 +1173,6 @@
}
mUpcomingActiveConfig = *desiredActiveConfig;
- const auto displayId = display->getPhysicalId();
ATRACE_INT("ActiveConfigFPS_HWC", refreshRate.getValue());
@@ -1184,12 +1183,11 @@
hal::VsyncPeriodChangeTimeline outTimeline;
const auto status =
- getHwComposer().setActiveModeWithConstraints(displayId, mUpcomingActiveConfig.configId,
- constraints, &outTimeline);
+ display->initiateModeChange(mUpcomingActiveConfig.configId, constraints, &outTimeline);
if (status != NO_ERROR) {
- // setActiveModeWithConstraints may fail if a hotplug event is just about
+ // initiateModeChange may fail if a hotplug event is just about
// to be sent. We just log the error in this case.
- ALOGW("setActiveModeWithConstraints failed: %d", status);
+ ALOGW("initiateModeChange failed: %d", status);
return;
}
@@ -2431,8 +2429,9 @@
const DisplayDeviceState& state,
const sp<compositionengine::DisplaySurface>& displaySurface,
const sp<IGraphicBufferProducer>& producer) {
- DisplayDeviceCreationArgs creationArgs(this, displayToken, compositionDisplay);
+ DisplayDeviceCreationArgs creationArgs(this, getHwComposer(), displayToken, compositionDisplay);
creationArgs.sequenceId = state.sequenceId;
+ creationArgs.hwComposer = getHwComposer();
creationArgs.isSecure = state.isSecure;
creationArgs.displaySurface = displaySurface;
creationArgs.hasWideColorGamut = false;
@@ -6044,7 +6043,7 @@
if (!display->isPrimary()) {
// TODO(b/144711714): For non-primary displays we should be able to set an active config
- // as well. For now, just call directly to setActiveModeWithConstraints but ideally
+ // as well. For now, just call directly to initiateModeChange but ideally
// it should go thru setDesiredActiveConfig, similar to primary display.
ALOGV("setAllowedDisplayConfigsInternal for non-primary display");
const auto displayId = display->getPhysicalId();
@@ -6054,8 +6053,8 @@
constraints.seamlessRequired = false;
hal::VsyncPeriodChangeTimeline timeline = {0, 0, 0};
- if (getHwComposer().setActiveModeWithConstraints(displayId, policy->defaultConfig,
- constraints, &timeline) < 0) {
+ if (display->initiateModeChange(policy->defaultConfig, constraints, &timeline) !=
+ NO_ERROR) {
return BAD_VALUE;
}
if (timeline.refreshRequired) {
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 71986fe..1e24c0a 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -35,6 +35,7 @@
#include "DisplayHardware/DisplayMode.h"
#include "DisplayHardware/HWComposer.h"
+#include "DisplayHardware/Hal.h"
#include "mock/DisplayHardware/MockComposer.h"
// TODO(b/129481165): remove the #pragma below and fix conversion issues
@@ -230,7 +231,7 @@
constraints.seamlessRequired = false;
hal::VsyncPeriodChangeTimeline timeline = {0, 0, 0};
- constexpr DisplayModeId kConfigIndex(0);
+ constexpr Config kConfigIndex = 0;
const auto status =
hwc.setActiveModeWithConstraints(physicalId, kConfigIndex, constraints, &timeline);
EXPECT_EQ(NO_ERROR, status);
@@ -243,8 +244,10 @@
hwc.allocatePhysicalDisplay(hwcId, physicalId);
for (size_t configIndex = 0; configIndex < kConfigs.size(); configIndex++) {
- const auto status = hwc.setActiveModeWithConstraints(physicalId, DisplayModeId(configIndex),
- constraints, &timeline);
+ const auto status =
+ hwc.setActiveModeWithConstraints(physicalId,
+ static_cast<hal::HWConfigId>(configIndex),
+ constraints, &timeline);
EXPECT_EQ(NO_ERROR, status) << "Error when switching to config " << configIndex;
}
}
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 9a9eeab..fc284ff 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -598,7 +598,8 @@
std::optional<DisplayConnectionType> connectionType,
std::optional<hal::HWDisplayId> hwcDisplayId, bool isPrimary)
: mFlinger(flinger),
- mCreationArgs(flinger.mFlinger.get(), mDisplayToken, compositionDisplay),
+ mCreationArgs(flinger.mFlinger.get(), flinger.mFlinger->getHwComposer(),
+ mDisplayToken, compositionDisplay),
mHwcDisplayId(hwcDisplayId) {
mCreationArgs.connectionType = connectionType;
mCreationArgs.isPrimary = isPrimary;