Re: [PATCH v6 5/6] 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 757939afc7a64783d9f3384321cd0d56046b5326
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 396204f2e3e45..3fa842c22a508 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]
As reported by Sashiko on v5 [1], does xe_late_bind_init() check the right
Kconfig symbol for Crescent Island when xe_device_probe() ->
xe_late_bind_init() runs with late_bind_mask enabled in cri_desc?

In drivers/gpu/drm/xe/xe_late_bind_fw.c, xe_late_bind_init() currently gates
initialization on CONFIG_INTEL_MEI_GSC:

drivers/gpu/drm/xe/xe_late_bind_fw.c:xe_late_bind_init() {
	if (!xe->info.late_bind_mask)
		return 0;

	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;
	}
}

Unlike Battlemage (bmg_desc), which sets has_heci_cscfi = 1 and registers an
auxiliary device bound by the mei-gsc driver under CONFIG_INTEL_MEI_GSC,
cri_desc has both has_heci_gscfi and has_heci_cscfi set to false. Instead,
Crescent Island exposes a standalone PCI MEI CSC controller bound by
mei_csc_driver under 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) },
	{ }
};

If CONFIG_INTEL_MEI_LB and CONFIG_INTEL_MEI_CSC are enabled while
CONFIG_INTEL_MEI_GSC is disabled, 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 late
bind component and queue xe_late_bind_work(), where each payload worker waits
LB_INIT_TIMEOUT_MS (20 seconds per payload, up to 40 seconds total on the
ordered workqueue) for a component bind that never happens, stalling
xe_late_bind_wait_for_worker_completion() during xe_late_bind_pm_suspend() or
xe_late_bind_remove().

Should xe_late_bind_init() check CONFIG_INTEL_MEI_CSC for Crescent Island
instead of CONFIG_INTEL_MEI_GSC?

[1] https://lore.kernel.org/r/20261028181951.1093283-11-badal.nilawar@intel.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030144547.2532309-8-badal.nilawar@intel.com?part=5