| Message ID | 20170428080222.6147-1-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----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
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 > >
> -----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 > > > >
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 --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));
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(-)