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

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 04 Nov 2022 17:32:35 -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.

Hi Umesh,

I think it would be good to add to the commit message:

* Gitlab bug number if any
* Paste of the actual dmesg error in the commit message
* Also adapt the above commit message to the fact that we've now added the
  optimized 64 bit read

With that this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

If you want me to review the new commit message I can do that too.

Thanks.
--
Ashutosh


>
> 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
>