diff mbox

drm/i915/glk: Fix DSI "*ERROR* ULPS is still active" messages

Message ID 20170428080222.6147-1-ander.conselvan.de.oliveira@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira April 28, 2017, 8:02 a.m. UTC 
The sequence in glk_dsi_device_ready() enters ULPS then waits until it is
*not* active to then disable it. The correct sequence according to the
spec is to enter ULPS then wait until the GLK_ULPS_NOT_ACTIVE bit is
zero, i.e., ULPS is active, and then disable ULPS.

Fixing the codition gets rid of the following spurious error messages:

[drm:glk_dsi_device_ready [i915]] *ERROR* ULPS is still active

Fixes: 4644848369c0 ("drm/i915/glk: Add MIPIIO Enable/disable sequence")
Cc: Deepak M <m.deepak@intel.com>
Cc: Madhav Chauhan <madhav.chauhan@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <drm-intel-fixes@lists.freedesktop.org>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Chauhan, Madhav April 28, 2017, 8:50 a.m. UTC | #1 
> -----Original Message-----
> From: Conselvan De Oliveira, Ander
> Sent: Friday, April 28, 2017 1:32 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> Deepak M <m.deepak@intel.com>; Chauhan, Madhav
> <madhav.chauhan@intel.com>; Nikula, Jani <jani.nikula@intel.com>; Vetter,
> Daniel <daniel.vetter@intel.com>; Jani Nikula <jani.nikula@linux.intel.com>;
> drm-intel-fixes@lists.freedesktop.org
> Subject: [PATCH] drm/i915/glk: Fix DSI "*ERROR* ULPS is still active"
> messages
> 
> The sequence in glk_dsi_device_ready() enters ULPS then waits until it is
> *not* active to then disable it. The correct sequence according to the spec is
> to enter ULPS then wait until the GLK_ULPS_NOT_ACTIVE bit is zero, i.e., ULPS
> is active, and then disable ULPS.
> 
> Fixing the codition gets rid of the following spurious error messages:

Please correct the typo error to "condition", otherwise looks good.
Thanks for catching this.

> 
> [drm:glk_dsi_device_ready [i915]] *ERROR* ULPS is still active
> 
> Fixes: 4644848369c0 ("drm/i915/glk: Add MIPIIO Enable/disable sequence")
> Cc: Deepak M <m.deepak@intel.com>
> Cc: Madhav Chauhan <madhav.chauhan@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: <drm-intel-fixes@lists.freedesktop.org>
> Signed-off-by: Ander Conselvan de Oliveira
> <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 3ffe8b1..fc0ef49 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -410,11 +410,10 @@ static void glk_dsi_device_ready(struct
> intel_encoder *encoder)
>  		val |= (ULPS_STATE_ENTER | DEVICE_READY);
>  		I915_WRITE(MIPI_DEVICE_READY(port), val);
> 
> -		/* Wait for ULPS Not active */
> +		/* Wait for ULPS active */
>  		if (intel_wait_for_register(dev_priv,
> -				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
> -				GLK_ULPS_NOT_ACTIVE, 20))
> -			DRM_ERROR("ULPS is still active\n");
> +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, 0,
> 20))
> +			DRM_ERROR("ULPS not active\n");
> 
>  		/* Exit ULPS */
>  		val = I915_READ(MIPI_DEVICE_READY(port));
> --
> 2.9.3
Ander Conselvan de Oliveira April 28, 2017, 8:57 a.m. UTC | #2 
On Fri, 2017-04-28 at 08:50 +0000, Chauhan, Madhav wrote:
> > -----Original Message-----
> > From: Conselvan De Oliveira, Ander
> > Sent: Friday, April 28, 2017 1:32 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira@intel.com>;
> > Deepak M <m.deepak@intel.com>; Chauhan, Madhav
> > <madhav.chauhan@intel.com>; Nikula, Jani <jani.nikula@intel.com>; Vetter,
> > Daniel <daniel.vetter@intel.com>; Jani Nikula <jani.nikula@linux.intel.com>;
> > drm-intel-fixes@lists.freedesktop.org
> > Subject: [PATCH] drm/i915/glk: Fix DSI "*ERROR* ULPS is still active"
> > messages
> > 
> > The sequence in glk_dsi_device_ready() enters ULPS then waits until it is
> > *not* active to then disable it. The correct sequence according to the spec is
> > to enter ULPS then wait until the GLK_ULPS_NOT_ACTIVE bit is zero, i.e., ULPS
> > is active, and then disable ULPS.
> > 
> > Fixing the codition gets rid of the following spurious error messages:
> 
> Please correct the typo error to "condition", otherwise looks good.
> Thanks for catching this.

