[Intel-gfx] [PATCH 2/2] drm/i915: Don't give up waiting on INVALID_MODE

[Shashank Sharma]

Our current logic to read LSPCON's current mode, stops retries and
breaks wait-loop, if it gets LSPCON_MODE_INVALID as return from the
core function. This doesn't allow us to try reading the mode again.

This patch removes this condition and allows retries reading
the currnt mode until timeout.

This also fixes/prevents some of the noise in form of debug messages
while running IGT CI test cases.

V2: rebase, added r-b
V2: changed some debug message levels from debug->error and
error->debug in lspcon_get_current_mode function.

Cc: Imre Deak 
Cc: Daniel Vetter 

Reviewed-by: Imre Deak 
Signed-off-by: Shashank Sharma 
Signed-off-by: Mahesh Kumar 
---
 drivers/gpu/drm/i915/intel_lspcon.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c 
b/drivers/gpu/drm/i915/intel_lspcon.c
index 8413a4c..0a61c0d 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -106,7 +106,7 @@ static enum drm_lspcon_mode lspcon_get_current_mode(struct 
intel_lspcon *lspcon)
struct i2c_adapter *adapter = &lspcon_to_intel_dp(lspcon)->aux.ddc;
 
if (drm_lspcon_get_mode(adapter, ¤t_mode)) {
-   DRM_ERROR("Error reading LSPCON mode\n");
+   DRM_DEBUG_KMS("Error reading LSPCON mode\n");
return DRM_LSPCON_MODE_INVALID;
}
return current_mode;
@@ -118,16 +118,15 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct 
intel_lspcon *lspcon,
enum drm_lspcon_mode current_mode;
 
current_mode = lspcon_get_current_mode(lspcon);
-   if (current_mode == mode || current_mode == DRM_LSPCON_MODE_INVALID)
+   if (current_mode == mode)
goto out;
 
DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
  lspcon_mode_name(mode));
 
-   wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode ||
-current_mode == DRM_LSPCON_MODE_INVALID, 100);
+   wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
if (current_mode != mode)
-   DRM_DEBUG_KMS("LSPCON mode hasn't settled\n");
+   DRM_ERROR("LSPCON mode hasn't settled\n");
 
 out:
DRM_DEBUG_KMS("Current LSPCON mode %s\n",
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Don't give up waiting on INVALID_MODE

[Sharma, Shashank]

Regards

Shashank


On 8/15/2017 5:51 AM, Pandiyan, Dhinakaran wrote:

On Fri, 2017-08-11 at 18:58 +0530, Shashank Sharma wrote:

Our current logic to read LSPCON's current mode, stops retries and
breaks wait-loop, if it gets LSPCON_MODE_INVALID as return from the
core function. This doesn't allow us to try reading the mode again.

This patch removes this condition and allows retries reading
the currnt mode until timeout.

This also fixes/prevents some of the noise in form of debug messages
while running IGT CI test cases.

Cc: Imre Deak 
Cc: Daniel Vetter 
Signed-off-by: Shashank Sharma 
Signed-off-by: Mahesh Kumar 
---
  drivers/gpu/drm/i915/intel_lspcon.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c 
b/drivers/gpu/drm/i915/intel_lspcon.c
index 8413a4c..e64a336 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -118,14 +118,13 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct 
intel_lspcon *lspcon,
enum drm_lspcon_mode current_mode;
  
  	current_mode = lspcon_get_current_mode(lspcon);

-   if (current_mode == mode || current_mode == DRM_LSPCON_MODE_INVALID)
+   if (current_mode == mode)
goto out;
  
  	DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",

  lspcon_mode_name(mode));
  
-	wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode ||

-current_mode == DRM_LSPCON_MODE_INVALID, 100);
+   wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);

Doesn't this do the job of retries that you are adding in patch 1/2 ?
I think Imre has responded on this comment already, and explained better 
than me :)

- Shashank

if (current_mode != mode)
DRM_DEBUG_KMS("LSPCON mode hasn't settled\n");
  


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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Don't give up waiting on INVALID_MODE

[Imre Deak]

On Tue, Aug 15, 2017 at 12:21:14AM +, Pandiyan, Dhinakaran wrote:
> On Fri, 2017-08-11 at 18:58 +0530, Shashank Sharma wrote:
> > Our current logic to read LSPCON's current mode, stops retries and
> > breaks wait-loop, if it gets LSPCON_MODE_INVALID as return from the
> > core function. This doesn't allow us to try reading the mode again.
> > 
> > This patch removes this condition and allows retries reading
> > the currnt mode until timeout.
> > 
> > This also fixes/prevents some of the noise in form of debug messages
> > while running IGT CI test cases.
> > 
> > Cc: Imre Deak 
> > Cc: Daniel Vetter 
> > Signed-off-by: Shashank Sharma 
> > Signed-off-by: Mahesh Kumar 
> > ---
> >  drivers/gpu/drm/i915/intel_lspcon.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lspcon.c 
> > b/drivers/gpu/drm/i915/intel_lspcon.c
> > index 8413a4c..e64a336 100644
> > --- a/drivers/gpu/drm/i915/intel_lspcon.c
> > +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> > @@ -118,14 +118,13 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct 
> > intel_lspcon *lspcon,
> > enum drm_lspcon_mode current_mode;
> >  
> > current_mode = lspcon_get_current_mode(lspcon);
> > -   if (current_mode == mode || current_mode == DRM_LSPCON_MODE_INVALID)
> > +   if (current_mode == mode)
> > goto out;
> >  
> > DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
> >   lspcon_mode_name(mode));
> >  
> > -   wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode ||
> > -current_mode == DRM_LSPCON_MODE_INVALID, 100);
> > +   wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
> 
> Doesn't this do the job of retries that you are adding in patch 1/2 ?

