Re: [PATCH v2 5/5] drm/xe/hwmon: Disable memory controller and PCIe temperatures on CRI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch uses an enum ordering check that inadvertently disables memory controller and PCIe temperatures on Nova Lake P, splitting the Novalake platform family.
--

commit 91ec5832293666790affd1f3a6202739f54f4e49
Author: Karthik Poosa <karthik.poosa@intel.com>

    drm/xe/hwmon: Disable memory controller and PCIe temperatures on CRI

    Temporarily disable memory controller and PCIe temperature reporting on
    CRI platforms. The definitions and interpretation of these temperature
    sources are not yet sufficiently clear, which may result in inaccurate
    reporting.

> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index c2b8d4c79783a..5b2cef292ffcc 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -1061,8 +1061,14 @@ static bool is_temp_available(struct xe_hwmon *hwmon, int channel)
>  
>  		return true;
>  	case CHANNEL_MCTRL:
> +		if (hwmon->xe->info.platform >= XE_CRESCENTISLAND)
> +			return false;
> +
>  		return hwmon->temp.count && !get_mc_temp(hwmon, NULL);
>  	case CHANNEL_PCIE:
> +		if (hwmon->xe->info.platform >= XE_CRESCENTISLAND)
> +			return false;
> +

[Severity: Medium]
Does this check inadvertently affect Nova Lake P?

Looking at the enum definition in xe_platform_types.h, XE_NOVALAKE_P is
defined after XE_CRESCENTISLAND, while XE_NOVALAKE_S is defined before it:

	XE_NOVALAKE_S,
	XE_CRESCENTISLAND,
	XE_NOVALAKE_P,

Using the >= operator will disable memory controller and PCIe temperatures on
Nova Lake P, while they remain enabled for Nova Lake S. This seems to
contradict the commit message which states this disables temperatures on CRI
platforms.

Should these checks use == XE_CRESCENTISLAND instead to avoid splitting the
Novalake family?

>  		return hwmon->temp.count && !get_pcie_temp(hwmon, NULL);
>  	default:
>  		return false;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902175507.3910573-1-karthik.poosa@intel.com?part=5