All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts
@ 2018-07-13 17:32 Nathan Ciobanu
  2018-07-13 18:25 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-07-13 21:22 ` [PATCH] " Rodrigo Vivi
  0 siblings, 2 replies; 7+ messages in thread
From: Nathan Ciobanu @ 2018-07-13 17:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: dhinakaran.pandiyan

Limit the link training clock recovery loop to 10 failed attempts at
LANEx_CR_DONE per DP 1.4 spec. Some USB-C MST hubs cause us to get
stuck in this loop on hot-plugging indefinitely as
drm_dp_clock_recovery_ok() never returns true and none of the
other conditions occur.

Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp_link_training.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 4da6e33c7fa1..66c1a70343ba 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -129,7 +129,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
 intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
 {
 	uint8_t voltage;
-	int voltage_tries, max_vswing_tries;
+	int voltage_tries, max_vswing_tries, cr_tries;
 	uint8_t link_config[2];
 	uint8_t link_bw, rate_select;
 
@@ -172,6 +172,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
 
 	voltage_tries = 1;
 	max_vswing_tries = 0;
+	cr_tries = 0;
 	for (;;) {
 		uint8_t link_status[DP_LINK_STATUS_SIZE];
 
@@ -215,6 +216,11 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
 		if (intel_dp_link_max_vswing_reached(intel_dp))
 			++max_vswing_tries;
 
+		if (cr_tries == 9) {
+			DRM_ERROR("Failed clock recovery 10 times, giving up!\n");
+			return false;
+		}
+		++cr_tries;
 	}
 }
 
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/dp: Give up link training clock recovery after 10 failed attempts
  2018-07-13 17:32 [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts Nathan Ciobanu
@ 2018-07-13 18:25 ` Patchwork
  2018-07-13 21:22 ` [PATCH] " Rodrigo Vivi
  1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-07-13 18:25 UTC (permalink / raw)
  To: Nathan Ciobanu; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: Give up link training clock recovery after 10 failed attempts
URL   : https://patchwork.freedesktop.org/series/46506/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4487 -> Patchwork_9651 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46506/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9651 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       PASS -> FAIL (fdo#103841)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      PASS -> DMESG-WARN (fdo#105998)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       PASS -> FAIL (fdo#103928)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#102614, fdo#106103)

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (46 -> 42) ==

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4487 -> Patchwork_9651

  CI_DRM_4487: 627ed020cac6a73f0a014537dac7191efbb57084 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4556: caea9c5b3aa1191c0152d7c0f22a94efca4fd048 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9651: cf5336bcc531278ec2b0046a05a474b4b630c206 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

cf5336bcc531 drm/i915/dp: Give up link training clock recovery after 10 failed attempts

== Logs ==

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts
  2018-07-13 17:32 [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts Nathan Ciobanu
  2018-07-13 18:25 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-07-13 21:22 ` Rodrigo Vivi
  2018-07-13 22:23   ` Dhinakaran Pandiyan
  2018-07-13 22:49   ` Nathan Ciobanu
  1 sibling, 2 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2018-07-13 21:22 UTC (permalink / raw)
  To: Nathan Ciobanu; +Cc: intel-gfx, dhinakaran.pandiyan

On Fri, Jul 13, 2018 at 10:32:15AM -0700, Nathan Ciobanu wrote:
> Limit the link training clock recovery loop to 10 failed attempts at
> LANEx_CR_DONE per DP 1.4 spec.

Where exactly in the spec?

> Some USB-C MST hubs cause us to get
> stuck in this loop on hot-plugging indefinitely as
> drm_dp_clock_recovery_ok() never returns true and none of the
> other conditions occur.

Although it seems really bad situation that we need to avoid...

