SF: Cache DisplayConnectionType on first call
The connection type is immutable, so avoid the unnecessary AIDL call to
getDisplayConnectionType for each setActiveConfigWithConstraints.
Fixes: 323905961
Test: HWComposerTest.getDisplayConnectionType
Change-Id: I925510e24bab6bd1665128e67a98611aa9b20cd4
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index bd5efc3..84f668d 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -282,19 +282,28 @@
return Error::NONE;
}
-Error Display::getConnectionType(ui::DisplayConnectionType* outType) const {
- if (mType != DisplayType::PHYSICAL) return Error::BAD_DISPLAY;
+ftl::Expected<ui::DisplayConnectionType, hal::Error> Display::getConnectionType() const {
+ if (!mConnectionType) {
+ mConnectionType = [this]() -> decltype(mConnectionType) {
+ if (mType != DisplayType::PHYSICAL) {
+ return ftl::Unexpected(Error::BAD_DISPLAY);
+ }
- using ConnectionType = Hwc2::IComposerClient::DisplayConnectionType;
- ConnectionType connectionType;
- const auto error = static_cast<Error>(mComposer.getDisplayConnectionType(mId, &connectionType));
- if (error != Error::NONE) {
- return error;
+ using ConnectionType = Hwc2::IComposerClient::DisplayConnectionType;
+ ConnectionType connectionType;
+
+ if (const auto error = static_cast<Error>(
+ mComposer.getDisplayConnectionType(mId, &connectionType));
+ error != Error::NONE) {
+ return ftl::Unexpected(error);
+ }
+
+ return connectionType == ConnectionType::INTERNAL ? ui::DisplayConnectionType::Internal
+ : ui::DisplayConnectionType::External;
+ }();
}
- *outType = connectionType == ConnectionType::INTERNAL ? ui::DisplayConnectionType::Internal
- : ui::DisplayConnectionType::External;
- return Error::NONE;
+ return *mConnectionType;
}
bool Display::hasCapability(DisplayCapability capability) const {
@@ -420,16 +429,11 @@
// FIXME (b/319505580): At least the first config set on an external display must be
// `setActiveConfig`, so skip over the block that calls `setActiveConfigWithConstraints`
// for simplicity.
- ui::DisplayConnectionType type = ui::DisplayConnectionType::Internal;
const bool connected_display = FlagManager::getInstance().connected_display();
- if (connected_display) {
- // Do not bail out on error, since the underlying API may return UNSUPPORTED on older HWCs.
- // TODO: b/323905961 - Remove this AIDL call.
- getConnectionType(&type);
- }
if (isVsyncPeriodSwitchSupported() &&
- (!connected_display || type != ui::DisplayConnectionType::External)) {
+ (!connected_display ||
+ getConnectionType().value_opt() != ui::DisplayConnectionType::External)) {
Hwc2::IComposerClient::VsyncPeriodChangeConstraints hwc2Constraints;
hwc2Constraints.desiredTimeNanos = constraints.desiredTimeNanos;
hwc2Constraints.seamlessRequired = constraints.seamlessRequired;
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h
index f907061..de044e0 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -18,6 +18,7 @@
#include <android-base/expected.h>
#include <android-base/thread_annotations.h>
+#include <ftl/expected.h>
#include <ftl/future.h>
#include <gui/HdrMetadata.h>
#include <math/mat4.h>
@@ -120,7 +121,8 @@
[[nodiscard]] virtual hal::Error getRequests(
hal::DisplayRequest* outDisplayRequests,
std::unordered_map<Layer*, hal::LayerRequest>* outLayerRequests) = 0;
- [[nodiscard]] virtual hal::Error getConnectionType(ui::DisplayConnectionType*) const = 0;
+ [[nodiscard]] virtual ftl::Expected<ui::DisplayConnectionType, hal::Error> getConnectionType()
+ const = 0;
[[nodiscard]] virtual hal::Error supportsDoze(bool* outSupport) const = 0;
[[nodiscard]] virtual hal::Error getHdrCapabilities(
android::HdrCapabilities* outCapabilities) const = 0;
@@ -213,7 +215,7 @@
hal::Error getRequests(
hal::DisplayRequest* outDisplayRequests,
std::unordered_map<HWC2::Layer*, hal::LayerRequest>* outLayerRequests) override;
- hal::Error getConnectionType(ui::DisplayConnectionType*) const override;
+ ftl::Expected<ui::DisplayConnectionType, hal::Error> getConnectionType() const override;
hal::Error supportsDoze(bool* outSupport) const override EXCLUDES(mDisplayCapabilitiesMutex);
hal::Error getHdrCapabilities(android::HdrCapabilities* outCapabilities) const override;
hal::Error getOverlaySupport(aidl::android::hardware::graphics::composer3::OverlayProperties*
@@ -294,6 +296,8 @@
const hal::HWDisplayId mId;
hal::DisplayType mType;
+ // Cached on first call to getConnectionType.
+ mutable std::optional<ftl::Expected<ui::DisplayConnectionType, hal::Error>> mConnectionType;
bool mIsConnected = false;
using Layers = std::unordered_map<hal::HWLayerId, std::weak_ptr<HWC2::impl::Layer>>;
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index bac24c7..cfa0339 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -364,15 +364,13 @@
RETURN_IF_INVALID_DISPLAY(displayId, ui::DisplayConnectionType::Internal);
const auto& hwcDisplay = mDisplayData.at(displayId).hwcDisplay;
- ui::DisplayConnectionType type;
- const auto error = hwcDisplay->getConnectionType(&type);
-
- const auto FALLBACK_TYPE = hwcDisplay->getId() == mPrimaryHwcDisplayId
- ? ui::DisplayConnectionType::Internal
- : ui::DisplayConnectionType::External;
-
- RETURN_IF_HWC_ERROR(error, displayId, FALLBACK_TYPE);
- return type;
+ if (const auto connectionType = hwcDisplay->getConnectionType()) {
+ return connectionType.value();
+ } else {
+ LOG_HWC_ERROR(__func__, connectionType.error(), displayId);
+ return hwcDisplay->getId() == mPrimaryHwcDisplayId ? ui::DisplayConnectionType::Internal
+ : ui::DisplayConnectionType::External;
+ }
}
bool HWComposer::isVsyncPeriodSwitchSupported(PhysicalDisplayId displayId) const {
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index a25804c..2cff2f2 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -102,6 +102,29 @@
ASSERT_TRUE(mHwc.isHeadless());
}
+TEST_F(HWComposerTest, getDisplayConnectionType) {
+ // Unknown display.
+ EXPECT_EQ(mHwc.getDisplayConnectionType(PhysicalDisplayId::fromPort(0)),
+ ui::DisplayConnectionType::Internal);
+
+ constexpr hal::HWDisplayId kHwcDisplayId = 1;
+ expectHotplugConnect(kHwcDisplayId);
+
+ const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
+ ASSERT_TRUE(info);
+
+ EXPECT_CALL(*mHal, getDisplayConnectionType(kHwcDisplayId, _))
+ .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::EXTERNAL),
+ Return(V2_4::Error::NONE)));
+
+ // The first call caches the connection type.
+ EXPECT_EQ(mHwc.getDisplayConnectionType(info->id), ui::DisplayConnectionType::External);
+
+ // Subsequent calls return the cached connection type.
+ EXPECT_EQ(mHwc.getDisplayConnectionType(info->id), ui::DisplayConnectionType::External);
+ EXPECT_EQ(mHwc.getDisplayConnectionType(info->id), ui::DisplayConnectionType::External);
+}
+
TEST_F(HWComposerTest, getActiveMode) {
// Unknown display.
EXPECT_EQ(mHwc.getActiveMode(PhysicalDisplayId::fromPort(0)), ftl::Unexpected(BAD_INDEX));
@@ -449,7 +472,7 @@
{kMetadata1Name, kMetadata1Mandatory},
{kMetadata2Name, kMetadata2Mandatory},
}),
- Return(hardware::graphics::composer::V2_4::Error::NONE)));
+ Return(V2_4::Error::NONE)));
EXPECT_CALL(*mHal, getOverlaySupport(_)).WillOnce(Return(HalError::NONE));
EXPECT_CALL(*mHal, getHdrConversionCapabilities(_)).WillOnce(Return(HalError::NONE));
@@ -467,8 +490,7 @@
TEST_F(HWComposerSetCallbackTest, handlesUnsupportedCallToGetLayerGenericMetadataKeys) {
EXPECT_CALL(*mHal, getCapabilities()).WillOnce(Return(std::vector<aidl::Capability>{}));
- EXPECT_CALL(*mHal, getLayerGenericMetadataKeys(_))
- .WillOnce(Return(hardware::graphics::composer::V2_4::Error::UNSUPPORTED));
+ EXPECT_CALL(*mHal, getLayerGenericMetadataKeys(_)).WillOnce(Return(V2_4::Error::UNSUPPORTED));
EXPECT_CALL(*mHal, getOverlaySupport(_)).WillOnce(Return(HalError::UNSUPPORTED));
EXPECT_CALL(*mHal, getHdrConversionCapabilities(_)).WillOnce(Return(HalError::UNSUPPORTED));
EXPECT_CALL(*mHal, registerCallback(_));
@@ -528,7 +550,7 @@
setLayerGenericMetadata(kDisplayId, kLayerId, kLayerGenericMetadata1Name,
kLayerGenericMetadata1Mandatory,
kLayerGenericMetadata1Value))
- .WillOnce(Return(hardware::graphics::composer::V2_4::Error::NONE));
+ .WillOnce(Return(V2_4::Error::NONE));
auto result = mLayer.setLayerGenericMetadata(kLayerGenericMetadata1Name,
kLayerGenericMetadata1Mandatory,
kLayerGenericMetadata1Value);
@@ -538,7 +560,7 @@
setLayerGenericMetadata(kDisplayId, kLayerId, kLayerGenericMetadata2Name,
kLayerGenericMetadata2Mandatory,
kLayerGenericMetadata2Value))
- .WillOnce(Return(hardware::graphics::composer::V2_4::Error::UNSUPPORTED));
+ .WillOnce(Return(V2_4::Error::UNSUPPORTED));
result = mLayer.setLayerGenericMetadata(kLayerGenericMetadata2Name,
kLayerGenericMetadata2Mandatory,
kLayerGenericMetadata2Value);
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
index 7413235..602bdfc 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h
@@ -53,7 +53,8 @@
MOCK_METHOD(hal::Error, getRequests,
(hal::DisplayRequest *, (std::unordered_map<Layer *, hal::LayerRequest> *)),
(override));
- MOCK_METHOD(hal::Error, getConnectionType, (ui::DisplayConnectionType *), (const, override));
+ MOCK_METHOD((ftl::Expected<ui::DisplayConnectionType, hal::Error>), getConnectionType, (),
+ (const, override));
MOCK_METHOD(hal::Error, supportsDoze, (bool *), (const, override));
MOCK_METHOD(hal::Error, getHdrCapabilities, (android::HdrCapabilities *), (const, override));
MOCK_METHOD(hal::Error, getDisplayedContentSamplingAttributes,