On Fri, 2016-09-02 at 14:20 +0300, Mika Kahola wrote:
> 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@xxxxxxxxx>
> > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > ---
> > 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.
>
Looks like we do. This check helps us to break out of the loop for link
rate reduction after adjusting drive setting.
> >
> > 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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
