Re: [Intel-gfx] [PATCH 1/3] drm/i915/hwmon: Get mutex and rpm ref just once in hwm_power_max_write

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

On Wed, Apr 05, 2023 at 09:45:20PM -0700, Ashutosh Dixit wrote:
> In preparation for follow-on patches, refactor hwm_power_max_write to take
> hwmon_lock and runtime pm wakeref at start of the function and release them
> at the end, therefore acquiring these just once each.
> 
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_hwmon.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 8e7dccc8d3a0e..7f44e809ca155 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -396,31 +396,33 @@ hwm_power_max_write(struct hwm_drvdata *ddat, long val)
>  {
>  	struct i915_hwmon *hwmon = ddat->hwmon;
>  	intel_wakeref_t wakeref;
> +	int ret = 0;
>  	u32 nval;
>  
> +	mutex_lock(&hwmon->hwmon_lock);
> +	wakeref = intel_runtime_pm_get(ddat->uncore->rpm);
> +
>  	/* Disable PL1 limit and verify, because the limit cannot be disabled on all platforms */
>  	if (val == PL1_DISABLE) {
> -		mutex_lock(&hwmon->hwmon_lock);
> -		with_intel_runtime_pm(ddat->uncore->rpm, wakeref) {
> -			intel_uncore_rmw(ddat->uncore, hwmon->rg.pkg_rapl_limit,
> -					 PKG_PWR_LIM_1_EN, 0);
> -			nval = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
> -		}
> -		mutex_unlock(&hwmon->hwmon_lock);
> +		intel_uncore_rmw(ddat->uncore, hwmon->rg.pkg_rapl_limit,
> +				 PKG_PWR_LIM_1_EN, 0);
> +		nval = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
>  
>  		if (nval & PKG_PWR_LIM_1_EN)
> -			return -ENODEV;
> -		return 0;
> +			ret = -ENODEV;
> +		goto exit;
>  	}
>  
>  	/* 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_EN | PKG_PWR_LIM_1,
> -					    nval);
> -	return 0;
> +	intel_uncore_rmw(ddat->uncore, hwmon->rg.pkg_rapl_limit,
> +			 PKG_PWR_LIM_1_EN | PKG_PWR_LIM_1, nval);
> +exit:
> +	intel_runtime_pm_put(ddat->uncore->rpm, wakeref);
> +	mutex_unlock(&hwmon->hwmon_lock);
> +	return ret;
>  }
>  
>  static int
> -- 
> 2.38.0
>