Re: [PATCH] Ring range argument improvements (v2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tom,

Thanks for debugging it!

I'll reapply and resend it.

Regards,
Luben

On 2019-10-24 9:44 a.m., StDenis, Tom wrote:
> 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@xxxxxxx>
>> ---
>>   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;

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux