Re: [Intel-gfx] [PATCH 1/2] drm/i915/selftest: Fix engine timestamp and ktime disparity

From: "Nilawar, Badal" <badal.nilawar@intel.com>
To: Anshuman Gupta <anshuman.gupta@intel.com>,
	<intel-gfx@lists.freedesktop.org>
Cc: Chris Wilson <chris.p.wilson@intel.com>
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/selftest: Fix engine timestamp and ktime disparity
Date: Wed, 1 Mar 2023 17:34:05 +0530	[thread overview]
Message-ID: <431c924a-0584-6871-c57d-c6a43ed2b206@intel.com> (raw)
In-Reply-To: <20230223100503.3323627-2-anshuman.gupta@intel.com>

LGTM

Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>

On 23-02-2023 15:35, Anshuman Gupta wrote:
> While reading the engine timestamps there can be uncontrollable
> concurrent mmio access via other i915 child drivers and by GuC,
> which is not truly atomic context as expected by this selftest,
> which may cause mmio latency to read the engine timestamps,
> Account such latency to calculate time to read engine timestamp
> such that selftest can validate the timestamp and ktime pair.
> 
> Cc: Chris Wilson <chris.p.wilson@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 2 +-
>   drivers/gpu/drm/i915/gt/selftest_rps.c   | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> index b46425aeb2f0..0971241707ce 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
> @@ -63,8 +63,8 @@ static void measure_clocks(struct intel_engine_cs *engine,
>   
>   		udelay(1000);
>   
> -		dt[i] = ktime_sub(ktime_get(), dt[i]);
>   		cycles[i] += read_timestamp(engine);
> +		dt[i] = ktime_sub(ktime_get(), dt[i]);
>   		local_irq_enable();
>   	}
>   
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index 6755bbc4ebda..c0cc0dd78c7c 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -299,13 +299,13 @@ int live_rps_clock_interval(void *arg)
>   			for (i = 0; i < 5; i++) {
>   				preempt_disable();
>   
> -				dt_[i] = ktime_get();
>   				cycles_[i] = -intel_uncore_read_fw(gt->uncore, GEN6_RP_CUR_UP_EI);
> +				dt_[i] = ktime_get();
>   
>   				udelay(1000);
>   
> -				dt_[i] = ktime_sub(ktime_get(), dt_[i]);
>   				cycles_[i] += intel_uncore_read_fw(gt->uncore, GEN6_RP_CUR_UP_EI);
> +				dt_[i] = ktime_sub(ktime_get(), dt_[i]);
>   
>   				preempt_enable();
>   			}