Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: fix overlay on i830M
@ 2012-10-22 10:44 Daniel Vetter
  2012-10-22 10:54 ` Chris Wilson
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2012-10-22 10:44 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, stable

The overlay on the i830M has a peculiar failure mode: It works the
first time around after boot-up, but consistenly hangs the second time
it's used.

Chris Wilson has dug out a nice errata:

"1.5.12 Clock Gating Disable for Display Register
Address Offset:	06200h–06203h

"Bit 3
Ovrunit Clock Gating Disable.
0 = Clock gating controlled by unit enabling logic
1 = Disable clock gating function
DevALM Errata ALM049: Overlay Clock Gating Must be Disabled:  Overlay
& L2 Cache clock gating must be disabled in order to prevent device
hangs when turning off overlay.SW must turn off Ovrunit clock gating
(6200h) and L2 Cache clock gating (C8h)."

Now I've nowhere found that 0xc8 register and hence couldn't apply the
l2 cache workaround. But I've remembered that part of the magic that
the OVERLAY_ON/OFF commands are supposed to do is to rearrange cache
allocations so that the overlay scaler has some scratch space.

And while pondering how that could explain the hang the 2nd time we
enable the overlay, I've remembered that the old ums overlay code did
_not_ issue the OVERLAY_OFF cmd.

And indeed, disabling the OFF cmd results in the overlay working
flawlessly, so I guess we can workaround the lack of the above
workaround by simply never disabling the overlay engine once it's
enabled.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47827
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_overlay.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 555912f..0604af2 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -342,9 +342,15 @@ static int intel_overlay_off(struct intel_overlay *overlay)
 	intel_ring_emit(ring, flip_addr);
 	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
 	/* turn overlay off */
-	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
-	intel_ring_emit(ring, flip_addr);
-	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+	if (IS_I830(dev)) {
+		intel_ring_emit(ring, MI_NOOP);
+		intel_ring_emit(ring, MI_NOOP);
+		intel_ring_emit(ring, MI_NOOP);
+	} else {
+		intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
+		intel_ring_emit(ring, flip_addr);
+		intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+	}
 	intel_ring_advance(ring);
 
 	return intel_overlay_do_wait_request(overlay, intel_overlay_off_tail);
-- 
1.7.11.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: fix overlay on i830M
  2012-10-22 10:44 [PATCH] drm/i915: fix overlay on i830M Daniel Vetter
@ 2012-10-22 10:54 ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2012-10-22 10:54 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, stable

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On Mon, 22 Oct 2012 12:44:18 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> The overlay on the i830M has a peculiar failure mode: It works the
> first time around after boot-up, but consistenly hangs the second time
> it's used.
> 
> Chris Wilson has dug out a nice errata:
> 
> "1.5.12 Clock Gating Disable for Display Register
> Address Offset:	06200h–06203h
> 
> "Bit 3
> Ovrunit Clock Gating Disable.
> 0 = Clock gating controlled by unit enabling logic
> 1 = Disable clock gating function
> DevALM Errata ALM049: Overlay Clock Gating Must be Disabled:  Overlay
> & L2 Cache clock gating must be disabled in order to prevent device
> hangs when turning off overlay.SW must turn off Ovrunit clock gating
> (6200h) and L2 Cache clock gating (C8h)."
> 
> Now I've nowhere found that 0xc8 register and hence couldn't apply the
> l2 cache workaround. But I've remembered that part of the magic that
> the OVERLAY_ON/OFF commands are supposed to do is to rearrange cache
> allocations so that the overlay scaler has some scratch space.
> 
> And while pondering how that could explain the hang the 2nd time we
> enable the overlay, I've remembered that the old ums overlay code did
> _not_ issue the OVERLAY_OFF cmd.
> 
> And indeed, disabling the OFF cmd results in the overlay working
> flawlessly, so I guess we can workaround the lack of the above
> workaround by simply never disabling the overlay engine once it's
> enabled.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47827
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

That looks fun, and indeed will probably work. So if we never issue the
OFF command, do we still need i830_init_clock_gating()?

And if we do still need i830_init_clock_gating() wouldn't it be better
applied as a quirk to intel_overlay_on()?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] drm/i915: fix overlay on i830M
@ 2012-10-22 10:55 Daniel Vetter
  2012-10-23  9:12 ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2012-10-22 10:55 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, stable

The overlay on the i830M has a peculiar failure mode: It works the
first time around after boot-up, but consistenly hangs the second time
it's used.

Chris Wilson has dug out a nice errata:

"1.5.12 Clock Gating Disable for Display Register
Address Offset:	06200h–06203h

"Bit 3
Ovrunit Clock Gating Disable.
0 = Clock gating controlled by unit enabling logic
1 = Disable clock gating function
DevALM Errata ALM049: Overlay Clock Gating Must be Disabled:  Overlay
& L2 Cache clock gating must be disabled in order to prevent device
hangs when turning off overlay.SW must turn off Ovrunit clock gating
(6200h) and L2 Cache clock gating (C8h)."

Now I've nowhere found that 0xc8 register and hence couldn't apply the
l2 cache workaround. But I've remembered that part of the magic that
the OVERLAY_ON/OFF commands are supposed to do is to rearrange cache
allocations so that the overlay scaler has some scratch space.

And while pondering how that could explain the hang the 2nd time we
enable the overlay, I've remembered that the old ums overlay code did
_not_ issue the OVERLAY_OFF cmd.

And indeed, disabling the OFF cmd results in the overlay working
flawlessly, so I guess we can workaround the lack of the above
workaround by simply never disabling the overlay engine once it's
enabled.

v2: Add a comment in the code.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47827
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_overlay.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 555912f..e3b095f 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -342,9 +342,17 @@ static int intel_overlay_off(struct intel_overlay *overlay)
 	intel_ring_emit(ring, flip_addr);
 	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
 	/* turn overlay off */
-	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
-	intel_ring_emit(ring, flip_addr);
-	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+	if (IS_I830(dev)) {
+		/* Workaround: Don't disable the overlay fully, since otherwise
+		 * it dies on the next OVERLAY_ON cmd. */
+		intel_ring_emit(ring, MI_NOOP);
+		intel_ring_emit(ring, MI_NOOP);
+		intel_ring_emit(ring, MI_NOOP);
+	} else {
+		intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
+		intel_ring_emit(ring, flip_addr);
+		intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+	}
 	intel_ring_advance(ring);
 
 	return intel_overlay_do_wait_request(overlay, intel_overlay_off_tail);
-- 
1.7.11.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: fix overlay on i830M
  2012-10-22 10:55 Daniel Vetter
@ 2012-10-23  9:12 ` Daniel Vetter
  2012-10-23  9:56   ` Chris Wilson
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2012-10-23  9:12 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, stable

On Mon, Oct 22, 2012 at 12:55:55PM +0200, Daniel Vetter wrote:
> The overlay on the i830M has a peculiar failure mode: It works the
> first time around after boot-up, but consistenly hangs the second time
> it's used.
> 
> Chris Wilson has dug out a nice errata:
> 
> "1.5.12 Clock Gating Disable for Display Register
> Address Offset:	06200h–06203h
> 
> "Bit 3
> Ovrunit Clock Gating Disable.
> 0 = Clock gating controlled by unit enabling logic
> 1 = Disable clock gating function
> DevALM Errata ALM049: Overlay Clock Gating Must be Disabled:  Overlay
> & L2 Cache clock gating must be disabled in order to prevent device
> hangs when turning off overlay.SW must turn off Ovrunit clock gating
> (6200h) and L2 Cache clock gating (C8h)."
> 
> Now I've nowhere found that 0xc8 register and hence couldn't apply the
> l2 cache workaround. But I've remembered that part of the magic that
> the OVERLAY_ON/OFF commands are supposed to do is to rearrange cache
> allocations so that the overlay scaler has some scratch space.
> 
> And while pondering how that could explain the hang the 2nd time we
> enable the overlay, I've remembered that the old ums overlay code did
> _not_ issue the OVERLAY_OFF cmd.
> 
> And indeed, disabling the OFF cmd results in the overlay working
> flawlessly, so I guess we can workaround the lack of the above
> workaround by simply never disabling the overlay engine once it's
> enabled.
> 
> v2: Add a comment in the code.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47827
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Tested-by: Rhys <rhyspuk@gmail.com>

> ---
>  drivers/gpu/drm/i915/intel_overlay.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index 555912f..e3b095f 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -342,9 +342,17 @@ static int intel_overlay_off(struct intel_overlay *overlay)
>  	intel_ring_emit(ring, flip_addr);
>  	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
>  	/* turn overlay off */
> -	intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
> -	intel_ring_emit(ring, flip_addr);
> -	intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
> +	if (IS_I830(dev)) {
> +		/* Workaround: Don't disable the overlay fully, since otherwise
> +		 * it dies on the next OVERLAY_ON cmd. */
> +		intel_ring_emit(ring, MI_NOOP);
> +		intel_ring_emit(ring, MI_NOOP);
> +		intel_ring_emit(ring, MI_NOOP);
> +	} else {
> +		intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
> +		intel_ring_emit(ring, flip_addr);
> +		intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
> +	}
>  	intel_ring_advance(ring);
>  
>  	return intel_overlay_do_wait_request(overlay, intel_overlay_off_tail);
> -- 
> 1.7.11.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: fix overlay on i830M
  2012-10-23  9:12 ` Daniel Vetter