No that's a workaround for a different issue, where the adaptor is
initially in LS mode, but switches on its own to PCON mode after some
delay.

> 
> > if (current_mode != mode)
> > DRM_DEBUG_KMS("LSPCON mode hasn't settled\n");
> >  
> ___
> 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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Don't give up waiting on INVALID_MODE

[Imre Deak]

On Fri, Aug 11, 2017 at 06:58:27PM +0530, Shashank Sharma wrote:
> Our current logic to read LSPCON's current mode, stops retries and
> breaks wait-loop, if it gets LSPCON_MODE_INVALID as return from the
> core function. This doesn't allow us to try reading the mode again.
> 
> This patch removes this condition and allows retries reading
> the currnt mode until timeout.
> 
> This also fixes/prevents some of the noise in form of debug messages
> while running IGT CI test cases.
> 
> Cc: Imre Deak 
> Cc: Daniel Vetter 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Mahesh Kumar 

Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/intel_lspcon.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c 
> b/drivers/gpu/drm/i915/intel_lspcon.c
> index 8413a4c..e64a336 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -118,14 +118,13 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct 
> intel_lspcon *lspcon,
>   enum drm_lspcon_mode current_mode;
>  
>   current_mode = lspcon_get_current_mode(lspcon);
> - if (current_mode == mode || current_mode == DRM_LSPCON_MODE_INVALID)
> + if (current_mode == mode)
>   goto out;
>  
>   DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
> lspcon_mode_name(mode));
>  
> - wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode ||
> -  current_mode == DRM_LSPCON_MODE_INVALID, 100);
> + wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
>   if (current_mode != mode)
>   DRM_DEBUG_KMS("LSPCON mode hasn't settled\n");
>  
> -- 
> 2.7.4
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Don't give up waiting on INVALID_MODE

[Pandiyan, Dhinakaran]

On Fri, 2017-08-11 at 18:58 +0530, Shashank Sharma wrote:
> Our current logic to read LSPCON's current mode, stops retries and
> breaks wait-loop, if it gets LSPCON_MODE_INVALID as return from the
> core function. This doesn't allow us to try reading the mode again.
> 
> This patch removes this condition and allows retries reading
> the currnt mode until timeout.
> 
> This also fixes/prevents some of the noise in form of debug messages
> while running IGT CI test cases.
> 
> Cc: Imre Deak 
> Cc: Daniel Vetter 
> Signed-off-by: Shashank Sharma 
> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/intel_lspcon.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c 
> b/drivers/gpu/drm/i915/intel_lspcon.c
> index 8413a4c..e64a336 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -118,14 +118,13 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct 
> intel_lspcon *lspcon,
>   enum drm_lspcon_mode current_mode;
>  
>   current_mode = lspcon_get_current_mode(lspcon);
> - if (current_mode == mode || current_mode == DRM_LSPCON_MODE_INVALID)
> + if (current_mode == mode)
>   goto out;
>  
>   DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
> lspcon_mode_name(mode));
>  
> - wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode ||
> -  current_mode == DRM_LSPCON_MODE_INVALID, 100);
> + wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);

Doesn't this do the job of retries that you are adding in patch 1/2 ?

>   if (current_mode != mode)
>   DRM_DEBUG_KMS("LSPCON mode hasn't settled\n");
>  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915: Don't give up waiting on INVALID_MODE

[Shashank Sharma]

Our current logic to read LSPCON's current mode, stops retries and
breaks wait-loop, if it gets LSPCON_MODE_INVALID as return from the
core function. This doesn't allow us to try reading the mode again.

This patch removes this condition and allows retries reading
the currnt mode until timeout.

This also fixes/prevents some of the noise in form of debug messages
while running IGT CI test cases.

Cc: Imre Deak 
Cc: Daniel Vetter 
Signed-off-by: Shashank Sharma 
Signed-off-by: Mahesh Kumar 
---
 drivers/gpu/drm/i915/intel_lspcon.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lspcon.c 
b/drivers/gpu/drm/i915/intel_lspcon.c
index 8413a4c..e64a336 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -118,14 +118,13 @@ static enum drm_lspcon_mode lspcon_wait_mode(struct 
intel_lspcon *lspcon,
enum drm_lspcon_mode current_mode;
 
current_mode = lspcon_get_current_mode(lspcon);
-   if (current_mode == mode || current_mode == DRM_LSPCON_MODE_INVALID)
+   if (current_mode == mode)
goto out;
 
DRM_DEBUG_KMS("Waiting for LSPCON mode %s to settle\n",
  lspcon_mode_name(mode));
 
-   wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode ||
-current_mode == DRM_LSPCON_MODE_INVALID, 100);
+   wait_for((current_mode = lspcon_get_current_mode(lspcon)) == mode, 100);
if (current_mode != mode)
DRM_DEBUG_KMS("LSPCON mode hasn't settled\n");
 
-- 
2.7.4

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