[PATCH i-g-t] intel_reg: Stop warning if register spec is not there

[Jani Nikula]

On Thu, 11 Apr 2024, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
> On Thu, Apr 11, 2024 at 11:53:03AM +0300, Jani Nikula wrote:
>>On Wed, 10 Apr 2024, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>>> Stop suffering when using igt from the git checkout, which is the case
>>> for all/most developers.
>>>
>>> 	$ sudo ./build/tools/intel_reg write 0x2358 0xdeadbeef
>>> 	Warning: stat '/usr/local/share/igt-gpu-tools/registers' failed: No such file or directory. Using builtin register spec.
>>>
>>> Pretty annoying when you have a bunch of commands to run and don't want
>>> to simply ignore stderr.  Keep the other warnings as if the file is
>>> there and can't be read for some reason, it's good to give a warning.
>>> However if the file simply isn't there, let it pass.
>>
>>The TL;DR is,
>>
>>Acked-by: Jani Nikula <jani.nikula at intel.com>
>>
>>---
>>
>>We have the "register specs" in plain text files so that anyone could
>>update them at runtime without building the source. And conceivably they
>>could be generated from a more authoritative source. It would be nice if
>>we could incorporate the info to the binary at build time as the builtin
>>register specs (so no data dir lookups at all), but also be able to
>>override them e.g. using a command-line parameter if the user wishes.
>>
>>I don't think that would even be so hard. But it would arguably be a bit
>>wasted effort as long as the register files under tools/registers/
>>aren't updated for newer platforms. (The current builtin register specs
>>are completely out of date.)
>
> I don't really get why one would want the tools/registers actually. When
> I'm using intel_reg I'm already working on a very low level interface
> with the hardware and can simply pass/read the raw offsets.

Something we wanted but never materialized was decoding register
contents, like the builtin decoding does. But I think with the pace of
registers added and meanings changing from platform to platform, etc,
with no automated import of the specs, it was just too laborous by hand.

And there's also the dumping, and it's kind of platform specific what
should be included. I think that's where the tools/registers files
originated.

BR,
Jani.




>
> I think a reg spec like that is nice when we are e.g. tracing, so we
> don't have to lookup all the registers that are showing up. I think
> Gustavo had something wip related to that. However for intel_reg I don't
> have a use case.
>
> Lucas De Marchi
>
>>
>>The tool could use some love again. It's been almost 10 years since
>>commit dfda0b6aecce ("intel_reg: introduce one intel_reg tool to rule
>>them all").
>>
>>BR,
>>Jani.
>>
>>>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>> ---
>>>  tools/intel_reg.c | 6 +-----
>>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
>>> index 6c37e14d1..ec311a05a 100644
>>> --- a/tools/intel_reg.c
>>> +++ b/tools/intel_reg.c
>>> @@ -1114,12 +1114,8 @@ static int read_reg_spec(struct config *config)
>>>  		path = IGT_DATADIR"/registers";
>>>
>>>  	r = stat(path, &st);
>>> -	if (r) {
>>> -		fprintf(stderr, "Warning: stat '%s' failed: %s. "
>>> -			"Using builtin register spec.\n",
>>> -			path, strerror(errno));
>>> +	if (r)
>>>  		goto builtin;
>>> -	}
>>>
>>>  	if (S_ISDIR(st.st_mode)) {
>>>  		r = get_reg_spec_file(buf, sizeof(buf), path, config->devid);
>>
>>-- 
>>Jani Nikula, Intel

-- 
Jani Nikula, Intel