drm/i915: Stop sampling rc6 from inside pmu_enable

Commit Message

Chris Wilson Dec. 3, 2020, 10:38 p.m. UTC

Since rc6 is sampling the device registers, we try to acquire the device
wakeref. However, since i915_pmu_enable may be called from hardirq, we
cannot actually wake the device up. So let's not, and hope our gt wake
tracking is accurate.

Signed-off-by: Chris Wilson <[e-mail]>
Cc: Tvrtko Ursulin <[e-mail]>
---
 drivers/gpu/drm/i915/i915_pmu.c | 20 --------------------
 1 file changed, 20 deletions(-)

Comments

Chris Wilson Dec. 3, 2020, 10:48 p.m. UTC | #1

Quoting Chris Wilson (2020-12-03 22:38:49)
> Since rc6 is sampling the device registers, we try to acquire the device
> wakeref. However, since i915_pmu_enable may be called from hardirq, we
> cannot actually wake the device up. So let's not, and hope our gt wake
> tracking is accurate.

    drm/i915/pmu: Correct the rc6 offset upon enabling

    The rc6 residency starts ticking from 0 from BIOS POST, but the kernel
    starts measuring the time from its boot. If we start measuruing
    I915_PMU_RC6_RESIDENCY while the GT is idle, we start our sampling from
    0 and then upon first activity (park/unpark) add in all the rc6
    residency since boot. After the first park with the sampler engaged, the
    sleep/active counters are aligned.

    v2: With a wakeref to be sure

    Closes: https://gitlab.freedesktop.org/drm/intel/issues/973

Hmm. Take 2.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 97bb4aaa5236..b3df73ba1cb0 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -103,11 +103,6 @@  static unsigned int event_bit(struct perf_event *event)
 	return config_bit(event->attr.config);
 }
 
-static bool event_read_needs_wakeref(const struct perf_event *event)
-{
-	return event->attr.config == I915_PMU_RC6_RESIDENCY;
-}
-
 static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
 {
 	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
@@ -655,15 +650,10 @@  static void i915_pmu_enable(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
-	bool need_wakeref = event_read_needs_wakeref(event);
 	struct i915_pmu *pmu = &i915->pmu;
-	intel_wakeref_t wakeref = 0;
 	unsigned long flags;
 	unsigned int bit;
 
-	if (need_wakeref)
-		wakeref = intel_runtime_pm_get(&i915->runtime_pm);
-
 	bit = event_bit(event);
 	if (bit == -1)
 		goto update;
@@ -678,13 +668,6 @@  static void i915_pmu_enable(struct perf_event *event)
 	GEM_BUG_ON(bit >= ARRAY_SIZE(pmu->enable_count));
 	GEM_BUG_ON(pmu->enable_count[bit] == ~0);
 
-	if (pmu->enable_count[bit] == 0 &&
-	    config_mask(I915_PMU_RC6_RESIDENCY) & BIT_ULL(bit)) {
-		pmu->sample[__I915_SAMPLE_RC6_LAST_REPORTED].cur = 0;
-		pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
-		pmu->sleep_last = ktime_get();
-	}
-
 	pmu->enable |= BIT_ULL(bit);
 	pmu->enable_count[bit]++;
 
@@ -726,9 +709,6 @@  static void i915_pmu_enable(struct perf_event *event)
 	 * an existing non-zero value.
 	 */
 	local64_set(&event->hw.prev_count, __i915_pmu_event_read(event));
-
-	if (wakeref)
-		intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
 static void i915_pmu_disable(struct perf_event *event)