* [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb
@ 2011-11-24 18:22 Chris Wilson
2012-01-16 22:48 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2011-11-24 18:22 UTC (permalink / raw)
To: intel-gfx
Staring at an error state such as:
PGTBL_ER: 0x00000400
Display B: Invalid tiling
fence[0] = 05001001
valid, x-tiled, pitch: 512, start: 0x05000000, size: 1048576
Pinned [2]:
00000000 131072 0001 0001 00000000 P uncached
00020000 4096000 0041 0000 00000000 P uncached (name: 1)
Plane [1]:
CNTR: c0000000 # enabled | gamma
STRIDE: 00001400
SIZE: 03ff04ff
POS: 00000000
ADDR: 05000000
Suggests that we did not clear the DSPBCNTR prior to unpinning the
framebuffer and reusing the GTT space. Impossible! Unless our DPMS
bookkeeping ran afoul again...
In the meantime add an assertion that the plane is decoupled from the
framebuffer prior to release.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5d4cbff..800b36e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -932,19 +932,24 @@ static void assert_pipe(struct drm_i915_private *dev_priv,
#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
-static void assert_plane_enabled(struct drm_i915_private *dev_priv,
- enum plane plane)
+static void assert_plane(struct drm_i915_private *dev_priv,
+ enum plane plane, bool state)
{
int reg;
u32 val;
+ bool cur_state;
reg = DSPCNTR(plane);
val = I915_READ(reg);
- WARN(!(val & DISPLAY_PLANE_ENABLE),
- "plane %c assertion failure, should be active but is disabled\n",
- plane_name(plane));
+ cur_state = !!(val & DISPLAY_PLANE_ENABLE);
+ WARN(cur_state != state,
+ "plane %c assertion failure (expected %s, current %s)\n",
+ plane_name(plane), state_string(state), state_string(cur_state));
}
+#define assert_plane_enabled(d, p) assert_plane(d, p, true)
+#define assert_plane_disabled(d, p) assert_plane(d, p, false)
+
static void assert_planes_disabled(struct drm_i915_private *dev_priv,
enum pipe pipe)
{
@@ -3320,6 +3325,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
+ assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane);
+ assert_pipe_disabled(dev->dev_private, to_intel_crtc(crtc)->pipe);
if (crtc->fb) {
mutex_lock(&dev->struct_mutex);
--
1.7.7.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb
2011-11-24 18:22 [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb Chris Wilson
@ 2012-01-16 22:48 ` Daniel Vetter
2012-01-16 23:01 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2012-01-16 22:48 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Thu, Nov 24, 2011 at 06:22:18PM +0000, Chris Wilson wrote:
> Staring at an error state such as:
>
> PGTBL_ER: 0x00000400
> Display B: Invalid tiling
> fence[0] = 05001001
> valid, x-tiled, pitch: 512, start: 0x05000000, size: 1048576
> Pinned [2]:
> 00000000 131072 0001 0001 00000000 P uncached
> 00020000 4096000 0041 0000 00000000 P uncached (name: 1)
> Plane [1]:
> CNTR: c0000000 # enabled | gamma
> STRIDE: 00001400
> SIZE: 03ff04ff
> POS: 00000000
> ADDR: 05000000
>
> Suggests that we did not clear the DSPBCNTR prior to unpinning the
> framebuffer and reusing the GTT space. Impossible! Unless our DPMS
> bookkeeping ran afoul again...
>
> In the meantime add an assertion that the plane is decoupled from the
> framebuffer prior to release.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Jesse acked this on irc, but it doesn't apply anymore. Care to rebase?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++-----
> 1 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5d4cbff..800b36e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -932,19 +932,24 @@ static void assert_pipe(struct drm_i915_private *dev_priv,
> #define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
> #define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
>
> -static void assert_plane_enabled(struct drm_i915_private *dev_priv,
> - enum plane plane)
> +static void assert_plane(struct drm_i915_private *dev_priv,
> + enum plane plane, bool state)
> {
> int reg;
> u32 val;
> + bool cur_state;
>
> reg = DSPCNTR(plane);
> val = I915_READ(reg);
> - WARN(!(val & DISPLAY_PLANE_ENABLE),
> - "plane %c assertion failure, should be active but is disabled\n",
> - plane_name(plane));
> + cur_state = !!(val & DISPLAY_PLANE_ENABLE);
> + WARN(cur_state != state,
> + "plane %c assertion failure (expected %s, current %s)\n",
> + plane_name(plane), state_string(state), state_string(cur_state));
> }
>
> +#define assert_plane_enabled(d, p) assert_plane(d, p, true)
> +#define assert_plane_disabled(d, p) assert_plane(d, p, false)
> +
> static void assert_planes_disabled(struct drm_i915_private *dev_priv,
> enum pipe pipe)
> {
> @@ -3320,6 +3325,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
> struct drm_device *dev = crtc->dev;
>
> crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
> + assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane);
> + assert_pipe_disabled(dev->dev_private, to_intel_crtc(crtc)->pipe);
>
> if (crtc->fb) {
> mutex_lock(&dev->struct_mutex);
> --
> 1.7.7.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb
2012-01-16 22:48 ` Daniel Vetter
@ 2012-01-16 23:01 ` Chris Wilson
2012-01-17 10:09 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2012-01-16 23:01 UTC (permalink / raw)
To: intel-gfx
Staring at an error state such as:
PGTBL_ER: 0x00000400
Display B: Invalid tiling
fence[0] = 05001001
valid, x-tiled, pitch: 512, start: 0x05000000, size: 1048576
Pinned [2]:
00000000 131072 0001 0001 00000000 P uncached
00020000 4096000 0041 0000 00000000 P uncached (name: 1)
Plane [1]:
CNTR: c0000000 # enabled | gamma
STRIDE: 00001400
SIZE: 03ff04ff
POS: 00000000
ADDR: 05000000
Suggests that we did not clear the DSPBCNTR prior to unpinning the
framebuffer and reusing the GTT space. Impossible! Unless our DPMS
bookkeeping ran afoul again...
In the meantime add an assertion that the plane is decoupled from the
framebuffer prior to release.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 29743de..708a181 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -930,19 +930,24 @@ void assert_pipe(struct drm_i915_private *dev_priv,
pipe_name(pipe), state_string(state), state_string(cur_state));
}
-static void assert_plane_enabled(struct drm_i915_private *dev_priv,
- enum plane plane)
+static void assert_plane(struct drm_i915_private *dev_priv,
+ enum plane plane, bool state)
{
int reg;
u32 val;
+ bool cur_state;
reg = DSPCNTR(plane);
val = I915_READ(reg);
- WARN(!(val & DISPLAY_PLANE_ENABLE),
- "plane %c assertion failure, should be active but is disabled\n",
- plane_name(plane));
+ cur_state = !!(val & DISPLAY_PLANE_ENABLE);
+ WARN(cur_state != state,
+ "plane %c assertion failure (expected %s, current %s)\n",
+ plane_name(plane), state_string(state), state_string(cur_state));
}
+#define assert_plane_enabled(d, p) assert_plane(d, p, true)
+#define assert_plane_disabled(d, p) assert_plane(d, p, false)
+
static void assert_planes_disabled(struct drm_i915_private *dev_priv,
enum pipe pipe)
{
@@ -3321,6 +3326,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
+ assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane);
+ assert_pipe_disabled(dev->dev_private, to_intel_crtc(crtc)->pipe);
if (crtc->fb) {
mutex_lock(&dev->struct_mutex);
--
1.7.8.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb
2012-01-16 23:01 ` Chris Wilson
@ 2012-01-17 10:09 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2012-01-17 10:09 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Mon, Jan 16, 2012 at 11:01:13PM +0000, Chris Wilson wrote:
> Staring at an error state such as:
>
> PGTBL_ER: 0x00000400
> Display B: Invalid tiling
> fence[0] = 05001001
> valid, x-tiled, pitch: 512, start: 0x05000000, size: 1048576
> Pinned [2]:
> 00000000 131072 0001 0001 00000000 P uncached
> 00020000 4096000 0041 0000 00000000 P uncached (name: 1)
> Plane [1]:
> CNTR: c0000000 # enabled | gamma
> STRIDE: 00001400
> SIZE: 03ff04ff
> POS: 00000000
> ADDR: 05000000
>
> Suggests that we did not clear the DSPBCNTR prior to unpinning the
> framebuffer and reusing the GTT space. Impossible! Unless our DPMS
> bookkeeping ran afoul again...
>
> In the meantime add an assertion that the plane is decoupled from the
> framebuffer prior to release.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-01-17 10:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-24 18:22 [PATCH] drm/i915: Check that plane/pipe is disabled before removing the fb Chris Wilson
2012-01-16 22:48 ` Daniel Vetter
2012-01-16 23:01 ` Chris Wilson
2012-01-17 10:09 ` 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.