[Intel-gfx] [PATCH v6] drm/i915/dp: Limit link training clock recovery loop

[Marc Herbert]

Compliance aside I had a really hard time understanding the gap between 80
and 10. I mean if up to 80 tries might be needed pre-1.4, then how come 1.4
is supposed to succeed in less than 10? Confusing.

After asking Nathan and DK (thx) I suggest rephrasing the comment below with
something like this:

/* For devices pre-DP 1.4 we set the retry limit to an extremely lenient 80
  max iterations = 4 (voltage levels) x 4 (preemphasis levels) x 5 (same
  voltage retries). It should never take that many attempts (TODO: why not?)
  but we let's be lax and tolerate all kinds of sinks because the specs
  don't specify any limit (and allow infinite loops).
  On the other hand, 1.4 devices should be smart and/or compliant enough and
  the expectations are now higher; they're must succeed in less than 10 tries
  because they know how <TODO>
*/

My 2 cents - you get the idea.

BTW I'm curious how long 80 tries typically take.


On 20/07/2018 13:15, Nathan Ciobanu wrote:
> Limit the link training clock recovery loop to 10 attempts at
> LANEx_CR_DONE per DP 1.4 spec section 3.5.1.2.2 and 80 attempts for
> pre-DP 1.4 (4 voltage levels x 4 preemphasis levels x
> x 5 identical voltages tries). Some faulty USB-C MST hubs can
> cause us to get stuck in this loop indefinitely requesting something
> like:

I heard that loop was not just the hub's fault after all.

Of course no hub should be able to trigger an infinite loop anyway,
not even a hypothetical hub.


>     voltage swing: 0, pre-emphasis level: 2
>     voltage swing: 1, pre-emphasis level: 2
>     voltage swing: 0, pre-emphasis level: 3
> 
> over and over so max_vswing would never be reached,
> drm_dp_clock_recovery_ok() would never return true and voltage_tries
> would always get reset to 1. The driver sends those values to the hub
> but the hub keeps requesting new values every time.
> 

> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c

>  
> @@ -170,9 +170,20 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
>  		return false;
>  	}
>  
> +	/*
> +	 * DP 1.4 spec clock recovery retries defined but
> +	 * for devices pre-DP 1.4 we set the retry limit
> +	 * to 4 (voltage levels) x 4 (preemphasis levels) x
> +	 * x 5 (same voltage retries) = 80 (max iterations)
> +	 */
> +	if (intel_dp->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14)
> +		max_cr_tries = 10;
> +	else
> +		max_cr_tries = 80;
> +
>  	voltage_tries = 1;