[PATCH] Ring range argument improvements (v2)
Tuikov, Luben
Luben.Tuikov at amd.com
Thu Oct 24 18:09:54 UTC 2019
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 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