diff mbox series

[1/2] drm/i915/selftest: Fix engine timestamp and ktime disparity

Message ID 20230223100503.3323627-2-anshuman.gupta@intel.com (mailing list archive)
State New, archived
Headers show
Series Selftest fixes | expand

Commit Message

Gupta, Anshuman Feb. 23, 2023, 10:05 a.m. UTC
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(-)

Comments

Nilawar, Badal March 1, 2023, 12:04 p.m. UTC | #1
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();
>   			}
diff mbox series

Patch

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();
 			}