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 <[email protected]>
> ---
>  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 <[email protected]>

> +}
> +
>  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
> 

Reply via email to