@ 2012-10-23  9:56   ` Chris Wilson
  2012-10-23 10:59     ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2012-10-23  9:56 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development; +Cc: Daniel Vetter, stable

[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]

On Tue, 23 Oct 2012 11:12:02 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Oct 22, 2012 at 12:55:55PM +0200, Daniel Vetter wrote:
> > The overlay on the i830M has a peculiar failure mode: It works the
> > first time around after boot-up, but consistenly hangs the second time
> > it's used.
> > 
> > Chris Wilson has dug out a nice errata:
> > 
> > "1.5.12 Clock Gating Disable for Display Register
> > Address Offset:	06200h–06203h
> > 
> > "Bit 3
> > Ovrunit Clock Gating Disable.
> > 0 = Clock gating controlled by unit enabling logic
> > 1 = Disable clock gating function
> > DevALM Errata ALM049: Overlay Clock Gating Must be Disabled:  Overlay
> > & L2 Cache clock gating must be disabled in order to prevent device
> > hangs when turning off overlay.SW must turn off Ovrunit clock gating
> > (6200h) and L2 Cache clock gating (C8h)."
> > 
> > Now I've nowhere found that 0xc8 register and hence couldn't apply the
> > l2 cache workaround. But I've remembered that part of the magic that
> > the OVERLAY_ON/OFF commands are supposed to do is to rearrange cache
> > allocations so that the overlay scaler has some scratch space.
> > 
> > And while pondering how that could explain the hang the 2nd time we
> > enable the overlay, I've remembered that the old ums overlay code did
> > _not_ issue the OVERLAY_OFF cmd.
> > 
> > And indeed, disabling the OFF cmd results in the overlay working
> > flawlessly, so I guess we can workaround the lack of the above
> > workaround by simply never disabling the overlay engine once it's
> > enabled.
> > 
> > v2: Add a comment in the code.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47827
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Tested-by: Rhys <rhyspuk@gmail.com>

If you care to mention that you are not turning off the old w/a in case
it has further side-effects,

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: fix overlay on i830M
  2012-10-23  9:56   ` Chris Wilson
@ 2012-10-23 10:59     ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-10-23 10:59 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, Intel Graphics Development, stable

On Tue, Oct 23, 2012 at 10:56:46AM +0100, Chris Wilson wrote:
> On Tue, 23 Oct 2012 11:12:02 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Oct 22, 2012 at 12:55:55PM +0200, Daniel Vetter wrote:
> > > The overlay on the i830M has a peculiar failure mode: It works the
> > > first time around after boot-up, but consistenly hangs the second time
> > > it's used.
> > > 
> > > Chris Wilson has dug out a nice errata:
> > > 
> > > "1.5.12 Clock Gating Disable for Display Register
> > > Address Offset:	06200h–06203h
> > > 
> > > "Bit 3
> > > Ovrunit Clock Gating Disable.
> > > 0 = Clock gating controlled by unit enabling logic
> > > 1 = Disable clock gating function
> > > DevALM Errata ALM049: Overlay Clock Gating Must be Disabled:  Overlay
> > > & L2 Cache clock gating must be disabled in order to prevent device
> > > hangs when turning off overlay.SW must turn off Ovrunit clock gating
> > > (6200h) and L2 Cache clock gating (C8h)."
> > > 
> > > Now I've nowhere found that 0xc8 register and hence couldn't apply the
> > > l2 cache workaround. But I've remembered that part of the magic that
> > > the OVERLAY_ON/OFF commands are supposed to do is to rearrange cache
> > > allocations so that the overlay scaler has some scratch space.
> > > 
> > > And while pondering how that could explain the hang the 2nd time we
> > > enable the overlay, I've remembered that the old ums overlay code did
> > > _not_ issue the OVERLAY_OFF cmd.
> > > 
> > > And indeed, disabling the OFF cmd results in the overlay working
> > > flawlessly, so I guess we can workaround the lack of the above
> > > workaround by simply never disabling the overlay engine once it's
> > > enabled.
> > > 
> > > v2: Add a comment in the code.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47827
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > Tested-by: Rhys <rhyspuk@gmail.com>
> 
> If you care to mention that you are not turning off the old w/a in case
> it has further side-effects,
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Small note about the existing w/a added to the commit message and patch
merged to -fixes. Thanks for the review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-23 10:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-22 10:44 [PATCH] drm/i915: fix overlay on i830M Daniel Vetter
2012-10-22 10:54 ` Chris Wilson
  -- strict thread matches above, loose matches on Subject: below --
2012-10-22 10:55 Daniel Vetter
2012-10-23  9:12 ` Daniel Vetter
2012-10-23  9:56   ` Chris Wilson
2012-10-23 10:59     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox