[PATCH v2] drm/xe/hwmon: Fix xe_hwmon_power_max_write

[Riana Tauro]

On 6/9/2025 11:43 AM, Karthik Poosa wrote:
> Prevent the power limit time interval from being overwritten with 0.
> This issue was due to a missing read and modify of current power limit,
> before setting a requested mailbox power limit, which is added in this
> patch.

Maybe prevent other bits from being overwritten or add a rmw version for 
pcode mailbox

Got confused because of power limit interval being mentioned

> 
> v2:
>   - Improve commit message. (Anshuman)
> 
> Signed-off-by: Karthik Poosa <karthik.poosa at intel.com>
> Fixes: 7596d839f6228 ("drm/xe/hwmon: Add support to manage power limits though mailbox")
> ---
>   drivers/gpu/drm/xe/regs/xe_mchbar_regs.h | 1 +
>   drivers/gpu/drm/xe/xe_hwmon.c            | 7 +++++--
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h b/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> index 5394a1373a6b..ef2bf984723f 100644
> --- a/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> @@ -40,6 +40,7 @@
>   #define PCU_CR_PACKAGE_RAPL_LIMIT		XE_REG(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
>   #define   PWR_LIM_VAL				REG_GENMASK(14, 0)
>   #define   PWR_LIM_EN				REG_BIT(15)
> +#define   PWR_LIM				REG_GENMASK(15, 0)
>   #define   PWR_LIM_TIME				REG_GENMASK(23, 17)
>   #define   PWR_LIM_TIME_X			REG_GENMASK(23, 22)
>   #define   PWR_LIM_TIME_Y			REG_GENMASK(21, 17)
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 0d32e977537c..abb5c3693437 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -377,11 +377,14 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
>   
>   	reg_val = PWR_LIM_EN | REG_FIELD_PREP(PWR_LIM_VAL, reg_val);
>   
> -	if (hwmon->xe->info.has_mbx_power_limits)
> +	if (hwmon->xe->info.has_mbx_power_limits) {
> +		xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &max);
> +		reg_val = (max & ~PWR_LIM) | reg_val;
If you re-use this in the future, maybe add a different helper function.
Looks good for now

Reviewed-by: Riana Tauro <riana.tauro at intel.com>


>   		ret = xe_hwmon_pcode_write_power_limit(hwmon, attr, channel, reg_val);
> -	else
> +	} else {
>   		reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM_EN | PWR_LIM_VAL,
>   					reg_val);
> +	}
>   unlock:
>   	mutex_unlock(&hwmon->hwmon_lock);
>   	return ret;