Re: [Intel-gfx] [PATCH v4 1/6] drm/i915/dp_link_training: Add a final failing state to link training fallback

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Aug 24, 2023 at 04:50:16PM -0400, Gil Dekel wrote:
> Instead of silently giving up when all link-training fallback values are
> exhausted, this patch modifies the fallback's failure branch to reduces
> both max_link_lane_count and max_link_rate to zero (0) and continues to
> emit uevents until userspace stops attempting to modeset.
> 
> By doing so, we ensure the failing connector, which is in
> link-status=Bad, has all its modes pruned (due to effectively having a
> bandwidth of 0Gbps).
> 
> It is then the userspace's responsibility to ignore connectors with no
> modes, even if they are marked as connected.
> 
> Signed-off-by: Gil Dekel <gildekel@chromium.org>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7067ee3a4bd3..2152ddbab557 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -276,8 +276,12 @@ static int intel_dp_common_len_rate_limit(const struct intel_dp *intel_dp,
> 
>  static int intel_dp_common_rate(struct intel_dp *intel_dp, int index)
>  {
> +	/* This occurs when max link rate drops to 0 via link training fallback*/
> +	if (index < 0)
> +		return 0;

I'm not comfortable with handling negative input as normal here
and no warning or msg.
Maybe I'm missing the cases in which this will get to negative and
it would be acceptable? In this case probably better to improve the
commit message.

> +
>  	if (drm_WARN_ON(&dp_to_i915(intel_dp)->drm,
> -			index < 0 || index >= intel_dp->num_common_rates))
> +			index >= intel_dp->num_common_rates))
>  		return 162000;
> 
>  	return intel_dp->common_rates[index];
> @@ -318,6 +322,9 @@ static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
>  int intel_dp_max_lane_count(struct intel_dp *intel_dp)
>  {
>  	switch (intel_dp->max_link_lane_count) {
> +	/* This occurs when max link lane count drops to 0 via link training fallback*/
> +	case 0:
> +		return 0;
>  	case 1:
>  	case 2:
>  	case 4:
> @@ -672,7 +679,14 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  		intel_dp->max_link_lane_count = lane_count >> 1;
>  	} else {
>  		drm_err(&i915->drm, "Link Training Unsuccessful\n");
> -		return -1;
> +		/*
> +		 * Ensure all of the connector's modes are pruned in the next
> +		 * probe by effectively reducing its bandwidth to 0 so userspace
> +		 * can ignore it within the next modeset attempt.
> +		 */
> +		intel_dp->max_link_rate = 0;
> +		intel_dp->max_link_lane_count = 0;
> +		return 0;
>  	}
> 
>  	return 0;
> --
> Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics