Re: [Intel-gfx] [PATCH] drm/i915/selftest: Bump up sample period for busy stats selftest

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Thu, Nov 03, 2022 at 10:01:11AM -0700, Dixit, Ashutosh wrote:
>On Wed, 02 Nov 2022 17:11:49 -0700, Umesh Nerlige Ramappa wrote:
>>
>> Engine busyness samples around a 10ms period is failing with busyness
>> ranging approx. from 87% to 115%. The expected range is +/- 5% of the
>> sample period.
>>
>> When determining busyness of active engine, the GuC based engine
>> busyness implementation relies on a 64 bit timestamp register read. The
>> latency incurred by this register read causes the failure.
>>
>> On DG1, when the test fails, the observed latencies range from 900us -
>> 1.5ms.
>>
>> One solution tried was to reduce the latency between reg read and
>> CPU timestamp capture, but such optimization does not add value to user
>> since the CPU timestamp obtained here is only used for (1) selftest and
>> (2) i915 rps implementation specific to execlist scheduler. Also, this
>> solution only reduces the frequency of failure and does not eliminate
>> it.
>>
>> In order to make the selftest more robust and account for such
>> latencies, increase the sample period to 100 ms.
>
>Does it make sense, and also by way of documenting, to use 10 ms for
>execlists and 100 ms for GuC? Maybe a comment in the code would be nice
>too. Thanks.

I was hoping to keep the same logic for execlist/guc backends. I can add 
it to the comments though.

sadly, this is the 2nd time we are bumping this up. This was originally 
100us for execlists. With the GuC backend, there is a latency by design 
since active busyness is calculated using GT timestamp register. 
Execlists relied solely on ktime_get() to check for active busyness and 
that seemed to have negligible latency. I see no robust option here.

Thanks,
Umesh

>
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/selftest_engine_pm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
>> index 0dcb3ed44a73..87c94314cf67 100644
>> --- a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
>> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
>> @@ -317,7 +317,7 @@ static int live_engine_busy_stats(void *arg)
>>		ENGINE_TRACE(engine, "measuring busy time\n");
>>		preempt_disable();
>>		de = intel_engine_get_busy_time(engine, &t[0]);
>> -		mdelay(10);
>> +		mdelay(100);
>>		de = ktime_sub(intel_engine_get_busy_time(engine, &t[1]), de);
>>		preempt_enable();
>>		dt = ktime_sub(t[1], t[0]);
>> --
>> 2.36.1
>>