diff mbox

[3/3,v3] intel_error_decode: Fix ACTHD/HEAD mess with libdrm

Message ID 1366679472-7824-3-git-send-email-ben@bwidawsk.net (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky April 23, 2013, 1:11 a.m. UTC
This patch is an enormous mess, and I'd be fine if people didn't want
it. However I have made the code do what we want at least on the two
error dumps I've tried.

The way that it works is it attempts to identify which ACTHD belongs to
the ring, or batch, and add the appropriate offset as necessary so the
libdrm decoder can do the right thing.

What I do is put each ACTHD in a fixed part of the array, and assume the
error dump will dump each ring in gtt ascending order (ie. RCS offset <
VCS offset < BCS offset). I know, its hacky.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 tools/intel_error_decode.c | 97 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 92 insertions(+), 5 deletions(-)

Comments

Chris Wilson April 23, 2013, 8:30 a.m. UTC | #1
On Mon, Apr 22, 2013 at 06:11:12PM -0700, Ben Widawsky wrote:
> This patch is an enormous mess, and I'd be fine if people didn't want
> it. However I have made the code do what we want at least on the two
> error dumps I've tried.
> 
> The way that it works is it attempts to identify which ACTHD belongs to
> the ring, or batch, and add the appropriate offset as necessary so the
> libdrm decoder can do the right thing.
> 
> What I do is put each ACTHD in a fixed part of the array, and assume the
> error dump will dump each ring in gtt ascending order (ie. RCS offset <
> VCS offset < BCS offset). I know, its hacky.
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---

> +static void emit_acthd(struct drm_intel_decode *decode_ctx,
> +		uint64_t gtt_offset, uint64_t size, int is_batch)
> +{
> +	/* XXX: This depends on always being in ascending gtt space order */
> +	static int which_acthd = 0;

You have ring_name available to sort on.
I would prefer the use of RING_HEAD when decoding the ringbuffers, so
that the current HEAD is always printed there - acting like a frame
pointer in a stacktrace. Making that distinction between rings and
batches should make this code simplier.
-Chris
Ben Widawsky April 23, 2013, 3:41 p.m. UTC | #2
On Tue, Apr 23, 2013 at 09:30:33AM +0100, Chris Wilson wrote:
> On Mon, Apr 22, 2013 at 06:11:12PM -0700, Ben Widawsky wrote:
> > This patch is an enormous mess, and I'd be fine if people didn't want
> > it. However I have made the code do what we want at least on the two
> > error dumps I've tried.
> > 
> > The way that it works is it attempts to identify which ACTHD belongs to
> > the ring, or batch, and add the appropriate offset as necessary so the
> > libdrm decoder can do the right thing.
> > 
> > What I do is put each ACTHD in a fixed part of the array, and assume the
> > error dump will dump each ring in gtt ascending order (ie. RCS offset <
> > VCS offset < BCS offset). I know, its hacky.
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> 
> > +static void emit_acthd(struct drm_intel_decode *decode_ctx,
> > +		uint64_t gtt_offset, uint64_t size, int is_batch)
> > +{
> > +	/* XXX: This depends on always being in ascending gtt space order */
> > +	static int which_acthd = 0;
> 
> You have ring_name available to sort on.
> I would prefer the use of RING_HEAD when decoding the ringbuffers, so
> that the current HEAD is always printed there - acting like a frame
> pointer in a stacktrace. Making that distinction between rings and
> batches should make this code simplier.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

Any issue with me pushing the first 2, and stalling on this one?
Chris Wilson April 23, 2013, 4:10 p.m. UTC | #3
On Tue, Apr 23, 2013 at 08:41:10AM -0700, Ben Widawsky wrote:
> Any issue with me pushing the first 2, and stalling on this one?

None at all, the improvements are worthwhile.
-Chris
diff mbox

Patch

diff --git a/tools/intel_error_decode.c b/tools/intel_error_decode.c
index 434c13a..3d500d6 100644
--- a/tools/intel_error_decode.c
+++ b/tools/intel_error_decode.c
@@ -55,11 +55,29 @@ 
 #include "intel_gpu_tools.h"
 #include "instdone.h"
 
+#define MAX_RINGS 10 /* I really hope this never... */
+struct head {
+	uint32_t rsvd : 2;
+	uint32_t head : 19; /* In dwords */
+	uint32_t wraps : 11;
+} __attribute__ ((__packed__));
+
+struct acthd {
+	uint64_t val;
+	enum {
+		INVALID=0,
+		RING=1,
+		BATCH=2
+	} type;
+} ring_acthds[MAX_RINGS * 2]; /* Cheap trick to put ring acthd in fixed slots */
+
+
 static uint32_t
 print_head(unsigned int reg)
 {
-	printf("    head = 0x%08x, wraps = %d\n", reg & (0x7ffff<<2), reg >> 21);
-	return reg & (0x7ffff<<2);
+	struct head head = *((struct head *)&reg);
+	printf("    head = 0x%08x, wraps = %d\n", head.head<<2, head.wraps);
+	return head.head << 2;
 }
 
 static void
@@ -280,7 +298,6 @@  print_fence(unsigned int devid, uint64_t fence)
 	}
 }
 
-#define MAX_RINGS 10 /* I really hope this never... */
 uint32_t head[MAX_RINGS];
 int head_ndx = 0;
 int num_rings = -1;
@@ -294,6 +311,59 @@  static void print_batch(int is_batch, const char *ring_name, uint32_t gtt_offset
 	}
 }
 
