Re: [PATCH 10/14] drm/i915: Make DP link training channel equalization DP 1.2 Spec compliant

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Patches 1 to 10 merged on dinq last week. Thanks for patches and
reviews and sorry for the delayed update.

On Wed, Sep 7, 2016 at 12:50 AM, Mika Kahola <mika.kahola@xxxxxxxxx> wrote:
> Reviewed-by: Mika Kahola <mika.kahola@xxxxxxxxx>
>
> On Fri, 2016-09-02 at 22:05 +0300, Pandiyan, Dhinakaran wrote:
>> 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(in
>> > > tel_
>> > > dp);
>> > > -                 intel_dp_set_link_train(intel_dp,
>> > > -                                         training_pattern
>> > > |
>> > > -                                         DP_LINK_SCRAMBLI
>> > > NG_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.
> You're right we do that.
>>
>>
>> >
>> > >
>> > >
>> > >           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(in
>> > > tel_
>> > > dp);
>> > > -                 intel_dp_set_link_train(intel_dp,
>> > > -                                         training_pattern
>> > > |
>> > > -                                         DP_LINK_SCRAMBLI
>> > > NG_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;
> --
> Mika Kahola - Intel OTC
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux