| Message ID | 1366585279-18263-1-git-send-email-ben@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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 --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]);
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(-)