[Intel-gfx] [PATCH 0/3] drm/i915/hwmon: PL1 power limit fixes for ATSM

[Intel-gfx] [PATCH 0/3] drm/i915/hwmon: PL1 power limit fixes for ATSM

[Ashutosh Dixit]

Previous PL1 power limit implementation assumed that the PL1 limit is
always enabled in HW. However we now find this not to be the case on ATSM
where the PL1 limit is disabled at power up. This requires changes in the
previous PL1 implementation.

Ashutosh Dixit (3):
  drm/i915/hwmon: Replace hwm_field_scale_and_write with
    hwm_power_max_write
  drm/i915/hwmon: Enable PL1 limit when writing limit value to HW
  drm/i915/hwmon: Use -1 to designate disabled PL1 power limit

 .../ABI/testing/sysfs-driver-intel-i915-hwmon |  3 +-
 drivers/gpu/drm/i915/i915_hwmon.c             | 61 +++++++++++++------
 2 files changed, 43 insertions(+), 21 deletions(-)

-- 
2.38.0

[Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Replace hwm_field_scale_and_write with hwm_power_max_write

[Ashutosh Dixit]

hwm_field_scale_and_write has a single caller hwm_power_write and is
specific to hwm_power_write but makes it appear that it is a general
function which can have multiple callers. Replace the function with
hwm_power_max_write which is specific to hwm_power_write and use that in
future patches where the function needs to be extended.

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

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 1225bc432f0d5..85195d61f89c7 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -99,20 +99,6 @@ hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
 	return mul_u64_u32_shr(reg_value, scale_factor, nshift);
 }
 
