[Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Ashutosh Dixit]

Previous documentation suggested that PL1 power limit is always
enabled. However we now find this not to be the case on some
platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
initialization.

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

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 1225bc432f0d5..4683a5b96eff1 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
 		for_each_gt(gt, i915, i)
 			hwm_energy(&hwmon->ddat_gt[i], &energy);
 	}
+
+	/* Enable PL1 power limit */
+	if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
+		hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
+						    PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
 }
 
 void i915_hwmon_register(struct drm_i915_private *i915)
-- 
2.38.0

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/hwmon: Enable PL1 power limit

[Patchwork]

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

== Series Details ==

Series: drm/i915/hwmon: Enable PL1 power limit
URL   : https://patchwork.freedesktop.org/series/113578/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12681 -> Patchwork_113578v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (25 -> 25)
------------------------------

  Additional (1): fi-apl-guc 
  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@hangcheck:
    - {bat-rpls-1}:       [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12681/bat-rpls-1/igt@i915_selftest@live@hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113578v1/bat-rpls-1/igt@i915_selftest@live@hangcheck.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@fbdev@write:
    - fi-blb-e6850:       [PASS][3] -> [SKIP][4] ([fdo#109271]) +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12681/fi-blb-e6850/igt@fbdev@write.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113578v1/fi-blb-e6850/igt@fbdev@write.html

  * igt@gem_lmem_swapping@basic:
    - fi-apl-guc:         NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113578v1/fi-apl-guc/igt@gem_lmem_swapping@basic.html

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

  * igt@kms_chamelium_hpd@vga-hpd-fast:
    - fi-apl-guc:         NOTRUN -> [SKIP][7] ([fdo#109271]) +21 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113578v1/fi-apl-guc/igt@kms_chamelium_hpd@vga-hpd-fast.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
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7852]: https://gitlab.freedesktop.org/drm/intel/issues/7852
  [i915#7977]: https://gitlab.freedesktop.org/drm/intel/issues/7977


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

  * Linux: CI_DRM_12681 -> Patchwork_113578v1

  CI-20190529: 20190529
  CI_DRM_12681: 8ee2ec597aa4b8331124bf852432c2ca2fd7b8d1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7143: c7b12dcc460fc2348e1fa7f4dcb791bb82e29e44 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_113578v1: 8ee2ec597aa4b8331124bf852432c2ca2fd7b8d1 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

0153ee93659c drm/i915/hwmon: Enable PL1 power limit

== Logs ==

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

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

Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Gwan-gyeong Mun]

looks good to me, but could you please add bpsec# to commit log?

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

On 2/2/23 4:52 AM, Ashutosh Dixit wrote:
> Previous documentation suggested that PL1 power limit is always
> enabled. However we now find this not to be the case on some
> platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
> initialization.
> 
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 1225bc432f0d5..4683a5b96eff1 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
>   		for_each_gt(gt, i915, i)
>   			hwm_energy(&hwmon->ddat_gt[i], &energy);
>   	}
> +
> +	/* Enable PL1 power limit */
> +	if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
> +		hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> +						    PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
>   }
>   
>   void i915_hwmon_register(struct drm_i915_private *i915)

[Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Ashutosh Dixit]

Previous documentation suggested that PL1 power limit is always
enabled. However we now find this not to be the case on some
platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
initialization.

Bspec: 51864

v2: Add Bspec reference (Gwan-gyeong)

Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 1225bc432f0d5..4683a5b96eff1 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
 		for_each_gt(gt, i915, i)
 			hwm_energy(&hwmon->ddat_gt[i], &energy);
 	}
+
+	/* Enable PL1 power limit */
+	if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
+		hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
+						    PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
 }
 
 void i915_hwmon_register(struct drm_i915_private *i915)
-- 
2.38.0

[Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Ashutosh Dixit]

Previous documentation suggested that PL1 power limit is always
enabled. However we now find this not to be the case on some
platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
initialization.

Bspec: 51864

v2: Add Bspec reference (Gwan-gyeong)
v3: Add Fixes tag

Fixes: 99f55efb79114 ("drm/i915/hwmon: Power PL1 limit and TDP setting")
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 1225bc432f0d5..4683a5b96eff1 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
 		for_each_gt(gt, i915, i)
 			hwm_energy(&hwmon->ddat_gt[i], &energy);
 	}
+
+	/* Enable PL1 power limit */
+	if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
+		hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
+						    PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
 }
 
 void i915_hwmon_register(struct drm_i915_private *i915)
-- 
2.38.0

Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Matthew Auld]

On Fri, 3 Feb 2023 at 15:54, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
>
> Previous documentation suggested that PL1 power limit is always
> enabled. However we now find this not to be the case on some
> platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
> initialization.

For some reason it looks like this change is impacting the atsm in CI:
https://intel-gfx-ci.01.org/tree/drm-tip/bat-atsm-1.html

>
> Bspec: 51864
>
> v2: Add Bspec reference (Gwan-gyeong)
> v3: Add Fixes tag
>
> Fixes: 99f55efb79114 ("drm/i915/hwmon: Power PL1 limit and TDP setting")
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 1225bc432f0d5..4683a5b96eff1 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
>                 for_each_gt(gt, i915, i)
>                         hwm_energy(&hwmon->ddat_gt[i], &energy);
>         }
> +
> +       /* Enable PL1 power limit */
> +       if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
> +               hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> +                                                   PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
>  }
>
>  void i915_hwmon_register(struct drm_i915_private *i915)
> --
> 2.38.0
>

Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Dixit, Ashutosh]

