| Message ID | 1303032761-2991-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, 17 Apr 2011 10:32:41 +0100 Chris Wilson <chris@chris-wilson.co.uk> 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! > I like it (though now the comment talks about DSPADDR and leaves DSPSURF out in the cold). Sounds like our dpms code may be causing trouble for this x11perf run perhaps? What else would cause us to change base addrs during a bench run like that?
On Mon, 18 Apr 2011 08:54:44 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > I like it (though now the comment talks about DSPADDR and leaves > DSPSURF out in the cold). D'oh. That'll teach me to try and write a comment to explain a function first! > Sounds like our dpms code may be causing trouble for this x11perf run > perhaps? What else would cause us to change base addrs during a bench > run like that? Right. The early observation is that we don't assign a framebuffer to the CRTC borrowed for VGA/TV load detection. And I can postulate that we contrived to access a PTE during a hotplug poll whilst rewriting the GTT (or something similar to the object following the ringbuffer). I think we can just attach the fbcon for the purposes of load detection. -Chris
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7734d1e..ab80046 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1274,6 +1274,34 @@ static void intel_disable_pipe(struct drm_i915_private *dev_priv, intel_wait_for_pipe_off(dev_priv->dev, pipe); } +/* Check that the DSPADDR points to the right framebufffer for the pipe. */ +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 @@ -1290,6 +1318,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);
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 <chris@chris-wilson.co.uk> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> --- v2: Remember that gen4 splits the base + offset between different regs... --- drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++++++++++++ 1 files changed, 29 insertions(+), 0 deletions(-)