All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
@ 2023-05-12  1:53 ` Ashutosh Dixit
  0 siblings, 0 replies; 8+ messages in thread
From: Ashutosh Dixit @ 2023-05-12  1:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Tvrtko Ursulin, Umesh Nerlige Ramappa, dri-devel

pmu_needs_timer() keeps the timer running even when GT is parked,
ostensibly to sample requested/actual frequencies. However
frequency_sample() has the following:

	/* Report 0/0 (actual/requested) frequency while parked. */
	if (!intel_gt_pm_get_if_awake(gt))
		return;

The above code prevents frequencies to be sampled while the GT is
parked. So we might as well turn off the sampling timer itself in this
case and save CPU cycles/power.

Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 7ece883a7d956..8db1d681cf4ab 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -124,11 +124,14 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
 		  ENGINE_SAMPLE_MASK;
 
 	/*
-	 * When the GPU is idle per-engine counters do not need to be
-	 * running so clear those bits out.
+	 * When GPU is idle, frequency or per-engine counters do not need
+	 * to be running so clear those bits out.
 	 */
-	if (!gpu_active)
-		enable &= ~ENGINE_SAMPLE_MASK;
+	if (!gpu_active) {
+		enable &= ~(config_mask(I915_PMU_ACTUAL_FREQUENCY) |
+			    config_mask(I915_PMU_REQUESTED_FREQUENCY) |
+			    ENGINE_SAMPLE_MASK);
+	}
 	/*
 	 * Also there is software busyness tracking available we do not
 	 * need the timer for I915_SAMPLE_BUSY counter.
-- 
2.38.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
@ 2023-05-12  1:53 ` Ashutosh Dixit
  0 siblings, 0 replies; 8+ messages in thread
From: Ashutosh Dixit @ 2023-05-12  1:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

pmu_needs_timer() keeps the timer running even when GT is parked,
ostensibly to sample requested/actual frequencies. However
frequency_sample() has the following:

	/* Report 0/0 (actual/requested) frequency while parked. */
	if (!intel_gt_pm_get_if_awake(gt))
		return;

The above code prevents frequencies to be sampled while the GT is
parked. So we might as well turn off the sampling timer itself in this
case and save CPU cycles/power.

Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 7ece883a7d956..8db1d681cf4ab 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -124,11 +124,14 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
 		  ENGINE_SAMPLE_MASK;
 
 	/*
-	 * When the GPU is idle per-engine counters do not need to be
-	 * running so clear those bits out.
+	 * When GPU is idle, frequency or per-engine counters do not need
+	 * to be running so clear those bits out.
 	 */
-	if (!gpu_active)
-		enable &= ~ENGINE_SAMPLE_MASK;
+	if (!gpu_active) {
+		enable &= ~(config_mask(I915_PMU_ACTUAL_FREQUENCY) |
+			    config_mask(I915_PMU_REQUESTED_FREQUENCY) |
+			    ENGINE_SAMPLE_MASK);
+	}
 	/*
 	 * Also there is software busyness tracking available we do not
 	 * need the timer for I915_SAMPLE_BUSY counter.
-- 
2.38.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
  2023-05-12  1:53 ` [Intel-gfx] " Ashutosh Dixit
  (?)
@ 2023-05-12  2:55 ` Patchwork
  -1 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-05-12  2:55 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 6483 bytes --]

== Series Details ==

Series: drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
URL   : https://patchwork.freedesktop.org/series/117658/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13140 -> Patchwork_117658v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (41 -> 39)
------------------------------

  Missing    (2): fi-kbl-soraka fi-snb-2520m 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-apl-guc:         [PASS][1] -> [DMESG-FAIL][2] ([i915#5334])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-2:         [PASS][3] -> [ABORT][4] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#7981] / [i915#8347])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-rpls-2/igt@i915_selftest@live@reset.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-rpls-2/igt@i915_selftest@live@reset.html

  * igt@i915_selftest@live@slpc:
    - bat-atsm-1:         [PASS][5] -> [INCOMPLETE][6] ([i915#7913])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-atsm-1/igt@i915_selftest@live@slpc.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-atsm-1/igt@i915_selftest@live@slpc.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence:
    - bat-adlp-9:         NOTRUN -> [SKIP][7] ([i915#3546]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-adlp-9/igt@kms_pipe_crc_basic@read-crc-frame-sequence.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rplp-1:         NOTRUN -> [ABORT][8] ([i915#4579] / [i915#8260])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-rplp-1/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_engines:
    - bat-atsm-1:         [FAIL][9] ([i915#6268]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-atsm-1/igt@i915_selftest@live@gt_engines.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-atsm-1/igt@i915_selftest@live@gt_engines.html

  * igt@i915_selftest@live@requests:
    - {bat-mtlp-6}:       [ABORT][11] ([i915#4983] / [i915#7920] / [i915#7953]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-mtlp-6/igt@i915_selftest@live@requests.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-mtlp-6/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@slpc:
    - {bat-mtlp-8}:       [DMESG-WARN][13] ([i915#6367] / [i915#7953]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-mtlp-8/igt@i915_selftest@live@slpc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-mtlp-8/igt@i915_selftest@live@slpc.html
    - bat-rpls-1:         [DMESG-WARN][15] ([i915#6367] / [i915#7953]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-rpls-1/igt@i915_selftest@live@slpc.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1:
    - bat-dg2-8:          [FAIL][17] ([i915#7932]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html

  
#### Warnings ####

  * igt@kms_psr@primary_mmap_gtt:
    - bat-rplp-1:         [ABORT][19] -> [SKIP][20] ([i915#1072])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/bat-rplp-1/igt@kms_psr@primary_mmap_gtt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/bat-rplp-1/igt@kms_psr@primary_mmap_gtt.html

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

  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#7953]: https://gitlab.freedesktop.org/drm/intel/issues/7953
  [i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
  [i915#8260]: https://gitlab.freedesktop.org/drm/intel/issues/8260
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347


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

  * Linux: CI_DRM_13140 -> Patchwork_117658v1

  CI-20190529: 20190529
  CI_DRM_13140: 42375f267a2451a1b2a1d5dc753785d7f1b7d9b3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7285: d1cbf2bad9c2664ab8bd3bd0946510a52800912f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117658v1: 42375f267a2451a1b2a1d5dc753785d7f1b7d9b3 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

e24e4e2bbd5f drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked

== Logs ==

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

[-- Attachment #2: Type: text/html, Size: 7401 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
  2023-05-12  1:53 ` [Intel-gfx] " Ashutosh Dixit
@ 2023-05-12  8:59   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2023-05-12  8:59 UTC (permalink / raw)
  To: Ashutosh Dixit, intel-gfx; +Cc: Umesh Nerlige Ramappa, dri-devel


On 12/05/2023 02:53, Ashutosh Dixit wrote:
> pmu_needs_timer() keeps the timer running even when GT is parked,
> ostensibly to sample requested/actual frequencies. However
> frequency_sample() has the following:
> 
> 	/* Report 0/0 (actual/requested) frequency while parked. */
> 	if (!intel_gt_pm_get_if_awake(gt))
> 		return;
> 
> The above code prevents frequencies to be sampled while the GT is
> parked. So we might as well turn off the sampling timer itself in this
> case and save CPU cycles/power.

The confusing situation seems to be the consequence of b66ecd0438bf ("drm/i915/pmu: Report frequency as zero while GPU is sleeping").

Before that commit we were deliberately sampling the frequencies as GPU minimum during the parked periods and to do so leaving the timer running.

But then some RPS changes exposed that approach as questionable (AFAIR software tracked state stopped being reset to min freq and so created wild PMU readings) and we went the route of reporting zero when parked.

At which point running the timer stopped making sense, so really that commit should/could have made the change you now propose.

> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_pmu.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 7ece883a7d956..8db1d681cf4ab 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -124,11 +124,14 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>   		  ENGINE_SAMPLE_MASK;
>   
>   	/*
> -	 * When the GPU is idle per-engine counters do not need to be
> -	 * running so clear those bits out.
> +	 * When GPU is idle, frequency or per-engine counters do not need
> +	 * to be running so clear those bits out.
>   	 */
> -	if (!gpu_active)
> -		enable &= ~ENGINE_SAMPLE_MASK;
> +	if (!gpu_active) {
> +		enable &= ~(config_mask(I915_PMU_ACTUAL_FREQUENCY) |
> +			    config_mask(I915_PMU_REQUESTED_FREQUENCY) |
> +			    ENGINE_SAMPLE_MASK);
> +	}
>   	/*
>   	 * Also there is software busyness tracking available we do not
>   	 * need the timer for I915_SAMPLE_BUSY counter.

LGTM.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Or maybe it is possible to simplify since it looks there is no way to return true if gt is parked. So that could be:

pmu_needs_timer(..)
{
	...

	if (!gpu_active)
		return false;

	...
	enable = pmu->enable;

	...
	enable &= config_mask(I915_PMU_ACTUAL_FREQUENCY) |
		  config_mask(I915_PMU_REQUESTED_FREQUENCY) |
		  ENGINE_SAMPLE_MASK;

	...
	if (i915->caps.scheduler & I915_SCHEDULER_CAP_ENGINE_BUSY_STATS)
		enable &= ~BIT(I915_SAMPLE_BUSY);

	return enable;
}

Not sure it is any better, your call.

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
@ 2023-05-12  8:59   ` Tvrtko Ursulin
  0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2023-05-12  8:59 UTC (permalink / raw)
  To: Ashutosh Dixit, intel-gfx; +Cc: dri-devel


On 12/05/2023 02:53, Ashutosh Dixit wrote:
> pmu_needs_timer() keeps the timer running even when GT is parked,
> ostensibly to sample requested/actual frequencies. However
> frequency_sample() has the following:
> 
> 	/* Report 0/0 (actual/requested) frequency while parked. */
> 	if (!intel_gt_pm_get_if_awake(gt))
> 		return;
> 
> The above code prevents frequencies to be sampled while the GT is
> parked. So we might as well turn off the sampling timer itself in this
> case and save CPU cycles/power.

The confusing situation seems to be the consequence of b66ecd0438bf ("drm/i915/pmu: Report frequency as zero while GPU is sleeping").

Before that commit we were deliberately sampling the frequencies as GPU minimum during the parked periods and to do so leaving the timer running.

But then some RPS changes exposed that approach as questionable (AFAIR software tracked state stopped being reset to min freq and so created wild PMU readings) and we went the route of reporting zero when parked.

At which point running the timer stopped making sense, so really that commit should/could have made the change you now propose.

> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_pmu.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 7ece883a7d956..8db1d681cf4ab 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -124,11 +124,14 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>   		  ENGINE_SAMPLE_MASK;
>   
>   	/*
> -	 * When the GPU is idle per-engine counters do not need to be
> -	 * running so clear those bits out.
> +	 * When GPU is idle, frequency or per-engine counters do not need
> +	 * to be running so clear those bits out.
>   	 */
> -	if (!gpu_active)
> -		enable &= ~ENGINE_SAMPLE_MASK;
> +	if (!gpu_active) {
> +		enable &= ~(config_mask(I915_PMU_ACTUAL_FREQUENCY) |
> +			    config_mask(I915_PMU_REQUESTED_FREQUENCY) |
> +			    ENGINE_SAMPLE_MASK);
> +	}
>   	/*
>   	 * Also there is software busyness tracking available we do not
>   	 * need the timer for I915_SAMPLE_BUSY counter.

LGTM.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Or maybe it is possible to simplify since it looks there is no way to return true if gt is parked. So that could be:

pmu_needs_timer(..)
{
	...

	if (!gpu_active)
		return false;

	...
	enable = pmu->enable;

	...
	enable &= config_mask(I915_PMU_ACTUAL_FREQUENCY) |
		  config_mask(I915_PMU_REQUESTED_FREQUENCY) |
		  ENGINE_SAMPLE_MASK;

	...
	if (i915->caps.scheduler & I915_SCHEDULER_CAP_ENGINE_BUSY_STATS)
		enable &= ~BIT(I915_SAMPLE_BUSY);

	return enable;
}

Not sure it is any better, your call.

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
  2023-05-12  1:53 ` [Intel-gfx] " Ashutosh Dixit
                   ` (2 preceding siblings ...)
  (?)
@ 2023-05-12  9:48 ` Patchwork
  -1 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-05-12  9:48 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 15229 bytes --]

== Series Details ==

Series: drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
URL   : https://patchwork.freedesktop.org/series/117658/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13140_full -> Patchwork_117658v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (8 -> 7)
------------------------------

  Missing    (1): shard-rkl0 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_barrier_race@remote-request@rcs0:
    - shard-glk:          [PASS][1] -> [ABORT][2] ([i915#7461] / [i915#8211])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-glk6/igt@gem_barrier_race@remote-request@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk2/igt@gem_barrier_race@remote-request@rcs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-glk:          NOTRUN -> [FAIL][3] ([i915#2842])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_lmem_evict@dontneed-evict-race:
    - shard-glk:          NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@gem_lmem_evict@dontneed-evict-race.html

  * igt@kms_ccs@pipe-a-crc-primary-basic-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#3886])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl2/igt@kms_ccs@pipe-a-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#3886]) +2 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium_color@ctm-negative:
    - shard-glk:          NOTRUN -> [SKIP][7] ([fdo#109271]) +37 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@kms_chamelium_color@ctm-negative.html

  * igt@kms_cursor_crc@cursor-offscreen-max-size:
    - shard-apl:          NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#4579])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl2/igt@kms_cursor_crc@cursor-offscreen-max-size.html

  * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#2122])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-glk5/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk5/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-mmap-wc:
    - shard-apl:          NOTRUN -> [SKIP][11] ([fdo#109271]) +12 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl2/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-mmap-wc.html

  * igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1:
    - shard-apl:          [PASS][12] -> [FAIL][13] ([i915#1188])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-apl4/igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl7/igt@kms_hdr@bpc-switch-dpms@pipe-a-dp-1.html

  * igt@kms_panel_fitting@legacy:
    - shard-glk:          NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#4579]) +3 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@kms_panel_fitting@legacy.html

  * igt@kms_plane_alpha_blend@alpha-transparent-fb@pipe-a-hdmi-a-1:
    - shard-glk:          NOTRUN -> [FAIL][15] ([i915#4573]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@kms_plane_alpha_blend@alpha-transparent-fb@pipe-a-hdmi-a-1.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area:
    - shard-glk:          NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#658])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html

  * igt@kms_writeback@writeback-invalid-parameters:
    - shard-glk:          NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#2437])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk6/igt@kms_writeback@writeback-invalid-parameters.html

  * igt@perf_pmu@module-unload:
    - shard-apl:          [PASS][18] -> [DMESG-WARN][19] ([i915#1982])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-apl4/igt@perf_pmu@module-unload.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl7/igt@perf_pmu@module-unload.html

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic-nohangcheck:
    - {shard-rkl}:        [FAIL][20] ([i915#6268]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-rkl-6/igt@gem_ctx_exec@basic-nohangcheck.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-rkl-3/igt@gem_ctx_exec@basic-nohangcheck.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-glk:          [FAIL][22] ([i915#2846]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-glk6/igt@gem_exec_fair@basic-deadline.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk9/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [FAIL][24] ([i915#2842]) -> [PASS][25] +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-apl7/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl1/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_lmem_swapping@smem-oom@lmem0:
    - {shard-dg1}:        [TIMEOUT][26] ([i915#5493]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-dg1-12/igt@gem_lmem_swapping@smem-oom@lmem0.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-dg1-15/igt@gem_lmem_swapping@smem-oom@lmem0.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - {shard-rkl}:        [SKIP][28] ([i915#1937] / [i915#4579]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-rkl-4/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-rkl-7/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - {shard-rkl}:        [SKIP][30] ([i915#1397]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-rkl-7/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-rkl-6/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html

  * igt@i915_selftest@live@workarounds:
    - {shard-dg1}:        [ABORT][32] ([i915#4983]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-dg1-17/igt@i915_selftest@live@workarounds.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-dg1-13/igt@i915_selftest@live@workarounds.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [ABORT][34] -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-apl7/igt@i915_suspend@sysfs-reader.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl2/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [FAIL][36] ([i915#2346]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-apl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@forked-bo@pipe-b:
    - {shard-rkl}:        [INCOMPLETE][38] ([i915#8011]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-rkl-7/igt@kms_cursor_legacy@forked-bo@pipe-b.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-rkl-6/igt@kms_cursor_legacy@forked-bo@pipe-b.html

  * igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2:
    - shard-glk:          [FAIL][40] ([i915#79]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-glk8/igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-glk1/igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2.html

  * igt@perf_pmu@idle@rcs0:
    - {shard-rkl}:        [FAIL][42] ([i915#4349]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13140/shard-rkl-6/igt@perf_pmu@idle@rcs0.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117658v1/shard-rkl-3/igt@perf_pmu@idle@rcs0.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4573]: https://gitlab.freedesktop.org/drm/intel/issues/4573
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
  [i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7582]: https://gitlab.freedesktop.org/drm/intel/issues/7582
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
  [i915#8411]: https://gitlab.freedesktop.org/drm/intel/issues/8411


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

  * Linux: CI_DRM_13140 -> Patchwork_117658v1

  CI-20190529: 20190529
  CI_DRM_13140: 42375f267a2451a1b2a1d5dc753785d7f1b7d9b3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7285: d1cbf2bad9c2664ab8bd3bd0946510a52800912f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117658v1: 42375f267a2451a1b2a1d5dc753785d7f1b7d9b3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

[-- Attachment #2: Type: text/html, Size: 13771 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
  2023-05-12  8:59   ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-05-23 15:27     ` Dixit, Ashutosh
  -1 siblings, 0 replies; 8+ messages in thread
From: Dixit, Ashutosh @ 2023-05-23 15:27 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx, Umesh Nerlige Ramappa, dri-devel

On Fri, 12 May 2023 01:59:08 -0700, Tvrtko Ursulin wrote:
>

Hi Tvrtko,

> On 12/05/2023 02:53, Ashutosh Dixit wrote:
> > pmu_needs_timer() keeps the timer running even when GT is parked,
> > ostensibly to sample requested/actual frequencies. However
> > frequency_sample() has the following:
> >
> >	/* Report 0/0 (actual/requested) frequency while parked. */
> >	if (!intel_gt_pm_get_if_awake(gt))
> >		return;
> >
> > The above code prevents frequencies to be sampled while the GT is
> > parked. So we might as well turn off the sampling timer itself in this
> > case and save CPU cycles/power.
>
> The confusing situation seems to be the consequence of b66ecd0438bf
> ("drm/i915/pmu: Report frequency as zero while GPU is sleeping").
>
> Before that commit we were deliberately sampling the frequencies as GPU
> minimum during the parked periods and to do so leaving the timer running.
>
> But then some RPS changes exposed that approach as questionable (AFAIR
> software tracked state stopped being reset to min freq and so created
> wild PMU readings) and we went the route of reporting zero when parked.
>
> At which point running the timer stopped making sense, so really that
> commit should/could have made the change you now propose.
>
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_pmu.c | 11 +++++++----
> >   1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> > index 7ece883a7d956..8db1d681cf4ab 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -124,11 +124,14 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
> >		  ENGINE_SAMPLE_MASK;
> >		/*
> > -	 * When the GPU is idle per-engine counters do not need to be
> > -	 * running so clear those bits out.
> > +	 * When GPU is idle, frequency or per-engine counters do not need
> > +	 * to be running so clear those bits out.
> >	 */
> > -	if (!gpu_active)
> > -		enable &= ~ENGINE_SAMPLE_MASK;
> > +	if (!gpu_active) {
> > +		enable &= ~(config_mask(I915_PMU_ACTUAL_FREQUENCY) |
> > +			    config_mask(I915_PMU_REQUESTED_FREQUENCY) |
> > +			    ENGINE_SAMPLE_MASK);
> > +	}
> >	/*
> >	 * Also there is software busyness tracking available we do not
> >	 * need the timer for I915_SAMPLE_BUSY counter.
>
> LGTM.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Or maybe it is possible to simplify since it looks there is no way to return true if gt is parked. So that could be:
>
> pmu_needs_timer(..)
> {
>	...
>
>	if (!gpu_active)
>		return false;
>
>	...
>	enable = pmu->enable;
>
>	...
>	enable &= config_mask(I915_PMU_ACTUAL_FREQUENCY) |
>		  config_mask(I915_PMU_REQUESTED_FREQUENCY) |
>		  ENGINE_SAMPLE_MASK;
>
>	...
>	if (i915->caps.scheduler & I915_SCHEDULER_CAP_ENGINE_BUSY_STATS)
>		enable &= ~BIT(I915_SAMPLE_BUSY);
>
>	return enable;
> }
>
> Not sure it is any better, your call.

I have made this change in v2 of the patch submitted here:

https://patchwork.freedesktop.org/series/118225/

And I have retained your R-b since the patch is essentially a copy of your
code above.

The original version of the patch was here:

https://patchwork.freedesktop.org/series/117658/

Thanks.
--
Ashutosh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked
@ 2023-05-23 15:27     ` Dixit, Ashutosh
  0 siblings, 0 replies; 8+ messages in thread
From: Dixit, Ashutosh @ 2023-05-23 15:27 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx, dri-devel

On Fri, 12 May 2023 01:59:08 -0700, Tvrtko Ursulin wrote:
>

Hi Tvrtko,

> On 12/05/2023 02:53, Ashutosh Dixit wrote:
> > pmu_needs_timer() keeps the timer running even when GT is parked,
> > ostensibly to sample requested/actual frequencies. However
> > frequency_sample() has the following:
> >
> >	/* Report 0/0 (actual/requested) frequency while parked. */
> >	if (!intel_gt_pm_get_if_awake(gt))
> >		return;
> >
> > The above code prevents frequencies to be sampled while the GT is
> > parked. So we might as well turn off the sampling timer itself in this
> > case and save CPU cycles/power.
>
> The confusing situation seems to be the consequence of b66ecd0438bf
> ("drm/i915/pmu: Report frequency as zero while GPU is sleeping").
>
> Before that commit we were deliberately sampling the frequencies as GPU
> minimum during the parked periods and to do so leaving the timer running.
>
> But then some RPS changes exposed that approach as questionable (AFAIR
> software tracked state stopped being reset to min freq and so created
> wild PMU readings) and we went the route of reporting zero when parked.
>
> At which point running the timer stopped making sense, so really that
> commit should/could have made the change you now propose.
>
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_pmu.c | 11 +++++++----
> >   1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> > index 7ece883a7d956..8db1d681cf4ab 100644
> > --- a/drivers/gpu/drm/i915/i915_pmu.c
> > +++ b/drivers/gpu/drm/i915/i915_pmu.c
> > @@ -124,11 +124,14 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
> >		  ENGINE_SAMPLE_MASK;
> >		/*
> > -	 * When the GPU is idle per-engine counters do not need to be
> > -	 * running so clear those bits out.
> > +	 * When GPU is idle, frequency or per-engine counters do not need
> > +	 * to be running so clear those bits out.
> >	 */
> > -	if (!gpu_active)
> > -		enable &= ~ENGINE_SAMPLE_MASK;
> > +	if (!gpu_active) {
> > +		enable &= ~(config_mask(I915_PMU_ACTUAL_FREQUENCY) |
> > +			    config_mask(I915_PMU_REQUESTED_FREQUENCY) |
> > +			    ENGINE_SAMPLE_MASK);
> > +	}
> >	/*
> >	 * Also there is software busyness tracking available we do not
> >	 * need the timer for I915_SAMPLE_BUSY counter.
>
> LGTM.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Or maybe it is possible to simplify since it looks there is no way to return true if gt is parked. So that could be:
>
> pmu_needs_timer(..)
> {
>	...
>
>	if (!gpu_active)
>		return false;
>
>	...
>	enable = pmu->enable;
>
>	...
>	enable &= config_mask(I915_PMU_ACTUAL_FREQUENCY) |
>		  config_mask(I915_PMU_REQUESTED_FREQUENCY) |
>		  ENGINE_SAMPLE_MASK;
>
>	...
>	if (i915->caps.scheduler & I915_SCHEDULER_CAP_ENGINE_BUSY_STATS)
>		enable &= ~BIT(I915_SAMPLE_BUSY);
>
>	return enable;
> }
>
> Not sure it is any better, your call.

I have made this change in v2 of the patch submitted here:

https://patchwork.freedesktop.org/series/118225/

And I have retained your R-b since the patch is essentially a copy of your
code above.

The original version of the patch was here:

https://patchwork.freedesktop.org/series/117658/

Thanks.
--
Ashutosh

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-23 15:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-12  1:53 [PATCH] drm/i915/pmu: Turn off the timer to sample frequencies when GT is parked Ashutosh Dixit
2023-05-12  1:53 ` [Intel-gfx] " Ashutosh Dixit
2023-05-12  2:55 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2023-05-12  8:59 ` [PATCH] " Tvrtko Ursulin
2023-05-12  8:59   ` [Intel-gfx] " Tvrtko Ursulin
2023-05-23 15:27   ` Dixit, Ashutosh
2023-05-23 15:27     ` [Intel-gfx] " Dixit, Ashutosh
2023-05-12  9:48 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.