If you upgrade the above statement to a Reviewed-by, I can fix the typo while
merging?

Ander

> 
> > 
> > [drm:glk_dsi_device_ready [i915]] *ERROR* ULPS is still active
> > 
> > Fixes: 4644848369c0 ("drm/i915/glk: Add MIPIIO Enable/disable sequence")
> > Cc: Deepak M <m.deepak@intel.com>
> > Cc: Madhav Chauhan <madhav.chauhan@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: <drm-intel-fixes@lists.freedesktop.org>
> > Signed-off-by: Ander Conselvan de Oliveira
> > <ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 3ffe8b1..fc0ef49 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -410,11 +410,10 @@ static void glk_dsi_device_ready(struct
> > intel_encoder *encoder)
> >  		val |= (ULPS_STATE_ENTER | DEVICE_READY);
> >  		I915_WRITE(MIPI_DEVICE_READY(port), val);
> > 
> > -		/* Wait for ULPS Not active */
> > +		/* Wait for ULPS active */
> >  		if (intel_wait_for_register(dev_priv,
> > -				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
> > -				GLK_ULPS_NOT_ACTIVE, 20))
> > -			DRM_ERROR("ULPS is still active\n");
> > +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, 0,
> > 20))
> > +			DRM_ERROR("ULPS not active\n");
> > 
> >  		/* Exit ULPS */
> >  		val = I915_READ(MIPI_DEVICE_READY(port));
> > --
> > 2.9.3
> 
>
Chauhan, Madhav April 28, 2017, 12:44 p.m. UTC | #3 
> -----Original Message-----

> From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]

> Sent: Friday, April 28, 2017 2:28 PM

> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-

> gfx@lists.freedesktop.org

> Cc: Deepak M <m.deepak@intel.com>; Nikula, Jani <jani.nikula@intel.com>;

> Vetter, Daniel <daniel.vetter@intel.com>; Jani Nikula

> <jani.nikula@linux.intel.com>; drm-intel-fixes@lists.freedesktop.org

> Subject: Re: [PATCH] drm/i915/glk: Fix DSI "*ERROR* ULPS is still active"

> messages

> 

> On Fri, 2017-04-28 at 08:50 +0000, Chauhan, Madhav wrote:

> > > -----Original Message-----

> > > From: Conselvan De Oliveira, Ander

> > > Sent: Friday, April 28, 2017 1:32 PM

> > > To: intel-gfx@lists.freedesktop.org

> > > Cc: Conselvan De Oliveira, Ander

> > > <ander.conselvan.de.oliveira@intel.com>;

> > > Deepak M <m.deepak@intel.com>; Chauhan, Madhav

> > > <madhav.chauhan@intel.com>; Nikula, Jani <jani.nikula@intel.com>;

> > > Vetter, Daniel <daniel.vetter@intel.com>; Jani Nikula

> > > <jani.nikula@linux.intel.com>; drm-intel-fixes@lists.freedesktop.org

> > > Subject: [PATCH] drm/i915/glk: Fix DSI "*ERROR* ULPS is still active"

> > > messages

> > >

> > > The sequence in glk_dsi_device_ready() enters ULPS then waits until

> > > it is

