[Intel-gfx] [PATCH] i915/selftest: Disable irq to calc eng timestamp

[Nilawar, Badal]

A couple of comments below, after addressing those this is:
Reviewed-by: Badal Nilawar <badal.nilawar at intel.com>

On 26-10-2021 19:10, Anshuman Gupta wrote:
> gt_pm selftest calculates engine ticks cycles and wall time
> cycles by delta of respective engine elapsed TIMESTAMP and ktime
> for period of 1000us.
> It compares the engine ticks cycles with wall time cycles.
>
> Disable local cpu interrupt so that interrupt handler
> should not preempt the measure_clocks() to calculate
> correct engine ticks cycles.
>
> Suggested-by: Chris P Wilson<chris.p.wilson at intel.com>
> Signed-off-by: Anshuman Gupta<anshuman.gupta at intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> index b9441217ca3d..9cf76398bdf5 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> @@ -43,6 +43,7 @@ static void measure_clocks(struct intel_engine_cs *engine,
>   	int i;
>   
>   	for (i = 0; i < 5; i++) {
> +		local_irq_disable();

How about saving interrupt state before disabling it.

Use local_irq_save here.

>   		preempt_disable();
>   		cycles[i] = -ENGINE_READ_FW(engine, RING_TIMESTAMP);
>   		dt[i] = ktime_get();
> @@ -52,6 +53,7 @@ static void measure_clocks(struct intel_engine_cs *engine,
>   		dt[i] = ktime_sub(ktime_get(), dt[i]);
>   		cycles[i] += ENGINE_READ_FW(engine, RING_TIMESTAMP);
>   		preempt_enable();
> +		local_irq_enable();
Use local_irq_restore here.
>   	}
>   
>   	/* Use the median of both cycle/dt; close enough */
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20211109/308d2407/attachment.htm>