diff mbox

drm/i915: Decode RING_BUFFER_HEAD in error state

Message ID 1366585279-18263-1-git-send-email-ben@bwidawsk.net (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky April 21, 2013, 11:01 p.m. UTC
I consistently forget how to decode the HEAD pointer. If we put it in
the error state, one doesn't need to look in docs to find the relevant
info.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Chris Wilson April 22, 2013, 1:16 p.m. UTC | #1
On Sun, Apr 21, 2013 at 04:01:19PM -0700, Ben Widawsky wrote:
> I consistently forget how to decode the HEAD pointer. If we put it in
> the error state, one doesn't need to look in docs to find the relevant
> info.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Turn the offset into an address and that would be really useful - then
once can do a quick search on the value and jump straight into the right
point in the ring.
  error->head[ring] & (0x7ffff << 2) + error->ring[ring].batchbuffer->gtt_offset
-Chris
Daniel Vetter April 22, 2013, 3:17 p.m. UTC | #2
On Mon, Apr 22, 2013 at 02:16:34PM +0100, Chris Wilson wrote:
> On Sun, Apr 21, 2013 at 04:01:19PM -0700, Ben Widawsky wrote:
> > I consistently forget how to decode the HEAD pointer. If we put it in
> > the error state, one doesn't need to look in docs to find the relevant
> > info.
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 
> Turn the offset into an address and that would be really useful - then
> once can do a quick search on the value and jump straight into the right
> point in the ring.
>   error->head[ring] & (0x7ffff << 2) + error->ring[ring].batchbuffer->gtt_offset

Also, shouldn't that kind of decode magic be done in intel_error_decode,
where we have all the other stuff? Or is there a special reason?
-Daniel
Ben Widawsky April 22, 2013, 4:57 p.m. UTC | #3
On Mon, Apr 22, 2013 at 05:17:41PM +0200, Daniel Vetter wrote:
> On Mon, Apr 22, 2013 at 02:16:34PM +0100, Chris Wilson wrote:
> > On Sun, Apr 21, 2013 at 04:01:19PM -0700, Ben Widawsky wrote:
> > > I consistently forget how to decode the HEAD pointer. If we put it in
> > > the error state, one doesn't need to look in docs to find the relevant
> > > info.
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > Turn the offset into an address and that would be really useful - then
> > once can do a quick search on the value and jump straight into the right
> > point in the ring.
> >   error->head[ring] & (0x7ffff << 2) + error->ring[ring].batchbuffer->gtt_offset
> 
> Also, shouldn't that kind of decode magic be done in intel_error_decode,
> where we have all the other stuff? Or is there a special reason?
> -Daniel

Originally, I had 4 excuses, but 2 turned out to false.
1. I usually don't use error decode, and I suspect others don't as well.
2. HEAD decoding hasn't changed for as long as I can tell.
3. (false) I thought we already decoded other registers in the error
4. (false) It's easier to add this to the error dump than the tool.

Personally, I felt Chris' suggestion is something along the lines of
what belongs in IGT, whereas this patch was the level of decoding worth
doing in error dump.

Personally, I'm not opposed to decoding more registers that improve the
usefulness of the error state without needing a tool, but I also
understand why you wouldn't take it, and how it's a fuzzy line about
which registers are appropriate to add vs. not.

I'm willing to add it to error dumper.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e913d32..dd5a64e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -643,7 +643,10 @@  static void i915_ring_error_state(struct seq_file *m,
 {
 	BUG_ON(ring >= I915_NUM_RINGS); /* shut up confused gcc */
 	seq_printf(m, "%s command stream:\n", ring_str(ring));
-	seq_printf(m, "  HEAD: 0x%08x\n", error->head[ring]);
+	seq_printf(m, "  HEAD: 0x%08x (wraps: %d; offset: 0x%08x)\n",
+		   error->head[ring],
+		   error->head[ring] >> 21,
+		   error->head[ring] & (0x7ffff << 2));
 	seq_printf(m, "  TAIL: 0x%08x\n", error->tail[ring]);
 	seq_printf(m, "  CTL: 0x%08x\n", error->ctl[ring]);
 	seq_printf(m, "  ACTHD: 0x%08x\n", error->acthd[ring]);