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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

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
>