Re: [Intel-xe] [PATCH v6 5/5] drm/xe/hwmon: Expose power1_max_interval

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Badal,

[...]

> +static ssize_t
> +xe_hwmon_power1_max_interval_store(struct device *dev, struct device_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	struct xe_hwmon *hwmon = dev_get_drvdata(dev);
> +	u32 x, y, rxy, x_w = 2; /* 2 bits */
> +	u64 tau4, r, max_win;
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Max HW supported tau in '1.x * power(2,y)' format, x = 0, y = 0x12
> +	 * The hwmon->scl_shift_time default of 0xa results in a max tau of 256 seconds
> +	 */
> +#define PKG_MAX_WIN_DEFAULT 0x12ull
> +
> +	/*
> +	 * val must be < max in hwmon interface units. The steps below are
> +	 * explained in xe_hwmon_power1_max_interval_show()
> +	 */
> +	r = FIELD_PREP(PKG_MAX_WIN, PKG_MAX_WIN_DEFAULT);
> +	x = REG_FIELD_GET(PKG_MAX_WIN_X, r);
> +	y = REG_FIELD_GET(PKG_MAX_WIN_Y, r);
> +	tau4 = ((1 << x_w) | x) << y;
> +	max_win = mul_u64_u32_shr(tau4, SF_TIME, hwmon->scl_shift_time + x_w);
> +
> +	if (val > max_win)
> +		return -EINVAL;
> +
> +	/* val in hw units */
> +	val = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_time, SF_TIME);
> +	/* Convert to 1.x * power(2,y) */
> +	if (!val) {
> +		/* Avoid ilog2(0) */
> +		y = 0;
> +		x = 0;
> +	} else {
> +		y = ilog2(val);
> +		/* x = (val - (1 << y)) >> (y - 2); */

this is some spurious development comment, can you please remove
it?

> +		x = (val - (1ul << y)) << x_w >> y;
> +	}
> +
> +	rxy = REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_X, x) | REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_Y, y);
> +
> +	xe_device_mem_access_get(gt_to_xe(hwmon->gt));
> +
> +	mutex_lock(&hwmon->hwmon_lock);
> +
> +	xe_hwmon_process_reg(hwmon, REG_PKG_RAPL_LIMIT, REG_RMW, (u32 *)&r,
> +			     PKG_PWR_LIM_1_TIME, rxy);
> +
> +	mutex_unlock(&hwmon->hwmon_lock);

why are we locking here?

Andi

> +
> +	xe_device_mem_access_put(gt_to_xe(hwmon->gt));
> +
> +	return count;
> +}