Re: [PATCH v5 4/5] drm/xe/xe_late_bind_fw: Enable late binding support for CRI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: wrong Kconfig check in xe_late_bind_init() for CRI
--

commit 4506929d511f2d47da604ed04955ac5e9cacd5bb
Author: Badal Nilawar <badal.nilawar@intel.com>

drm/xe/xe_late_bind_fw: Enable late binding support for CRI

This commit enables late binding firmware support for fan control and ocode on
Crescent Island (CRI) by setting .late_bind_mask in cri_desc.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 33e0ea0d406a0..190eaee5250eb 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -488,6 +488,8 @@ static const struct xe_device_desc cri_desc = {
>  	.has_sriov = true,
>  	.has_sysctrl = true,
>  	.has_temp_sentinel = true,
> +	.late_bind_mask = BIT(XE_LB_FW_FAN_CONTROL) |
> +			  BIT(XE_LB_FW_OCODE),

[Severity: Medium]
Does xe_late_bind_init() also need to be updated to check for
CONFIG_INTEL_MEI_CSC when initializing late binding on CRI?

Unlike Battlemage (bmg_desc), which sets .has_heci_cscfi = 1 and registers an
auxiliary device bound by the mei-gsc driver (CONFIG_INTEL_MEI_GSC), Crescent
Island exposes its MEI CSC controller as a standalone PCI device bound by the
mei-csc driver (CONFIG_INTEL_MEI_CSC):

drivers/misc/mei/pci-csc.c:
static const struct pci_device_id mei_csc_pci_tbl[] = {
	{ PCI_DEVICE_DATA(INTEL, MEI_CRI, MEI_ME_CSC_CFG) },
	{ }
};

However, xe_device_probe() -> xe_late_bind_init() unconditionally checks
CONFIG_INTEL_MEI_GSC:

drivers/gpu/drm/xe/xe_late_bind_fw.c:xe_late_bind_init() {
	...
	if (!IS_ENABLED(CONFIG_INTEL_MEI_LB) || !IS_ENABLED(CONFIG_INTEL_MEI_GSC)) {
		drm_info(&xe->drm, "Can't init xe mei late bind missing mei component\n");
		return 0;
	}
	...
}

If CONFIG_INTEL_MEI_LB and CONFIG_INTEL_MEI_CSC are enabled without
CONFIG_INTEL_MEI_GSC, xe_late_bind_init() will bail out and skip loading the
fan_control and ocode firmware on CRI.

Conversely, if CONFIG_INTEL_MEI_LB and CONFIG_INTEL_MEI_GSC are enabled while
CONFIG_INTEL_MEI_CSC is disabled, xe_late_bind_init() will register the
component and queue xe_late_bind_work(), which will then sleep for
LB_INIT_TIMEOUT_MS (20 seconds) per firmware image waiting for a component
bind that never happens.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028181951.1093283-7-badal.nilawar@intel.com?part=4