[Intel-gfx] [PATCH 10/14] drm/i915: Make DP link training channel equalization DP 1.2 Spec compliant

[Mika Kahola]

On Thu, 2016-09-01 at 15:08 -0700, Manasi Navare wrote:
> Fix the number of tries in channel euqalization link training
> sequence
> according to DP 1.2 Spec. It returns a boolean depending on channel
> equalization pass or failure.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 57 ++++++++++-------
> ----------
>  drivers/gpu/drm/i915/intel_drv.h              |  1 +
>  2 files changed, 22 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 13a0341..07f0159 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -240,12 +240,12 @@ static u32 intel_dp_training_pattern(struct
> intel_dp *intel_dp)
>  	return training_pattern;
>  }
>  
> -static void
> +static bool
>  intel_dp_link_training_channel_equalization(struct intel_dp
> *intel_dp)
>  {
> -	bool channel_eq = false;
> -	int tries, cr_tries;
> +	int tries;
>  	u32 training_pattern;
> +	uint8_t link_status[DP_LINK_STATUS_SIZE];
>  
>  	training_pattern = intel_dp_training_pattern(intel_dp);
>  
> @@ -254,20 +254,11 @@
> intel_dp_link_training_channel_equalization(struct intel_dp
> *intel_dp)
>  				     training_pattern |
>  				     DP_LINK_SCRAMBLING_DISABLE)) {
>  		DRM_ERROR("failed to start channel equalization\n");
> -		return;
> +		return false;
>  	}
>  
> -	tries = 0;
> -	cr_tries = 0;
> -	channel_eq = false;
> -	for (;;) {
> -		uint8_t link_status[DP_LINK_STATUS_SIZE];
> -
> -		if (cr_tries > 5) {
> -			DRM_ERROR("failed to train DP, aborting\n");
> -			intel_dp_dump_link_status(link_status);
> -			break;
> -		}
> +	intel_dp->channel_eq_status = false;
> +	for (tries = 0; tries < 5; tries++) {
>  
>  		drm_dp_link_train_channel_eq_delay(intel_dp->dpcd);
>  		if (!intel_dp_get_link_status(intel_dp,
> link_status)) {
> @@ -278,44 +269,38 @@
> intel_dp_link_training_channel_equalization(struct intel_dp
> *intel_dp)
>  		/* Make sure clock is still ok */
>  		if (!drm_dp_clock_recovery_ok(link_status,
>  					      intel_dp->lane_count)) 
> {
> -			intel_dp_link_training_clock_recovery(intel_
> dp);
> -			intel_dp_set_link_train(intel_dp,
> -						training_pattern |
> -						DP_LINK_SCRAMBLING_D
> ISABLE);
> -			cr_tries++;
> -			continue;
> +			intel_dp_dump_link_status(link_status);
> +			DRM_DEBUG_KMS("Clock recovery check failed,
> cannot "
> +				      "continue channel
> equalization\n");
> +			break;
>  		}
This clock recovery check got me thinking. Do we really need to check
if clock recovery is still ok within a loop? Could we move this outside
the loop and return early if we have failed in clock recovery? One idea
that I have in mind is that we wouldn't need to enter in channel
equalization if we have failed with clock recovery earlier.

>  
>  		if (drm_dp_channel_eq_ok(link_status,
>  					 intel_dp->lane_count)) {
> -			channel_eq = true;
> +			intel_dp->channel_eq_status = true;
> +			DRM_DEBUG_KMS("Channel EQ done. DP Training
> "
> +				      "successful\n");
>  			break;
>  		}
>  
> -		/* Try 5 times, then try clock recovery if that
> fails */
> -		if (tries > 5) {
> -			intel_dp_link_training_clock_recovery(intel_
> dp);
> -			intel_dp_set_link_train(intel_dp,
> -						training_pattern |
> -						DP_LINK_SCRAMBLING_D
> ISABLE);
> -			tries = 0;
> -			cr_tries++;
> -			continue;
> -		}
> -
>  		/* Update training set as requested by target */
>  		intel_get_adjust_train(intel_dp, link_status);
>  		if (!intel_dp_update_link_train(intel_dp)) {
>  			DRM_ERROR("failed to update link
> training\n");
>  			break;
>  		}
> -		++tries;
> +	}
> +
> +	/* Try 5 times, else fail and try at lower BW */
> +	if (tries == 5) {
> +		intel_dp_dump_link_status(link_status);
> +		DRM_DEBUG_KMS("Channel equalization failed 5
> times\n");
>  	}
>  
>  	intel_dp_set_idle_link_train(intel_dp);
>  
> -	if (channel_eq)
> -		DRM_DEBUG_KMS("Channel EQ done. DP Training
> successful\n");
> +	return intel_dp->channel_eq_status;
> +
>  }
>  
>  void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index efcd80b..e5bc976 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -878,6 +878,7 @@ struct intel_dp {
>  	bool link_mst;
>  	bool has_audio;
>  	bool detect_done;
> +	bool channel_eq_status;
>  	enum hdmi_force_audio force_audio;
>  	bool limited_color_range;
>  	bool color_range_auto;
-- 
Mika Kahola - Intel OTC