* [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable @ 2014-04-10 18:04 Ben Widawsky 2014-04-10 18:12 ` Chris Wilson 2014-04-10 21:32 ` [PATCH] [v2] " Ben Widawsky 0 siblings, 2 replies; 7+ messages in thread From: Ben Widawsky @ 2014-04-10 18:04 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky, Art Runyan From: Ben Widawsky <benjamin.widawsky@linux.intel.com> This is a requirement added to the spec. This patch will present persistent corruption on the display. Cc: Art Runyan <arthur.j.runyan@intel.com> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_display.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7f02444..ebc84ee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3583,7 +3583,7 @@ void hsw_disable_ips(struct intel_crtc *crtc) return; assert_plane_enabled(dev_priv, crtc->plane); - if (IS_BROADWELL(crtc->base.dev)) { + if (IS_BROADWELL(dev)) { mutex_lock(&dev_priv->rps.hw_lock); WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); mutex_unlock(&dev_priv->rps.hw_lock); @@ -3594,6 +3594,10 @@ void hsw_disable_ips(struct intel_crtc *crtc) /* We need to wait for a vblank before we can disable the plane. */ intel_wait_for_vblank(dev, crtc->pipe); + + /* wait for pcode to finish disabling IPS, which may take up to 42ms */ + if (IS_BROADWELL(dev)) + msleep(42); } /** Loads the palette/gamma unit for the CRTC with the prepared values */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable 2014-04-10 18:04 [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable Ben Widawsky @ 2014-04-10 18:12 ` Chris Wilson 2014-04-10 18:41 ` Ben Widawsky 2014-04-10 21:32 ` [PATCH] [v2] " Ben Widawsky 1 sibling, 1 reply; 7+ messages in thread From: Chris Wilson @ 2014-04-10 18:12 UTC (permalink / raw) To: Ben Widawsky; +Cc: Ben Widawsky, Intel GFX, Art Runyan On Thu, Apr 10, 2014 at 11:04:17AM -0700, Ben Widawsky wrote: > From: Ben Widawsky <benjamin.widawsky@linux.intel.com> > > This is a requirement added to the spec. This patch will present > persistent corruption on the display. Prevent? I don't think we want to start showing corruption to the user. > Cc: Art Runyan <arthur.j.runyan@intel.com> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_display.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7f02444..ebc84ee 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3583,7 +3583,7 @@ void hsw_disable_ips(struct intel_crtc *crtc) > return; > > assert_plane_enabled(dev_priv, crtc->plane); > - if (IS_BROADWELL(crtc->base.dev)) { > + if (IS_BROADWELL(dev)) { > mutex_lock(&dev_priv->rps.hw_lock); > WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); > mutex_unlock(&dev_priv->rps.hw_lock); > @@ -3594,6 +3594,10 @@ void hsw_disable_ips(struct intel_crtc *crtc) > > /* We need to wait for a vblank before we can disable the plane. */ > intel_wait_for_vblank(dev, crtc->pipe); > + > + /* wait for pcode to finish disabling IPS, which may take up to 42ms */ > + if (IS_BROADWELL(dev)) > + msleep(42); Reading too much Douglas Adams? -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable 2014-04-10 18:12 ` Chris Wilson @ 2014-04-10 18:41 ` Ben Widawsky 2014-04-10 19:38 ` Runyan, Arthur J 0 siblings, 1 reply; 7+ messages in thread From: Ben Widawsky @ 2014-04-10 18:41 UTC (permalink / raw) To: Chris Wilson, Ben Widawsky, Intel GFX, Art Runyan On Thu, Apr 10, 2014 at 07:12:46PM +0100, Chris Wilson wrote: > On Thu, Apr 10, 2014 at 11:04:17AM -0700, Ben Widawsky wrote: > > From: Ben Widawsky <benjamin.widawsky@linux.intel.com> > > > > This is a requirement added to the spec. This patch will present > > persistent corruption on the display. s/present/prevent > > Prevent? I don't think we want to start showing corruption to the user. > > > Cc: Art Runyan <arthur.j.runyan@intel.com> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/intel_display.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 7f02444..ebc84ee 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3583,7 +3583,7 @@ void hsw_disable_ips(struct intel_crtc *crtc) > > return; > > > > assert_plane_enabled(dev_priv, crtc->plane); > > - if (IS_BROADWELL(crtc->base.dev)) { > > + if (IS_BROADWELL(dev)) { > > mutex_lock(&dev_priv->rps.hw_lock); > > WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); > > mutex_unlock(&dev_priv->rps.hw_lock); > > @@ -3594,6 +3594,10 @@ void hsw_disable_ips(struct intel_crtc *crtc) > > > > /* We need to wait for a vblank before we can disable the plane. */ > > intel_wait_for_vblank(dev, crtc->pipe); > > + > > + /* wait for pcode to finish disabling IPS, which may take up to 42ms */ > > + if (IS_BROADWELL(dev)) > > + msleep(42); > > Reading too much Douglas Adams? > -Chris > It's those damn display arTchitects -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable 2014-04-10 18:41 ` Ben Widawsky @ 2014-04-10 19:38 ` Runyan, Arthur J 0 siblings, 0 replies; 7+ messages in thread From: Runyan, Arthur J @ 2014-04-10 19:38 UTC (permalink / raw) To: Ben Widawsky, Chris Wilson, Widawsky, Benjamin, Intel GFX >-----Original Message----- >From: Ben Widawsky [mailto:benjamin.widawsky@linux.intel.com] >Sent: Thursday, April 10, 2014 11:41 AM >To: Chris Wilson; Widawsky, Benjamin; Intel GFX; Runyan, Arthur J >Subject: Re: [Intel-gfx] [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable > >On Thu, Apr 10, 2014 at 07:12:46PM +0100, Chris Wilson wrote: >> On Thu, Apr 10, 2014 at 11:04:17AM -0700, Ben Widawsky wrote: >> > From: Ben Widawsky <benjamin.widawsky@linux.intel.com> >> > >> > This is a requirement added to the spec. This patch will present >> > persistent corruption on the display. > >s/present/prevent > >> >> Prevent? I don't think we want to start showing corruption to the user. >> >> > Cc: Art Runyan <arthur.j.runyan@intel.com> >> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >> > --- >> > drivers/gpu/drm/i915/intel_display.c | 6 +++++- >> > 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> > index 7f02444..ebc84ee 100644 >> > --- a/drivers/gpu/drm/i915/intel_display.c >> > +++ b/drivers/gpu/drm/i915/intel_display.c >> > @@ -3583,7 +3583,7 @@ void hsw_disable_ips(struct intel_crtc *crtc) >> > return; >> > >> > assert_plane_enabled(dev_priv, crtc->plane); >> > - if (IS_BROADWELL(crtc->base.dev)) { >> > + if (IS_BROADWELL(dev)) { >> > mutex_lock(&dev_priv->rps.hw_lock); >> > WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, >0)); >> > mutex_unlock(&dev_priv->rps.hw_lock); >> > @@ -3594,6 +3594,10 @@ void hsw_disable_ips(struct intel_crtc *crtc) >> > >> > /* We need to wait for a vblank before we can disable the plane. */ >> > intel_wait_for_vblank(dev, crtc->pipe); >> > + >> > + /* wait for pcode to finish disabling IPS, which may take up to 42ms */ >> > + if (IS_BROADWELL(dev)) >> > + msleep(42); The added delay for broadwell should come before the wait for vblank. Also, you should check for IPS_CTL bit 31 (Enable IPS) to become 0, just in case the 42ms was a lie, or something got stuck. >> >> Reading too much Douglas Adams? >> -Chris >> > >It's those damn display arTchitects Hah.... I am an Art, but not one of those lofty architects. > >-- >Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] [v2] drm/i915/bdw: Add 42ms delay for IPS disable 2014-04-10 18:04 [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable Ben Widawsky 2014-04-10 18:12 ` Chris Wilson @ 2014-04-10 21:32 ` Ben Widawsky 2014-04-10 23:38 ` Runyan, Arthur J 1 sibling, 1 reply; 7+ messages in thread From: Ben Widawsky @ 2014-04-10 21:32 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky, Art Runyan From: Ben Widawsky <benjamin.widawsky@linux.intel.com> This is a requirement added to the spec. This patch will prevent persistent corruption on the display. v2: Make the wait before the vblank wait. (Art) Try to finish early by polling the register s/present/prevent (Chris) Cc: Art Runyan <arthur.j.runyan@intel.com> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/intel_display.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7f02444..05c60b1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3583,10 +3583,13 @@ void hsw_disable_ips(struct intel_crtc *crtc) return; assert_plane_enabled(dev_priv, crtc->plane); - if (IS_BROADWELL(crtc->base.dev)) { + if (IS_BROADWELL(dev)) { mutex_lock(&dev_priv->rps.hw_lock); WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); mutex_unlock(&dev_priv->rps.hw_lock); + /* wait for pcode to finish disabling IPS, which may take up to 42ms */ + if (wait_for((I915_READ(IPS_CTL) & IPS_ENABLE) == 0, 42)) + DRM_DEBUG_KMS("Timed out waiting for IPS disable\n"); } else { I915_WRITE(IPS_CTL, 0); POSTING_READ(IPS_CTL); -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] drm/i915/bdw: Add 42ms delay for IPS disable 2014-04-10 21:32 ` [PATCH] [v2] " Ben Widawsky @ 2014-04-10 23:38 ` Runyan, Arthur J 2014-04-11 9:12 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: Runyan, Arthur J @ 2014-04-10 23:38 UTC (permalink / raw) To: Widawsky, Benjamin, Intel GFX; +Cc: Ben Widawsky Ben explained some of the fine details of the code to me, and I'm happy. Reviewed-by: Art Runyan <arthur.j.runyan@intel.com> > >From: Ben Widawsky <benjamin.widawsky@linux.intel.com> > >This is a requirement added to the spec. This patch will prevent >persistent corruption on the display. > >v2: Make the wait before the vblank wait. (Art) >Try to finish early by polling the register >s/present/prevent (Chris) > >Cc: Art Runyan <arthur.j.runyan@intel.com> >Signed-off-by: Ben Widawsky <ben@bwidawsk.net> >--- > drivers/gpu/drm/i915/intel_display.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >index 7f02444..05c60b1 100644 >--- a/drivers/gpu/drm/i915/intel_display.c >+++ b/drivers/gpu/drm/i915/intel_display.c >@@ -3583,10 +3583,13 @@ void hsw_disable_ips(struct intel_crtc *crtc) > return; > > assert_plane_enabled(dev_priv, crtc->plane); >- if (IS_BROADWELL(crtc->base.dev)) { >+ if (IS_BROADWELL(dev)) { > mutex_lock(&dev_priv->rps.hw_lock); > WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, >0)); > mutex_unlock(&dev_priv->rps.hw_lock); >+ /* wait for pcode to finish disabling IPS, which may take up to 42ms */ >+ if (wait_for((I915_READ(IPS_CTL) & IPS_ENABLE) == 0, 42)) >+ DRM_DEBUG_KMS("Timed out waiting for IPS disable\n"); > } else { > I915_WRITE(IPS_CTL, 0); > POSTING_READ(IPS_CTL); >-- >1.9.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] drm/i915/bdw: Add 42ms delay for IPS disable 2014-04-10 23:38 ` Runyan, Arthur J @ 2014-04-11 9:12 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2014-04-11 9:12 UTC (permalink / raw) To: Runyan, Arthur J; +Cc: Ben Widawsky, Intel GFX, Widawsky, Benjamin On Thu, Apr 10, 2014 at 11:38:21PM +0000, Runyan, Arthur J wrote: > Ben explained some of the fine details of the code to me, and I'm happy. > Reviewed-by: Art Runyan <arthur.j.runyan@intel.com> > > > > >From: Ben Widawsky <benjamin.widawsky@linux.intel.com> > > > >This is a requirement added to the spec. This patch will prevent > >persistent corruption on the display. > > > >v2: Make the wait before the vblank wait. (Art) > >Try to finish early by polling the register > >s/present/prevent (Chris) > > > >Cc: Art Runyan <arthur.j.runyan@intel.com> > >Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Queued for -next, thanks for the patch. > >--- > > drivers/gpu/drm/i915/intel_display.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >index 7f02444..05c60b1 100644 > >--- a/drivers/gpu/drm/i915/intel_display.c > >+++ b/drivers/gpu/drm/i915/intel_display.c > >@@ -3583,10 +3583,13 @@ void hsw_disable_ips(struct intel_crtc *crtc) > > return; > > > > assert_plane_enabled(dev_priv, crtc->plane); > >- if (IS_BROADWELL(crtc->base.dev)) { > >+ if (IS_BROADWELL(dev)) { > > mutex_lock(&dev_priv->rps.hw_lock); > > WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, > >0)); > > mutex_unlock(&dev_priv->rps.hw_lock); > >+ /* wait for pcode to finish disabling IPS, which may take up to 42ms */ > >+ if (wait_for((I915_READ(IPS_CTL) & IPS_ENABLE) == 0, 42)) > >+ DRM_DEBUG_KMS("Timed out waiting for IPS disable\n"); I've upgraded this to an ERROR to make sure we'll get noticed if it happens. -Daniel > > } else { > > I915_WRITE(IPS_CTL, 0); > > POSTING_READ(IPS_CTL); > >-- > >1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-04-11 9:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-10 18:04 [PATCH] drm/i915/bdw: Add 42ms delay for IPS disable Ben Widawsky 2014-04-10 18:12 ` Chris Wilson 2014-04-10 18:41 ` Ben Widawsky 2014-04-10 19:38 ` Runyan, Arthur J 2014-04-10 21:32 ` [PATCH] [v2] " Ben Widawsky 2014-04-10 23:38 ` Runyan, Arthur J 2014-04-11 9:12 ` Daniel Vetter
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.