On 23/02/17 01:41 PM, Andres Rodriguez wrote:
>
>
> On 2017-02-23 09:46 AM, Tom St Denis wrote:
>> To simplify scripting you can now use * for ipnames as well.
>
> Thanks for the patch, this change is very welcome.
>
>>
>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>> ---
>> doc/umr.1 | 6 ++-
>> src/app/main.c | 5 ++-
>> src/app/scan.c | 21 ++++++++++
>> src/app/set_bit.c | 117
>> +++++++++++++++++++++++++++---------------------------
>> src/app/set_reg.c | 91 +++++++++++++++++++++---------------------
>> 5 files changed, 131 insertions(+), 109 deletions(-)
>>
>> diff --git a/doc/umr.1 b/doc/umr.1
>> index 5ed2c3bb93ae..a12dfb0a00ff 100644
>> --- a/doc/umr.1
>> +++ b/doc/umr.1
>> @@ -34,7 +34,7 @@ Look up an MMIO register by address and bitfield
>> decode the value specified.
>> Write a value specified in hex to a register specified with a complete
>> register path in the form <
>> .B asicname.ipname.regname
>> ->. For example, fiji.uvd6.mmUVD_CGC_GATE. The value of asicname can be
>> +>. For example, fiji.uvd6.mmUVD_CGC_GATE. The value of asicname
>> and/or ipname can be
>> .B *
>> to simplify scripting. This command can be used multiple times to
>> write to multiple registers in a single invocation.
>> @@ -52,7 +52,9 @@ command but also allows
>> for the regname field.
>> .IP "--scan, -s <string>"
>> Scan and print an IP block by name, for example,
>> -.B uvd5.
>> +.B uvd6
>> +or
>> +.B carrizo.uvd6.
>> Can be used multiple times in a single invocation.
>> .IP "--ring, -R <string>(from:to)"
>> Read the contents of a ring named by the string without the
>> diff --git a/src/app/main.c b/src/app/main.c
>> index 8364d54c5227..99e014888571 100644
>> --- a/src/app/main.c
>> +++ b/src/app/main.c
>> @@ -364,12 +364,13 @@ int main(int argc, char **argv)
>> "\n\t--write, -w <address> <number>\n\t\tWrite a value in hex to a
>> register specified as a register path in the"
>> "\n\t\tform <asicname.ipname.regname>. For instance
>> \"tonga.uvd5.mmUVD_SOFT_RESET\"."
>> "\n\t\tCan be used multiple times to set multiple registers. You
>> can"
>> - "\n\t\tspecify * for asicname to simplify scripts.\n"
>> + "\n\t\tspecify * for asicname and/or ipname to simplify scripts.\n"
>> "\n\t--writebit, -wb <string> <number>\n\t\tWrite a value in hex to a
>> register bitfield specified as in --write but"
>> "\n\t\tthe addition of the bitfield name. For instance:
>> \"*.gfx80.mmRLC_PG_CNTL.PG_OVERRIDE\"\n"
>> "\n\t--read, -r <string>\n\t\tRead a value from a register and print
>> it to stdout. This command"
>> "\n\t\tuses the same path notation as --write. It also accepts *
>> for regname.\n"
>> -"\n\t--scan, -s <string>\n\t\tScan and print an ip block by name,
>> e.g. \"uvd5\". Can be used multiple times.\n"
>> +"\n\t--scan, -s <string>\n\t\tScan and print an ip block by name,
>> e.g. \"uvd6\" or \"carrizo.uvd6\"."
>> + "\n\t\tCan be used multiple times.\n"
>> "\n\t--ring, -R <string>([from:to])\n\t\tRead the contents of a ring
>> named by the string without the amdgpu_ring_ prefix. "
>> "\n\t\tBy default it will read and display the entire ring. A
>> starting and ending "
>> "\n\t\taddress can be specified in decimal or a '.' can be used
>> to indicate relative "
>> diff --git a/src/app/scan.c b/src/app/scan.c
>> index 19c97fe1499d..fbb8f5838069 100644
>> --- a/src/app/scan.c
>> +++ b/src/app/scan.c
>> @@ -31,6 +31,27 @@ int umr_scan_asic(struct umr_asic *asic, char
>> *asicname, char *ipname, char *reg
>> uint64_t addr, scale;
>> char buf[256];
>>
>> + // if ipname == '*' scan for a match and stop on first one
>> + if (ipname[0] == '*' && regname[0] != '*') {
>> + for (i = 0; i < asic->no_blocks; i++) {
>> + for (j = 0; j < asic->blocks[i]->no_regs; j++) {
>> + if (!strcmp(regname,
>> asic->blocks[i]->regs[j].regname) ||
>> + (options.many &&
>> strstr(asic->blocks[i]->regs[j].regname, regname))) {
>> + ipname = asic->blocks[i]->ipname;
>> + goto ip_found;
>
> Since the star globs usually imply a wildcard match, I think it is a
> little weird to truncate the output to only the first match.
>
> If a star glob is requested by the user, I think it should be okay to
> loop and print multiple outputs as usually happens when options.many is
> enabled.
>
> Alternative approach would be to print an 'ambiguous argument' error.
> That way the user will at least have some feedback. Although I think the
> behaviour of printing multiple results is preferable.
Now that I think about it is possible that future asics have multiples
of IP blocks so ideally we should continue on for as many collisions.
I think the '-O named' option should then print the IP name as well.
I'll (v2) this shortly.
Thanks,
Tom