On Tue, 07 Feb 2023 01:32:44 -0800, Matthew Auld wrote:
>
> On Fri, 3 Feb 2023 at 15:54, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> >
> > Previous documentation suggested that PL1 power limit is always
> > enabled. However we now find this not to be the case on some
> > platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
> > initialization.
>
> For some reason it looks like this change is impacting the atsm in CI:
> https://intel-gfx-ci.01.org/tree/drm-tip/bat-atsm-1.html

Hmm, the change was meant for ATSM. Anyway let me try to get hold of an
ATSM and see if I can figure out what might be going on with these
seemingly unrelated failures and if I can repro them locally. Thanks!

>
> >
> > Bspec: 51864
> >
> > v2: Add Bspec reference (Gwan-gyeong)
> > v3: Add Fixes tag
> >
> > Fixes: 99f55efb79114 ("drm/i915/hwmon: Power PL1 limit and TDP setting")
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> > index 1225bc432f0d5..4683a5b96eff1 100644
> > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > @@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> >                 for_each_gt(gt, i915, i)
> >                         hwm_energy(&hwmon->ddat_gt[i], &energy);
> >         }
> > +
> > +       /* Enable PL1 power limit */
> > +       if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
> > +               hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > +                                                   PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
> >  }
> >
> >  void i915_hwmon_register(struct drm_i915_private *i915)
> > --
> > 2.38.0
> >

Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Dixit, Ashutosh]

On Tue, 07 Feb 2023 08:12:25 -0800, Dixit, Ashutosh wrote:
>
> On Tue, 07 Feb 2023 01:32:44 -0800, Matthew Auld wrote:
> >
> > On Fri, 3 Feb 2023 at 15:54, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> > >
> > > Previous documentation suggested that PL1 power limit is always
> > > enabled. However we now find this not to be the case on some
> > > platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
> > > initialization.
> >
> > For some reason it looks like this change is impacting the atsm in CI:
> > https://intel-gfx-ci.01.org/tree/drm-tip/bat-atsm-1.html
>
> Hmm, the change was meant for ATSM. Anyway let me try to get hold of an
> ATSM and see if I can figure out what might be going on with these
> seemingly unrelated failures and if I can repro them locally. Thanks!

Rodrigo/Matt,

I am proposing we revert this now and remerge again after investigating,
even getting ATSM systems to investigate is not easy so it might take a few
days to investigate. What do you guys think?

Thanks.
--
Ashutosh


>
> >
> > >
> > > Bspec: 51864
> > >
> > > v2: Add Bspec reference (Gwan-gyeong)
> > > v3: Add Fixes tag
> > >
> > > Fixes: 99f55efb79114 ("drm/i915/hwmon: Power PL1 limit and TDP setting")
> > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> > > index 1225bc432f0d5..4683a5b96eff1 100644
> > > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > > @@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> > >                 for_each_gt(gt, i915, i)
> > >                         hwm_energy(&hwmon->ddat_gt[i], &energy);
> > >         }
> > > +
> > > +       /* Enable PL1 power limit */
> > > +       if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
> > > +               hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > > +                                                   PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
> > >  }
> > >
> > >  void i915_hwmon_register(struct drm_i915_private *i915)
> > > --
> > > 2.38.0
> > >

Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Enable PL1 power limit

[Matthew Auld]

On Tue, 7 Feb 2023 at 17:19, Dixit, Ashutosh <ashutosh.dixit@intel.com> wrote:
>
> On Tue, 07 Feb 2023 08:12:25 -0800, Dixit, Ashutosh wrote:
> >
> > On Tue, 07 Feb 2023 01:32:44 -0800, Matthew Auld wrote:
> > >
> > > On Fri, 3 Feb 2023 at 15:54, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> > > >
> > > > Previous documentation suggested that PL1 power limit is always
> > > > enabled. However we now find this not to be the case on some
> > > > platforms (such as ATSM). Therefore enable PL1 power limit during hwmon
> > > > initialization.
> > >
> > > For some reason it looks like this change is impacting the atsm in CI:
> > > https://intel-gfx-ci.01.org/tree/drm-tip/bat-atsm-1.html
> >
> > Hmm, the change was meant for ATSM. Anyway let me try to get hold of an
> > ATSM and see if I can figure out what might be going on with these
> > seemingly unrelated failures and if I can repro them locally. Thanks!
>
> Rodrigo/Matt,
>
> I am proposing we revert this now and remerge again after investigating,
> even getting ATSM systems to investigate is not easy so it might take a few
> days to investigate. What do you guys think?

Yeah, maybe just revert for now.

>
> Thanks.
> --
> Ashutosh
>
>
> >
> > >
> > > >
> > > > Bspec: 51864
> > > >
> > > > v2: Add Bspec reference (Gwan-gyeong)
> > > > v3: Add Fixes tag
> > > >
> > > > Fixes: 99f55efb79114 ("drm/i915/hwmon: Power PL1 limit and TDP setting")
> > > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > > Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> > > > index 1225bc432f0d5..4683a5b96eff1 100644
> > > > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > > > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > > > @@ -687,6 +687,11 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> > > >                 for_each_gt(gt, i915, i)
> > > >                         hwm_energy(&hwmon->ddat_gt[i], &energy);
> > > >         }
> > > > +
> > > > +       /* Enable PL1 power limit */
> > > > +       if (i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
> > > > +               hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > > > +                                                   PKG_PWR_LIM_1_EN, PKG_PWR_LIM_1_EN);
> > > >  }
> > > >
> > > >  void i915_hwmon_register(struct drm_i915_private *i915)
> > > > --
> > > > 2.38.0
> > > >