[PATCH] drm/i915/dp: Fix the channel equalization failure condition during Link Training

[PATCH] drm/i915/dp: Fix the channel equalization failure condition during Link Training

[Manasi Navare]

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(-)

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dp: Fix the channel equalization failure condition during Link Training

[Patchwork]

== Series Details ==

Series: drm/i915/dp: Fix the channel equalization failure condition during Link Training
URL   : https://patchwork.freedesktop.org/series/28960/
State : success

== Summary ==

Series 28960v1 drm/i915/dp: Fix the channel equalization failure condition during Link Training
https://patchwork.freedesktop.org/api/1.0/series/28960/revisions/1/mbox/

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:453s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:435s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:361s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:557s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:520s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:519s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:611s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:447s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:421s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:424s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:503s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:602s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:599s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:523s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:477s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:479s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:485s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:439s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:477s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:405s

cdafe36f7b6f8fa0ae1a90babce68cbd8ccc98cb drm-tip: 2017y-08m-17d-21h-02m-32s UTC integration manifest
48c01fe55f07 drm/i915/dp: Fix the channel equalization failure condition during Link Training

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5432/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp: Fix the channel equalization failure condition during Link Training

[Jim Bride]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp: Fix the channel equalization failure condition during Link Training

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp: Fix the channel equalization failure condition during Link Training

[Manasi Navare]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp: Fix the channel equalization failure condition during Link Training

[Manasi Navare]

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@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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx