Re: [PATCH v2] drm/amdgpu/dc: Avoid PSR AUX WARN on unhealthy eDP link

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