[Intel-gfx] [PATCH] drm/i915: Stop sampling rc6 from inside pmu_enable

[Intel-gfx] [PATCH] drm/i915: Stop sampling rc6 from inside pmu_enable

[Chris Wilson]

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 <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 20 --------------------
 1 file changed, 20 deletions(-)

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)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Stop sampling rc6 from inside pmu_enable

[Chris Wilson]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Stop sampling rc6 from inside pmu_enable

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 2374 bytes --]

== Series Details ==

Series: drm/i915: Stop sampling rc6 from inside pmu_enable
URL   : https://patchwork.freedesktop.org/series/84549/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9438 -> Patchwork_19053
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19053/index.html

New tests
---------

  New tests have been introduced between CI_DRM_9438 and Patchwork_19053:

### New CI tests (1) ###

  * boot:
    - Statuses : 1 fail(s) 38 pass(s)
    - Exec time: [0.0] s

  

Known issues
------------

  Here are the changes found in Patchwork_19053 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@create-close:
    - fi-tgl-y:           [PASS][1] -> [DMESG-WARN][2] ([i915#402]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9438/fi-tgl-y/igt@gem_basic@create-close.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19053/fi-tgl-y/igt@gem_basic@create-close.html

  
#### Possible fixes ####

  * {igt@fbdev@read}:
    - fi-tgl-y:           [DMESG-WARN][3] ([i915#402]) -> [PASS][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9438/fi-tgl-y/igt@fbdev@read.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19053/fi-tgl-y/igt@fbdev@read.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402


Participating hosts (43 -> 39)
------------------------------

  Missing    (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u 


Build changes
-------------

  * Linux: CI_DRM_9438 -> Patchwork_19053

  CI-20190529: 20190529
  CI_DRM_9438: a07470999401550eadc26fc3954afbe5a39f753e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5880: d67bad61dc9a7515f94a7eecadd3bcd6b4f9d49e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19053: f80329b5088f43f7beb8922c83910595bce90772 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f80329b5088f drm/i915: Stop sampling rc6 from inside pmu_enable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19053/index.html

[-- Attachment #1.2: Type: text/html, Size: 3100 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Stop sampling rc6 from inside pmu_enable

[Chris Wilson]

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 and so much find another way to
calibrate the rc6 bias.

Having thought of no ideal solution, let's just always record the latest
rc6 measurement when parking, and so we will have an accurate value
should the user start measuring during an idle period -- at the cost of
an extra bunch of forcewaked register regs on every GT park.

Fortunately, we have changed the explicit forcewake holds to immediately
release rather than incur a timer delay and so it does not noticeable
increase power consumption for light testing.

Fixes: f4e9894b6952 ("drm/i915/pmu: Correct the rc6 offset upon enabling")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 38 +--------------------------------
 1 file changed, 1 insertion(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 97bb4aaa5236..2477068fffb5 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);
@@ -166,8 +161,6 @@ static u64 __get_rc6(struct intel_gt *gt)
 	return val;
 }
 
-#if IS_ENABLED(CONFIG_PM)
-
 static inline s64 ktime_since(const ktime_t kt)
 {
 	return ktime_to_ns(ktime_sub(ktime_get(), kt));
@@ -217,23 +210,10 @@ static void park_rc6(struct drm_i915_private *i915)
 {
 	struct i915_pmu *pmu = &i915->pmu;
 
-	if (pmu->enable & config_mask(I915_PMU_RC6_RESIDENCY))
-		pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
-
+	pmu->sample[__I915_SAMPLE_RC6].cur = __get_rc6(&i915->gt);
 	pmu->sleep_last = ktime_get();
 }
 
-#else
-
-static u64 get_rc6(struct intel_gt *gt)
-{
-	return __get_rc6(gt);
-}
-
-static void park_rc6(struct drm_i915_private *i915) {}
-
-#endif
-
 static void __i915_pmu_maybe_start_timer(struct i915_pmu *pmu)
 {
 	if (!pmu->timer_enabled && pmu_needs_timer(pmu, true)) {
@@ -655,15 +635,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 +653,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 +694,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)
@@ -1175,7 +1140,6 @@ void i915_pmu_register(struct drm_i915_private *i915)
 		&i915_pmu_cpumask_attr_group,
 		NULL
 	};
-
 	int ret = -ENOMEM;
 
 	if (INTEL_GEN(i915) <= 2) {
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Stop sampling rc6 from inside pmu_enable (rev2)

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 3165 bytes --]

== Series Details ==

Series: drm/i915: Stop sampling rc6 from inside pmu_enable (rev2)
URL   : https://patchwork.freedesktop.org/series/84549/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9439 -> Patchwork_19054
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_19054 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_19054, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19054/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_19054:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-snb-2600:        [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9439/fi-snb-2600/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19054/fi-snb-2600/igt@gem_exec_suspend@basic-s3.html

  
New tests
---------

  New tests have been introduced between CI_DRM_9439 and Patchwork_19054:

### New CI tests (1) ###

  * boot:
    - Statuses : 1 fail(s) 37 pass(s)
    - Exec time: [0.0] s

  

Known issues
------------

  Here are the changes found in Patchwork_19054 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@i915_selftest@live@execlists:
    - fi-kbl-x1275:       [INCOMPLETE][3] ([i915#1037] / [i915#794]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9439/fi-kbl-x1275/igt@i915_selftest@live@execlists.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19054/fi-kbl-x1275/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@gt_timelines:
    - fi-apl-guc:         [INCOMPLETE][5] -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9439/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19054/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html

  
  [i915#1037]: https://gitlab.freedesktop.org/drm/intel/issues/1037
  [i915#794]: https://gitlab.freedesktop.org/drm/intel/issues/794


Participating hosts (42 -> 38)
------------------------------

  Additional (1): fi-tgl-y 
  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_9439 -> Patchwork_19054

  CI-20190529: 20190529
  CI_DRM_9439: 17ead179a640cc74313ebdd72c51eee3d2d803d6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5880: d67bad61dc9a7515f94a7eecadd3bcd6b4f9d49e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19054: bc6f455af929894c1be333a41cd397e3c5232829 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bc6f455af929 drm/i915: Stop sampling rc6 from inside pmu_enable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19054/index.html

[-- Attachment #1.2: Type: text/html, Size: 3873 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx