[PATCH] Ring range argument improvements (v2)
StDenis, Tom
Tom.StDenis at amd.com
Thu Oct 24 13:44:43 UTC 2019
This diff fixes your patch, can you resend please?
diff --git a/src/app/ring_read.c b/src/app/ring_read.c
index 9cbecb0..c1c9187 100644
--- a/src/app/ring_read.c
+++ b/src/app/ring_read.c
@@ -120,16 +120,16 @@ void umr_read_ring(struct umr_asic *asic, char
*ringpath)
asic->asicname, ringname, (unsigned long)wptr >> 2,
asic->asicname, ringname, (unsigned long)drv_wptr >> 2);
+ if (enable_decoder) {
+ decoder.pm4.cur_opcode = 0xFFFFFFFF;
+ decoder.sdma.cur_opcode = 0xFFFFFFFF;
+ }
do {
value = ring_data[(start+12)>>2];
printf("%s.%s.ring[%s%4lu%s] == %s0x%08lx%s ",
asic->asicname, ringname,
BLUE, (unsigned long)start >> 2, RST,
YELLOW, (unsigned long)value, RST);
- if (enable_decoder) {
- decoder.pm4.cur_opcode = 0xFFFFFFFF;
- decoder.sdma.cur_opcode = 0xFFFFFFFF;
- }
printf(" %c%c%c ",
(start == rptr) ? 'r' : '.',
(start == wptr) ? 'w' : '.',
On 2019-10-23 5:11 p.m., Tuikov, Luben wrote:
> 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;
More information about the amd-gfx
mailing list