Blob Blame History Raw
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Date: Fri, 24 Nov 2017 09:49:59 +0000
Subject: drm/i915/pmu: Stop averaging with the previous sample
Git-commit: 8ee4f19c47031f23340055da4d9f2af537de23f4
Patch-mainline: v4.16-rc1
References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166

Averaging with the previous sample brings a small statistical improvement
to sampling counters, but can leek a little bit of state from a current
client to the next which mulls the border between past and present for
observing clients.

This is because on event enable clients record the current counter value
and use it as reference, but with rapid off-on event cycles, and due the
delayed nature of sampling timer self-disarm, previous sample value does
not get cleared under these circumstances.

Solution is to stop averaging with the previous sample. This has a small
downside of losing some precision with short and spiky signals, but the
alternatives look too complicated for the benefit.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171124094959.10725-1-tvrtko.ursulin@linux.intel.com

Acked-by: Petr Tesarik <ptesarik@suse.com>
---
 drivers/gpu/drm/i915/i915_pmu.c |   29 ++---------------------------
 drivers/gpu/drm/i915/i915_pmu.h |    1 -
 2 files changed, 2 insertions(+), 28 deletions(-)

--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -181,12 +181,7 @@ static bool grab_forcewake(struct drm_i9
 static void
 update_sample(struct i915_pmu_sample *sample, u32 unit, u32 val)
 {
-	/*
-	 * Since we are doing stochastic sampling for these counters,
-	 * average the delta with the previous value for better accuracy.
-	 */
-	sample->cur += div_u64(mul_u32_u32(sample->prev + val, unit), 2);
-	sample->prev = val;
+	sample->cur += mul_u32_u32(val, unit);
 }
 
 static void engines_sample(struct drm_i915_private *dev_priv)
@@ -262,31 +257,13 @@ static void frequency_sample(struct drm_
 	}
 }
 
-static void pmu_init_previous_samples(struct drm_i915_private *i915)
-{
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-	unsigned int i;
-
-	for_each_engine(engine, i915, id) {
-		for (i = 0; i < ARRAY_SIZE(engine->pmu.sample); i++)
-			engine->pmu.sample[i].prev = 0;
-	}
-
-	for (i = 0; i < ARRAY_SIZE(i915->pmu.sample); i++)
-		i915->pmu.sample[i].prev = i915->gt_pm.rps.idle_freq;
-}
-
 static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
 {
 	struct drm_i915_private *i915 =
 		container_of(hrtimer, struct drm_i915_private, pmu.timer);
 
-	if (!READ_ONCE(i915->pmu.timer_enabled)) {
-		pmu_init_previous_samples(i915);
-
+	if (!READ_ONCE(i915->pmu.timer_enabled))
 		return HRTIMER_NORESTART;
-	}
 
 	engines_sample(i915);
 	frequency_sample(i915);
@@ -857,8 +834,6 @@ void i915_pmu_register(struct drm_i915_p
 	hrtimer_init(&i915->pmu.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	i915->pmu.timer.function = i915_sample;
 
-	pmu_init_previous_samples(i915);
-
 	for_each_engine(engine, i915, id)
 		INIT_DELAYED_WORK(&engine->pmu.disable_busy_stats,
 				  __disable_busy_stats);
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -41,7 +41,6 @@ enum {
 
 struct i915_pmu_sample {
 	u64 cur;
-	u32 prev;
 };
 
 struct i915_pmu {