SF: Handle BAD_CONFIG for getActiveConfig
Propagate the error as NO_INIT, which SF::loadDisplayModes will handle
in a future CL.
Also, fix confusion between hal::HWDisplayId and hal::HWConfigId in SF::
loadDisplayModes.
Bug: 305813445
Test: HWComposerTest.getActiveMode
Change-Id: I2f1510ec3d883977fd77c01315e2406788f4ac90
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
index 9e35717..351cc53 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h
@@ -97,7 +97,7 @@
MOCK_CONST_METHOD1(isConnected, bool(PhysicalDisplayId));
MOCK_CONST_METHOD2(getModes,
std::vector<HWComposer::HWCDisplayMode>(PhysicalDisplayId, int32_t));
- MOCK_CONST_METHOD1(getActiveMode, std::optional<hal::HWConfigId>(PhysicalDisplayId));
+ MOCK_CONST_METHOD1(getActiveMode, ftl::Expected<hal::HWConfigId, status_t>(PhysicalDisplayId));
MOCK_CONST_METHOD1(getColorModes, std::vector<ui::ColorMode>(PhysicalDisplayId));
MOCK_METHOD3(setActiveColorMode, status_t(PhysicalDisplayId, ui::ColorMode, ui::RenderIntent));
MOCK_CONST_METHOD0(isUsingVrComposer, bool());
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index cf1c3c1..0c29569 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -343,14 +343,18 @@
return modes;
}
-std::optional<hal::HWConfigId> HWComposer::getActiveMode(PhysicalDisplayId displayId) const {
- RETURN_IF_INVALID_DISPLAY(displayId, std::nullopt);
+ftl::Expected<hal::HWConfigId, status_t> HWComposer::getActiveMode(
+ PhysicalDisplayId displayId) const {
+ RETURN_IF_INVALID_DISPLAY(displayId, ftl::Unexpected(BAD_INDEX));
const auto hwcId = *fromPhysicalDisplayId(displayId);
hal::HWConfigId configId;
const auto error = static_cast<hal::Error>(mComposer->getActiveConfig(hwcId, &configId));
+ if (error == hal::Error::BAD_CONFIG) {
+ return ftl::Unexpected(NO_INIT);
+ }
- RETURN_IF_HWC_ERROR_FOR("getActiveConfig", error, displayId, std::nullopt);
+ RETURN_IF_HWC_ERROR_FOR("getActiveConfig", error, displayId, ftl::Unexpected(UNKNOWN_ERROR));
return configId;
}
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index fb32ff4..6d1d0a5 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -25,6 +25,7 @@
#include <vector>
#include <android-base/thread_annotations.h>
+#include <ftl/expected.h>
#include <ftl/future.h>
#include <ui/DisplayIdentification.h>
#include <ui/FenceTime.h>
@@ -237,7 +238,7 @@
virtual std::vector<HWCDisplayMode> getModes(PhysicalDisplayId,
int32_t maxFrameIntervalNs) const = 0;
- virtual std::optional<hal::HWConfigId> getActiveMode(PhysicalDisplayId) const = 0;
+ virtual ftl::Expected<hal::HWConfigId, status_t> getActiveMode(PhysicalDisplayId) const = 0;
virtual std::vector<ui::ColorMode> getColorModes(PhysicalDisplayId) const = 0;
@@ -424,7 +425,7 @@
std::vector<HWCDisplayMode> getModes(PhysicalDisplayId,
int32_t maxFrameIntervalNs) const override;
- std::optional<hal::HWConfigId> getActiveMode(PhysicalDisplayId) const override;
+ ftl::Expected<hal::HWConfigId, status_t> getActiveMode(PhysicalDisplayId) const override;
std::vector<ui::ColorMode> getColorModes(PhysicalDisplayId) const override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 47e7474..82a46be 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3293,7 +3293,7 @@
std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes(
PhysicalDisplayId displayId) const {
std::vector<HWComposer::HWCDisplayMode> hwcModes;
- std::optional<hal::HWDisplayId> activeModeHwcId;
+ std::optional<hal::HWConfigId> activeModeHwcIdOpt;
int attempt = 0;
constexpr int kMaxAttempts = 3;
@@ -3301,10 +3301,10 @@
hwcModes = getHwComposer().getModes(displayId,
scheduler::RefreshRateSelector::kMinSupportedFrameRate
.getPeriodNsecs());
- activeModeHwcId = getHwComposer().getActiveMode(displayId);
+ activeModeHwcIdOpt = getHwComposer().getActiveMode(displayId).value_opt();
- const auto isActiveMode = [activeModeHwcId](const HWComposer::HWCDisplayMode& mode) {
- return mode.hwcId == activeModeHwcId;
+ const auto isActiveMode = [activeModeHwcIdOpt](const HWComposer::HWCDisplayMode& mode) {
+ return mode.hwcId == activeModeHwcIdOpt;
};
if (std::any_of(hwcModes.begin(), hwcModes.end(), isActiveMode)) {
@@ -3314,7 +3314,7 @@
if (attempt == kMaxAttempts) {
const std::string activeMode =
- activeModeHwcId ? std::to_string(*activeModeHwcId) : "unknown"s;
+ activeModeHwcIdOpt ? std::to_string(*activeModeHwcIdOpt) : "unknown"s;
ALOGE("HWC failed to report an active mode that is supported: activeModeHwcId=%s, "
"hwcModes={%s}",
activeMode.c_str(), base::Join(hwcModes, ", ").c_str());
@@ -3358,8 +3358,8 @@
// Keep IDs if modes have not changed.
const auto& modes = sameModes ? oldModes : newModes;
const DisplayModePtr activeMode =
- std::find_if(modes.begin(), modes.end(), [activeModeHwcId](const auto& pair) {
- return pair.second->getHwcId() == activeModeHwcId;
+ std::find_if(modes.begin(), modes.end(), [activeModeHwcIdOpt](const auto& pair) {
+ return pair.second->getHwcId() == activeModeHwcIdOpt;
})->second;
return {modes, activeMode};
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index a5c0657..a25804c 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -104,7 +104,7 @@
TEST_F(HWComposerTest, getActiveMode) {
// Unknown display.
- EXPECT_EQ(mHwc.getActiveMode(PhysicalDisplayId::fromPort(0)), std::nullopt);
+ EXPECT_EQ(mHwc.getActiveMode(PhysicalDisplayId::fromPort(0)), ftl::Unexpected(BAD_INDEX));
constexpr hal::HWDisplayId kHwcDisplayId = 2;
expectHotplugConnect(kHwcDisplayId);
@@ -117,14 +117,20 @@
EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
.WillOnce(Return(HalError::BAD_DISPLAY));
- EXPECT_EQ(mHwc.getActiveMode(info->id), std::nullopt);
+ EXPECT_EQ(mHwc.getActiveMode(info->id), ftl::Unexpected(UNKNOWN_ERROR));
+ }
+ {
+ EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
+ .WillOnce(Return(HalError::BAD_CONFIG));
+
+ EXPECT_EQ(mHwc.getActiveMode(info->id), ftl::Unexpected(NO_INIT));
}
{
constexpr hal::HWConfigId kConfigId = 42;
EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
.WillOnce(DoAll(SetArgPointee<1>(kConfigId), Return(HalError::NONE)));
- EXPECT_EQ(mHwc.getActiveMode(info->id), kConfigId);
+ EXPECT_EQ(mHwc.getActiveMode(info->id).value_opt(), kConfigId);
}
}