Skip to content

Commit 5155aa6

Browse files
Lucas BerthouAndroid (Google) Code Review
authored andcommitted
Merge "SF: Handle Link training failure in SF and DM" into main
2 parents 2d1ee0c + a41a592 commit 5155aa6

4 files changed

Lines changed: 37 additions & 10 deletions

File tree

services/surfaceflinger/DisplayHardware/HWComposer.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplug(hal::HWDisplayId
145145
case HotplugEvent::Disconnected:
146146
return onHotplugDisconnect(hwcDisplayId);
147147
case HotplugEvent::LinkUnstable:
148-
return {};
148+
return onHotplugLinkTrainingFailure(hwcDisplayId);
149149
}
150150
}
151151

@@ -1245,6 +1245,16 @@ std::optional<DisplayIdentificationInfo> HWComposer::onHotplugDisconnect(
12451245
return DisplayIdentificationInfo{.id = *displayId};
12461246
}
12471247

1248+
std::optional<DisplayIdentificationInfo> HWComposer::onHotplugLinkTrainingFailure(
1249+
hal::HWDisplayId hwcDisplayId) {
1250+
const auto displayId = toPhysicalDisplayId(hwcDisplayId);
1251+
if (!displayId) {
1252+
LOG_HWC_DISPLAY_ERROR(hwcDisplayId, "Invalid HWC display");
1253+
return {};
1254+
}
1255+
return DisplayIdentificationInfo{.id = *displayId};
1256+
}
1257+
12481258
void HWComposer::loadCapabilities() {
12491259
static_assert(sizeof(hal::Capability) == sizeof(int32_t), "Capability size has changed");
12501260
auto capabilities = mComposer->getCapabilities();

services/surfaceflinger/DisplayHardware/HWComposer.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,7 @@ class HWComposer final : public android::HWComposer {
544544

545545
std::optional<DisplayIdentificationInfo> onHotplugConnect(hal::HWDisplayId);
546546
std::optional<DisplayIdentificationInfo> onHotplugDisconnect(hal::HWDisplayId);
547+
std::optional<DisplayIdentificationInfo> onHotplugLinkTrainingFailure(hal::HWDisplayId);
547548
bool shouldIgnoreHotplugConnect(hal::HWDisplayId, uint8_t port,
548549
bool hasDisplayIdentificationData) const;
549550

services/surfaceflinger/SurfaceFlinger.cpp

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2339,9 +2339,19 @@ void SurfaceFlinger::onComposerHalHotplugEvent(hal::HWDisplayId hwcDisplayId,
23392339
return;
23402340
}
23412341

2342-
if (event == DisplayHotplugEvent::ERROR_LINK_UNSTABLE &&
2343-
!FlagManager::getInstance().display_config_error_hal()) {
2344-
return;
2342+
if (event == DisplayHotplugEvent::ERROR_LINK_UNSTABLE) {
2343+
if (!FlagManager::getInstance().display_config_error_hal()) {
2344+
return;
2345+
}
2346+
{
2347+
std::lock_guard<std::mutex> lock(mHotplugMutex);
2348+
mPendingHotplugEvents.push_back(
2349+
HotplugEvent{hwcDisplayId, HWComposer::HotplugEvent::LinkUnstable});
2350+
}
2351+
if (mScheduler) {
2352+
mScheduler->scheduleConfigure();
2353+
}
2354+
// do not return to also report the error.
23452355
}
23462356

23472357
// TODO(b/311403559): use enum type instead of int
@@ -3719,11 +3729,12 @@ bool SurfaceFlinger::configureLocked() {
37193729
const auto displayId = info->id;
37203730
const ftl::Concat displayString("display ", displayId.value, "(HAL ID ", hwcDisplayId,
37213731
')');
3722-
3723-
if (event == HWComposer::HotplugEvent::Connected) {
3732+
// TODO: b/393126541 - replace if with switch as all cases are handled.
3733+
if (event == HWComposer::HotplugEvent::Connected ||
3734+
event == HWComposer::HotplugEvent::LinkUnstable) {
37243735
const auto activeModeIdOpt =
37253736
processHotplugConnect(displayId, hwcDisplayId, std::move(*info),
3726-
displayString.c_str());
3737+
displayString.c_str(), event);
37273738
if (!activeModeIdOpt) {
37283739
mScheduler->dispatchHotplugError(
37293740
static_cast<int32_t>(DisplayHotplugEvent::ERROR_UNKNOWN));
@@ -3749,7 +3760,7 @@ bool SurfaceFlinger::configureLocked() {
37493760
LOG_ALWAYS_FATAL_IF(!snapshotOpt);
37503761

37513762
mDisplayModeController.registerDisplay(*snapshotOpt, *activeModeIdOpt, config);
3752-
} else {
3763+
} else { // event == HWComposer::HotplugEvent::Disconnected
37533764
// Unregister before destroying the DisplaySnapshot below.
37543765
mDisplayModeController.unregisterDisplay(displayId);
37553766

@@ -3764,7 +3775,8 @@ bool SurfaceFlinger::configureLocked() {
37643775
std::optional<DisplayModeId> SurfaceFlinger::processHotplugConnect(PhysicalDisplayId displayId,
37653776
hal::HWDisplayId hwcDisplayId,
37663777
DisplayIdentificationInfo&& info,
3767-
const char* displayString) {
3778+
const char* displayString,
3779+
HWComposer::HotplugEvent event) {
37683780
auto [displayModes, activeMode] = loadDisplayModes(displayId);
37693781
if (!activeMode) {
37703782
ALOGE("Failed to hotplug %s", displayString);
@@ -3799,6 +3811,9 @@ std::optional<DisplayModeId> SurfaceFlinger::processHotplugConnect(PhysicalDispl
37993811
state.physical->port = port;
38003812
ALOGI("Reconnecting %s", displayString);
38013813
return activeModeId;
3814+
} else if (event == HWComposer::HotplugEvent::LinkUnstable) {
3815+
ALOGE("Failed to reconnect unknown %s", displayString);
3816+
return std::nullopt;
38023817
}
38033818

38043819
const sp<IBinder> token = sp<BBinder>::make();

services/surfaceflinger/SurfaceFlinger.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1067,7 +1067,8 @@ class SurfaceFlinger : public BnSurfaceComposer,
10671067
// Returns the active mode ID, or nullopt on hotplug failure.
10681068
std::optional<DisplayModeId> processHotplugConnect(PhysicalDisplayId, hal::HWDisplayId,
10691069
DisplayIdentificationInfo&&,
1070-
const char* displayString)
1070+
const char* displayString,
1071+
HWComposer::HotplugEvent event)
10711072
REQUIRES(mStateLock, kMainThreadContext);
10721073
void processHotplugDisconnect(PhysicalDisplayId, const char* displayString)
10731074
REQUIRES(mStateLock, kMainThreadContext);

0 commit comments

Comments
 (0)