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

[Pandiyan, Dhinakaran]

On Thu, 2016-08-04 at 10:46 +0300, Jani Nikula wrote:
> On Thu, 04 Aug 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan at 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 at 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.
> 

Thanks for the tip Jani, will keep that in mind. I will split this patch
when I send the next version of the series.
> > +
> >  /* intel_dp_aux_backlight.c */
> >  int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
>