Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Callers of `dpcd_set_link_settings()` ignore its return value, turning the new early aborts into a logic flaw that causes link training to fail or fall back to lower bandwidths on transient AUX errors. --
--- Patch [1]: [PATCH v2] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link --- commit 3a9a75a9b46189406373e0f0c7bc3d7ed3668c23 Author: Kean Ren <[email protected]> drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link This commit addresses a kernel warning caused by PSR AUX timeouts on an unhealthy eDP link after resume. It adds early returns to dpcd_set_link_settings() when DPCD writes fail, and adds a check for active links in edp_setup_psr(). > diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c > b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c > --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c > +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c [ ... ] > @@ -1117,15 +1117,25 @@ enum dc_status dpcd_set_link_settings( > > link->dpcd_caps.max_ln_count.bits.POST_LT_ADJ_REQ_SUPPORTED; > } > > + /* Bail out on the first DPCD write failure so callers can react and > + * subsequent operations (e.g. PSR setup) do not keep poking an > + * unhealthy AUX channel. Without this, a transient AUX/HPD glitch > + * during resume leads to a cascade of DPCD errors and ultimately a > + * WARN at dce_aux_transfer_raw() because AUX_SW_DONE never asserts. > + */ > status = core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL, > - &downspread.raw, sizeof(downspread)); > - if (status != DC_OK) > + &downspread.raw, sizeof(downspread)); > + if (status != DC_OK) { > DC_LOG_ERROR("%s:%d: core_link_write_dpcd (DP_DOWNSPREAD_CTRL) > failed\n", __func__, __LINE__); > + return status; > + } [Severity: High] By returning early here, do we cause link training to permanently fall back to lower bandwidths if a transient AUX error occurs? It looks like the callers of dpcd_set_link_settings() do not check its return value. For example, in dp_perform_8b_10b_link_training(): else dpcd_set_link_settings(link, lt_settings); if (lt_settings->lttpr_mode == LTTPR_MODE_NON_TRANSPARENT) { And similarly in dp_perform_128b_132b_link_training(): dpcd_set_link_settings(link, lt_settings); if (result == LINK_TRAINING_SUCCESS) { result = dp_perform_128b_132b_channel_eq_done_sequence(...); If a transient error occurs on the first DPCD write, dpcd_set_link_settings() now bails out early and skips critical configurations like DP_LANE_COUNT_SET and DP_LINK_BW_SET. Because the callers ignore the return value, they will blindly proceed with clock recovery on a sink that was not fully configured. Does this guarantee a CR lock failure, which triggers the driver's fallback logic to permanently downgrade the link bandwidth? Should the callers be updated to handle the error return from dpcd_set_link_settings() so they can abort or retry properly? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
