On Fri, Apr 04, 2025 at 05:34:34PM -0700, Khaled Almahallawy wrote:
> During LT fallback, sometimes the sink/LTTPR reports a different lane
> count than what it reported previously. This can cause
> intel_dp->link.num_configs to have a different value than before,
> meaning that the previous link_rate/lane_config no longer exists, which
> results in the LT fallback failing prematurely and blank screen.
> 
> This issue was observed on an MTL RVP using an Asus DP2.1 monitor:
> 
> [  226.207376] AUX USBC2/DDI TC2/PHY TC2: 0xf0000 AUX -> (ret=  8) 20 1e 80 
> aa 04 00 01 03
> [  226.222334] AUX USBC2/DDI TC2/PHY TC2: 0x00000 AUX -> (ret= 15) 12 14 c4 
> 81 01 00 03 c0 02 00 06 00 00 00 82
> [  226.236312] intel_dp_link_config_init 711 num_common_rates=5 
> num_common_lanes_configs=3 num_configs=15
> 
> [  227.305515] AUX USBC2/DDI TC2/PHY TC2: 0xf0000 AUX -> (ret=  8) 20 1e 62 
> aa 01 00 01 00
> [  227.308100] AUX USBC2/DDI TC2/PHY TC2: 0x00000 AUX -> (ret= 15) 12 14 c4 
> 81 01 00 03 c0 02 00 06 00 00 00 82

It's odd that the LTTPR changes the link capability on the fly and it's
not signaled by a hotplug for instance - which should be handled then
by userspace by redecting everything and retrying the modeset.

> [  227.315679] intel_dp_link_config_init 711 num_common_rates=5 
> num_common_lanes_configs=1 num_configs=5
> 
> Fix this by choosing the lowest config (link_rate/lane_count).
> 
> Cc: Imre Deak <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Signed-off-by: Charlton Lin <[email protected]>
> Signed-off-by: Khaled Almahallawy <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 26db4c49deec..7d2f1e540699 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1199,6 +1199,15 @@ static bool reduce_link_params_in_bw_order(struct 
> intel_dp *intel_dp,
>       int i;
>  
>       i = intel_dp_link_config_index(intel_dp, crtc_state->port_clock, 
> crtc_state->lane_count);
> +
> +     if (i < 0) {
> +             /* Old config is not located. Skip to the lowest*/
> +             intel_dp_link_config_get(intel_dp, 0, &link_rate, &lane_count);
> +             *new_link_rate = link_rate;
> +             *new_lane_count = lane_count;
> +             return true;
> +     }

I don't think the changed link capability should be handled here based
on the above. Could you open a ticket with the logs, so we can see the
full sequence?

> +
>       for (i--; i >= 0; i--) {
>               intel_dp_link_config_get(intel_dp, i, &link_rate, &lane_count);
>  
> -- 
> 2.43.0
> 

Reply via email to