On Thu, Aug 24, 2017 at 10:15:42AM -0700, Manasi Navare wrote:
> On Thu, Aug 24, 2017 at 03:33:27PM +0300, Ville Syrjälä wrote:
> > On Thu, Aug 17, 2017 at 08:03:04PM -0700, Manasi Navare wrote:
> > > In the channel EQ retry loop, we break from the loop in case
> > > of failure to get link status or failure in clock recovery or
> > > failure to update link training. In these cases channel_eq_status
> > > is still false even though the retry loop has not reached max retries.
> > > So we need to consider this as a failure condition.
> >
> > This just prints a debug message, and each break in the loop also
> > prints some kind of debug/error message. So to me this just seems to
> > make it harder to see why things failed because we now point the finger
> > at channel EQ even when the actual error was something else.
> >
>
> Thanks for your feedback.
> The idea is to have one common place for failure so that
> failure handling like in case of PSR can be done only
> in that one place.
> The debug prints for each failure case would still give the
> exact reason for failure followed by the end result debug print
> saying that "Channel EQ failed" since any of those failures will
> result in Ch EQ fail.
>
> Manasi
>
Ville, we would need this common exit point for the Channel EQ failures
especially in case where we set the train_set_valid to false in case of
any Channel EQ failures like in the patch:
"drm/i915/edp: Be less aggressive about changing link config on eDP"
So we need to move all the failure handling to the common place like I do
here in this patch if (tries == 5 || !intel_dp->channel_eq_status)
So I am not just adding the DEBUG print but also adding this condition
that if tries ==5 or if channel_eq_status is still false.
This is the place where, train_set_valid gets set to false.
Manasi
> > >
> > > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > > Cc: Jim Bride <jim.bride@xxxxxxxxxxxxxxx>
> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> > > Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp_link_training.c | 4 ++--
> > > 1 file changed, 2 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 05907fa..3fef219 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > @@ -294,9 +294,9 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> > > }
> > >
> > > /* Try 5 times, else fail and try at lower BW */
> > > - if (tries == 5) {
> > > + if (tries == 5 || !intel_dp->channel_eq_status) {
> > > intel_dp_dump_link_status(link_status);
> > > - DRM_DEBUG_KMS("Channel equalization failed 5 times\n");
> > > + DRM_DEBUG_KMS("Channel equalization failed\n");
> > > }
> > >
> > > intel_dp_set_idle_link_train(intel_dp);
> > > --
> > > 2.1.4
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