-static void
-hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
-			  int nshift, unsigned int scale_factor, long lval)
-{
-	u32 nval;
-
-	/* Computation in 64-bits to avoid overflow. Round to nearest. */
-	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
-
-	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
-					    PKG_PWR_LIM_1,
-					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
-}
-
 /*
  * hwm_energy - Obtain energy value
  *
@@ -391,6 +377,21 @@ hwm_power_max_read(struct hwm_drvdata *ddat, long *val)
 	return 0;
 }
 
+static int
+hwm_power_max_write(struct hwm_drvdata *ddat, long val)
+{
+	struct i915_hwmon *hwmon = ddat->hwmon;
+	u32 nval;
+
+	/* Computation in 64-bits to avoid overflow. Round to nearest. */
+	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
+
+	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
+					    PKG_PWR_LIM_1,
+					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
+	return 0;
+}
+
 static int
 hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
 {
@@ -425,16 +426,11 @@ hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
 static int
 hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
 {
-	struct i915_hwmon *hwmon = ddat->hwmon;
 	u32 uval;
 
 	switch (attr) {
 	case hwmon_power_max:
-		hwm_field_scale_and_write(ddat,
-					  hwmon->rg.pkg_rapl_limit,
-					  hwmon->scl_shift_power,
-					  SF_POWER, val);
-		return 0;
+		return hwm_power_max_write(ddat, val);
 	case hwmon_power_crit:
 		uval = DIV_ROUND_CLOSEST_ULL(val << POWER_SETUP_I1_SHIFT, SF_POWER);
 		return hwm_pcode_write_i1(ddat->uncore->i915, uval);
-- 
2.38.0

Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Replace hwm_field_scale_and_write with hwm_power_max_write

[Rodrigo Vivi]

On Mon, Feb 13, 2023 at 01:00:47PM -0800, Ashutosh Dixit wrote:
> hwm_field_scale_and_write has a single caller hwm_power_write and is
> specific to hwm_power_write but makes it appear that it is a general
> function which can have multiple callers. Replace the function with
> hwm_power_max_write which is specific to hwm_power_write and use that in
> future patches where the function needs to be extended.
> 
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_hwmon.c | 36 ++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 1225bc432f0d5..85195d61f89c7 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -99,20 +99,6 @@ hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
>  	return mul_u64_u32_shr(reg_value, scale_factor, nshift);
>  }
>  
> -static void
> -hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
> -			  int nshift, unsigned int scale_factor, long lval)
> -{
> -	u32 nval;
> -
> -	/* Computation in 64-bits to avoid overflow. Round to nearest. */
> -	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
> -
> -	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
> -					    PKG_PWR_LIM_1,
> -					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> -}
> -
>  /*
>   * hwm_energy - Obtain energy value
>   *
> @@ -391,6 +377,21 @@ hwm_power_max_read(struct hwm_drvdata *ddat, long *val)
>  	return 0;
>  }
>  
> +static int
> +hwm_power_max_write(struct hwm_drvdata *ddat, long val)
 +{
> +	struct i915_hwmon *hwmon = ddat->hwmon;
> +	u32 nval;
> +
> +	/* Computation in 64-bits to avoid overflow. Round to nearest. */
> +	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> +
> +	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> +					    PKG_PWR_LIM_1,
> +					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> +	return 0;

Let's keep this function as void and the return 0 in the previous spot.
With that change:

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

> +}
> +
>  static int
>  hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
>  {
> @@ -425,16 +426,11 @@ hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
>  static int
>  hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
>  {
> -	struct i915_hwmon *hwmon = ddat->hwmon;
>  	u32 uval;
>  
>  	switch (attr) {
>  	case hwmon_power_max:
> -		hwm_field_scale_and_write(ddat,
> -					  hwmon->rg.pkg_rapl_limit,
> -					  hwmon->scl_shift_power,
> -					  SF_POWER, val);
> -		return 0;
> +		return hwm_power_max_write(ddat, val);
>  	case hwmon_power_crit:
>  		uval = DIV_ROUND_CLOSEST_ULL(val << POWER_SETUP_I1_SHIFT, SF_POWER);
>  		return hwm_pcode_write_i1(ddat->uncore->i915, uval);
> -- 
> 2.38.0
> 

Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Replace hwm_field_scale_and_write with hwm_power_max_write

[Dixit, Ashutosh]

On Tue, 14 Feb 2023 06:50:44 -0800, Rodrigo Vivi wrote:
>
> > +static int
> > +hwm_power_max_write(struct hwm_drvdata *ddat, long val)
>  +{
> > +	struct i915_hwmon *hwmon = ddat->hwmon;
> > +	u32 nval;
> > +
> > +	/* Computation in 64-bits to avoid overflow. Round to nearest. */
> > +	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> > +
> > +	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > +					    PKG_PWR_LIM_1,
> > +					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> > +	return 0;
>
> Let's keep this function as void and the return 0 in the previous spot.

Hmm, see your point. Though there is an identical situation for
hwm_power_max_read read too (in hwm_power_read). Maybe I'll change it there
too in the same patch to keep things symmetrical and retain your R-b?

> With that change:
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks.

Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Replace hwm_field_scale_and_write with hwm_power_max_write

[Rodrigo Vivi]

On Tue, Feb 14, 2023 at 12:20:36PM -0800, Dixit, Ashutosh wrote:
> On Tue, 14 Feb 2023 06:50:44 -0800, Rodrigo Vivi wrote:
> >
> > > +static int
> > > +hwm_power_max_write(struct hwm_drvdata *ddat, long val)
> >  +{
> > > +	struct i915_hwmon *hwmon = ddat->hwmon;
> > > +	u32 nval;
> > > +
> > > +	/* Computation in 64-bits to avoid overflow. Round to nearest. */
> > > +	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> > > +
> > > +	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > > +					    PKG_PWR_LIM_1,
> > > +					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> > > +	return 0;
> >
> > Let's keep this function as void and the return 0 in the previous spot.
> 
> Hmm, see your point. Though there is an identical situation for
> hwm_power_max_read read too (in hwm_power_read). Maybe I'll change it there
> too in the same patch to keep things symmetrical and retain your R-b?

okay then. let's move this one as is and fix both functions in a follow up.

Thanks,
Rodrigo.

> 
> > With that change:
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Thanks.

[Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Enable PL1 limit when writing limit value to HW

[Ashutosh Dixit]

Previous documentation suggested that the PL1 power limit is always enabled
in HW. However we now find this not to be the case on some platforms (such
as ATSM). Therefore enable the PL1 power limit (by setting the enable bit)
when writing the PL1 limit value to HW.

Bspec: 51864

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

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 85195d61f89c7..7c20a6f47b92e 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -385,10 +385,11 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val)
 
 	/* Computation in 64-bits to avoid overflow. Round to nearest. */
 	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
+	nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval);
 
 	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
-					    PKG_PWR_LIM_1,
-					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
+					    PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1,
+					    nval);
 	return 0;
 }
 
-- 
2.38.0

Re: [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Enable PL1 limit when writing limit value to HW

[Rodrigo Vivi]

On Mon, Feb 13, 2023 at 01:00:48PM -0800, Ashutosh Dixit wrote:
> Previous documentation suggested that the PL1 power limit is always enabled
> in HW. However we now find this not to be the case on some platforms (such
> as ATSM). Therefore enable the PL1 power limit (by setting the enable bit)
> when writing the PL1 limit value to HW.
> 
> Bspec: 51864
> 
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 85195d61f89c7..7c20a6f47b92e 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -385,10 +385,11 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val)
>  
>  	/* Computation in 64-bits to avoid overflow. Round to nearest. */
>  	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> +	nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval);
>  
>  	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> -					    PKG_PWR_LIM_1,
> -					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> +					    PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1,
> +					    nval);

This patch looks right to me. But could you please open up on what exactly
failed on that reverted patch? Why do we need to set the bits together?

>  	return 0;
>  }
>  
> -- 
> 2.38.0
> 

Re: [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Enable PL1 limit when writing limit value to HW

[Dixit, Ashutosh]

On Tue, 14 Feb 2023 06:51:37 -0800, Rodrigo Vivi wrote:
>

Hi Rodrigo,

> On Mon, Feb 13, 2023 at 01:00:48PM -0800, Ashutosh Dixit wrote:
> > Previous documentation suggested that the PL1 power limit is always enabled
> > in HW. However we now find this not to be the case on some platforms (such
> > as ATSM). Therefore enable the PL1 power limit (by setting the enable bit)
> > when writing the PL1 limit value to HW.
> >
> > Bspec: 51864
> >
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> > index 85195d61f89c7..7c20a6f47b92e 100644
> > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > @@ -385,10 +385,11 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val)
> >
> >	/* Computation in 64-bits to avoid overflow. Round to nearest. */
> >	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> > +	nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval);
> >
> >	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > -					    PKG_PWR_LIM_1,
> > -					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> > +					    PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1,
> > +					    nval);
>
> This patch looks right to me. But could you please open up on what exactly
> failed on that reverted patch? Why do we need to set the bits together?

I had explained a bit here:

https://gitlab.freedesktop.org/drm/intel/-/issues/8062#note_1761070

But will repeat. On ATSM, at power-up, PCODE sets the PL1 power limit to 0
but disables the PL1 power limit. The earlier patch had enabled the the PL1
power limit during module load itself but had left the PL1 power limit set
to 0 (since there is no easy way to find out what it should be set to, on
ATSM PCODE sets even the max power (which could have been used to set the
PL1 limit) to 0). You can see that patch here:

https://patchwork.freedesktop.org/patch/521321/?series=113578&rev=4

Now the PL1 power limit being 0 (and enabled) implies that HW will work
with minimum power and therefore the lowest effective frequency. This means
all workloads will run slower and this was resulting in various IGT tests
timing out and GuC FW load (on resets) timing out on ATSM and that is why
we had to revert that patch.

In this patch I have changed the strategy and instead of enabling the PL1
power limit on module load we now enable it only when the limit is set by
userspace. So at least the default CI will not be affected in this case. We
can see that there no regressions on ATSM this time here:

https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113984v1/bat-all.html?

Thanks.
--
Ashutosh



>
> >	return 0;
> >  }
> >
> > --
> > 2.38.0
> >

Re: [Intel-gfx] [PATCH 2/3] drm/i915/hwmon: Enable PL1 limit when writing limit value to HW

[Rodrigo Vivi]

On Tue, Feb 14, 2023 at 12:47:23PM -0800, Dixit, Ashutosh wrote:
> On Tue, 14 Feb 2023 06:51:37 -0800, Rodrigo Vivi wrote:
> >
> 
> Hi Rodrigo,
> 
> > On Mon, Feb 13, 2023 at 01:00:48PM -0800, Ashutosh Dixit wrote:
> > > Previous documentation suggested that the PL1 power limit is always enabled
> > > in HW. However we now find this not to be the case on some platforms (such
> > > as ATSM). Therefore enable the PL1 power limit (by setting the enable bit)
> > > when writing the PL1 limit value to HW.
> > >
> > > Bspec: 51864
> > >
> > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_hwmon.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> > > index 85195d61f89c7..7c20a6f47b92e 100644
> > > --- a/drivers/gpu/drm/i915/i915_hwmon.c
> > > +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> > > @@ -385,10 +385,11 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val)
> > >
> > >	/* Computation in 64-bits to avoid overflow. Round to nearest. */
> > >	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> > > +	nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval);
> > >
> > >	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> > > -					    PKG_PWR_LIM_1,
> > > -					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
> > > +					    PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1,
> > > +					    nval);
> >
> > This patch looks right to me. But could you please open up on what exactly
> > failed on that reverted patch? Why do we need to set the bits together?
> 
> I had explained a bit here:
> 
> https://gitlab.freedesktop.org/drm/intel/-/issues/8062#note_1761070
> 
> But will repeat. On ATSM, at power-up, PCODE sets the PL1 power limit to 0
> but disables the PL1 power limit. The earlier patch had enabled the the PL1
> power limit during module load itself but had left the PL1 power limit set
> to 0 (since there is no easy way to find out what it should be set to, on
> ATSM PCODE sets even the max power (which could have been used to set the
> PL1 limit) to 0). You can see that patch here:
> 
> https://patchwork.freedesktop.org/patch/521321/?series=113578&rev=4
> 
> Now the PL1 power limit being 0 (and enabled) implies that HW will work
> with minimum power and therefore the lowest effective frequency. This means
> all workloads will run slower and this was resulting in various IGT tests
> timing out and GuC FW load (on resets) timing out on ATSM and that is why
> we had to revert that patch.
> 
> In this patch I have changed the strategy and instead of enabling the PL1
> power limit on module load we now enable it only when the limit is set by
> userspace. So at least the default CI will not be affected in this case. We
> can see that there no regressions on ATSM this time here:
> 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113984v1/bat-all.html?
> 

