diff options
5 files changed, 63 insertions, 34 deletions
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp index bd5efc379c..84f668d9df 100644 --- a/services/surfaceflinger/DisplayHardware/HWC2.cpp +++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp @@ -282,19 +282,28 @@ Error Display::getRequests(HWC2::DisplayRequest* outDisplayRequests, 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 @@ Error Display::setActiveConfigWithConstraints(hal::HWConfigId configId, // 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 f907061774..de044e0b76 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 @@ public: [[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 @@ public: 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 @@ private: 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 bac24c701e..cfa03397d6 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -364,15 +364,13 @@ ui::DisplayConnectionType HWComposer::getDisplayConnectionType(PhysicalDisplayId 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 a25804c909..2cff2f2929 100644 --- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp +++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp @@ -102,6 +102,29 @@ TEST_F(HWComposerTest, isHeadless) { 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 @@ TEST_F(HWComposerSetCallbackTest, loadsLayerMetadataSupport) { {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, loadsLayerMetadataSupport) { 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 @@ TEST_F(HWComposerLayerGenericMetadataTest, forwardsSupportedMetadata) { 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 @@ TEST_F(HWComposerLayerGenericMetadataTest, forwardsSupportedMetadata) { 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 7413235c19..602bdfc152 100644 --- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h +++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h @@ -53,7 +53,8 @@ public: 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, |