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

[Riana Tauro]

Hi Karthik

On 8/7/2025 7:36 PM, 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.
> 
> v2:
>   - Address below review comments. (Riana)
>   - Split comments into multiple sentences.
>   - Use local variables for readability.
>   - Add a debug log.
>   - Use u64 instead of unsigned long.
> 
> v3:
>   - Change drm_dbg logs to drm_info. (Badal)
> 
> 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 | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index f08fc4377d25..a43bc8b91fea 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -332,6 +332,7 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
>   	int ret = 0;
>   	u32 reg_val, max;
>   	struct xe_reg rapl_limit;
> +	u64 max_mbx_power_limit = 0;
>   
>   	mutex_lock(&hwmon->hwmon_lock);
>   
> @@ -356,6 +357,20 @@ 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).
> +	 * This is 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.
> +	 */
> +	max_mbx_power_limit = ((PWR_LIM_VAL) >> hwmon->scl_shift_power) * SF_POWER;
> +	if (value > max_mbx_power_limit) {
> +		value = max_mbx_power_limit;
> +		drm_info(&hwmon->xe->drm,
> +			 "Sysfs value for ch %d %s exceeds limit; clamped to supported maximum\n",
> +			 channel, PWR_ATTR_TO_STR(attr));

Why semicolon here?
Expand ch to channel. Unnecessary to mention "Sysfs"

With the above changes
Reviewed-by: Riana Tauro <riana.tauro at intel.com>

> +	}
> +
>   	/* 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 +754,23 @@ static int xe_hwmon_power_curr_crit_write(struct xe_hwmon *hwmon, int channel,
>   {
>   	int ret;
>   	u32 uval;
> +	u64 max_crit_power_curr = 0;
>   
>   	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).
> +	 * This is 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_info(&hwmon->xe->drm,
> +			 "Sysfs value for ch %d exceeds limit; clamped to supported maximum\n",
> +			 channel);
> +	}
>   	uval = DIV_ROUND_CLOSEST_ULL(value << POWER_SETUP_I1_SHIFT, scale_factor);
>   	ret = xe_hwmon_pcode_write_i1(hwmon, uval);
>