this makes total sense. Thank you!

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


> Thanks.
> --
> Ashutosh
> 
> 
> 
> >
> > >	return 0;
> > >  }
> > >
> > > --
> > > 2.38.0
> > >

[Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Use -1 to designate disabled PL1 power limit

[Ashutosh Dixit]

On ATSM the PL1 limit is disabled at power up. The previous uapi assumed
that the PL1 limit is always enabled and therefore did not have a notion of
a disabled PL1 limit. This results in erroneous PL1 limit values when the
PL1 limit is disabled. For example at power up, the disabled ATSM PL1 limit
was previously shown as 0 which means a low PL1 limit whereas the limit
being disabled actually implies a high effective PL1 limit value.

To get round this problem, the PL1 limit uapi is expanded to include a
special value, -1, to designate a disabled PL1 limit.

Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 .../ABI/testing/sysfs-driver-intel-i915-hwmon |  3 ++-
 drivers/gpu/drm/i915/i915_hwmon.c             | 24 +++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index 2d6a472eef885..7386c39c65cd9 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -14,7 +14,8 @@ Description:	RW. Card reactive sustained  (PL1/Tau) power limit in microwatts.
 
 		The power controller will throttle the operating frequency
 		if the power averaged over a window (typically seconds)
-		exceeds this limit.
+		exceeds this limit. A value of -1 indicates that the PL1
+		power limit is disabled.
 
 		Only supported for particular Intel i915 graphics platforms.
 
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 7c20a6f47b92e..e926f2feaef4b 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -345,6 +345,8 @@ hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
 	}
 }
 
+#define PL1_DISABLE	-1
+
 /*
  * HW allows arbitrary PL1 limits to be set but silently clamps these values to
  * "typical but not guaranteed" min/max values in rg.pkg_power_sku. Follow the
@@ -358,6 +360,14 @@ hwm_power_max_read(struct hwm_drvdata *ddat, long *val)
 	intel_wakeref_t wakeref;
 	u64 r, min, max;
 
+	/* Check if PL1 limit is disabled */
+	with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
+		r = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
+	if (!(r & PKG_PWR_LIM_1_EN)) {
+		*val = PL1_DISABLE;
+		return 0;
+	}
+
 	*val = hwm_field_read_and_scale(ddat,
 					hwmon->rg.pkg_rapl_limit,
 					PKG_PWR_LIM_1,
@@ -381,8 +391,22 @@ static int
 hwm_power_max_write(struct hwm_drvdata *ddat, long val)
 {
 	struct i915_hwmon *hwmon = ddat->hwmon;
+	intel_wakeref_t wakeref;
 	u32 nval;
 
+	if (val == PL1_DISABLE) {
+		/* Disable PL1 limit */
+		hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
+						    PKG_PWR_LIM_1_EN, 0);
+
+		/* Verify, because PL1 limit cannot be disabled on all platforms */
+		with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
+			nval = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
+		if (nval & PKG_PWR_LIM_1_EN)
+			return -EPERM;
+		return 0;
+	}
+
 	/* Computation in 64-bits to avoid overflow. Round to nearest. */
 	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
 	nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval);
-- 
2.38.0

Re: [Intel-gfx] [PATCH 3/3] drm/i915/hwmon: Use -1 to designate disabled PL1 power limit

[Dixit, Ashutosh]

On Mon, 13 Feb 2023 13:00:49 -0800, Ashutosh Dixit wrote:
>
> On ATSM the PL1 limit is disabled at power up. The previous uapi assumed
> that the PL1 limit is always enabled and therefore did not have a notion of
> a disabled PL1 limit. This results in erroneous PL1 limit values when the
> PL1 limit is disabled. For example at power up, the disabled ATSM PL1 limit
> was previously shown as 0 which means a low PL1 limit whereas the limit
> being disabled actually implies a high effective PL1 limit value.
>
> To get round this problem, the PL1 limit uapi is expanded to include a
> special value, -1, to designate a disabled PL1 limit.

Please don't review this patch. I have replaced this patch with a different
one in v2. Will send v2 out in a bit. Thanks!

