From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Karthik Poosa <karthik.poosa@intel.com>
Cc: <intel-xe@lists.freedesktop.org>, <anshuman.gupta@intel.com>,
<badal.nilawar@intel.com>, <raag.jadav@intel.com>,
<riana.tauro@intel.com>, <sk.anirban@intel.com>,
<mallesh.koujalagi@intel.com>, <soham.purkait@intel.com>
Subject: Re: [PATCH v3 2/5] drm/xe/hwmon: Use VRAM temperature sensor count from thermal config on CRI
Date: Fri, 18 Sep 2026 05:50:20 -0400 [thread overview]
Message-ID: <aq0JXGomUXDbQghg@intel.com> (raw)
In-Reply-To: <20260910182756.638830-3-karthik.poosa@intel.com>
On Thu, Sep 10, 2026 at 11:57:53PM +0530, Karthik Poosa wrote:
> Read the number of VRAM temperature sensor channels from the second byte
> of READ_THERMAL_CONFIG on CRI platforms. Use the reported count to avoid
> exposing hwmon attributes for unavailable VRAM temperature sensors, while
> retaining the maximum supported channel count on non-CRI platforms.
>
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> ---
>
> v2:
> - Address review comments from sashiko-bot@kernel.org.
> - Address review comments from Badal and Raag.
> - Update HWMON_CHANNEL_INFO() to supports maximum VRAM channels of CRI.
>
> v3:
> - Address review comments from sashiko-bot@kernel.org.
> - Address review comments from Rodrigo.
>
> drivers/gpu/drm/xe/xe_device_types.h | 4 +
> drivers/gpu/drm/xe/xe_hwmon.c | 115 +++++++++++++++++++++++++--
> drivers/gpu/drm/xe/xe_pci.c | 2 +
> drivers/gpu/drm/xe/xe_pci_types.h | 1 +
> drivers/gpu/drm/xe/xe_pcode_api.h | 1 +
> 5 files changed, 115 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index f88bacf63c83..74037a9c32f7 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -178,6 +178,10 @@ struct xe_device {
> u8 has_drm_ras:1;
> /** @info.has_fan_control: Device supports fan control */
> u8 has_fan_control:1;
> + /*
/**
then Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> + * @info.has_fixed_vram_channels: Device has fixed VRAM temperature channel count.
> + */
> + u8 has_fixed_vram_channels:1;
> /** @info.has_flat_ccs: Whether flat CCS metadata is used */
> u8 has_flat_ccs:1;
> /** @info.has_gsc_nvm: Device has gsc non-volatile memory */
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index faed2f5da394..a60a6a731d88 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -39,7 +39,8 @@ enum xe_hwmon_reg_operation {
> REG_READ64,
> };
>
> -#define MAX_VRAM_CHANNELS (16)
> +#define MAX_VRAM_CHANNELS 80
> +#define FIXED_VRAM_CHANNELS 16
>
> enum xe_hwmon_channel {
> CHANNEL_CARD,
> @@ -48,6 +49,7 @@ enum xe_hwmon_channel {
> CHANNEL_MCTRL,
> CHANNEL_PCIE,
> CHANNEL_VRAM_N,
> + /* Compile-time upper bound; actual channel count is hwmon->temp.vram_count */
> CHANNEL_VRAM_N_MAX = CHANNEL_VRAM_N + MAX_VRAM_CHANNELS - 1,
> CHANNEL_MAX,
> };
> @@ -142,12 +144,14 @@ struct xe_hwmon_thermal_info {
> /** @data: temperature limits in dwords */
> u32 data[DIV_ROUND_UP(TEMP_LIMIT_MAX, sizeof(u32))];
> };
> - /** @count: no of temperature sensors available for the platform */
> + /** @count: temperature sensors count from READ_THERMAL_CONFIG */
> u8 count;
> + /** @vram_count: number of VRAM temperature sensors available for the platform */
> + u8 vram_count;
> /** @value: signed value from each sensor */
> s8 value[U8_MAX];
> - /** @vram_label: vram label names */
> - char vram_label[MAX_VRAM_CHANNELS][MAX_LABEL_SIZE];
> + /** @vram_label: vram label names, dynamically allocated based on vram_count */
> + char (*vram_label)[MAX_LABEL_SIZE];
> };
>
> /**
> @@ -271,7 +275,7 @@ static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg
> return BMG_PACKAGE_TEMPERATURE;
> else if (channel == CHANNEL_VRAM)
> return BMG_VRAM_TEMPERATURE;
> - else if (in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
> + else if (in_range(channel, CHANNEL_VRAM_N, hwmon->temp.vram_count))
> return BMG_VRAM_TEMPERATURE_N(channel - CHANNEL_VRAM_N);
> } else if (xe->info.platform == XE_DG2) {
> if (channel == CHANNEL_PKG)
> @@ -777,6 +781,70 @@ static const struct hwmon_channel_info * const hwmon_info[] = {
> HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> + HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_CRIT | HWMON_T_EMERGENCY | HWMON_T_INPUT | HWMON_T_LABEL),
> HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_LABEL | HWMON_P_CRIT |
> HWMON_P_CAP,
> @@ -794,6 +862,14 @@ static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
> u32 config = 0;
> int ret;
>
> + /*
> + * Fixed-channel platforms don't derive the VRAM count from the mailbox
> + * config, so set it up front to keep the sensors visible even if a read
> + * below fails.
> + */
> + if (hwmon->xe->info.has_fixed_vram_channels)
> + hwmon->temp.vram_count = FIXED_VRAM_CHANNELS;
> +
> ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_THERMAL_INFO, READ_THERMAL_LIMITS, 0),
> &hwmon->temp.data[0], &hwmon->temp.data[1]);
> if (ret)
> @@ -810,6 +886,16 @@ static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
> drm_dbg(&hwmon->xe->drm, "thermal config count 0x%x\n", config);
> hwmon->temp.count = REG_FIELD_GET(TEMP_MASK, config);
>
> + if (!hwmon->xe->info.has_fixed_vram_channels) {
> + hwmon->temp.vram_count = REG_FIELD_GET(VRAM_COUNT_MASK, config);
> + if (hwmon->temp.vram_count > MAX_VRAM_CHANNELS) {
> + drm_warn(&hwmon->xe->drm,
> + "VRAM channel count %d exceeds max %d, clamping\n",
> + hwmon->temp.vram_count, MAX_VRAM_CHANNELS);
> + hwmon->temp.vram_count = MAX_VRAM_CHANNELS;
> + }
> + }
> +
> return ret;
> }
>
> @@ -1505,7 +1591,7 @@ static int xe_hwmon_read_label(struct device *dev,
> *str = "mctrl";
> else if (channel == CHANNEL_PCIE)
> *str = "pcie";
> - else if (in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
> + else if (in_range(channel, CHANNEL_VRAM_N, hwmon->temp.vram_count))
> *str = hwmon->temp.vram_label[channel - CHANNEL_VRAM_N];
> return 0;
> case hwmon_power:
> @@ -1634,6 +1720,15 @@ int xe_hwmon_register(struct xe_device *xe)
>
> xe_hwmon_get_preregistration_info(hwmon);
>
> + if (hwmon->temp.vram_count) {
> + hwmon->temp.vram_label = devm_kcalloc(dev, hwmon->temp.vram_count,
> + MAX_LABEL_SIZE, GFP_KERNEL);
> + if (!hwmon->temp.vram_label) {
> + ret = -ENOMEM;
> + goto err_null_hwmon;
> + }
> + }
> +
> drm_dbg(&xe->drm, "Register xe hwmon interface\n");
>
> /* hwmon_dev points to device hwmon<i> */
> @@ -1642,10 +1737,14 @@ int xe_hwmon_register(struct xe_device *xe)
> hwmon_groups);
> if (IS_ERR(hwmon->hwmon_dev)) {
> drm_err(&xe->drm, "Failed to register xe hwmon (%pe)\n", hwmon->hwmon_dev);
> - xe->hwmon = NULL;
> - return PTR_ERR(hwmon->hwmon_dev);
> + ret = PTR_ERR(hwmon->hwmon_dev);
> + goto err_null_hwmon;
> }
>
> return 0;
> +
> +err_null_hwmon:
> + xe->hwmon = NULL;
> + return ret;
> }
> MODULE_IMPORT_NS("INTEL_PMT_TELEMETRY");
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 1e04e8ef2611..d98b240e1753 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -416,6 +416,7 @@ static const struct xe_device_desc bmg_desc = {
> .dma_mask_size = 46,
> .has_display = true,
> .has_fan_control = true,
> + .has_fixed_vram_channels = true,
> .has_flat_ccs = 1,
> .has_mbx_power_limits = true,
> .has_mbx_thermal_info = true,
> @@ -794,6 +795,7 @@ static int xe_info_init_early(struct xe_device *xe,
> xe->info.has_cached_pt = desc->has_cached_pt;
> xe->info.has_drm_ras = desc->has_drm_ras;
> xe->info.has_fan_control = desc->has_fan_control;
> + xe->info.has_fixed_vram_channels = desc->has_fixed_vram_channels;
> /* runtime fusing may force flat_ccs to disabled later */
> xe->info.has_flat_ccs = desc->has_flat_ccs;
> xe->info.has_mbx_power_limits = desc->has_mbx_power_limits;
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
> index fed509ff601e..87b8121b3e51 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -42,6 +42,7 @@ struct xe_device_desc {
> u8 has_display:1;
> u8 has_drm_ras:1;
> u8 has_fan_control:1;
> + u8 has_fixed_vram_channels:1;
> u8 has_flat_ccs:1;
> u8 has_gsc_nvm:1;
> u8 has_heci_gscfi:1;
> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
> index 94575c476e3d..e1079eff72c6 100644
> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
> @@ -57,6 +57,7 @@
> #define PCODE_THERMAL_INFO 0x25
> #define READ_THERMAL_LIMITS 0x0
> #define READ_THERMAL_CONFIG 0x1
> +#define VRAM_COUNT_MASK REG_GENMASK(15, 8)
> #define READ_THERMAL_DATA 0x2
> #define PCIE_SENSOR_GROUP_ID 0x2
> #define PCIE_SENSOR_MASK REG_GENMASK(31, 16)
> --
> 2.25.1
>
next prev parent reply other threads:[~2026-09-18 9:51 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-10 18:27 [PATCH v3 0/5] drm/xe/hwmon: Update hwmon thermal mailbox handling Karthik Poosa
2026-09-10 18:27 ` [PATCH v3 1/5] drm/xe/hwmon: Detect unavailable temperature sensors Karthik Poosa
2026-09-10 18:42 ` sashiko-bot
2026-09-17 11:02 ` Poosa, Karthik
2026-09-18 9:40 ` Rodrigo Vivi
2026-09-23 15:49 ` Poosa, Karthik
2026-09-10 18:27 ` [PATCH v3 2/5] drm/xe/hwmon: Use VRAM temperature sensor count from thermal config on CRI Karthik Poosa
2026-09-10 18:47 ` sashiko-bot
2026-09-18 9:50 ` Rodrigo Vivi [this message]
2026-09-10 18:27 ` [PATCH v3 3/5] drm/xe/hwmon: Fix memory controller thermal data handling Karthik Poosa
2026-09-18 10:00 ` Rodrigo Vivi
2026-09-10 18:27 ` [PATCH v3 4/5] drm/xe/hwmon: Decode CRI temperature registers as IEEE-754 Karthik Poosa
2026-09-18 10:13 ` Rodrigo Vivi
2026-09-24 6:54 ` Poosa, Karthik
2026-09-10 18:27 ` [PATCH v3 5/5] drm/xe/hwmon: Disable memory controller and PCIe temperatures on CRI Karthik Poosa
2026-09-18 10:20 ` Rodrigo Vivi
2026-09-23 18:37 ` Poosa, Karthik
2026-09-10 18:47 ` ✓ CI.KUnit: success for drm/xe/hwmon: Update hwmon thermal mailbox handling (rev3) Patchwork
2026-09-10 19:52 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-11 4:49 ` ✓ Xe.CI.FULL: " Patchwork
2026-09-24 11:16 ` [PATCH v3 0/5] drm/xe/hwmon: Update hwmon thermal mailbox handling Poosa, Karthik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aq0JXGomUXDbQghg@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=badal.nilawar@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=karthik.poosa@intel.com \
--cc=mallesh.koujalagi@intel.com \
--cc=raag.jadav@intel.com \
--cc=riana.tauro@intel.com \
--cc=sk.anirban@intel.com \
--cc=soham.purkait@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox