Re: [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Oct 12, 2026 at 01:42:45PM +0300, Constantinos Chalkias wrote:
> On Battlemage, thermal telemetry can remain stale after the GT returns
> to RC6 following sustained workloads.
> 
> On an Intel Arc Pro B70, package and VRAM temperature telemetry may stop
> refreshing after a long inference workload. Fan telemetry can also
> become invalid while the physical blower remains elevated.
> 
> Acquire the GT force-wake domain around Battlemage thermal MMIO reads
> and allow a short settling interval before reading the register. Route
> package, VRAM and VRAM-channel availability reads through the helper.
> 
> Testing showed that an immediate read, as well as a 100-300 us delay,
> could leave telemetry stale. A 500-1000 us delay recovered correctly
> across repeated inference-to-idle cycles.
> 
> The change is restricted to Battlemage, where the behavior has been
> reproduced and validated.
> 
> Signed-off-by: Constantinos Chalkias <cch@e-psn.eu>

Cc: Bata Roland Krisztián <batarolandkrisztian@gmail.com>

https://lore.kernel.org/intel-xe/179041811564.526373.13204522833274380927.b70-hwmon-rfc@gmail.com/

I believe you guys are facing the same dificulties and arriving to similar
conclusions.

If the consensus is that we need this forcewake, let's do it.

Let's just avoid duplication of efforts in here.

And thank you all for the contributions.

> ---
>  drivers/gpu/drm/xe/xe_hwmon.c | 51 ++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 5edeac961ec3..fee358ab9a6b 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -3,6 +3,7 @@
>   * Copyright © 2023 Intel Corporation
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/hwmon-sysfs.h>
>  #include <linux/hwmon.h>
>  #include <linux/jiffies.h>
> @@ -14,6 +15,7 @@
>  #include "regs/xe_mchbar_regs.h"
>  #include "regs/xe_pcode_regs.h"
>  #include "xe_device.h"
> +#include "xe_force_wake.h"
>  #include "xe_hwmon.h"
>  #include "xe_mmio.h"
>  #include "xe_pcode.h"
> @@ -946,14 +948,43 @@ static void xe_hwmon_get_voltage(struct xe_hwmon *hwmon, int channel, long *valu
>  	*value = DIV_ROUND_CLOSEST(REG_FIELD_GET(VOLTAGE_MASK, reg_val) * 2500, SF_VOLTAGE);
>  }
>  
> -static inline bool is_vram_ch_available(struct xe_hwmon *hwmon, int channel)
> +static int xe_hwmon_temp_mmio_read(struct xe_hwmon *hwmon,
> +				   struct xe_reg reg, u32 *reg_val)
>  {
>  	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
> +
> +	if (hwmon->xe->info.platform == XE_BATTLEMAGE) {
> +		struct xe_gt *gt = xe_root_mmio_gt(hwmon->xe);
> +
> +		CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GT);
> +
> +		if (!xe_force_wake_ref_has_domain(fw_ref.domains, XE_FW_GT))
> +			return -ETIMEDOUT;
> +
> +		/*
> +		 * Allow thermal telemetry to refresh after leaving RC6
> +		 * before reading the temperature register.
> +		 */
> +		usleep_range(500, 1000);
> +
> +		*reg_val = xe_mmio_read32(mmio, reg);
> +		return 0;
> +	}
> +
> +	*reg_val = xe_mmio_read32(mmio, reg);
> +
> +	return 0;
> +}
> +
> +static inline bool is_vram_ch_available(struct xe_hwmon *hwmon, int channel)
> +{
>  	int vram_id = channel - CHANNEL_VRAM_N;
>  	struct xe_reg vram_reg;
> +	u32 reg_val;
>  
>  	vram_reg = xe_hwmon_get_reg(hwmon, REG_TEMP, channel);
> -	if (!xe_reg_is_valid(vram_reg) || !xe_mmio_read32(mmio, vram_reg))
> +	if (!xe_reg_is_valid(vram_reg) ||
> +	    xe_hwmon_temp_mmio_read(hwmon, vram_reg, &reg_val) || !reg_val)
>  		return false;
>  
>  	/* Create label only for available vram channel */
> @@ -1025,15 +1056,19 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  static int
>  xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>  {
> -	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
> -	u64 reg_val;
> +	u32 reg_val;
> +	int ret;
>  
>  	switch (attr) {
>  	case hwmon_temp_input:
>  		switch (channel) {
>  		case CHANNEL_PKG:
>  		case CHANNEL_VRAM:
> -			reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_TEMP, channel));
> +			ret = xe_hwmon_temp_mmio_read(hwmon,
> +						      xe_hwmon_get_reg(hwmon, REG_TEMP, channel),
> +						      &reg_val);
> +			if (ret)
> +				return ret;
>  
>  			/* HW register value is in degrees Celsius, convert to millidegrees. */
>  			*val = REG_FIELD_GET(TEMP_MASK, reg_val) * MILLIDEGREE_PER_DEGREE;
> @@ -1043,7 +1078,11 @@ xe_hwmon_temp_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>  		case CHANNEL_PCIE:
>  			return get_pcie_temp(hwmon, val);
>  		case CHANNEL_VRAM_N...CHANNEL_VRAM_N_MAX:
> -			reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_TEMP, channel));
> +			ret = xe_hwmon_temp_mmio_read(hwmon,
> +						      xe_hwmon_get_reg(hwmon, REG_TEMP, channel),
> +						      &reg_val);
> +			if (ret)
> +				return ret;
>  			/*
>  			 * This temperature format is 24 bit [31:8] signed integer and 8 bit
>  			 * [7:0] fraction.
> -- 
> 2.56.0
>