Re: [Intel-gfx] [PATCH 4/4] drm/i915/dp: Dump DP link status when link training stages fails

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 04 Aug 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> A full dump of link status can be handy in debugging link training
> failures. Let's add that to the debug messages when link training fails.
>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 11 +++++++++++
>  drivers/gpu/drm/i915/intel_drv.h              |  6 ++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index c0a858d..ab7d1a6 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -24,6 +24,15 @@
>  #include "intel_drv.h"
>  
>  static void
> +intel_dp_dump_link_status(const uint8_t link_status[DP_LINK_STATUS_SIZE])
> +{
> +
> +	DRM_DEBUG_KMS("ln0_1:0x%x ln2_3:0x%x align:0x%x sink:0x%x adj_req0_1:0x%x adj_req2_3:0x%x",
> +		      link_status[0], link_status[1], link_status[2],
> +		      link_status[3], link_status[4], link_status[5]);
> +}
> +
> +static void
>  intel_get_adjust_train(struct intel_dp *intel_dp,
>  		       const uint8_t link_status[DP_LINK_STATUS_SIZE])
>  {
> @@ -170,6 +179,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
>  			++loop_tries;
>  			if (loop_tries == 5) {
>  				DRM_ERROR("too many full retries, give up\n");
> +				intel_dp_dump_link_status(link_status);
>  				break;
>  			}
>  			intel_dp_reset_link_train(intel_dp,
> @@ -257,6 +267,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
>  
>  		if (cr_tries > 5) {
>  			DRM_ERROR("failed to train DP, aborting\n");
> +			intel_dp_dump_link_status(link_status);
>  			break;
>  		}
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 87069ba..549a8fd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1356,8 +1356,6 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  			     struct intel_connector *intel_connector);
>  void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  			      const struct intel_crtc_state *pipe_config);
> -void intel_dp_start_link_train(struct intel_dp *intel_dp);
> -void intel_dp_stop_link_train(struct intel_dp *intel_dp);
>  void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
> @@ -1409,6 +1407,10 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>  	return ~((1 << lane_count) - 1) & 0xf;
>  }
>  
> +/* intel_dp_link_training.c */
> +void intel_dp_start_link_train(struct intel_dp *intel_dp);
> +void intel_dp_stop_link_train(struct intel_dp *intel_dp);

Unrelated cleanup change. Should be a (standalone) separate patch, and
if it were, it could have been merged already.

Pro-tip: In general, you'll want to organize your series in a way that
allows the early patches to be merged even when the review rounds are
still in progress on the later patches. Crucial fixes first (so they can
be backported without conflicts), trivial and non-controversial things
next, and so on. You'll have a feeling of making progress, you'll have
fewer rebases and conflicts later on, and it'll be easier for the
reviewers too as the number of patches in later versions shrinks.

BR,
Jani.

> +
>  /* intel_dp_aux_backlight.c */
>  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel