Skip to content

Commit 5906db4

Browse files
Treehugger RobotAndroid (Google) Code Review
Treehugger Robot
authored and
Android (Google) Code Review
committed
Merge "SF: prerequisite refactor to remove deprecated HIDL enum" into main
2 parents 5920fd2 + 13f4858 commit 5906db4

10 files changed

Lines changed: 54 additions & 52 deletions

‎services/surfaceflinger/DisplayHardware/HWComposer.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,13 @@ bool HWComposer::hasDisplayCapability(HalDisplayId displayId, DisplayCapability
138138
}
139139

140140
std::optional<DisplayIdentificationInfo> HWComposer::onHotplug(hal::HWDisplayId hwcDisplayId,
141-
hal::Connection connection) {
142-
switch (connection) {
143-
case hal::Connection::CONNECTED:
141+
HotplugEvent event) {
142+
switch (event) {
143+
case HotplugEvent::Connected:
144144
return onHotplugConnect(hwcDisplayId);
145-
case hal::Connection::DISCONNECTED:
145+
case HotplugEvent::Disconnected:
146146
return onHotplugDisconnect(hwcDisplayId);
147-
case hal::Connection::INVALID:
147+
case HotplugEvent::LinkUnstable:
148148
return {};
149149
}
150150
}

‎services/surfaceflinger/DisplayHardware/HWComposer.h‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,12 @@ class HWComposer {
231231

232232
// Events handling ---------------------------------------------------------
233233

234-
// Returns stable display ID (and display name on connection of new or previously disconnected
235-
// display), or std::nullopt if hotplug event was ignored.
234+
enum class HotplugEvent { Connected, Disconnected, LinkUnstable };
235+
236+
// Returns the stable display ID of the display for which the hotplug event was received, or
237+
// std::nullopt if hotplug event was ignored.
236238
// This function is called from SurfaceFlinger.
237-
virtual std::optional<DisplayIdentificationInfo> onHotplug(hal::HWDisplayId,
238-
hal::Connection) = 0;
239+
virtual std::optional<DisplayIdentificationInfo> onHotplug(hal::HWDisplayId, HotplugEvent) = 0;
239240

240241
// If true we'll update the DeviceProductInfo on subsequent hotplug connected events.
241242
// TODO(b/157555476): Remove when the framework has proper support for headless mode
@@ -435,9 +436,7 @@ class HWComposer final : public android::HWComposer {
435436

436437
// Events handling ---------------------------------------------------------
437438

438-
// Returns PhysicalDisplayId (and display name on connection of new or previously disconnected
439-
// display), or std::nullopt if hotplug event was ignored.
440-
std::optional<DisplayIdentificationInfo> onHotplug(hal::HWDisplayId, hal::Connection) override;
439+
std::optional<DisplayIdentificationInfo> onHotplug(hal::HWDisplayId, HotplugEvent) override;
441440

442441
bool updatesDeviceProductInfoOnHotplugReconnect() const override;
443442

‎services/surfaceflinger/SurfaceFlinger.cpp‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@
135135
#include "DisplayDevice.h"
136136
#include "DisplayHardware/ComposerHal.h"
137137
#include "DisplayHardware/FramebufferSurface.h"
138-
#include "DisplayHardware/HWComposer.h"
139138
#include "DisplayHardware/Hal.h"
140139
#include "DisplayHardware/VirtualDisplaySurface.h"
141140
#include "DisplayRenderArea.h"
@@ -2292,12 +2291,12 @@ void SurfaceFlinger::onComposerHalVsync(hal::HWDisplayId hwcDisplayId, int64_t t
22922291
void SurfaceFlinger::onComposerHalHotplugEvent(hal::HWDisplayId hwcDisplayId,
22932292
DisplayHotplugEvent event) {
22942293
if (event == DisplayHotplugEvent::CONNECTED || event == DisplayHotplugEvent::DISCONNECTED) {
2295-
hal::Connection connection = (event == DisplayHotplugEvent::CONNECTED)
2296-
? hal::Connection::CONNECTED
2297-
: hal::Connection::DISCONNECTED;
2294+
const HWComposer::HotplugEvent hotplugEvent = event == DisplayHotplugEvent::CONNECTED
2295+
? HWComposer::HotplugEvent::Connected
2296+
: HWComposer::HotplugEvent::Disconnected;
22982297
{
22992298
std::lock_guard<std::mutex> lock(mHotplugMutex);
2300-
mPendingHotplugEvents.push_back(HotplugEvent{hwcDisplayId, connection});
2299+
mPendingHotplugEvents.push_back(HotplugEvent{hwcDisplayId, hotplugEvent});
23012300
}
23022301

23032302
if (mScheduler) {
@@ -3658,13 +3657,13 @@ bool SurfaceFlinger::configureLocked() {
36583657
events = std::move(mPendingHotplugEvents);
36593658
}
36603659

3661-
for (const auto [hwcDisplayId, connection] : events) {
3662-
if (auto info = getHwComposer().onHotplug(hwcDisplayId, connection)) {
3660+
for (const auto [hwcDisplayId, event] : events) {
3661+
if (auto info = getHwComposer().onHotplug(hwcDisplayId, event)) {
36633662
const auto displayId = info->id;
36643663
const ftl::Concat displayString("display ", displayId.value, "(HAL ID ", hwcDisplayId,
36653664
')');
36663665

3667-
if (connection == hal::Connection::CONNECTED) {
3666+
if (event == HWComposer::HotplugEvent::Connected) {
36683667
const auto activeModeIdOpt =
36693668
processHotplugConnect(displayId, hwcDisplayId, std::move(*info),
36703669
displayString.c_str());

‎services/surfaceflinger/SurfaceFlinger.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
#include "Display/VirtualDisplaySnapshot.h"
7676
#include "DisplayDevice.h"
7777
#include "DisplayHardware/HWC2.h"
78+
#include "DisplayHardware/HWComposer.h"
7879
#include "DisplayIdGenerator.h"
7980
#include "Effects/Daltonizer.h"
8081
#include "FrontEnd/DisplayInfo.h"
@@ -126,7 +127,6 @@ class FlagManager;
126127
class FpsReporter;
127128
class TunnelModeEnabledReporter;
128129
class HdrLayerInfoReporter;
129-
class HWComposer;
130130
class IGraphicBufferProducer;
131131
class Layer;
132132
class MessageBase;
@@ -1279,7 +1279,7 @@ class SurfaceFlinger : public BnSurfaceComposer,
12791279

12801280
struct HotplugEvent {
12811281
hal::HWDisplayId hwcDisplayId;
1282-
hal::Connection connection = hal::Connection::INVALID;
1282+
HWComposer::HotplugEvent event;
12831283
};
12841284

12851285
bool mIsHdcpViaNegVsync = false;

‎services/surfaceflinger/tests/unittests/DisplayModeControllerTest.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ class DisplayModeControllerTest : public testing::Test {
6767
setVsyncEnabled(kHwcDisplayId, hal::IComposerClient::Vsync::DISABLE));
6868
EXPECT_CALL(*mComposerHal, onHotplugConnect(kHwcDisplayId));
6969

70-
const auto infoOpt = mComposer->onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
70+
const auto infoOpt =
71+
mComposer->onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
7172
ASSERT_TRUE(infoOpt);
7273

7374
mDisplayId = infoOpt->id;

‎services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,10 @@ struct HwcDisplayVariant {
360360
// The HWC active configuration id
361361
static constexpr hal::HWConfigId HWC_ACTIVE_CONFIG_ID = 2001;
362362

363-
static void injectPendingHotplugEvent(DisplayTransactionTest* test, Connection connection) {
363+
static void injectPendingHotplugEvent(DisplayTransactionTest* test,
364+
HWComposer::HotplugEvent event) {
364365
test->mFlinger.mutablePendingHotplugEvents().emplace_back(
365-
TestableSurfaceFlinger::HotplugEvent{HWC_DISPLAY_ID, connection});
366+
TestableSurfaceFlinger::HotplugEvent{HWC_DISPLAY_ID, event});
366367
}
367368

368369
// Called by tests to inject a HWC display setup

‎services/surfaceflinger/tests/unittests/HWComposerTest.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ TEST_F(HWComposerTest, isHeadless) {
9494
constexpr hal::HWDisplayId kHwcDisplayId = 1;
9595
expectHotplugConnect(kHwcDisplayId);
9696

97-
const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
97+
const auto info = mHwc.onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
9898
ASSERT_TRUE(info);
9999

100100
ASSERT_FALSE(mHwc.isHeadless());
@@ -111,7 +111,7 @@ TEST_F(HWComposerTest, getDisplayConnectionType) {
111111
constexpr hal::HWDisplayId kHwcDisplayId = 1;
112112
expectHotplugConnect(kHwcDisplayId);
113113

114-
const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
114+
const auto info = mHwc.onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
115115
ASSERT_TRUE(info);
116116

117117
EXPECT_CALL(*mHal, getDisplayConnectionType(kHwcDisplayId, _))
@@ -133,7 +133,7 @@ TEST_F(HWComposerTest, getActiveMode) {
133133
constexpr hal::HWDisplayId kHwcDisplayId = 2;
134134
expectHotplugConnect(kHwcDisplayId);
135135

136-
const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
136+
const auto info = mHwc.onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
137137
ASSERT_TRUE(info);
138138

139139
{
@@ -164,7 +164,7 @@ TEST_F(HWComposerTest, getModesWithLegacyDisplayConfigs) {
164164
constexpr int32_t kMaxFrameIntervalNs = 50000000; // 20Fps
165165

166166
expectHotplugConnect(kHwcDisplayId);
167-
const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
167+
const auto info = mHwc.onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
168168
ASSERT_TRUE(info);
169169
ASSERT_TRUE(info->preferredDetailedTimingDescriptor.has_value());
170170

@@ -266,7 +266,7 @@ TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_OFF) {
266266
constexpr int32_t kMaxFrameIntervalNs = 50000000; // 20Fps
267267

268268
expectHotplugConnect(kHwcDisplayId);
269-
const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
269+
const auto info = mHwc.onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
270270
ASSERT_TRUE(info);
271271

272272
EXPECT_CALL(*mHal, isVrrSupported()).WillRepeatedly(Return(false));
@@ -364,7 +364,7 @@ TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_ON) {
364364
constexpr hal::HWConfigId kConfigId = 42;
365365
constexpr int32_t kMaxFrameIntervalNs = 50000000; // 20Fps
366366
expectHotplugConnect(kHwcDisplayId);
367-
const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
367+
const auto info = mHwc.onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
368368
ASSERT_TRUE(info);
369369

370370
EXPECT_CALL(*mHal, isVrrSupported()).WillRepeatedly(Return(true));
@@ -452,7 +452,7 @@ TEST_F(HWComposerTest, onVsync) {
452452
constexpr hal::HWDisplayId kHwcDisplayId = 1;
453453
expectHotplugConnect(kHwcDisplayId);
454454

455-
const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
455+
const auto info = mHwc.onHotplug(kHwcDisplayId, HWComposer::HotplugEvent::Connected);
456456
ASSERT_TRUE(info);
457457

458458
const auto physicalDisplayId = info->id;

‎services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayTransactionCommitTest.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ void DisplayTransactionCommitTest::processesHotplugConnectCommon() {
163163
setupCommonPreconditions<Case>();
164164

165165
// A hotplug connect event is enqueued for a display
166-
Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
166+
Case::Display::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
167167

168168
// --------------------------------------------------------------------
169169
// Call Expectations
@@ -197,7 +197,7 @@ void DisplayTransactionCommitTest::ignoresHotplugConnectCommon() {
197197
setupCommonPreconditions<Case>();
198198

199199
// A hotplug connect event is enqueued for a display
200-
Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
200+
Case::Display::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
201201

202202
// --------------------------------------------------------------------
203203
// Invocation
@@ -219,7 +219,7 @@ void DisplayTransactionCommitTest::processesHotplugDisconnectCommon() {
219219
setupCommonPreconditions<Case>();
220220

221221
// A hotplug disconnect event is enqueued for a display
222-
Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
222+
Case::Display::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Disconnected);
223223

224224
// The display is already completely set up.
225225
Case::Display::injectHwcDisplay(this);
@@ -327,9 +327,10 @@ TEST_F(DisplayTransactionCommitTest, processesHotplugConnectThenDisconnectPrimar
327327
setupCommonPreconditions<Case>();
328328

329329
// A hotplug connect event is enqueued for a display
330-
Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
330+
Case::Display::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
331331
// A hotplug disconnect event is also enqueued for the same display
332-
Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
332+
Case::Display::injectPendingHotplugEvent(this,
333+
HWComposer::HotplugEvent::Disconnected);
333334

334335
// --------------------------------------------------------------------
335336
// Call Expectations
@@ -378,9 +379,10 @@ TEST_F(DisplayTransactionCommitTest, processesHotplugDisconnectThenConnectPrimar
378379
existing.inject();
379380

380381
// A hotplug disconnect event is enqueued for a display
381-
Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
382+
Case::Display::injectPendingHotplugEvent(this,
383+
HWComposer::HotplugEvent::Disconnected);
382384
// A hotplug connect event is also enqueued for the same display
383-
Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
385+
Case::Display::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
384386

385387
// --------------------------------------------------------------------
386388
// Call Expectations

‎services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ TEST_F(HotplugTest, schedulesConfigureToProcessHotplugEvents) {
4141
const auto& pendingEvents = mFlinger.mutablePendingHotplugEvents();
4242
ASSERT_EQ(2u, pendingEvents.size());
4343
EXPECT_EQ(hwcDisplayId1, pendingEvents[0].hwcDisplayId);
44-
EXPECT_EQ(Connection::CONNECTED, pendingEvents[0].connection);
44+
EXPECT_EQ(HWComposer::HotplugEvent::Connected, pendingEvents[0].event);
4545
EXPECT_EQ(hwcDisplayId2, pendingEvents[1].hwcDisplayId);
46-
EXPECT_EQ(Connection::DISCONNECTED, pendingEvents[1].connection);
46+
EXPECT_EQ(HWComposer::HotplugEvent::Disconnected, pendingEvents[1].event);
4747
}
4848

4949
TEST_F(HotplugTest, schedulesFrameToCommitDisplayTransaction) {
@@ -64,7 +64,7 @@ TEST_F(HotplugTest, createsDisplaySnapshotsForDisplaysWithIdentificationData) {
6464
using PrimaryDisplay = InnerDisplayVariant;
6565
PrimaryDisplay::setupHwcHotplugCallExpectations(this);
6666
PrimaryDisplay::setupHwcGetActiveConfigCallExpectations(this);
67-
PrimaryDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
67+
PrimaryDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
6868

6969
// TODO(b/241286146): Remove this unnecessary call.
7070
EXPECT_CALL(*mComposer,
@@ -80,7 +80,7 @@ TEST_F(HotplugTest, createsDisplaySnapshotsForDisplaysWithIdentificationData) {
8080
using ExternalDisplay = ExternalDisplayWithIdentificationVariant;
8181
ExternalDisplay::setupHwcHotplugCallExpectations(this);
8282
ExternalDisplay::setupHwcGetActiveConfigCallExpectations(this);
83-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
83+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
8484

8585
// TODO(b/241286146): Remove this unnecessary call.
8686
EXPECT_CALL(*mComposer,
@@ -123,7 +123,7 @@ TEST_F(HotplugTest, createsDisplaySnapshotsForDisplaysWithoutIdentificationData)
123123
using PrimaryDisplay = PrimaryDisplayVariant;
124124
PrimaryDisplay::setupHwcHotplugCallExpectations(this);
125125
PrimaryDisplay::setupHwcGetActiveConfigCallExpectations(this);
126-
PrimaryDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
126+
PrimaryDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
127127

128128
// TODO(b/241286146): Remove this unnecessary call.
129129
EXPECT_CALL(*mComposer,
@@ -139,7 +139,7 @@ TEST_F(HotplugTest, createsDisplaySnapshotsForDisplaysWithoutIdentificationData)
139139
using ExternalDisplay = ExternalDisplayWithIdentificationVariant;
140140
ExternalDisplay::setupHwcHotplugCallExpectations(this);
141141
ExternalDisplay::setupHwcGetActiveConfigCallExpectations(this);
142-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
142+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
143143

144144
// TODO(b/241286146): Remove this unnecessary call.
145145
EXPECT_CALL(*mComposer,
@@ -206,15 +206,15 @@ TEST_F(HotplugTest, ignoresDuplicateDisconnection) {
206206
// A single commit should be scheduled for both configure calls.
207207
EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
208208

209-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
209+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
210210
mFlinger.configure();
211211

212212
EXPECT_TRUE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID));
213213

214214
// Disconnecting a display that was already disconnected should be a no-op.
215-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
216-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
217-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
215+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Disconnected);
216+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Disconnected);
217+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Disconnected);
218218
mFlinger.configure();
219219

220220
// The display should be scheduled for removal during the next commit. At this point, it should
@@ -249,14 +249,14 @@ TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) {
249249

250250
EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1);
251251

252-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED);
252+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Connected);
253253
mFlinger.configure();
254254

255255
// The hotplug should be rejected, so no HWComposer::DisplayData should be created.
256256
EXPECT_FALSE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID));
257257

258258
// Disconnecting a display that does not exist should be a no-op.
259-
ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
259+
ExternalDisplay::injectPendingHotplugEvent(this, HWComposer::HotplugEvent::Disconnected);
260260
mFlinger.configure();
261261

262262
EXPECT_FALSE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID));

‎services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWComposer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class HWComposer : public android::HWComposer {
8181
(PhysicalDisplayId, float, float, const Hwc2::Composer::DisplayBrightnessOptions&),
8282
(override));
8383
MOCK_METHOD(std::optional<DisplayIdentificationInfo>, onHotplug,
84-
(hal::HWDisplayId, hal::Connection), (override));
84+
(hal::HWDisplayId, HWComposer::HotplugEvent), (override));
8585
MOCK_METHOD(bool, updatesDeviceProductInfoOnHotplugReconnect, (), (const, override));
8686
MOCK_METHOD(std::optional<PhysicalDisplayId>, onVsync, (hal::HWDisplayId, int64_t));
8787
MOCK_METHOD(void, setVsyncEnabled, (PhysicalDisplayId, hal::Vsync), (override));

0 commit comments

Comments
 (0)