[PATCH v1] drm/i915/selftest: Change throttle criteria for rps

[PATCH v1] drm/i915/selftest: Change throttle criteria for rps

[Raag Jadav]

Current live_rps_control() implementation errors out on throttling.
This was done with the assumption that throttling to minimum frequency
is a catastrophic failure, which is incorrect. Throttling can happen
due to variety of reasons and often times out of our control. Also,
the resulting frequency can be at any given point below the maximum
allowed. Change throttle criteria to reflect this logic and drop the
error, as it doesn't necessarily mean selftest failure.

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_rps.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
index dcef8d498919..7aac90c1679e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_rps.c
+++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
@@ -477,12 +477,13 @@ int live_rps_control(void *arg)
 			limit, intel_gpu_freq(rps, limit),
 			min, max, ktime_to_ns(min_dt), ktime_to_ns(max_dt));
 
-		if (limit == rps->min_freq) {
-			pr_err("%s: GPU throttled to minimum!\n",
-			       engine->name);
+		if (limit != rps->max_freq) {
+			u32 throttle = intel_uncore_read(gt->uncore,
+							 intel_gt_perf_limit_reasons_reg(gt));
+
+			pr_warn("%s: GPU throttled with reasons 0x%08x\n",
+				engine->name, throttle & GT0_PERF_LIMIT_REASONS_MASK);
 			show_pstate_limits(rps);
-			err = -ENODEV;
-			break;
 		}
 
 		if (igt_flush_test(gt->i915)) {
-- 
2.34.1

✓ i915.CI.BAT: success for drm/i915/selftest: Change throttle criteria for rps

[Patchwork]

== Series Details ==

Series: drm/i915/selftest: Change throttle criteria for rps
URL   : https://patchwork.freedesktop.org/series/143060/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_15892 -> Patchwork_143060v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

  Missing    (2): bat-twl-1 fi-snb-2520m 


Changes
-------

  No changes found


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

  * IGT: IGT_8172 -> None
  * Linux: CI_DRM_15892 -> Patchwork_143060v1

  CI-20190529: 20190529
  CI_DRM_15892: 08bd590935a5258ffd79355c59adffd72fb2c642 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8172: 9112581619aa198fa03041d5c7e18e02f42ac00f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_143060v1: 08bd590935a5258ffd79355c59adffd72fb2c642 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

✓ i915.CI.Full: success for drm/i915/selftest: Change throttle criteria for rps

[Patchwork]

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

== Series Details ==

Series: drm/i915/selftest: Change throttle criteria for rps
URL   : https://patchwork.freedesktop.org/series/143060/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_15892_full -> Patchwork_143060v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (11 -> 9)
------------------------------

  Missing    (2): shard-snb-0 shard-glk-0 


Changes
-------

  No changes found


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

  * IGT: IGT_8172 -> None
  * Linux: CI_DRM_15892 -> Patchwork_143060v1

  CI-20190529: 20190529
  CI_DRM_15892: 08bd590935a5258ffd79355c59adffd72fb2c642 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8172: 9112581619aa198fa03041d5c7e18e02f42ac00f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_143060v1: 08bd590935a5258ffd79355c59adffd72fb2c642 @ 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_143060v1/index.html

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

Re: [PATCH v1] drm/i915/selftest: Change throttle criteria for rps

[Raag Jadav]

On Thu, Jan 02, 2025 at 04:36:18PM +0530, Raag Jadav wrote:
> Current live_rps_control() implementation errors out on throttling.
> This was done with the assumption that throttling to minimum frequency
> is a catastrophic failure, which is incorrect. Throttling can happen
> due to variety of reasons and often times out of our control. Also,
> the resulting frequency can be at any given point below the maximum
> allowed. Change throttle criteria to reflect this logic and drop the
> error, as it doesn't necessarily mean selftest failure.
> 
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---

Rodrigo, does this look okay?

Raag

>  drivers/gpu/drm/i915/gt/selftest_rps.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index dcef8d498919..7aac90c1679e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -477,12 +477,13 @@ int live_rps_control(void *arg)
>  			limit, intel_gpu_freq(rps, limit),
>  			min, max, ktime_to_ns(min_dt), ktime_to_ns(max_dt));
>  
> -		if (limit == rps->min_freq) {
> -			pr_err("%s: GPU throttled to minimum!\n",
> -			       engine->name);
> +		if (limit != rps->max_freq) {
> +			u32 throttle = intel_uncore_read(gt->uncore,
> +							 intel_gt_perf_limit_reasons_reg(gt));
> +
> +			pr_warn("%s: GPU throttled with reasons 0x%08x\n",
> +				engine->name, throttle & GT0_PERF_LIMIT_REASONS_MASK);
>  			show_pstate_limits(rps);
> -			err = -ENODEV;
> -			break;
>  		}
>  
>  		if (igt_flush_test(gt->i915)) {
> -- 
> 2.34.1
> 

Re: [PATCH v1] drm/i915/selftest: Change throttle criteria for rps

[Belgaumkar, Vinay]

On 1/2/2025 3:06 AM, Raag Jadav wrote:
> Current live_rps_control() implementation errors out on throttling.
> This was done with the assumption that throttling to minimum frequency
> is a catastrophic failure, which is incorrect. Throttling can happen
> due to variety of reasons and often times out of our control. Also,
> the resulting frequency can be at any given point below the maximum
> allowed. Change throttle criteria to reflect this logic and drop the
> error, as it doesn't necessarily mean selftest failure.

LGTM, CI systems are especially susceptible to thermal issues.

Reviewed-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_rps.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index dcef8d498919..7aac90c1679e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -477,12 +477,13 @@ int live_rps_control(void *arg)
>   			limit, intel_gpu_freq(rps, limit),
>   			min, max, ktime_to_ns(min_dt), ktime_to_ns(max_dt));
>   
> -		if (limit == rps->min_freq) {
> -			pr_err("%s: GPU throttled to minimum!\n",
> -			       engine->name);
> +		if (limit != rps->max_freq) {
> +			u32 throttle = intel_uncore_read(gt->uncore,
> +							 intel_gt_perf_limit_reasons_reg(gt));
> +
> +			pr_warn("%s: GPU throttled with reasons 0x%08x\n",
> +				engine->name, throttle & GT0_PERF_LIMIT_REASONS_MASK);
>   			show_pstate_limits(rps);
> -			err = -ENODEV;
> -			break;
>   		}
>   
>   		if (igt_flush_test(gt->i915)) {

Re: [PATCH v1] drm/i915/selftest: Change throttle criteria for rps

[Rodrigo Vivi]

On Wed, Jan 08, 2025 at 11:28:47AM +0200, Raag Jadav wrote:
> On Thu, Jan 02, 2025 at 04:36:18PM +0530, Raag Jadav wrote:
> > Current live_rps_control() implementation errors out on throttling.
> > This was done with the assumption that throttling to minimum frequency
> > is a catastrophic failure, which is incorrect. Throttling can happen
> > due to variety of reasons and often times out of our control. Also,
> > the resulting frequency can be at any given point below the maximum
> > allowed. Change throttle criteria to reflect this logic and drop the
> > error, as it doesn't necessarily mean selftest failure.
> > 
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > ---
> 
> Rodrigo, does this look okay?

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Raag
> 
> >  drivers/gpu/drm/i915/gt/selftest_rps.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c b/drivers/gpu/drm/i915/gt/selftest_rps.c
> > index dcef8d498919..7aac90c1679e 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> > @@ -477,12 +477,13 @@ int live_rps_control(void *arg)
> >  			limit, intel_gpu_freq(rps, limit),
> >  			min, max, ktime_to_ns(min_dt), ktime_to_ns(max_dt));
> >  
> > -		if (limit == rps->min_freq) {
> > -			pr_err("%s: GPU throttled to minimum!\n",
> > -			       engine->name);
> > +		if (limit != rps->max_freq) {
> > +			u32 throttle = intel_uncore_read(gt->uncore,
> > +							 intel_gt_perf_limit_reasons_reg(gt));
> > +
> > +			pr_warn("%s: GPU throttled with reasons 0x%08x\n",
> > +				engine->name, throttle & GT0_PERF_LIMIT_REASONS_MASK);
> >  			show_pstate_limits(rps);
> > -			err = -ENODEV;
> > -			break;
> >  		}
> >  
> >  		if (igt_flush_test(gt->i915)) {
> > -- 
> > 2.34.1
> > 

Re: [PATCH v1] drm/i915/selftest: Change throttle criteria for rps

[Andi Shyti]

Hi Raag,

On Thu, Jan 02, 2025 at 04:36:18PM +0530, Raag Jadav wrote:
> Current live_rps_control() implementation errors out on throttling.
> This was done with the assumption that throttling to minimum frequency
> is a catastrophic failure, which is incorrect. Throttling can happen
> due to variety of reasons and often times out of our control. Also,
> the resulting frequency can be at any given point below the maximum
> allowed. Change throttle criteria to reflect this logic and drop the
> error, as it doesn't necessarily mean selftest failure.
> 
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>

Reviewed and merged to drm-intel-gt-next.

Thanks,
Andi

Re: [PATCH v1] drm/i915/selftest: Change throttle criteria for rps

[Raag Jadav]

On Sat, Jan 11, 2025 at 09:41:47PM +0100, Andi Shyti wrote:
> Hi Raag,
> 
> On Thu, Jan 02, 2025 at 04:36:18PM +0530, Raag Jadav wrote:
> > Current live_rps_control() implementation errors out on throttling.
> > This was done with the assumption that throttling to minimum frequency
> > is a catastrophic failure, which is incorrect. Throttling can happen
> > due to variety of reasons and often times out of our control. Also,
> > the resulting frequency can be at any given point below the maximum
> > allowed. Change throttle criteria to reflect this logic and drop the
> > error, as it doesn't necessarily mean selftest failure.
> > 
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> 
> Reviewed and merged to drm-intel-gt-next.

Thank you.

Sorry I didn't pick your tag as I considered this a different patch.

Raag