>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>  .../ABI/testing/sysfs-driver-intel-i915-hwmon |  3 ++-
>  drivers/gpu/drm/i915/i915_hwmon.c             | 24 +++++++++++++++++++
>  2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index 2d6a472eef885..7386c39c65cd9 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -14,7 +14,8 @@ Description:	RW. Card reactive sustained  (PL1/Tau) power limit in microwatts.
>
>		The power controller will throttle the operating frequency
>		if the power averaged over a window (typically seconds)
> -		exceeds this limit.
> +		exceeds this limit. A value of -1 indicates that the PL1
> +		power limit is disabled.
>
>		Only supported for particular Intel i915 graphics platforms.
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 7c20a6f47b92e..e926f2feaef4b 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -345,6 +345,8 @@ hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
>	}
>  }
>
> +#define PL1_DISABLE	-1
> +
>  /*
>   * HW allows arbitrary PL1 limits to be set but silently clamps these values to
>   * "typical but not guaranteed" min/max values in rg.pkg_power_sku. Follow the
> @@ -358,6 +360,14 @@ hwm_power_max_read(struct hwm_drvdata *ddat, long *val)
>	intel_wakeref_t wakeref;
>	u64 r, min, max;
>
> +	/* Check if PL1 limit is disabled */
> +	with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> +		r = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
> +	if (!(r & PKG_PWR_LIM_1_EN)) {
> +		*val = PL1_DISABLE;
> +		return 0;
> +	}
> +
>	*val = hwm_field_read_and_scale(ddat,
>					hwmon->rg.pkg_rapl_limit,
>					PKG_PWR_LIM_1,
> @@ -381,8 +391,22 @@ static int
>  hwm_power_max_write(struct hwm_drvdata *ddat, long val)
>  {
>	struct i915_hwmon *hwmon = ddat->hwmon;
> +	intel_wakeref_t wakeref;
>	u32 nval;
>
> +	if (val == PL1_DISABLE) {
> +		/* Disable PL1 limit */
> +		hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> +						    PKG_PWR_LIM_1_EN, 0);
> +
> +		/* Verify, because PL1 limit cannot be disabled on all platforms */
> +		with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> +			nval = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
> +		if (nval & PKG_PWR_LIM_1_EN)
> +			return -EPERM;
> +		return 0;
> +	}
> +
>	/* Computation in 64-bits to avoid overflow. Round to nearest. */
>	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
>	nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval);
> --
> 2.38.0
>

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

[Patchwork]

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

== Series Details ==