> 
> Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 4da6e33c7fa1..66c1a70343ba 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -129,7 +129,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
>  intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
>  {
>  	uint8_t voltage;
> -	int voltage_tries, max_vswing_tries;
> +	int voltage_tries, max_vswing_tries, cr_tries;
>  	uint8_t link_config[2];
>  	uint8_t link_bw, rate_select;
>  
> @@ -172,6 +172,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
>  
>  	voltage_tries = 1;
>  	max_vswing_tries = 0;
> +	cr_tries = 0;
>  	for (;;) {
>  		uint8_t link_status[DP_LINK_STATUS_SIZE];
>  
> @@ -215,6 +216,11 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
>  		if (intel_dp_link_max_vswing_reached(intel_dp))
>  			++max_vswing_tries;
>  
> +		if (cr_tries == 9) {
> +			DRM_ERROR("Failed clock recovery 10 times, giving up!\n");
> +			return false;
> +		}
> +		++cr_tries;

If I understood correctly this is a global thing for the for(;;) right?

Shouldn't we make then like a:

- for(;;)
+ for(cr_tries = 0; cr_tries < 10; cr_tries++)
	{
	}

+ DRM_ERROR("Failed clock recovery 10 times, giving up!\n"); 
+ return false;
}

Thanks,
Rodrigo.

>  	}
>  }
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts
  2018-07-13 22:23   ` Dhinakaran Pandiyan
@ 2018-07-13 22:18     ` Manasi Navare
  2018-07-13 23:17       ` Nathan Ciobanu
  0 siblings, 1 reply; 7+ messages in thread
From: Manasi Navare @ 2018-07-13 22:18 UTC (permalink / raw)
  To: Dhinakaran Pandiyan, Nathan Ciobanu; +Cc: intel-gfx, Rodrigo Vivi

On Fri, Jul 13, 2018 at 03:23:41PM -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-07-13 at 14:22 -0700, Rodrigo Vivi wrote:
> > On Fri, Jul 13, 2018 at 10:32:15AM -0700, Nathan Ciobanu wrote:
> > > 
> > > Limit the link training clock recovery loop to 10 failed attempts
> > > at
> > > LANEx_CR_DONE per DP 1.4 spec.
> > Where exactly in the spec?

I see that this is specified in 3.5.1.2.2 Clock Recovery Sequence section
in DP 1.4 spec. Might be a good idea to mention this expliitly in the commit message.

> > 
> > > 
> > > Some USB-C MST hubs cause us to get
> > > stuck in this loop on hot-plugging indefinitely as

This is likely to occur in case of USB Type C cases where only fewer lanes
might be connected whereas the max lane count returned is still 4.
If this is the case, explicitly mentioning this is also a good idea.

Doesnt this get recovered at all after the fallback when it will eventually
try with the lower lane count?

> 
> Also include the information (the vswing toggling part) about why it is
> stuck in the loop. 
> 
> > > drm_dp_clock_recovery_ok() never returns true and none of the
> > > other conditions occur.
> > Although it seems really bad situation that we need to avoid...
> > 
> > > 
> > > 
> > > Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > index 4da6e33c7fa1..66c1a70343ba 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > @@ -129,7 +129,7 @@ static bool
> > > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> > >  intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > >  {
> > >  	uint8_t voltage;
> > > -	int voltage_tries, max_vswing_tries;
> > > +	int voltage_tries, max_vswing_tries, cr_tries;
> > >  	uint8_t link_config[2];
> > >  	uint8_t link_bw, rate_select;
> > >  
> > > @@ -172,6 +172,7 @@ static bool
> > > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> > >  
> > >  	voltage_tries = 1;
> > >  	max_vswing_tries = 0;
> > > +	cr_tries = 0;
> > >  	for (;;) {
> > >  		uint8_t link_status[DP_LINK_STATUS_SIZE];
> > >  
> > > @@ -215,6 +216,11 @@ static bool
> > > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> > >  		if (intel_dp_link_max_vswing_reached(intel_dp))
> > >  			++max_vswing_tries;
> > >  
> > > +		if (cr_tries == 9) {
> > > +			DRM_ERROR("Failed clock recovery 10 times,
> > > giving up!\n");
> > > +			return false;
> > > +		}
> > > +		++cr_tries;
> > If I understood correctly this is a global thing for the for(;;)
> > right?
> > 
> > Shouldn't we make then like a:
> > 
> > - for(;;)
> > + for(cr_tries = 0; cr_tries < 10; cr_tries++)
> > 	{
> > 	}

I think incrementing it at the end of the loop makes sense because we
are checking for the 10 total read requests for lane_cr_done
and adjust_request_lane.
But if the max voltage swing is reached before that it should exit earlier.

Manasi

> > 
> > + DRM_ERROR("Failed clock recovery 10 times, giving up!\n"); 
> > + return false;
> > }
> > 
> > Thanks,
> > Rodrigo.
> > 
> > > 
> > >  	}
> > >  }
> > >  
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts
  2018-07-13 21:22 ` [PATCH] " Rodrigo Vivi
