On Thu, 2016-08-04 at 10:46 +0300, Jani Nikula wrote:
> On Thu, 04 Aug 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
