SF: Update the cached display modes in HWComposer on hotplug
Currently configs in HWComposer are cached only once
per display and never updated on subseqent onHotplug
events. This may cause setActiveConfigsWithConstraints
to fail for a valid config.
This CL also removes the test HandleTransactionLockedTest::
processesHotplugConnectPrimaryDisplayWithExternalAlreadyConnected,
since it's testing an unsupported use case and its
incompatible with the tests we've added.
Bug: 159590486
Test: atest HWComposerConfigsTest
Change-Id: Ifb22c33ba5078bde35ae20a2f94a8630316da024
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 5fa72b8..6f3987f 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -74,6 +74,7 @@
namespace hal = android::hardware::graphics::composer::hal;
+namespace android {
namespace {
using android::hardware::Return;
@@ -88,47 +89,46 @@
mSequenceId(sequenceId),
mVsyncSwitchingSupported(vsyncSwitchingSupported) {}
- android::hardware::Return<void> onHotplug(hal::HWDisplayId display,
- hal::Connection conn) override {
+ Return<void> onHotplug(hal::HWDisplayId display, hal::Connection conn) override {
mCallback->onHotplugReceived(mSequenceId, display, conn);
- return android::hardware::Void();
+ return Void();
}
- android::hardware::Return<void> onRefresh(hal::HWDisplayId display) override {
+ Return<void> onRefresh(hal::HWDisplayId display) override {
mCallback->onRefreshReceived(mSequenceId, display);
- return android::hardware::Void();
+ return Void();
}
- android::hardware::Return<void> onVsync(hal::HWDisplayId display, int64_t timestamp) override {
+ Return<void> onVsync(hal::HWDisplayId display, int64_t timestamp) override {
if (!mVsyncSwitchingSupported) {
mCallback->onVsyncReceived(mSequenceId, display, timestamp, std::nullopt);
} else {
ALOGW("Unexpected onVsync callback on composer >= 2.4, ignoring.");
}
- return android::hardware::Void();
+ return Void();
}
- android::hardware::Return<void> onVsync_2_4(hal::HWDisplayId display, int64_t timestamp,
- hal::VsyncPeriodNanos vsyncPeriodNanos) override {
+ Return<void> onVsync_2_4(hal::HWDisplayId display, int64_t timestamp,
+ hal::VsyncPeriodNanos vsyncPeriodNanos) override {
if (mVsyncSwitchingSupported) {
mCallback->onVsyncReceived(mSequenceId, display, timestamp,
std::make_optional(vsyncPeriodNanos));
} else {
ALOGW("Unexpected onVsync_2_4 callback on composer <= 2.3, ignoring.");
}
- return android::hardware::Void();
+ return Void();
}
- android::hardware::Return<void> onVsyncPeriodTimingChanged(
+ Return<void> onVsyncPeriodTimingChanged(
hal::HWDisplayId display,
const hal::VsyncPeriodChangeTimeline& updatedTimeline) override {
mCallback->onVsyncPeriodTimingChangedReceived(mSequenceId, display, updatedTimeline);
- return android::hardware::Void();
+ return Void();
}
- android::hardware::Return<void> onSeamlessPossible(hal::HWDisplayId display) override {
+ Return<void> onSeamlessPossible(hal::HWDisplayId display) override {
mCallback->onSeamlessPossible(mSequenceId, display);
- return android::hardware::Void();
+ return Void();
}
private:
@@ -139,8 +139,6 @@
} // namespace
-namespace android {
-
HWComposer::~HWComposer() = default;
namespace impl {
@@ -295,6 +293,7 @@
hal::DisplayType::PHYSICAL);
newDisplay->setConnected(true);
displayData.hwcDisplay = std::move(newDisplay);
+ displayData.configs = displayData.hwcDisplay->getConfigs();
mPhysicalDisplayIdMap[hwcDisplayId] = displayId;
}
@@ -335,14 +334,9 @@
PhysicalDisplayId displayId) const {
RETURN_IF_INVALID_DISPLAY(displayId, {});
- const auto& displayData = mDisplayData.at(displayId);
- auto configs = displayData.hwcDisplay->getConfigs();
- if (displayData.configMap.empty()) {
- for (size_t i = 0; i < configs.size(); ++i) {
- displayData.configMap[i] = configs[i];
- }
- }
- return configs;
+ // We cache the configs when the DisplayData is created on hotplug. If the configs need to
+ // change HWC will send a hotplug event which will recreate displayData.
+ return mDisplayData.at(displayId).configs;
}
std::shared_ptr<const HWC2::Display::Config> HWComposer::getActiveConfig(
@@ -653,13 +647,13 @@
RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
auto& displayData = mDisplayData[displayId];
- if (displayData.configMap.count(configId) == 0) {
+ if (configId >= displayData.configs.size()) {
LOG_DISPLAY_ERROR(displayId, ("Invalid config " + std::to_string(configId)).c_str());
return BAD_INDEX;
}
auto error =
- displayData.hwcDisplay->setActiveConfigWithConstraints(displayData.configMap[configId],
+ displayData.hwcDisplay->setActiveConfigWithConstraints(displayData.configs[configId],
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 d8af5bf..81750de 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -183,7 +183,6 @@
virtual nsecs_t getRefreshTimestamp(PhysicalDisplayId) const = 0;
virtual bool isConnected(PhysicalDisplayId) const = 0;
- // Non-const because it can update configMap inside of mDisplayData
virtual std::vector<std::shared_ptr<const HWC2::Display::Config>> getConfigs(
PhysicalDisplayId) const = 0;
@@ -319,7 +318,6 @@
nsecs_t getRefreshTimestamp(PhysicalDisplayId) const override;
bool isConnected(PhysicalDisplayId) const override;
- // Non-const because it can update configMap inside of mDisplayData
std::vector<std::shared_ptr<const HWC2::Display::Config>> getConfigs(
PhysicalDisplayId) const override;
@@ -378,8 +376,7 @@
std::unordered_map<HWC2::Layer*, sp<Fence>> releaseFences;
buffer_handle_t outbufHandle = nullptr;
sp<Fence> outbufAcquireFence = Fence::NO_FENCE;
- mutable std::unordered_map<int32_t,
- std::shared_ptr<const HWC2::Display::Config>> configMap;
+ std::vector<std::shared_ptr<const HWC2::Display::Config>> configs;
bool validateWasSkipped;
hal::Error presentError;