| Message ID | 1503025384-7379-1-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > Reviewed-by: Jim Bride <jim.bride@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Cc: Jim Bride <jim.bride@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > --- > 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
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. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Cc: Jim Bride <jim.bride@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > --- > 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
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 > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Cc: Jim Bride <jim.bride@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > --- > > 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
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);
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. Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> Cc: Jim Bride <jim.bride@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp_link_training.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)