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

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

[Constantinos Chalkias]

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>
---
 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: fan telemetry lacks Battlemage thermal force-wake workaround
--

commit a9faaf30cbf789280ad52add1cf188270e8ccad9
Author: Constantinos Chalkias <cch@e-psn.eu>

drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake

This commit works around stale thermal telemetry on Battlemage by
acquiring the GT force-wake domain and waiting 500-1000 us before
reading temperature registers.

> drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
> 
> 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.

[Severity: Medium]
The commit message states that fan telemetry can become invalid, but the
patch only applies the force-wake and delay workaround to temperature and
VRAM channels via xe_hwmon_temp_mmio_read().

Does xe_hwmon_fan_input_read() also need this workaround? Currently, it
continues to read REG_FAN_SPEED directly via xe_mmio_read32() without
acquiring force-wake or delaying:

drivers/gpu/drm/xe/xe_hwmon.c:xe_hwmon_fan_input_read() {
    ...
	mutex_lock(&hwmon->hwmon_lock);

	reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_FAN_SPEED, channel));
	time_now = get_jiffies_64();
    ...
}

Will fan telemetry still remain susceptible to staleness on Battlemage
without the workaround?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012104245.2108030-1-cch@e-psn.eu?part=1

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

[Constantinos Chalkias]

Thanks for catching this.
On the B70 I observed two distinct symptoms after a sustained workload:
- package/VRAM temperature values could remain stale while the physical 
blower stayed elevated even after the device had returned to idle;
- fan1_input could independently report implausibly low or zero RPM 
while the physical blower was still running fast.
This patch only addresses the temperature MMIO path. Holding XE_FW_GT 
for a short settling interval before the temperature read causes the 
temperature telemetry to refresh and, in my testing, the physical blower 
then settles normally as well.
The REG_FAN_SPEED path remains a separate direct MMIO read and can still 
show unreliable values, so the current patch should not be read as 
fixing fan RPM telemetry itself.
I can reword the commit message in a v2 to make that distinction 
explicit, and investigate the fan telemetry path separately.

On 2026-10-12 13:59, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential 
> issue(s) to consider:
> - [Medium] drm/xe: fan telemetry lacks Battlemage thermal force-wake 
> workaround
> --
> 
> commit a9faaf30cbf789280ad52add1cf188270e8ccad9
> Author: Constantinos Chalkias <cch@e-psn.eu>
> 
> drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
> 
> This commit works around stale thermal telemetry on Battlemage by
> acquiring the GT force-wake domain and waiting 500-1000 us before
> reading temperature registers.
> 
>> drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
>> 
>> 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.
> 
> [Severity: Medium]
> The commit message states that fan telemetry can become invalid, but 
> the
> patch only applies the force-wake and delay workaround to temperature 
> and
> VRAM channels via xe_hwmon_temp_mmio_read().
> 
> Does xe_hwmon_fan_input_read() also need this workaround? Currently, it
> continues to read REG_FAN_SPEED directly via xe_mmio_read32() without
> acquiring force-wake or delaying:
> 
> drivers/gpu/drm/xe/xe_hwmon.c:xe_hwmon_fan_input_read() {
>     ...
> 	mutex_lock(&hwmon->hwmon_lock);
> 
> 	reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_FAN_SPEED, 
> channel));
> 	time_now = get_jiffies_64();
>     ...
> }
> 
> Will fan telemetry still remain susceptible to staleness on Battlemage
> without the workaround?

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

[Simon Richter]

