[PATCH] Ring range argument improvements (v2)

[Tuikov, Luben]

The valid contents of rings is invariably the
range [rptr, wptr). Augment the ring range to
interpret the '.' ("here") notation to mean rptr
or wptr when given on the left or right of the
range limits. This augments the range notation as
follows:

1) No range given, print the whole ring.

2) [.] or [.:.], print [rptr, wptr],

3) [.:k], k >= 0, print [rptr, rptr + k], this is
a range relative to the left limit, rptr, the
consumer pointer.

4) [k:.] or [k], k >= 0, print [wptr - k, wptr], this is
a range relative to the right limit, wptr, the
producer pointer.

5) [k:r], both greater than 0, print [k,r] of the
named ring. This is an absolute range limit
notation.

In any case, the ring contents is interpreted, if
the ring contents can be interpreted.

v2: Fix spelling mistake in the commit message:
"then" --> "than".

Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>
---
 doc/umr.1           | 33 +++++++++++++++++++---------
 src/app/ring_read.c | 52 ++++++++++++++++++++++++++++-----------------
 2 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/doc/umr.1 b/doc/umr.1
index 1e585fa..137ff1e 100644
--- a/doc/umr.1
+++ b/doc/umr.1
@@ -216,17 +216,30 @@ Disassemble 'size' bytes (in hex) from a given address (in hex).  The size can b
 specified as zero to have umr try and compute the shader size.
 
 .SH Ring and PM4 Decoding
-.IP "--ring, -R <string>(from:to)"
-Read the contents of a ring named by the string without the
-.B amdgpu_ring_
-prefix.  By default it will read and display the entire ring.  A
-starting and ending address can be specified in decimal or a '.' can
-be used to indicate relative to the current
+.IP "--ring, -R <string>[range]"
+Read the contents of the ring named by the string
+.B amdgpu_ring_<string>,
+i.e. without the
+.B amdgpu_ring
+prefix. By default it reads and prints the entire ring.  A
+range is optional and has the format '[start:end]'. The
+starting and ending address are non-negative integers or
+the '.' (dot) symbol, which indicates the
+.B rptr
+when on the left side and
 .B wptr
-pointer.  For example, "-R gfx" would read the entire gfx ring,
-"-R gfx[0:16]" would display the contents from 0 to 16 inclusively, and
-"-R gfx[.]" or "-R gfx[.:.]" would display the last 32 words relative
-to rptr.
+when on the right side of the range.
+For instance,
+"-R gfx" prints the entire gfx ring, "-R gfx[0:16]" prints
+the contents from 0 to 16 inclusively, and "-R gfx[.]" or
+"-R gfx[.:.]" prints the range [rptr,wptr]. When one of
+the range limits is a number while the other is the dot, '.',
+then the number indicates the relative range before or after the
+corresponding ring pointer. For instance, "-R sdma0[16:.]"
+prints [wptr-16, wptr] words of the SDMA0 ring, and
+"-R sdma1[.:32]" prints [rptr, rptr+32] double-words of the
+SDMA1 ring. The contents of the ring is always interpreted,
+if it can be interpreted.
 .IP "--dump-ib, -di [vmid@]address length [pm]"
 Dump an IB packet at an address with an optional VMID.  The length is specified
 in bytes.  The type of decoder <pm> is optional and defaults to PM4 packets.
diff --git a/src/app/ring_read.c b/src/app/ring_read.c
index ef0c711..9cbecb0 100644
--- a/src/app/ring_read.c
+++ b/src/app/ring_read.c
@@ -28,7 +28,7 @@
 void umr_read_ring(struct umr_asic *asic, char *ringpath)
 {
 	char ringname[32], from[32], to[32];
-	int use_decoder, enable_decoder, gprs;
+	int  enable_decoder, gprs;
 	uint32_t wptr, rptr, drv_wptr, ringsize, start, end, value,
 		 *ring_data;
 	struct umr_ring_decoder decoder, *pdecoder, *ppdecoder;
@@ -73,33 +73,46 @@ void umr_read_ring(struct umr_asic *asic, char *ringpath)
 	drv_wptr = ring_data[2]<<2;
 
 	/* default to reading entire ring */
-	use_decoder = 0;
 	if (!from[0]) {
 		start = 0;
 		end   = ringsize-4;
 	} else {
-		if (from[0] == '.' || !to[0] || to[0] == '.') {
-			/* start from 32 words prior to rptr up to wptr */
-			end = wptr;
-			if (rptr < (31*4)) {
-				start = rptr - 31*4;
-				start += ringsize;
+		if (from[0] == '.') {
+			if (to[0] == 0 || to[0] == '.') {
+				/* Notation: [.] or [.:.], meaning
+				 * [rptr, wptr].
+				 */
+				start = rptr;
+				end = wptr;
 			} else {
-				start = rptr - 31*4;
+				/* Notation: [.:k], k >=0, meaning
+				 * [rptr, rtpr+k] double-words.
+				 */
+				start = rptr;
+				sscanf(to, "%"SCNu32, &end);
+				end *= 4;
+				end = (start + end + ringsize) % ringsize;
 			}
-
 		} else {
 			sscanf(from, "%"SCNu32, &start);
-			sscanf(to, "%"SCNu32, &end);
 			start *= 4;
-			end *= 4;
-			use_decoder = 1;
-			decoder.pm4.cur_opcode = 0xFFFFFFFF;
-			decoder.sdma.cur_opcode = 0xFFFFFFFF;
+
+			if (to[0] != 0 && to[0] != '.') {
+				/* [k:r] ==> absolute [k, r].
+				 */
+				sscanf(to, "%"SCNu32, &end);
+				end *= 4;
+				start %= ringsize;
+				end %= ringsize;
+			} else {
+				/* to[0] is 0 or '.',
+				 * [k] or [k:.] ==> [wptr - k, wptr]
+				 */
+				start = (wptr - start + ringsize) % ringsize;
+				end = wptr;
+			}
 		}
 	}
-	end %= ringsize;
-	start %= ringsize;
 
 	/* dump data */
 	printf("\n%s.%s.rptr == %lu\n%s.%s.wptr == %lu\n%s.%s.drv_wptr == %lu\n",
@@ -113,8 +126,7 @@ void umr_read_ring(struct umr_asic *asic, char *ringpath)
 			asic->asicname, ringname,
 			BLUE, (unsigned long)start >> 2, RST,
 			YELLOW, (unsigned long)value, RST);
-		if (enable_decoder && start == rptr && start != wptr) {
-			use_decoder = 1;
+		if (enable_decoder) {
 			decoder.pm4.cur_opcode = 0xFFFFFFFF;
 			decoder.sdma.cur_opcode = 0xFFFFFFFF;
 		}
@@ -123,7 +135,7 @@ void umr_read_ring(struct umr_asic *asic, char *ringpath)
 			(start == wptr) ? 'w' : '.',
 			(start == drv_wptr) ? 'D' : '.');
 		decoder.next_ib_info.addr = start / 4;
-		if (use_decoder)
+		if (enable_decoder)
 			umr_print_decode(asic, &decoder, value);
 		printf("\n");
 		start += 4;
-- 
2.23.0.385.gbc12974a89