[PATCH] drm/xe/hwmon: Add SW clamp for power limits writes

[Riana Tauro]

Hi Karthik

On 8/1/2025 1:06 AM, Karthik Poosa wrote:
> Clamp writes to power limits powerX_crit/currX_crit, powerX_cap,
> powerX_max, to the maximum supported by the pcode mailbox
> when sysfs-provided values exceed this limit.
> Although the pcode already performs clamping, values beyond the pcode
> mailbox's supported range get truncated, leading to incorrect
> critical power settings.
> This patch ensures proper clamping to prevent such truncation.
> 
> Signed-off-by: Karthik Poosa <karthik.poosa at intel.com>
> Fixes: 92d44a422d0d ("drm/xe/hwmon: Expose card reactive critical power")
> Fixes: fb1b70607f73 ("drm/xe/hwmon: Expose power attributes")
> ---
>   drivers/gpu/drm/xe/xe_hwmon.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index f08fc4377d25..6f56f9192c6b 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -356,6 +356,15 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
>   		goto unlock;
>   	}
>   
> +	/*
> +	 * If the sysfs value exceeds the pcode mailbox cmd WRITE_PSYSGPU/PACKAGE_POWER_LIMIT
> +	 * max supported value, clamp it to the command's max (U12.3 format) to avoid
> +	 * truncation during reg_val calculation below and ensure the valid power limit is sent for
> +	 * pcode which would clamp it to card-supported value.
> +	 */

Split into two sentences

> +	if (value > (((PWR_LIM_VAL) >> hwmon->scl_shift_power) * SF_POWER))

Follow same format in both checks. Use a local variable for readability 
and add a log
> +		value = (((PWR_LIM_VAL) >> hwmon->scl_shift_power) * SF_POWER);
> +
>   	/* Computation in 64-bits to avoid overflow. Round to nearest. */
>   	reg_val = DIV_ROUND_CLOSEST_ULL((u64)value << hwmon->scl_shift_power, SF_POWER);
>   
> @@ -739,9 +748,22 @@ static int xe_hwmon_power_curr_crit_write(struct xe_hwmon *hwmon, int channel,
>   {
>   	int ret;
>   	u32 uval;
> +	unsigned long max_crit_power_curr = 0;

u64 should be enough ?

Thanks
Riana >
>   	mutex_lock(&hwmon->hwmon_lock);
>   
> +	/*
> +	 * If the sysfs value exceeds the pcode mailbox cmd POWER_SETUP_SUBCOMMAND_WRITE_I1
> +	 * max supported value, clamp it to the command's max (U10.6 format) to avoid
> +	 * truncation during uval calculation below and ensure the valid power limit is sent for
> +	 * pcode which would clamp it to card-supported value.
> +	 */
> +	max_crit_power_curr = (POWER_SETUP_I1_DATA_MASK >> POWER_SETUP_I1_SHIFT) * scale_factor;
> +	if (value > max_crit_power_curr) {
> +		value = max_crit_power_curr;
> +		drm_dbg(&hwmon->xe->drm,
> +			"Clamping critical power/curr to max supported\n");
> +	}
>   	uval = DIV_ROUND_CLOSEST_ULL(value << POWER_SETUP_I1_SHIFT, scale_factor);
>   	ret = xe_hwmon_pcode_write_i1(hwmon, uval);
>