[-- Attachment #1.1: Type: text/plain, Size: 1127 bytes --]

Hi,

On 10/12/26 19:42, Constantinos Chalkias wrote:

> On Battlemage, thermal telemetry can remain stale after the GT returns
> to RC6 following sustained workloads.

That feels like it could be related to

https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/4560

> 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.

Is that necessary, or a workaround for what should really happen in 
firmware?

If the latter, it probably needs a comment to that effect, so it can be 
made conditional on a firmware version check.

Also, how does this behave if someone is querying multiple sensors? I 
have collectd reading all sensors every ten seconds on a machine where 
the GPU is usually idle with no clients connected, would that switch 
power state ten times?

Also, it seems for me only the `pkg` readout is stuck, the `vram` 
readings are fine, and so are fan speed measurements -- the fan speed 
just never changes.

    Simon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

✗ LGCI.VerificationFailed: failure for drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake

[Patchwork]

== Series Details ==

Series: drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
URL   : https://patchwork.freedesktop.org/series/175962/
State : failure

== Summary ==

Series author address 'cch@e-psn.eu' is not on the allowlist, which prevents CI from being automatically triggered.
If you want CI to run for this series, ask Patchwork project owners to click 'retest' on the series in Patchwork.
Exception occurred during validation, bailing out!
Build URL: http://intel-gfx-ci-public.igk.intel.com:8080/job/xe_pw_trigger/1310214/ (on master)

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

[Simon.Richter]

Hi,

did a cursory test, it seems the *change* in forcewake state is what 
makes it poll the sensors once, and the "settling" time is not a power 
state transition, but the time needed until the new values arrive.

If forcewake is set all the time, the sensors are still stuck.

I wonder If there is something else we could be doing to poke that loop.

   Simon

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

[Constantinos Chalkias]

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

Hi Simon,

For some context, I started investigating this because my Maxsun Arc Pro 
B70 would latch the blower at very high speed after a long inference 
workload and leave it there even when the node was otherwise idle. I had 
previously dealt with a separate idle-power issue where continuous Intel 
Level Zero Sysman/XPUM monitoring kept the card at elevated power with a 
model resident; after removing that polling, the card could return to 
low idle power normally. The persistent blower latch was the remaining 
reproducible problem.

I did some more targeted testing based on your comments.

I restricted the workaround to CHANNEL_PKG only and disabled both hwmon 
pollers so no other sensor reads could interfere.

With a 500-1000 us settling interval, the package-only workaround was 
not reliable. The physical blower could remain latched at high speed for 
several minutes after the workload ended.

I then repeated the test with a package-only 3000-3500 us settling 
interval. With monitoring disabled, the blower remained latched high for 
more than five minutes after inference stopped. A single temp2_input 
package-temperature read then immediately caused the blower to drop; the 
read returned 36 C.

So the workaround does not need to cover VRAM or individual VRAM-channel 
reads. A package-only force-wake appears sufficient, provided the 
settling interval is long enough.

I also confirmed that fan1_input is a separate issue: it can report 
implausibly low values while the physical blower is still running at 
high speed.

This makes the v1 patch broader than necessary. I'll narrow v2 to the 
package-temperature path and document the delay as an empirical 
workaround rather than an architectural timing requirement.

Thanks,
Constantinos

[-- Attachment #2: Type: text/html, Size: 2270 bytes --]

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

[Simon Richter]

Hi,

On 10/13/26 2:39 PM, Constantinos Chalkias wrote:

> For some context, I started investigating this because my Maxsun Arc Pro 
> B70 would latch the blower at very high speed after a long inference 
> workload and leave it there even when the node was otherwise idle.

Yes, that is because the loop polling the pkg sensor for some reason 
stops while no client is connected, but it can be triggered once by some 
other events, such as changing forcewake state, or monitor hotplug.

So I have a background task running that just does

     while true
     do
         sleep 5 >forcewake_all
         sleep 5
     done

and I get working fan control, regardless of whether I'm also polling 
the sensors.

> With a 500-1000 us settling interval, the package-only workaround was 
> not reliable. The physical blower could remain latched at high speed for 
> several minutes after the workload ended.

It works if the pulse is long enough to be noticed. If you send multiple 
pulses (because you are reading multiple sensors in turn) with only 
brief pauses in between, you're generating a longer pulse that has a 
higher chance of being noticed.

I'd bet it would also work to just assert forcewake for 10 ms instead of 
ten shorter intervals, so we can use a timer here to asynchronously 
reset instead of synchronously waiting in the hwmon readout -- we're not 
getting fresh values here anyway, because the loop has not woken up yet.

The real solution, of course, would be a firmware fix.

    Simon

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

[Rodrigo Vivi]

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
> 

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

[Constantinos Chalkias]

Hi Rodrigo,
Thanks for pointing this out. Yes, this appears to be the same issue, 
and I agree we should avoid parallel fixes.
My follow-up testing independently converged on a very similar result. I 
narrowed the workaround to CHANNEL_PKG only. A 500-1000 us GT force-wake 
interval was not reliable, while 3000-3500 us was.
In the most controlled test I disabled both hwmon pollers, stopped the 
inference workload, and the physical blower remained latched at high 
speed for more than five minutes while the GPU was otherwise idle. A 
single package-temperature read using XE_FW_GT with a 3000-3500 us 
interval immediately caused the blower to return to normal.
I also saw the later RFC follow-up showing that both GT-only and 
RENDER-only longer pulses could refresh the telemetry and were followed 
by fan slowdown, which seems consistent with this result.
I won't send a competing v2. I'm happy to contribute these measurements 
to the existing RFC effort and help test whatever implementation you 
prefer to converge on.
Thanks,
Constantinos

On 2026-10-14 23:12, Rodrigo Vivi wrote:
> 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
>> 

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

[Simon Richter]

Hi,

On 10/15/26 5:12 AM, Rodrigo Vivi wrote:

> If the consensus is that we need this forcewake, let's do it.
It feels like we haven't really understood the problem yet, we just know 
something that appears to work.

If I run

     while true; do sleep 5 >forcewake_all; sleep 5; done & watch sensors

then the temperature sensors *do* update, but several of the VRAM 
channels take over a minute, so the long forcewake does not solve the 
problem. If shorter ones do, then I'd really like to understand why; my 
theory is that it's not the forcewake itself that does the job, but the 
fact that it's toggling, and possibly generating some events in some 
microcontroller. It may be sufficient to send some no-op commands 
instead of playing with the power rails.

Starting the loop above with pretty much everything at a nice toasty 
equilibrium temperature of 82 degrees (which is way too much IMO for a 
card that has no client connected and is just displaying a console 
login), the fans immediately come on, and apparently every ten seconds 
one of the VRAM channels updates to a lower temperature, so one 
forcewake event pretty much exactly corresponds to one sensor update.

That would explain why the patch works -- it generates the same number 
of forcewake events as sensor reads, and the delay makes sure they are 
registered as separate events. However, there is no correspondence 
between the event and the sensor, so if I do my normal telemetry 
recording (every sensor once every ten seconds) I can still get data 
that is ten seconds old, and I also wake up the GPU once every ten 
seconds even if there is nothing to do.

That is an improvement over the status quo, but my concern is that it 
will push us over into "good enough" territory where the actual problem 
is never addressed.

    Simon