| Message ID | 1470280061-640-5-git-send-email-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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);
On Thu, 2016-08-04 at 10:46 +0300, Jani Nikula wrote: > 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. > 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); >
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); + /* intel_dp_aux_backlight.c */ int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
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(-)