@ 2018-07-13 22:23   ` Dhinakaran Pandiyan
  2018-07-13 22:18     ` Manasi Navare
  2018-07-13 22:49   ` Nathan Ciobanu
  1 sibling, 1 reply; 7+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-13 22:23 UTC (permalink / raw)
  To: Rodrigo Vivi, Nathan Ciobanu; +Cc: intel-gfx

On Fri, 2018-07-13 at 14:22 -0700, Rodrigo Vivi wrote:
> On Fri, Jul 13, 2018 at 10:32:15AM -0700, Nathan Ciobanu wrote:
> > 
> > Limit the link training clock recovery loop to 10 failed attempts
> > at
> > LANEx_CR_DONE per DP 1.4 spec.
> Where exactly in the spec?
> 
> > 
> > Some USB-C MST hubs cause us to get
> > stuck in this loop on hot-plugging indefinitely as

Also include the information (the vswing toggling part) about why it is
stuck in the loop. 

> > drm_dp_clock_recovery_ok() never returns true and none of the
> > other conditions occur.
> Although it seems really bad situation that we need to avoid...
> 
> > 
> > 
> > Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index 4da6e33c7fa1..66c1a70343ba 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -129,7 +129,7 @@ static bool
> > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> >  intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> >  {
> >  	uint8_t voltage;
> > -	int voltage_tries, max_vswing_tries;
> > +	int voltage_tries, max_vswing_tries, cr_tries;
> >  	uint8_t link_config[2];
> >  	uint8_t link_bw, rate_select;
> >  
> > @@ -172,6 +172,7 @@ static bool
> > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> >  
> >  	voltage_tries = 1;
> >  	max_vswing_tries = 0;
> > +	cr_tries = 0;
> >  	for (;;) {
> >  		uint8_t link_status[DP_LINK_STATUS_SIZE];
> >  
> > @@ -215,6 +216,11 @@ static bool
> > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> >  		if (intel_dp_link_max_vswing_reached(intel_dp))
> >  			++max_vswing_tries;
> >  
> > +		if (cr_tries == 9) {
> > +			DRM_ERROR("Failed clock recovery 10 times,
> > giving up!\n");
> > +			return false;
> > +		}
> > +		++cr_tries;
> If I understood correctly this is a global thing for the for(;;)
> right?
> 
> Shouldn't we make then like a:
> 
> - for(;;)
> + for(cr_tries = 0; cr_tries < 10; cr_tries++)
> 	{
> 	}
> 
> + DRM_ERROR("Failed clock recovery 10 times, giving up!\n"); 
> + return false;
> }
> 
> Thanks,
> Rodrigo.
> 
> > 
> >  	}
> >  }
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts
  2018-07-13 21:22 ` [PATCH] " Rodrigo Vivi
  2018-07-13 22:23   ` Dhinakaran Pandiyan
@ 2018-07-13 22:49   ` Nathan Ciobanu
  1 sibling, 0 replies; 7+ messages in thread
From: Nathan Ciobanu @ 2018-07-13 22:49 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, dhinakaran.pandiyan

On Fri, Jul 13, 2018 at 02:22:03PM -0700, Rodrigo Vivi wrote:
> On Fri, Jul 13, 2018 at 10:32:15AM -0700, Nathan Ciobanu wrote:
> > Limit the link training clock recovery loop to 10 failed attempts at
> > LANEx_CR_DONE per DP 1.4 spec.
> 
> Where exactly in the spec?
I'll add the section number to the commit message.
> 
> > Some USB-C MST hubs cause us to get
> > stuck in this loop on hot-plugging indefinitely as
> > drm_dp_clock_recovery_ok() never returns true and none of the
> > other conditions occur.
> 
> Although it seems really bad situation that we need to avoid...
> 
> > 
> > Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index 4da6e33c7fa1..66c1a70343ba 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -129,7 +129,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> >  intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> >  {
> >  	uint8_t voltage;
> > -	int voltage_tries, max_vswing_tries;
> > +	int voltage_tries, max_vswing_tries, cr_tries;
> >  	uint8_t link_config[2];
> >  	uint8_t link_bw, rate_select;
> >  
> > @@ -172,6 +172,7 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> >  
> >  	voltage_tries = 1;
> >  	max_vswing_tries = 0;
> > +	cr_tries = 0;
> >  	for (;;) {
> >  		uint8_t link_status[DP_LINK_STATUS_SIZE];
> >  
> > @@ -215,6 +216,11 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> >  		if (intel_dp_link_max_vswing_reached(intel_dp))
> >  			++max_vswing_tries;
> >  
> > +		if (cr_tries == 9) {
> > +			DRM_ERROR("Failed clock recovery 10 times, giving up!\n");
> > +			return false;
> > +		}
> > +		++cr_tries;
> 
> If I understood correctly this is a global thing for the for(;;) right?
> 
> Shouldn't we make then like a:
> 
> - for(;;)
> + for(cr_tries = 0; cr_tries < 10; cr_tries++)
> 	{
> 	}
> 
> + DRM_ERROR("Failed clock recovery 10 times, giving up!\n"); 
> + return false;
> }
That was my thought initially as well but I was worried that
it will not be immediately obvious why I'm returning false after
the loop - although the error message tells you why. I'll change
this. 

-Nathan
> 
> Thanks,
> Rodrigo.
> 
> >  	}
> >  }
> >  
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts
  2018-07-13 22:18     ` Manasi Navare
@ 2018-07-13 23:17       ` Nathan Ciobanu
  0 siblings, 0 replies; 7+ messages in thread
