[igt-dev] [PATCH 4/5] tools/i915-perf: Add mmapped OA buffer support to i915-perf-recorder

[Umesh Nerlige Ramappa]

On Mon, Aug 23, 2021 at 06:45:22PM -0700, Dixit, Ashutosh wrote:
>On Tue, 03 Aug 2021 13:07:36 -0700, Umesh Nerlige Ramappa wrote:
>>
>> +	/*
>> +	 * We do not have permissions to update the OA HEAD register, so we
>> +	 * would end up with a buffer lost error once the OA buffer fills up. To
>
>s/buffer lost/data lost/ ?
>
>> +	 * avoid that, drain the OA buffer into a zero mem device. The drain
>> +	 * eventually updates the head register in i915.
>> +	 */
>> +	while (read(ctx->perf_fd, ctx->zero_mem, ctx->oa_buffer_size) > 0 || errno == EINTR);
>
>Also this looks strange that in this mmap mode userspace cannot update the
>head and it must re-read the data using read() which it has already
>previously read from the mmap'd buffer. Is this an acceptable interface and
>has it been reviewed? Maybe when the OA buffer is mmap'd i915 should not
>report the data lost error (and silently clear it) and leave it to
>userspace to manage this? And anyway I think the mmap and the read() modes
>are mutually exclusive so the data lost error will be seen only in the
>read() mode not in the mmap mode correct? So I don't understand why this
>drain operation needs to be done. Thanks.

Looks like I can do away with the drain and things still work the same 
way. I was trying to resolve the buffer overflow error, but didn't think 
that we can just ignore the error in the mmap case. Since this code 
already maintains a head pointer and reads the buffer periodically, we 
should be good.

Thanks,
Umesh