[Intel-gfx] [igt-dev] [RFC i-g-t] intel-gpu-top: Rewrite the tool to be safe to use

[Tvrtko Ursulin]

On 28/03/2018 21:11, Rinat Ibragimov wrote:
> 
>> Среда, 28 марта 2018, 21:30 +03:00 от Tvrtko Ursulin <tursulin at ursulin.net>:
>>
> 
>> +static struct engines *discover_engines(void)
>>   {
>> -uint32_t devid = pci_dev->device_id;
>> -uint16_t gcfgc;
>> +const char *sysfs_root = "/sys/devices/i915/events";
> 
> Just a question.
> I think, I have Linux 4.15.11 (from Debian testing) now. And there are no such files.
> Are there any estimates about when this feature is expected to be available?

4.17 I think. I could make it work with 4.16 as well, if there would be 
demand, just need to ignore counters enumerated in sysfs but not 
actually present in hardware.

>> -static void ring_init(struct ring *ring)
>> +static uint64_t pmu_read_multi(int fd, unsigned int num, uint64_t *val)
>>   {
>> -ring->size = (((ring_read(ring, RING_LEN) & RING_NR_PAGES) >> 12) + 1) * 4096;
>> +uint64_t buf[2 + num];
>> +unsigned int i;
>> +
>> +assert(read(fd, buf, sizeof(buf)) == sizeof(buf));
> 
> Will have undesired effects with NDEBUG.
> 
>> -int full;
>> +uint64_t data[2];
>>   
>> -if (!ring->size)
>> -return;
>> +assert(read(fd, data, sizeof(data)) == sizeof(data));
> 
> Same here.

Thanks, got a bit disconnected from userspace development patterns over 
the years.

> 
>> +/* Get terminal size. */
>> +if (ioctl(0, TIOCGWINSZ, &ws) != -1) {
>> +con_w = ws.ws_col;
>> +con_h = ws.ws_row;
>> +}
> 
> If you move this into the loop itself, the tool will adapt to changes in
> terminal width and height dynamically.

Makes sense, done in v2.

Regards,

Tvrtko