[Intel-gfx] [PATCH i-g-t 1/2] tests/perf_pmu: Verify busyness when PMU is enabled after engine got busy

[Tvrtko Ursulin]

On 09/01/2018 21:28, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-01-09 16:16:20)
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Make sure busyness is correctly reported when PMU is enabled after the
>> engine is already busy with a single long batch.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   tests/perf_pmu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 42 insertions(+)
>>
>> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
>> index 45e2f6148453..e1f449d48808 100644
>> --- a/tests/perf_pmu.c
>> +++ b/tests/perf_pmu.c
>> @@ -157,6 +157,41 @@ single(int gem_fd, const struct intel_execution_engine2 *e, bool busy)
>>          gem_quiescent_gpu(gem_fd);
>>   }
>>   
>> +static void
>> +busy_start(int gem_fd, const struct intel_execution_engine2 *e)
>> +{
>> +       unsigned long slept;
>> +       igt_spin_t *spin;
>> +       uint64_t val;
>> +       int fd;
>> +
>> +       /*
>> +        * Defeat the busy stats delayed disable, we need to guarantee we are
>> +        * the first user.
>> +        */
>> +       if (gem_has_execlists(gem_fd))
>> +               sleep(2);
> 
> I don't have a better idea than sleep, but I don't like tying this to
> execlists. Make the sleep unconditional for now. Is there anyway we can
> export the knowledge of the implementation through the perf api?
> Different counters, or now different attrs for different busy-stats?

I can't come up with any reasonable idea. Best hope I have is that we 
could also expose busyness via sysfs one day and then I would move this 
test out of the PMU specific ones to a new home.

To sleep unconditionally I am also torn because the less backend 
knowledge and variability in the tests the better, but also I would 
prefer not to waste time when not necessary.

>> +
>> +       spin = __igt_spin_batch_new(gem_fd, 0, e2ring(gem_fd, e), 0);
>> +
>> +       /*
>> +        * Sleep for a bit after making the engine busy to make sure the PMU
>> +        * gets enabled when the batch is already running.
>> +        */
>> +       usleep(500000);
> 
> Just a request for 500e3.
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

Thanks! Shard run shows it is catching the current issue fine.

Do you plan to respin your fix so the other subtest passes as well?

Regards,

Tvrtko