Series: drm/i915/hwmon: PL1 power limit fixes for ATSM
URL   : https://patchwork.freedesktop.org/series/113972/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12733 -> Patchwork_113972v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (38 -> 35)
------------------------------

  Missing    (3): bat-atsm-1 fi-snb-2520m fi-pnv-d510 

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0@smem:
    - {bat-adlm-1}:       [DMESG-WARN][1] ([i915#2867]) -> [PASS][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/bat-adlm-1/igt@gem_exec_suspend@basic-s0@smem.html
    - {bat-rpls-1}:       [ABORT][3] ([i915#6311]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@i915_selftest@live@hangcheck:
    - fi-skl-guc:         [DMESG-WARN][5] ([i915#8073]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/fi-skl-guc/igt@i915_selftest@live@hangcheck.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/fi-skl-guc/igt@i915_selftest@live@hangcheck.html

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

  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6311]: https://gitlab.freedesktop.org/drm/intel/issues/6311
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#8073]: https://gitlab.freedesktop.org/drm/intel/issues/8073


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

  * Linux: CI_DRM_12733 -> Patchwork_113972v1

  CI-20190529: 20190529
  CI_DRM_12733: e6aa8599c967748cfd3f9e5393ff45c31049e20f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7160: 45da871dd2684227e93a2fc002b87dfc58bd5fd9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_113972v1: e6aa8599c967748cfd3f9e5393ff45c31049e20f @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

c154a7e77d8b drm/i915/hwmon: Use -1 to designate disabled PL1 power limit
6e1c9c7e26c9 drm/i915/hwmon: Enable PL1 limit when writing limit value to HW
973690678b17 drm/i915/hwmon: Replace hwm_field_scale_and_write with hwm_power_max_write

== Logs ==

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

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

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/hwmon: PL1 power limit fixes for ATSM

[Patchwork]

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

== Series Details ==

Series: drm/i915/hwmon: PL1 power limit fixes for ATSM
URL   : https://patchwork.freedesktop.org/series/113972/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12733_full -> Patchwork_113972v1_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_113972v1_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_113972v1_full, 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_113972v1/index.html

Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_userptr_blits@huge-split:
    - shard-apl:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-apl3/igt@gem_userptr_blits@huge-split.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl7/igt@gem_userptr_blits@huge-split.html

  
New tests
---------

  New tests have been introduced between CI_DRM_12733_full and Patchwork_113972v1_full:

### New IGT tests (1) ###

  * igt@kms_cursor_edge_walk@128x128-left-edge@pipe-b-edp-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [PASS][3] -> [FAIL][4] ([i915#2842])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-glk7/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-glk9/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_lmem_swapping@smem-oom:
    - shard-apl:          NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl3/igt@gem_lmem_swapping@smem-oom.html

  * igt@i915_selftest@live@dmabuf:
    - shard-apl:          [PASS][6] -> [DMESG-FAIL][7] ([i915#7562])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-apl1/igt@i915_selftest@live@dmabuf.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl2/igt@i915_selftest@live@dmabuf.html

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#3886]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl3/igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_flip@2x-wf_vblank-ts-check-interruptible:
    - shard-snb:          NOTRUN -> [SKIP][9] ([fdo#109271]) +6 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-snb2/igt@kms_flip@2x-wf_vblank-ts-check-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2:
    - shard-glk:          [PASS][10] -> [FAIL][11] ([i915#79]) +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-glk2/igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-glk2/igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-indfb-draw-mmap-gtt:
    - shard-glk:          NOTRUN -> [SKIP][12] ([fdo#109271]) +8 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-glk1/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-dp-1:
    - shard-apl:          NOTRUN -> [FAIL][13] ([i915#4573]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl3/igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-dp-1.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-apl:          NOTRUN -> [SKIP][14] ([fdo#109271]) +40 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl3/igt@kms_psr@psr2_primary_page_flip.html

  
#### Possible fixes ####

  * igt@drm_read@short-buffer-block:
    - {shard-rkl}:        [SKIP][15] ([i915#4098]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@drm_read@short-buffer-block.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@drm_read@short-buffer-block.html

  * igt@fbdev@info:
    - {shard-rkl}:        [SKIP][17] ([i915#2582]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@fbdev@info.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@fbdev@info.html

  * igt@gem_ctx_persistence@legacy-engines-hang@blt:
    - {shard-rkl}:        [SKIP][19] ([i915#6252]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-5/igt@gem_ctx_persistence@legacy-engines-hang@blt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-2/igt@gem_ctx_persistence@legacy-engines-hang@blt.html

  * igt@gem_eio@in-flight-external:
    - {shard-rkl}:        [ABORT][21] ([i915#7811]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-2/igt@gem_eio@in-flight-external.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-1/igt@gem_eio@in-flight-external.html

  * igt@gem_eio@in-flight-suspend:
    - {shard-rkl}:        [FAIL][23] ([i915#5115] / [i915#7052]) -> [PASS][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@gem_eio@in-flight-suspend.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@gem_eio@in-flight-suspend.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-glk:          [FAIL][25] ([i915#2842]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-glk1/igt@gem_exec_fair@basic-none-share@rcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-glk6/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-apl:          [FAIL][27] ([i915#2842]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-apl2/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl7/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_flush@basic-batch-kernel-default-cmd:
    - {shard-rkl}:        [SKIP][29] ([fdo#109313]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-5/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html

  * igt@gem_exec_reloc@basic-write-gtt:
    - {shard-rkl}:        [SKIP][31] ([i915#3281]) -> [PASS][32] +6 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@gem_exec_reloc@basic-write-gtt.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-5/igt@gem_exec_reloc@basic-write-gtt.html

  * igt@gem_partial_pwrite_pread@writes-after-reads-uncached:
    - {shard-rkl}:        [SKIP][33] ([i915#3282]) -> [PASS][34] +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@gem_partial_pwrite_pread@writes-after-reads-uncached.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-5/igt@gem_partial_pwrite_pread@writes-after-reads-uncached.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-glk:          [ABORT][35] ([i915#5566]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-glk4/igt@gen9_exec_parse@allowed-single.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-glk5/igt@gen9_exec_parse@allowed-single.html

  * igt@gen9_exec_parse@valid-registers:
    - {shard-rkl}:        [SKIP][37] ([i915#2527]) -> [PASS][38] +3 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@gen9_exec_parse@valid-registers.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-5/igt@gen9_exec_parse@valid-registers.html

  * igt@i915_hangman@engine-engine-error@bcs0:
    - {shard-rkl}:        [SKIP][39] ([i915#6258]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-5/igt@i915_hangman@engine-engine-error@bcs0.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-4/igt@i915_hangman@engine-engine-error@bcs0.html

  * igt@i915_pm_dc@dc9-dpms:
    - {shard-rkl}:        [SKIP][41] ([i915#3361]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-5/igt@i915_pm_dc@dc9-dpms.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-3/igt@i915_pm_dc@dc9-dpms.html

  * igt@i915_pm_rpm@i2c:
    - {shard-rkl}:        [SKIP][43] ([fdo#109308]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@i915_pm_rpm@i2c.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@i915_pm_rpm@i2c.html

  * igt@i915_pm_rps@engine-order:
    - shard-apl:          [FAIL][45] ([i915#6537]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-apl1/igt@i915_pm_rps@engine-order.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl2/igt@i915_pm_rps@engine-order.html

  * igt@kms_big_fb@x-tiled-32bpp-rotate-0:
    - {shard-rkl}:        [SKIP][47] ([i915#1845] / [i915#4098]) -> [PASS][48] +30 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@kms_big_fb@x-tiled-32bpp-rotate-0.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@kms_big_fb@x-tiled-32bpp-rotate-0.html

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
    - shard-apl:          [FAIL][49] ([i915#2346]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-apl2/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html

  * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1:
    - shard-glk:          [FAIL][51] ([i915#79]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-glk2/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-glk2/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
    - shard-apl:          [ABORT][53] -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-apl2/igt@kms_flip@flip-vs-suspend@b-dp1.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-apl3/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_frontbuffer_tracking@psr-indfb-scaledprimary:
    - {shard-rkl}:        [SKIP][55] ([i915#1849] / [i915#4098]) -> [PASS][56] +14 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@kms_frontbuffer_tracking@psr-indfb-scaledprimary.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@kms_frontbuffer_tracking@psr-indfb-scaledprimary.html

  * igt@kms_plane@plane-panning-top-left@pipe-a-planes:
    - {shard-rkl}:        [SKIP][57] ([i915#1849]) -> [PASS][58] +4 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@kms_plane@plane-panning-top-left@pipe-a-planes.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@kms_plane@plane-panning-top-left@pipe-a-planes.html

  * igt@kms_psr@cursor_mmap_gtt:
    - {shard-rkl}:        [SKIP][59] ([i915#1072]) -> [PASS][60] +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@kms_psr@cursor_mmap_gtt.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@kms_psr@cursor_mmap_gtt.html

  * igt@kms_psr_stress_test@invalidate-primary-flip-overlay:
    - {shard-rkl}:        [SKIP][61] ([i915#5461]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@kms_psr_stress_test@invalidate-primary-flip-overlay.html

  * igt@kms_universal_plane@cursor-fb-leak-pipe-b:
    - {shard-rkl}:        [SKIP][63] ([i915#1845] / [i915#4070] / [i915#4098]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@kms_universal_plane@cursor-fb-leak-pipe-b.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@kms_universal_plane@cursor-fb-leak-pipe-b.html

  * igt@perf@mi-rpc:
    - {shard-rkl}:        [SKIP][65] ([i915#2434]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@perf@mi-rpc.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-5/igt@perf@mi-rpc.html

  * igt@perf_pmu@idle@rcs0:
    - {shard-rkl}:        [FAIL][67] ([i915#4349]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-1/igt@perf_pmu@idle@rcs0.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-6/igt@perf_pmu@idle@rcs0.html

  * igt@prime_vgem@basic-write:
    - {shard-rkl}:        [SKIP][69] ([fdo#109295] / [i915#3291] / [i915#3708]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12733/shard-rkl-4/igt@prime_vgem@basic-write.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113972v1/shard-rkl-5/igt@prime_vgem@basic-write.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#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
  [fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109314]: https://bugs.freedesktop.org/show_bug.cgi?id=109314
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110542]: https://bugs.freedesktop.org/show_bug.cgi?id=110542
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111644]: https://bugs.freedesktop.org/show_bug.cgi?id=111644
  [fdo#111656]: https://bugs.freedesktop.org/show_bug.cgi?id=111656
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#132]: https://gitlab.freedesktop.org/drm/intel/issues/132
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2434]: https://gitlab.freedesktop.org/drm/intel/issues/2434
  [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#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [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#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3361]: https://gitlab.freedesktop.org/drm/intel/issues/3361
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3528]: https://gitlab.freedesktop.org/drm/intel/issues/3528
  [i915#3536]: https://gitlab.freedesktop.org/drm/intel/issues/3536
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3547]: https://gitlab.freedesktop.org/drm/intel/issues/3547
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
  [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#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#426]: https://gitlab.freedesktop.org/drm/intel/issues/426
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4573]: https://gitlab.freedesktop.org/drm/intel/issues/4573
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4879]: https://gitlab.freedesktop.org/drm/intel/issues/4879
  [i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
  [i915#4881]: https://gitlab.freedesktop.org/drm/intel/issues/4881
  [i915#4885]: https://gitlab.freedesktop.org/drm/intel/issues/4885
  [i915#5115]: https://gitlab.freedesktop.org/drm/intel/issues/5115
  [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
  [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#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6230]: https://gitlab.freedesktop.org/drm/intel/issues/6230
  [i915#6248]: https://gitlab.freedesktop.org/drm/intel/issues/6248
  [i915#6252]: https://gitlab.freedesktop.org/drm/intel/issues/6252
  [i915#6258]: https://gitlab.freedesktop.org/drm/intel/issues/6258
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#6344]: https://gitlab.freedesktop.org/drm/intel/issues/6344
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#6497]: https://gitlab.freedesktop.org/drm/intel/issues/6497
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#6537]: https://gitlab.freedesktop.org/drm/intel/issues/6537
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
  [i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7037]: https://gitlab.freedesktop.org/drm/intel/issues/7037
  [i915#7052]: https://gitlab.freedesktop.org/drm/intel/issues/7052
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7562]: https://gitlab.freedesktop.org/drm/intel/issues/7562
  [i915#7582]: https://gitlab.freedesktop.org/drm/intel/issues/7582
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7811]: https://gitlab.freedesktop.org/drm/intel/issues/7811
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7949]: https://gitlab.freedesktop.org/drm/intel/issues/7949
  [i915#7957]: https://gitlab.freedesktop.org/drm/intel/issues/7957
  [i915#7984]: https://gitlab.freedesktop.org/drm/intel/issues/7984


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

  * Linux: CI_DRM_12733 -> Patchwork_113972v1

  CI-20190529: 20190529
  CI_DRM_12733: e6aa8599c967748cfd3f9e5393ff45c31049e20f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7160: 45da871dd2684227e93a2fc002b87dfc58bd5fd9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_113972v1: e6aa8599c967748cfd3f9e5393ff45c31049e20f @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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

[PATCH 0/3] PL1 power limit fixes for ATSM

[Ashutosh Dixit]

Previous PL1 power limit implementation assumed that the PL1 limit is
always enabled in HW. However we now find this not to be the case on ATSM
where the PL1 limit is disabled at power up. This requires changes in the
previous PL1 limit implementation.

Submitting 3 patches for easier review but patches can be squashed if
needed.

Ashutosh Dixit (3):
  drm/i915/hwmon: Replace hwm_field_scale_and_write with
    hwm_power_max_write
  drm/i915/hwmon: Enable PL1 limit when writing limit value to HW
  drm/i915/hwmon: Expose power1_max_enable

 .../ABI/testing/sysfs-driver-intel-i915-hwmon |  7 ++
 drivers/gpu/drm/i915/i915_hwmon.c             | 85 +++++++++++++------
 2 files changed, 68 insertions(+), 24 deletions(-)

-- 
2.38.0

[Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Replace hwm_field_scale_and_write with hwm_power_max_write

[Ashutosh Dixit]

hwm_field_scale_and_write has a single caller hwm_power_write and is
specific to hwm_power_write but makes it appear that it is a general
function which can have multiple callers. Replace the function with
hwm_power_max_write which is specific to hwm_power_write and use that in
future patches where the function needs to be extended.

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

diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 1225bc432f0d5..85195d61f89c7 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -99,20 +99,6 @@ hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
 	return mul_u64_u32_shr(reg_value, scale_factor, nshift);
 }
 
-static void
-hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
-			  int nshift, unsigned int scale_factor, long lval)
-{
-	u32 nval;
-
-	/* Computation in 64-bits to avoid overflow. Round to nearest. */
-	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
-
-	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
-					    PKG_PWR_LIM_1,
-					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
-}
-
 /*
  * hwm_energy - Obtain energy value
  *
@@ -391,6 +377,21 @@ hwm_power_max_read(struct hwm_drvdata *ddat, long *val)
 	return 0;
 }
 
+static int
+hwm_power_max_write(struct hwm_drvdata *ddat, long val)
+{
+	struct i915_hwmon *hwmon = ddat->hwmon;
+	u32 nval;
+
+	/* Computation in 64-bits to avoid overflow. Round to nearest. */
+	nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
+
+	hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
+					    PKG_PWR_LIM_1,
+					    REG_FIELD_PREP(PKG_PWR_LIM_1, nval));
+	return 0;
+}
+
 static int
 hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
 {
@@ -425,16 +426,11 @@ hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
 static int
 hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
 {
-	struct i915_hwmon *hwmon = ddat->hwmon;
 	u32 uval;
 
 	switch (attr) {
 	case hwmon_power_max:
-		hwm_field_scale_and_write(ddat,
-					  hwmon->rg.pkg_rapl_limit,
-					  hwmon->scl_shift_power,
-					  SF_POWER, val);
-		return 0;
+		return hwm_power_max_write(ddat, val);
 	case hwmon_power_crit:
 		uval = DIV_ROUND_CLOSEST_ULL(val << POWER_SETUP_I1_SHIFT, SF_POWER);
 		return hwm_pcode_write_i1(ddat->uncore->i915, uval);
-- 
2.38.0