> > > *not* active to then disable it. The correct sequence according to

> > > the spec is to enter ULPS then wait until the GLK_ULPS_NOT_ACTIVE

> > > bit is zero, i.e., ULPS is active, and then disable ULPS.

> > >

> > > Fixing the codition gets rid of the following spurious error messages:

> >

> > Please correct the typo error to "condition", otherwise looks good.

> > Thanks for catching this.

> 

> If you upgrade the above statement to a Reviewed-by, I can fix the typo while

> merging?

> 

> Ander

> 

> >

> > >

> > > [drm:glk_dsi_device_ready [i915]] *ERROR* ULPS is still active

> > >

> > > Fixes: 4644848369c0 ("drm/i915/glk: Add MIPIIO Enable/disable

> > > sequence")

> > > Cc: Deepak M <m.deepak@intel.com>

> > > Cc: Madhav Chauhan <madhav.chauhan@intel.com>

> > > Cc: Jani Nikula <jani.nikula@intel.com>

> > > Cc: Daniel Vetter <daniel.vetter@intel.com>

> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>

> > > Cc: intel-gfx@lists.freedesktop.org

> > > Cc: <drm-intel-fixes@lists.freedesktop.org>

> > > Signed-off-by: Ander Conselvan de Oliveira

> > > <ander.conselvan.de.oliveira@intel.com>


Reviewed-by: Madhav Chauhan <madhav.chauhan@intel.com>



> > > ---

> > >  drivers/gpu/drm/i915/intel_dsi.c | 7 +++----

> > >  1 file changed, 3 insertions(+), 4 deletions(-)

> > >

> > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c

> > > b/drivers/gpu/drm/i915/intel_dsi.c

> > > index 3ffe8b1..fc0ef49 100644

> > > --- a/drivers/gpu/drm/i915/intel_dsi.c

> > > +++ b/drivers/gpu/drm/i915/intel_dsi.c

> > > @@ -410,11 +410,10 @@ static void glk_dsi_device_ready(struct

> > > intel_encoder *encoder)

> > >  		val |= (ULPS_STATE_ENTER | DEVICE_READY);

> > >  		I915_WRITE(MIPI_DEVICE_READY(port), val);

> > >

> > > -		/* Wait for ULPS Not active */

> > > +		/* Wait for ULPS active */

> > >  		if (intel_wait_for_register(dev_priv,

> > > -				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,

> > > -				GLK_ULPS_NOT_ACTIVE, 20))

> > > -			DRM_ERROR("ULPS is still active\n");

> > > +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, 0,

> > > 20))

> > > +			DRM_ERROR("ULPS not active\n");

> > >

> > >  		/* Exit ULPS */

> > >  		val = I915_READ(MIPI_DEVICE_READY(port));

> > > --

> > > 2.9.3

> >