+static void emit_acthd(struct drm_intel_decode *decode_ctx,
+		uint64_t gtt_offset, uint64_t size, int is_batch)
+{
+	/* XXX: This depends on always being in ascending gtt space order */
+	static int which_acthd = 0;
+	int i;
+	size <<=2;
+
+	if (!is_batch) {
+		struct acthd *cur_acthd = &ring_acthds[which_acthd + MAX_RINGS];
+		which_acthd++;
+		if (cur_acthd->type == INVALID) {
+			return;
+		}
+
+		cur_acthd->val += gtt_offset;
+		if ((cur_acthd->val >= gtt_offset) && (cur_acthd->val < gtt_offset + size)) {
+			drm_intel_decode_set_head_tail(decode_ctx, cur_acthd->val, 0xffffffff);
+			cur_acthd->type = INVALID;
+		}
+		return;
+	}
+
+	for (i = 0; i < MAX_RINGS; i++) {
+		uint64_t offset = ring_acthds[i].val;
+		if (ring_acthds[i].type == INVALID)
+			continue;
+
+		if ((offset >= gtt_offset) && (offset < gtt_offset + size)) {
+			drm_intel_decode_set_head_tail(decode_ctx, offset, 0xffffffff);
+			ring_acthds[i].type = INVALID;
+			return;
+		}
+	}
+}
+
+static int acthd_equals_head(uint32_t a, uint32_t h)
+{
+	struct head _acthd = *(struct head *)&a;
+	struct head _head = *(struct head *)&h;
+
+	if (a == h)
+		return 1;
+
+	/* Likely the same */
+	if (_acthd.wraps == _head.wraps ||
+			_head.wraps + 1 == _acthd.wraps) {
+		return 1;
+	}
+
+	return 0;
+}
+
 static void
 read_data_file(FILE *file)
 {
@@ -308,6 +378,8 @@  read_data_file(FILE *file)
 	uint32_t gtt_offset = 0, new_gtt_offset;
 	char *ring_name = NULL;
 	int is_batch = 1;
+	uint32_t raw_head = -1;
+	int i=0;
 
 	while (getline(&line, &line_size, file) > 0) {
 		char *dashes;
@@ -327,6 +399,7 @@  read_data_file(FILE *file)
 			if (matched == 1) {
 				if (count) {
 					print_batch(is_batch, ring_name, gtt_offset);
+					emit_acthd(decode_ctx, gtt_offset, count, is_batch);
 					drm_intel_decode_set_batch_pointer(decode_ctx,
 							data, gtt_offset,
 							count);
@@ -345,6 +418,7 @@  read_data_file(FILE *file)
 			if (matched == 1) {
 				if (count) {
 					print_batch(is_batch, ring_name, gtt_offset);
+					emit_acthd(decode_ctx, gtt_offset, count, is_batch);
 					drm_intel_decode_set_batch_pointer(decode_ctx,
 							data, gtt_offset,
 							count);
@@ -366,6 +440,7 @@  read_data_file(FILE *file)
 			/* display reg section is after the ringbuffers, don't mix them */
 			if (count) {
 				print_batch(is_batch, ring_name, gtt_offset);
+				emit_acthd(decode_ctx, gtt_offset, count, is_batch);
 				drm_intel_decode_set_batch_pointer(decode_ctx,
 						data, gtt_offset,
 						count);
@@ -394,11 +469,22 @@  read_data_file(FILE *file)
 			matched = sscanf(line, "  HEAD: 0x%08x\n", &reg);
 			if (matched == 1) {
 				head[head_ndx++] = print_head(reg);
+				raw_head = reg;
+				assert(head_ndx < MAX_RINGS);
 			}
 
 			matched = sscanf(line, "  ACTHD: 0x%08x\n", &reg);
-			if (matched == 1)
-				drm_intel_decode_set_head_tail(decode_ctx, reg, 0xffffffff);
+			if (matched == 1) {
+				assert(raw_head != -1);
+				if (acthd_equals_head(reg, raw_head)) {
+					ring_acthds[MAX_RINGS + head_ndx - 1].type = RING;
+					ring_acthds[MAX_RINGS + head_ndx - 1].val = print_head(reg);
+				} else {
+					ring_acthds[i].val = reg;
+					ring_acthds[i].type = BATCH;
+					i++;
+				}
+			}
 
 			matched = sscanf(line, "  PGTBL_ER: 0x%08x\n", &reg);
 			if (matched == 1 && reg)
@@ -435,6 +521,7 @@  read_data_file(FILE *file)
 
 	if (count) {
 		print_batch(is_batch, ring_name, gtt_offset);
+		emit_acthd(decode_ctx, gtt_offset, count, is_batch);
 		drm_intel_decode_set_batch_pointer(decode_ctx,
 				data, gtt_offset,
 				count);