[PATCH] drm/xe/hwmon: Clamp power1_crit to GPU firmware default

[Riana Tauro]

Hi Karthik

On 7/10/2025 6:23 PM, Karthik Poosa wrote:
> Clamp card critical power(power1_crit) to GPU firmware default
> when value set by sysfs exceeds the maximum supported by pcode
> mailbox.
> Though pcode already does clamping, values above pcode mailbox
> supported gets truncated resulting in incorrect critical power
> getting set.
> This patch avoids that truncation.
> 
> Signed-off-by: Karthik Poosa <karthik.poosa at intel.com>
> Fixes: 92d44a422d0d ("drm/xe/hwmon: Expose card reactive critical power")
> ---
>   drivers/gpu/drm/xe/xe_hwmon.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index f08fc4377d25..ce239bc00ee4 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -137,7 +137,8 @@ struct xe_hwmon {
>   	u32 pl1_on_boot[CHANNEL_MAX];
>   	/** @pl2_on_boot: power limit PL2 on boot */
>   	u32 pl2_on_boot[CHANNEL_MAX];
> -
> +	/** @psys_crit_on_boot: card critical power from GPU firmware */
> +	u32 psys_crit_on_boot;
>   };
>   
>   static int xe_hwmon_pcode_read_power_limit(const struct xe_hwmon *hwmon, u32 attr, int channel,
> @@ -742,7 +743,16 @@ static int xe_hwmon_power_curr_crit_write(struct xe_hwmon *hwmon, int channel,
>   
>   	mutex_lock(&hwmon->hwmon_lock);
>   
> -	uval = DIV_ROUND_CLOSEST_ULL(value << POWER_SETUP_I1_SHIFT, scale_factor);
> +	/* Maximum card critical power value supported by pcode mailbox. */
> +#define MAX_CRIT_POWER_CURR 1023000000ull

Add a comment about format. Can you try not to hardcode this and use the 
shift and data to generate the value?

> +
> +	if (value > MAX_CRIT_POWER_CURR) {
> +		uval = hwmon->psys_crit_on_boot;

As discussed offline, you can remove the reading on boot and use max 
value directly.

Thanks
Riana

> +		drm_info(&hwmon->xe->drm,
> +			 "Clamping critical power to GPU firmware default 0x%x\n", uval);
> +	} else {
> +		uval = DIV_ROUND_CLOSEST_ULL(value << POWER_SETUP_I1_SHIFT, scale_factor);
> +	}
>   	ret = xe_hwmon_pcode_write_i1(hwmon, uval);
>   
>   	mutex_unlock(&hwmon->hwmon_lock);
> @@ -1251,6 +1261,9 @@ xe_hwmon_get_preregistration_info(struct xe_hwmon *hwmon)
>   			hwmon->scl_shift_time = REG_FIELD_GET(PKG_TIME_UNIT, val_sku_unit);
>   		}
>   	}
> +	xe_hwmon_pcode_read_i1(hwmon, &hwmon->psys_crit_on_boot);
> +	hwmon->psys_crit_on_boot &= POWER_SETUP_I1_DATA_MASK;
> +
>   	/*
>   	 * Initialize 'struct xe_hwmon_energy_info', i.e. set fields to the
>   	 * first value of the energy register read