> >
Ander Conselvan de Oliveira April 28, 2017, 2:16 p.m. UTC | #4 
On Fri, 2017-04-28 at 12:44 +0000, Chauhan, Madhav wrote:
> > -----Original Message-----
> > From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
> > Sent: Friday, April 28, 2017 2:28 PM
> > To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Deepak M <m.deepak@intel.com>; Nikula, Jani <jani.nikula@intel.com>;
> > Vetter, Daniel <daniel.vetter@intel.com>; Jani Nikula
> > <jani.nikula@linux.intel.com>; drm-intel-fixes@lists.freedesktop.org
> > Subject: Re: [PATCH] drm/i915/glk: Fix DSI "*ERROR* ULPS is still active"
> > messages
> > 
> > On Fri, 2017-04-28 at 08:50 +0000, Chauhan, Madhav wrote:
> > > > -----Original Message-----
> > > > From: Conselvan De Oliveira, Ander
> > > > Sent: Friday, April 28, 2017 1:32 PM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: Conselvan De Oliveira, Ander
> > > > <ander.conselvan.de.oliveira@intel.com>;
> > > > Deepak M <m.deepak@intel.com>; Chauhan, Madhav
> > > > <madhav.chauhan@intel.com>; Nikula, Jani <jani.nikula@intel.com>;
> > > > Vetter, Daniel <daniel.vetter@intel.com>; Jani Nikula
> > > > <jani.nikula@linux.intel.com>; drm-intel-fixes@lists.freedesktop.org
> > > > Subject: [PATCH] drm/i915/glk: Fix DSI "*ERROR* ULPS is still active"
> > > > messages
> > > > 
> > > > The sequence in glk_dsi_device_ready() enters ULPS then waits until
> > > > it is
> > > > *not* active to then disable it. The correct sequence according to
> > > > the spec is to enter ULPS then wait until the GLK_ULPS_NOT_ACTIVE
> > > > bit is zero, i.e., ULPS is active, and then disable ULPS.
> > > > 
> > > > Fixing the codition gets rid of the following spurious error messages:
> > > 
> > > Please correct the typo error to "condition", otherwise looks good.
> > > Thanks for catching this.
> > 
> > If you upgrade the above statement to a Reviewed-by, I can fix the typo while
> > merging?
> > 
> > Ander
> > 
> > > 
> > > > 
> > > > [drm:glk_dsi_device_ready [i915]] *ERROR* ULPS is still active
> > > > 
> > > > Fixes: 4644848369c0 ("drm/i915/glk: Add MIPIIO Enable/disable
> > > > sequence")
> > > > Cc: Deepak M <m.deepak@intel.com>
> > > > Cc: Madhav Chauhan <madhav.chauhan@intel.com>
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > Cc: intel-gfx@lists.freedesktop.org
> > > > Cc: <drm-intel-fixes@lists.freedesktop.org>
> > > > Signed-off-by: Ander Conselvan de Oliveira
> > > > <ander.conselvan.de.oliveira@intel.com>
> 
> Reviewed-by: Madhav Chauhan <madhav.chauhan@intel.com>

Pushed with the typo fixed. Thanks for reviewing.

Ander

> 
> 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dsi.c | 7 +++----
> > > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > > > b/drivers/gpu/drm/i915/intel_dsi.c
> > > > index 3ffe8b1..fc0ef49 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > > > @@ -410,11 +410,10 @@ static void glk_dsi_device_ready(struct
> > > > intel_encoder *encoder)
> > > >  		val |= (ULPS_STATE_ENTER | DEVICE_READY);
> > > >  		I915_WRITE(MIPI_DEVICE_READY(port), val);
> > > > 
> > > > -		/* Wait for ULPS Not active */
> > > > +		/* Wait for ULPS active */
> > > >  		if (intel_wait_for_register(dev_priv,
> > > > -				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
> > > > -				GLK_ULPS_NOT_ACTIVE, 20))
> > > > -			DRM_ERROR("ULPS is still active\n");
> > > > +				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, 0,
> > > > 20))
> > > > +			DRM_ERROR("ULPS not active\n");
> > > > 
> > > >  		/* Exit ULPS */
> > > >  		val = I915_READ(MIPI_DEVICE_READY(port));
> > > > --
> > > > 2.9.3
> > > 
> > >
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 3ffe8b1..fc0ef49 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -410,11 +410,10 @@  static void glk_dsi_device_ready(struct intel_encoder *encoder)
 		val |= (ULPS_STATE_ENTER | DEVICE_READY);
 		I915_WRITE(MIPI_DEVICE_READY(port), val);
 
-		/* Wait for ULPS Not active */
+		/* Wait for ULPS active */
 		if (intel_wait_for_register(dev_priv,
-				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
-				GLK_ULPS_NOT_ACTIVE, 20))
-			DRM_ERROR("ULPS is still active\n");
+				MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE, 0, 20))
+			DRM_ERROR("ULPS not active\n");
 
 		/* Exit ULPS */
 		val = I915_READ(MIPI_DEVICE_READY(port));