Re: [PATCH v4 5/8] drm/xe/guc: Bump minimum required GuC version to v70.36.0

[John Harrison <john.c.harrison@intel.com>]

On 1/29/2025 02:16, Riana Tauro wrote:
> The VF API version for this release is 1.17.1
>
> Bump the minimum required version to v70.36.0 to support
> engine activity.
We can only bump the minimum recommended version, not the required version.

>
> Suggested-by: John Harrison <John.C.Harrison@Intel.com>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_uc_fw.c | 28 ++++++++++++++--------------
>   1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> index 18e06ee9e23f..d9ff285c5d1d 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> @@ -108,17 +108,17 @@ struct fw_blobs_by_type {
>   
>   #define XE_GUC_FIRMWARE_DEFS(fw_def, mmp_ver, major_ver)			\
>   	fw_def(PANTHERLAKE,	mmp_ver(xe,	guc,	ptl,	70, 38, 1))	\
> -	fw_def(BATTLEMAGE,	major_ver(xe,	guc,	bmg,	70, 29, 2))	\
> -	fw_def(LUNARLAKE,	major_ver(xe,	guc,	lnl,	70, 29, 2))	\
> -	fw_def(METEORLAKE,	major_ver(i915,	guc,	mtl,	70, 29, 2))	\
> -	fw_def(PVC,		mmp_ver(xe,	guc,	pvc,	70, 29, 2))	\
> -	fw_def(DG2,		major_ver(i915,	guc,	dg2,	70, 29, 2))	\
> -	fw_def(DG1,		major_ver(i915,	guc,	dg1,	70, 29, 2))	\
> -	fw_def(ALDERLAKE_N,	major_ver(i915,	guc,	tgl,	70, 29, 2))	\
> -	fw_def(ALDERLAKE_P,	major_ver(i915,	guc,	adlp,	70, 29, 2))	\
> -	fw_def(ALDERLAKE_S,	major_ver(i915,	guc,	tgl,	70, 29, 2))	\
> -	fw_def(ROCKETLAKE,	major_ver(i915,	guc,	tgl,	70, 29, 2))	\
> -	fw_def(TIGERLAKE,	major_ver(i915,	guc,	tgl,	70, 29, 2))
> +	fw_def(BATTLEMAGE,	major_ver(xe,	guc,	bmg,	70, 36, 0))	\
> +	fw_def(LUNARLAKE,	major_ver(xe,	guc,	lnl,	70, 36, 0))	\
> +	fw_def(METEORLAKE,	major_ver(i915,	guc,	mtl,	70, 36, 0))	\
> +	fw_def(PVC,		mmp_ver(xe,	guc,	pvc,	70, 36, 0))	\
> +	fw_def(DG2,		major_ver(i915,	guc,	dg2,	70, 36, 0))	\
> +	fw_def(DG1,		major_ver(i915,	guc,	dg1,	70, 36, 0))	\
> +	fw_def(ALDERLAKE_N,	major_ver(i915,	guc,	tgl,	70, 36, 0))	\
> +	fw_def(ALDERLAKE_P,	major_ver(i915,	guc,	adlp,	70, 36, 0))	\
> +	fw_def(ALDERLAKE_S,	major_ver(i915,	guc,	tgl,	70, 36, 0))	\
> +	fw_def(ROCKETLAKE,	major_ver(i915,	guc,	tgl,	70, 36, 0))	\
> +	fw_def(TIGERLAKE,	major_ver(i915,	guc,	tgl,	70, 36, 0))
This part is technically fine, but note that we have just discovered an 
issue with recent GuC releases which means we need to wait for a bug fix 
before updating.

Also note that the purpose of the minor/patch version numbers in this 
table is just to provide a notice to the user that they should update. 
So after the update to 70.36.0, if a user boots a LNL with 70.29.2, they 
will get a line in dmesg saying "please update". But the driver will 
still load and run with no negative effects.

>   
>   #define XE_HUC_FIRMWARE_DEFS(fw_def, mmp_ver, no_ver)		\
>   	fw_def(PANTHERLAKE,	mmp_ver(xe,	huc,		ptl, 10, 2, 1))	\
> @@ -320,9 +320,9 @@ static int guc_read_css_info(struct xe_uc_fw *uc_fw, struct uc_css_header *css)
>   
>   	xe_gt_assert(gt, uc_fw->type == XE_UC_FW_TYPE_GUC);
>   
> -	/* We don't support GuC releases older than 70.29.2 */
> -	if (MAKE_GUC_VER_STRUCT(*release) < MAKE_GUC_VER(70, 29, 2)) {
> -		xe_gt_err(gt, "Unsupported GuC v%u.%u.%u! v70.29.2 or newer is required\n",
> +	/* We don't support GuC releases older than 70.36.0 */
> +	if (MAKE_GUC_VER_STRUCT(*release) < MAKE_GUC_VER(70, 36, 0)) {
> +		xe_gt_err(gt, "Unsupported GuC v%u.%u.%u! v70.36.0 or newer is required\n",
This is definitely not allowed.

Once a GuC has been released for a given platform, it must be supported 
forever on that platform. Which means that LNL and BMG must forever be 
able to run on 70.29.2.

If we really need to, we can add a per platform variant of this check 
for new platforms. E.g. PTL must be at least 70.39.42 or whatever. But 
we can't ever change the Xe global base line version. And there isn't 
much point in adding a new platform baseline because we just don't push 
the firmware upstream for new platforms until we are ready to do an 
official release. So there simply aren't any other versions available to 
warrant a baseline version check.

Rather than bumping the baseline here, individual features must check 
for a suitable GuC version. Which is what you have in patch #3 - the 
check for GUC_VER(1.14.1). That ensures that the new feature will not 
try to run if a new enough GuC is not available on the user's system.

John.

>   			  release->major, release->minor, release->patch);
>   		return -EINVAL;
>   	}