Re: [PATCH v6 08/13] drm/xe/hwmon: Cache temperature availability to reduce probe time

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

On Fri, Sep 25, 2026 at 02:24:24AM +0530, Karthik Poosa wrote:
> Avoid probing temperature sensor availability from the hwmon is_visible()
> callback. For MCTRL and PCIe channels, that path can trigger repeated
> blocking mailbox reads while sysfs attributes are being created, increasing
> probe time and making attribute creation sensitive to transient failures.
> 
> Probe temperature availability once during preregistration and cache the
> result for subsequent visibility checks.

fine to cache, perhaps wroth checking that 0 temp case..

but this patch itself looks good:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> 
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> Assisted-by: LLM
> ---
>  drivers/gpu/drm/xe/xe_hwmon.c | 39 +++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 052b25a233d6..92280248f1e9 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -146,6 +146,8 @@ struct xe_hwmon_thermal_info {
>  	};
>  	/** @count: no of temperature sensors available for the platform */
>  	u8 count;
> +	/** @available: temperature sensor availability cached at registration */
> +	bool available[CHANNEL_MAX];
>  	union {
>  		/** @value: per-sensor raw mailbox temperature; bit7=sign, bits6:0=magnitude */
>  		u8 value[U8_MAX + 1];
> @@ -1002,7 +1004,7 @@ 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 bool is_temp_available(struct xe_hwmon *hwmon, int channel)
> +static bool xe_hwmon_temp_probe(struct xe_hwmon *hwmon, int channel)
>  {
>  	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
>  	struct xe_reg reg;
> @@ -1023,10 +1025,6 @@ static bool is_temp_available(struct xe_hwmon *hwmon, int channel)
>  			return false;
>  		}
>  
> -		if (channel >= CHANNEL_VRAM_N)
> -			sprintf(hwmon->temp.vram_label[channel - CHANNEL_VRAM_N],
> -				"vram_ch_%d", channel - CHANNEL_VRAM_N);
> -
>  		return true;
>  	case CHANNEL_MCTRL:
>  		return hwmon->temp.count && !get_mc_temp(hwmon, NULL);
> @@ -1037,6 +1035,22 @@ static bool is_temp_available(struct xe_hwmon *hwmon, int channel)
>  	}
>  }
>  
> +static void xe_hwmon_init_temp_info(struct xe_hwmon *hwmon)
> +{
> +	int channel;
> +
> +	if (hwmon->xe->info.has_mbx_thermal_info && xe_hwmon_pcode_read_thermal_info(hwmon))
> +		drm_warn(&hwmon->xe->drm, "Thermal mailbox not supported by card firmware\n");
> +
> +	for (channel = 0; channel < CHANNEL_MAX; channel++) {
> +		hwmon->temp.available[channel] = xe_hwmon_temp_probe(hwmon, channel);
> +		if (hwmon->temp.available[channel] &&
> +		    in_range(channel, CHANNEL_VRAM_N, MAX_VRAM_CHANNELS))
> +			snprintf(hwmon->temp.vram_label[channel - CHANNEL_VRAM_N], MAX_LABEL_SIZE,
> +				 "vram_ch_%d", channel - CHANNEL_VRAM_N);
> +	}
> +}
> +
>  static umode_t
>  xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  {
> @@ -1046,11 +1060,11 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  		case CHANNEL_PKG:
>  		case CHANNEL_MCTRL:
>  		case CHANNEL_PCIE:
> -			return (is_temp_available(hwmon, channel) &&
> +			return (hwmon->temp.available[channel] &&
>  				hwmon->temp.limit[TEMP_LIMIT_PKG_SHUTDOWN]) ? 0444 : 0;
>  		case CHANNEL_VRAM:
>  		case CHANNEL_VRAM_N...CHANNEL_VRAM_N_MAX:
> -			return (is_temp_available(hwmon, channel) &&
> +			return (hwmon->temp.available[channel] &&
>  				hwmon->temp.limit[TEMP_LIMIT_MEM_SHUTDOWN]) ? 0444 : 0;
>  		default:
>  			return 0;
> @@ -1060,11 +1074,11 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  		case CHANNEL_PKG:
>  		case CHANNEL_MCTRL:
>  		case CHANNEL_PCIE:
> -			return (is_temp_available(hwmon, channel) &&
> +			return (hwmon->temp.available[channel] &&
>  				hwmon->temp.limit[TEMP_LIMIT_PKG_CRIT]) ? 0444 : 0;
>  		case CHANNEL_VRAM:
>  		case CHANNEL_VRAM_N...CHANNEL_VRAM_N_MAX:
> -			return (is_temp_available(hwmon, channel) &&
> +			return (hwmon->temp.available[channel] &&
>  				hwmon->temp.limit[TEMP_LIMIT_MEM_CRIT]) ? 0444 : 0;
>  		default:
>  			return 0;
> @@ -1072,7 +1086,7 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  	case hwmon_temp_max:
>  		switch (channel) {
>  		case CHANNEL_PKG:
> -			return (is_temp_available(hwmon, channel) &&
> +			return (hwmon->temp.available[channel] &&
>  				hwmon->temp.limit[TEMP_LIMIT_PKG_MAX]) ? 0444 : 0;
>  		default:
>  			return 0;
> @@ -1085,7 +1099,7 @@ xe_hwmon_temp_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  		case CHANNEL_MCTRL:
>  		case CHANNEL_PCIE:
>  		case CHANNEL_VRAM_N...CHANNEL_VRAM_N_MAX:
> -			return is_temp_available(hwmon, channel) ? 0444 : 0;
> +			return hwmon->temp.available[channel] ? 0444 : 0;
>  		default:
>  			return 0;
>  		}
> @@ -1627,8 +1641,7 @@ xe_hwmon_get_preregistration_info(struct xe_hwmon *hwmon)
>  		if (xe_hwmon_is_visible(hwmon, hwmon_fan, hwmon_fan_input, channel))
>  			xe_hwmon_fan_input_read(hwmon, channel, &fan_speed);
>  
> -	if (hwmon->xe->info.has_mbx_thermal_info && xe_hwmon_pcode_read_thermal_info(hwmon))
> -		drm_warn(&hwmon->xe->drm, "Thermal mailbox not supported by card firmware\n");
> +	xe_hwmon_init_temp_info(hwmon);
>  }
>  
>  int xe_hwmon_register(struct xe_device *xe)
> -- 
> 2.25.1
>