[PATCH] Ring range argument improvements (v3)

StDenis, Tom Tom.StDenis at amd.com
Thu Oct 24 18:31:42 UTC 2019


Thanks, pushed out.


Cheers,

Tom

On 2019-10-24 2:27 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".
> v3: Fix decoding.
>
> Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>
> ---
>   doc/umr.1           | 33 +++++++++++++++++--------
>   src/app/ring_read.c | 59 +++++++++++++++++++++++++++------------------
>   2 files changed, 59 insertions(+), 33 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..b995188 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",
> @@ -107,23 +120,23 @@ 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 && start == rptr && start != wptr) {
> -			use_decoder = 1;
> -			decoder.pm4.cur_opcode = 0xFFFFFFFF;
> -			decoder.sdma.cur_opcode = 0xFFFFFFFF;
> -		}
>   		printf(" %c%c%c ",
>   			(start == rptr) ? 'r' : '.',
>   			(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