[igt-dev] [PATCH i-g-t 4/4] lib/i915/perf: don't forget last timeline element

[Lionel Landwerlin]

On 07/01/2021 03:07, Umesh Nerlige Ramappa wrote:
> On Mon, Dec 28, 2020 at 05:19:40AM +0200, Lionel Landwerlin wrote:
>> We're currently dropping the last element of the timeline.
>
> Inside the for-loop we append timeline events on context switches 
> indicating what context ran and for how long, but that's not the case 
> for the append_timeline_event outside the for-loop.
>
> Does the one outside indicate how long last_ctx_id ran before capture 
> ended (or we ran out of records)? Is it possible for the user to 
> distinguish the two? Just thinking out loud.


Let's say you have the following reports


report0 ctx=1

report1 ctx=2

report2 ctx=1


That generates 2 timeline events (report1 - report0 for ctx1), (report2 
- report1 for ctx2).

Those 2 are generated within the for loop because it notices a change in 
ctx value twice.


Now this :


report0 ctx=1

report1 ctx=2

report2 ctx=1

report3 ctx=1


This also generates 2 timeline events within the for loop.

But because the last report3 has the same ctx as report2, there is no 
additional timeline event generated for ctx1 within the for loop.

And we just quit the loop because we ran out of reports.

So we need to add one more item.


The gpu_ts_start & gpu_ts_end being generated before the continue; in 
the for loop, make the last append_timeline_event() add the right 
timestamps after the for loop for that last timeline.


-Lionel


>
> Otherwise:
>
> Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>
> Thanks,
> Umesh
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Fixes: 43116ee368585d ("lib/i915-perf: add i915 perf data reader")
>> ---
>> lib/i915/perf_data_reader.c | 27 ++++++++++++++++++---------
>> 1 file changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/i915/perf_data_reader.c b/lib/i915/perf_data_reader.c
>> index 4b68fb502..065fe6066 100644
>> --- a/lib/i915/perf_data_reader.c
>> +++ b/lib/i915/perf_data_reader.c
>> @@ -298,17 +298,23 @@ static void
>> generate_cpu_events(struct intel_perf_data_reader *reader)
>> {
>>     uint32_t last_header_idx = 0;
>> -    const struct drm_i915_perf_record_header *last_header = 
>> reader->records[0];
>> +    const struct drm_i915_perf_record_header *last_header = 
>> reader->records[0],
>> +        *current_header = reader->records[0];
>> +    const uint8_t *start_report, *end_report;
>> +    uint32_t last_ctx_id, current_ctx_id;
>> +    uint64_t gpu_ts_start, gpu_ts_end;
>>
>>     for (uint32_t i = 1; i < reader->n_records; i++) {
>> -        const struct drm_i915_perf_record_header *current_header =
>> -            reader->records[i];
>> -        const uint8_t *start_report = (const uint8_t *) (last_header 
>> + 1),
>> -            *end_report = (const uint8_t *) (current_header + 1);
>> -        uint32_t last_ctx_id = oa_report_ctx_id(&reader->devinfo, 
>> start_report),
>> -            current_ctx_id = oa_report_ctx_id(&reader->devinfo, 
>> end_report);
>> -        uint64_t gpu_ts_start = oa_report_timestamp(start_report),
>> -            gpu_ts_end = oa_report_timestamp(end_report);
>> +        current_header = reader->records[i];
>> +
>> +        start_report = (const uint8_t *) (last_header + 1);
>> +        end_report = (const uint8_t *) (current_header + 1);
>> +
>> +        last_ctx_id = oa_report_ctx_id(&reader->devinfo, start_report);
>> +        current_ctx_id = oa_report_ctx_id(&reader->devinfo, 
>> end_report);
>> +
>> +        gpu_ts_start = oa_report_timestamp(start_report);
>> +        gpu_ts_end = oa_report_timestamp(end_report);
>>
>>         if (last_ctx_id == current_ctx_id)
>>             continue;
>> @@ -318,6 +324,9 @@ generate_cpu_events(struct intel_perf_data_reader 
>> *reader)
>>         last_header = current_header;
>>         last_header_idx = i;
>>     }
>> +
>> +    if (last_header != current_header)
>> +        append_timeline_event(reader, gpu_ts_start, gpu_ts_end, 
>> last_header_idx, reader->n_records - 1, last_ctx_id);
>> }
>>
>> static void
>> -- 
>> 2.30.0.rc2
>>