[1/3] drm/i915: Check that the plane points to the pipe's framebuffer before enabling

Commit Message

Chris Wilson April 19, 2011, 8:32 p.m. UTC

Knut Petersen reported a GPU hang when he left x11perf running
overnight. The error state quite clearly indicates that plane A was
enabled without being fully setup:

PGTBL_ER: 0x00000010
    Display A: Invalid GTT PTE
Plane [0]:
  CNTR: c1000000
  STRIDE: 00000c80
  SIZE: 03ff04ff
  POS: 00000000
  ADDR: 00000000

[That GTT offset on his system being pinned for the ringbuffer.]

This is a simple debugging patch to assert that this cannot be so!

References: https://bugs.freedesktop.org/show_bug.cgi?id=36246
Signed-off-by: Chris Wilson <[e-mail]>
Cc: Jesse Barnes <[e-mail]>
---
 drivers/gpu/drm/i915/intel_display.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

Comments

Jesse Barnes April 19, 2011, 8:40 p.m. UTC | #1

On Tue, 19 Apr 2011 21:32:01 +0100
Chris Wilson <[e-mail]> wrote:

> Knut Petersen reported a GPU hang when he left x11perf running
> overnight. The error state quite clearly indicates that plane A was
> enabled without being fully setup:
> 
> PGTBL_ER: 0x00000010
>     Display A: Invalid GTT PTE
> Plane [0]:
>   CNTR: c1000000
>   STRIDE: 00000c80
>   SIZE: 03ff04ff
>   POS: 00000000
>   ADDR: 00000000
> 
> [That GTT offset on his system being pinned for the ringbuffer.]
> 
> This is a simple debugging patch to assert that this cannot be so!
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=36246
> Signed-off-by: Chris Wilson <[e-mail]>
> Cc: Jesse Barnes <[e-mail]>
> ---

I like it.

Reviewed-by: Jesse Barnes <[e-mail]>

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 73cec21..b7d63a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1560,6 +1560,34 @@  static void intel_disable_pipe(struct drm_i915_private *dev_priv,
 	intel_wait_for_pipe_off(dev_priv->dev, pipe);
 }
 
+/* Check that the display plane points to the pipe's framebuffer. */
+static void assert_fb_bound_to_plane(struct drm_i915_private *dev_priv,
+				     enum pipe pipe, enum plane plane)
+{
+	struct drm_crtc *crtc;
+	struct intel_framebuffer *intel_fb;
+	u32 val, base, size;
+
+	crtc = intel_get_crtc_for_pipe(dev_priv->dev, pipe);
+	if (WARN(crtc->fb == NULL,
+		 "no framebuffer attached to pipe %c\n",
+		 pipe_name(pipe)))
+		return;
+
+	intel_fb = to_intel_framebuffer(crtc->fb);
+	base = intel_fb->obj->gtt_offset;
+	size = intel_fb->obj->base.size;
+
+	if (dev_priv->info->gen >= 4)
+		val = I915_READ(DSPSURF(plane));
+	else
+		val = I915_READ(DSPADDR(plane));
+	WARN(val < base || val >= base + size,
+	     "mismatching framebuffer for plane %c attached to pipe %c, expected %x-%x found %x\n",
+	     plane_name(plane), pipe_name(pipe),
+	     base, base + size, val);
+}
+
 /**
  * intel_enable_plane - enable a display plane on a given pipe
  * @dev_priv: i915 private structure
@@ -1576,6 +1604,7 @@  static void intel_enable_plane(struct drm_i915_private *dev_priv,
 
 	/* If the pipe isn't enabled, we can't pump pixels and may hang */
 	assert_pipe_enabled(dev_priv, pipe);
+	assert_fb_bound_to_plane(dev_priv, pipe, plane);
 
 	reg = DSPCNTR(plane);
 	val = I915_READ(reg);