Re: [PATCH] drm/i915/guc: Test GuC v70.83.0 for DG2, MTL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/guc: guc_mmp() replacement breaks major-versioned firmware on MTL/DG2
--

commit 8a0f11765b445043e0614bba8e7df77aba661014
Author: Julia Filipchuk <julia.filipchuk@intel.com>

drm/i915/guc: Test GuC v70.83.0 for DG2, MTL

This commit updates the Meteor Lake and DG2 GuC firmware definitions in
intel_uc_fw.c from v70.53.0 using guc_maj() to v70.83.0 using guc_mmp() for
testing.

> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index d7c731fe972bf..c89a1b97108c0 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -89,8 +89,8 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>   * security fixes, etc. to be enabled.
>   */
>  #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
> -	fw_def(METEORLAKE,   0, guc_maj(mtl,  70, 53, 0)) \
> -	fw_def(DG2,          0, guc_maj(dg2,  70, 53, 0)) \
> +	fw_def(METEORLAKE,   0, guc_mmp(mtl,  70, 83, 0)) \
> +	fw_def(DG2,          0, guc_mmp(dg2,  70, 83, 0)) \

[Severity: High]
Does replacing guc_maj() with guc_mmp() here break GuC firmware loading on
Meteor Lake and DG2 systems that have standard linux-firmware installed?

With guc_mmp(), blobs_guc[] and the module firmware table only list
i915/mtl_guc_70.83.0.bin and i915/dg2_guc_70.83.0.bin, removing the
major-versioned i915/mtl_guc_70.bin and i915/dg2_guc_70.bin entries with no
fallback.

When intel_uc_fw_fetch() calls try_firmware_load() and those three-part
versioned test files are absent, the fallback loop in intel_uc_fw_fetch()
calls __uc_fw_auto_select():

drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:intel_uc_fw_fetch() {
	...
	while (err == -ENOENT) {
		old_ver = true;

		__uc_fw_auto_select(i915, uc_fw);
		if (!uc_fw->file_selected.path) {
			...
}

Because no other blobs_guc[] entry exists for Meteor Lake or DG2,
__uc_fw_auto_select() leaves file_selected.path unset and firmware fetch
fails. Since GuC submission is enabled by default on both platforms,
__uc_init_hw() then aborts hardware initialization:

drivers/gpu/drm/i915/gt/uc/intel_uc.c:__uc_init_hw() {
	...
	if (!intel_uc_fw_is_loadable(&guc->fw)) {
		ret = __uc_check_hw(uc) ||
		      intel_uc_fw_is_overridden(&guc->fw) ||
		      intel_uc_wants_guc_submission(uc) ?
		      intel_uc_fw_status_to_error(guc->fw.status) : 0;
		goto err_out;
	}
	...
}

Should these entries remain guc_maj(mtl, 70, 83, 0) and
guc_maj(dg2, 70, 83, 0) (or retain the guc_maj() entries as fallbacks)
before this is merged beyond testing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102221518.546010-2-julia.filipchuk@intel.com?part=1