From 5c1a4a5271f8a5650f9fd03fd44426f915a69aca Mon Sep 17 00:00:00 2001
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Date: Fri, 29 Nov 2019 10:54:36 +0000
Subject: drm/i915/pmu: Report frequency as zero while GPU is sleeping
Git-commit: b66ecd0438bf6079d08e8f2afea6114f468f56f4
Patch-mainline: v5.6-rc1
References: jsc#SLE-12680, jsc#SLE-12880, jsc#SLE-12882, jsc#SLE-12883, jsc#SLE-13496, jsc#SLE-15322

We used to report the minimum possible frequency as both requested and
active while GPU was in sleep state. This was a consequence of sampling
the value from the "current frequency" field in our software tracking.

This was strictly speaking wrong, but given that until recently the
current frequency in sleeping state used to be equal to minimum, it did
not stand out sufficiently to be noticed as such.

After some recent changes have made the current frequency be reported
as last active before GPU went to sleep, meaning both requested and active
frequencies could end up being reported at their maximum values for the
duration of the GPU idle state, it became much more obvious that this does
not make sense.

To fix this we will now sample the frequency counters only when the GPU is
awake. As a consequence reported frequencies could be reported as below
the GPU reported minimum but that should be much less confusing that the
current situation.

v2:
 * Split out early exit conditions for readability. (Chris)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Closes: https://gitlab.freedesktop.org/drm/intel/issues/675
Link: https://patchwork.freedesktop.org/patch/msgid/20191129105436.20100-1-tvrtko.ursulin@linux.intel.com
Signed-off-by: Patrik Jakobsson <pjakobsson@suse.de>
---
 drivers/gpu/drm/i915/i915_pmu.c | 52 ++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e16ef367b209..bdc6348e6468 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -337,6 +337,13 @@ add_sample_mult(struct i915_pmu_sample *sample, u32 val, u32 mul)
 	sample->cur += mul_u32_u32(val, mul);
 }
 
+static bool frequency_sampling_enabled(struct i915_pmu *pmu)
+{
+	return pmu->enable &
+	       (config_enabled_mask(I915_PMU_ACTUAL_FREQUENCY) |
+		config_enabled_mask(I915_PMU_REQUESTED_FREQUENCY));
+}
+
 static void
 frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 {
@@ -345,32 +352,33 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 	struct i915_pmu *pmu = &i915->pmu;
 	struct intel_rps *rps = &gt->rps;
 
+	if (!frequency_sampling_enabled(pmu))
+		return;
+
+	/* Report 0/0 (actual/requested) frequency while parked. */
+	if (!intel_gt_pm_get_if_awake(gt))
+		return;
+
 	if (pmu->enable & config_enabled_mask(I915_PMU_ACTUAL_FREQUENCY)) {
 		u32 val;
 
-		val = rps->cur_freq;
-		if (intel_gt_pm_get_if_awake(gt)) {
-			u32 stat;
-
-			/*
-			 * We take a quick peek here without using forcewake
-			 * so that we don't perturb the system under observation
-			 * (forcewake => !rc6 => increased power use). We expect
-			 * that if the read fails because it is outside of the
-			 * mmio power well, then it will return 0 -- in which
-			 * case we assume the system is running at the intended
-			 * frequency. Fortunately, the read should rarely fail!
-			 */
-			stat = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
-			if (stat)
-				val = intel_get_cagf(rps, stat);
-
-			intel_gt_pm_put(gt);
-		}
+		/*
+		 * We take a quick peek here without using forcewake
+		 * so that we don't perturb the system under observation
+		 * (forcewake => !rc6 => increased power use). We expect
+		 * that if the read fails because it is outside of the
+		 * mmio power well, then it will return 0 -- in which
+		 * case we assume the system is running at the intended
+		 * frequency. Fortunately, the read should rarely fail!
+		 */
+		val = intel_uncore_read_fw(uncore, GEN6_RPSTAT1);
+		if (val)
+			val = intel_get_cagf(rps, val);
+		else
+			val = rps->cur_freq;
 
 		add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT],
-				intel_gpu_freq(rps, val),
-				period_ns / 1000);
+				intel_gpu_freq(rps, val), period_ns / 1000);
 	}
 
 	if (pmu->enable & config_enabled_mask(I915_PMU_REQUESTED_FREQUENCY)) {
@@ -378,6 +386,8 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns)
 				intel_gpu_freq(rps, rps->cur_freq),
 				period_ns / 1000);
 	}
+
+	intel_gt_pm_put_async(gt);
 }
 
 static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
-- 
2.28.0