All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
@ 2026-10-12 10:42 Constantinos Chalkias
  2026-10-12 10:59 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Constantinos Chalkias @ 2026-10-12 10:42 UTC (permalink / raw)
  To: intel-xe
  Cc: matthew.brost, thomas.hellstrom, rodrigo.vivi, airlied, simona,
	dri-devel, linux-kernel

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
  2026-10-12 10:42 [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake Constantinos Chalkias
@ 2026-10-12 10:59 ` sashiko-bot
  2026-10-12 11:11   ` Constantinos Chalkias
  2026-10-12 15:15 ` Simon Richter
  2026-10-12 18:50 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
  2 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-10-12 10:59 UTC (permalink / raw)
  To: Constantinos Chalkias; +Cc: intel-xe, dri-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
  2026-10-12 10:59 ` sashiko-bot
@ 2026-10-12 11:11   ` Constantinos Chalkias
  0 siblings, 0 replies; 8+ messages in thread
From: Constantinos Chalkias @ 2026-10-12 11:11 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: intel-xe, dri-devel

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?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
  2026-10-12 10:42 [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake Constantinos Chalkias
  2026-10-12 10:59 ` sashiko-bot
@ 2026-10-12 15:15 ` Simon Richter
  2026-10-13  3:50   ` Simon.Richter
  2026-10-12 18:50 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
  2 siblings, 1 reply; 8+ messages in thread
From: Simon Richter @ 2026-10-12 15:15 UTC (permalink / raw)
  To: intel-xe


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✗ LGCI.VerificationFailed: failure for drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
  2026-10-12 10:42 [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake Constantinos Chalkias
  2026-10-12 10:59 ` sashiko-bot
  2026-10-12 15:15 ` Simon Richter
@ 2026-10-12 18:50 ` Patchwork
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2026-10-12 18:50 UTC (permalink / raw)
  To: Constantinos Chalkias; +Cc: intel-xe

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
  2026-10-12 15:15 ` Simon Richter
@ 2026-10-13  3:50   ` Simon.Richter
  0 siblings, 0 replies; 8+ messages in thread
From: Simon.Richter @ 2026-10-13  3:50 UTC (permalink / raw)
  To: intel-xe

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
@ 2026-10-13  5:39 Constantinos Chalkias
  2026-10-13 10:25 ` Simon Richter
  0 siblings, 1 reply; 8+ messages in thread
From: Constantinos Chalkias @ 2026-10-13  5:39 UTC (permalink / raw)
  To: simon.richter; +Cc: intel-xe, dri-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake
  2026-10-13  5:39 [PATCH] " Constantinos Chalkias
@ 2026-10-13 10:25 ` Simon Richter
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Richter @ 2026-10-13 10:25 UTC (permalink / raw)
  To: cch; +Cc: intel-xe, dri-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-13 10:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 10:42 [PATCH] drm/xe/hwmon: refresh Battlemage thermal telemetry under force-wake Constantinos Chalkias
2026-10-12 10:59 ` sashiko-bot
2026-10-12 11:11   ` Constantinos Chalkias
2026-10-12 15:15 ` Simon Richter
2026-10-13  3:50   ` Simon.Richter
2026-10-12 18:50 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2026-10-13  5:39 [PATCH] " Constantinos Chalkias
2026-10-13 10:25 ` Simon Richter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.