From: Nathan Ciobanu @ 2018-07-13 23:17 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx, Dhinakaran Pandiyan, Rodrigo Vivi

On Fri, Jul 13, 2018 at 03:18:32PM -0700, Manasi Navare wrote:
> On Fri, Jul 13, 2018 at 03:23:41PM -0700, Dhinakaran Pandiyan wrote:
> > On Fri, 2018-07-13 at 14:22 -0700, Rodrigo Vivi wrote:
> > > On Fri, Jul 13, 2018 at 10:32:15AM -0700, Nathan Ciobanu wrote:
> > > > 
> > > > Limit the link training clock recovery loop to 10 failed attempts
> > > > at
> > > > LANEx_CR_DONE per DP 1.4 spec.
> > > Where exactly in the spec?
> 
> I see that this is specified in 3.5.1.2.2 Clock Recovery Sequence section
> in DP 1.4 spec. Might be a good idea to mention this expliitly in the commit message.
Right, I'll do that.
> 
> > > 
> > > > 
> > > > Some USB-C MST hubs cause us to get
> > > > stuck in this loop on hot-plugging indefinitely as
> 
> This is likely to occur in case of USB Type C cases where only fewer lanes
> might be connected whereas the max lane count returned is still 4.
> If this is the case, explicitly mentioning this is also a good idea.
I'm pretty sure this device uses 4 lanes. I am getting voltage swing/pre-emphasis
values and status on all 4.
> 
> Doesnt this get recovered at all after the fallback when it will eventually
> try with the lower lane count?
Yes it does recover after the 10 attempts.
> 
> > 
> > Also include the information (the vswing toggling part) about why it is
> > stuck in the loop. 
> > 
> > > > drm_dp_clock_recovery_ok() never returns true and none of the
> > > > other conditions occur.
> > > Although it seems really bad situation that we need to avoid...
> > > 
> > > > 
> > > > 
> > > > Signed-off-by: Nathan Ciobanu <nathan.d.ciobanu@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp_link_training.c | 8 +++++++-
> > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > > b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > > index 4da6e33c7fa1..66c1a70343ba 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > > @@ -129,7 +129,7 @@ static bool
> > > > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> > > >  intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > > >  {
> > > >  	uint8_t voltage;
> > > > -	int voltage_tries, max_vswing_tries;
> > > > +	int voltage_tries, max_vswing_tries, cr_tries;
> > > >  	uint8_t link_config[2];
> > > >  	uint8_t link_bw, rate_select;
> > > >  
> > > > @@ -172,6 +172,7 @@ static bool
> > > > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> > > >  
> > > >  	voltage_tries = 1;
> > > >  	max_vswing_tries = 0;
> > > > +	cr_tries = 0;
> > > >  	for (;;) {
> > > >  		uint8_t link_status[DP_LINK_STATUS_SIZE];
> > > >  
> > > > @@ -215,6 +216,11 @@ static bool
> > > > intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> > > >  		if (intel_dp_link_max_vswing_reached(intel_dp))
> > > >  			++max_vswing_tries;
> > > >  
> > > > +		if (cr_tries == 9) {
> > > > +			DRM_ERROR("Failed clock recovery 10 times,
> > > > giving up!\n");
> > > > +			return false;
> > > > +		}
> > > > +		++cr_tries;
> > > If I understood correctly this is a global thing for the for(;;)
> > > right?
> > > 
> > > Shouldn't we make then like a:
> > > 
> > > - for(;;)
> > > + for(cr_tries = 0; cr_tries < 10; cr_tries++)
> > > 	{
> > > 	}
> 
> I think incrementing it at the end of the loop makes sense because we
> are checking for the 10 total read requests for lane_cr_done
> and adjust_request_lane.
> But if the max voltage swing is reached before that it should exit earlier.
> 
> Manasi
> 
> > > 
> > > + DRM_ERROR("Failed clock recovery 10 times, giving up!\n"); 
> > > + return false;
> > > }
> > > 
> > > Thanks,
> > > Rodrigo.
> > > 
> > > > 
> > > >  	}
> > > >  }
> > > >  
> > _______________________________________________
> > 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-07-13 23:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13 17:32 [PATCH] drm/i915/dp: Give up link training clock recovery after 10 failed attempts Nathan Ciobanu
2018-07-13 18:25 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-07-13 21:22 ` [PATCH] " Rodrigo Vivi
2018-07-13 22:23   ` Dhinakaran Pandiyan
2018-07-13 22:18     ` Manasi Navare
2018-07-13 23:17       ` Nathan Ciobanu
2018-07-13 22:49   ` Nathan Ciobanu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.