[igt-dev] [PATCH i-g-t v3] intel-gpu-top: Rewrite the tool to be safe to use

[Tvrtko Ursulin]

On 30/03/2018 20:15, Rinat Ibragimov wrote:
> 
> 
>> Четверг, 29 марта 2018, 21:46 +03:00 от Tvrtko Ursulin <tursulin at ursulin.net>:
>>
> 
>> +#define engine_ptr(engines, n) \
>> +((struct engine *)((unsigned char *)(&engines->engine) + (n) * sizeof(struct engine)))
> 
> I think (&engines->engine + (n)) is easier to read.

Absolutely agreed.

>> +if (fd < 0 && !cnt->optional)
>> +return -1;
> 
> I've tried to run it on Skylake on Linux 4.16, and intel_gpu_top is working, as long as
> I remove these lines. Otherwise it fails while trying "vcs1". Error message says about
> Linux 4.16, which is a bit confusing.
> 
> There are code that sets and tests "present" field of struct pmu_counter. So,
> I guess, it's fine to remove the code, and thus make all counters optional?
> 
>> +
>> +if (!cnt->present) {
>> +strncpy(buf, "---", bufsz);
>>   return;
>> +}
> 
> If you decide to make all counters optional, this will be used for "busy" numbers
> too. But "busy" is 6 characters wide, unlike "sema" and "wait", which are 3 each.

Yep I failed to implement this correctly. Fixed in v4 hopefully.

Definitely thanks a lot for this and previous feedback!

Regards,

Tvrtko