Re: [PATCH v2 2/4] drm/xe/guc: Introduce GUC_FIRMWARE_VER_AT_LEAST helper

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 12/15/2025 6:37 AM, Michal Wajdeczko wrote:
> There are already few places in the code where we need to check GuC
> firmware version. Wrap existing raw conditions into a named helper
> macro to make it clear and avoid explicit call of the MAKE_GUC_VER.
>
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> ---
> v1: split from [1] (Michal)
>      rename helper and add new macro (Daniele)
>
> [1] https://patchwork.freedesktop.org/patch/693953/?series=158874&rev=1
> ---
> note: there is yet another place that could be updated (xe_guc.c)
>        but since it seems already broken (VFs don't know FW version)
>        it should be fixed separately anyway.
> ---
>   drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c |  2 +-
>   drivers/gpu/drm/xe/xe_guc.h                   | 21 +++++++++++++++++++
>   drivers/gpu/drm/xe/xe_guc_ads.c               |  4 ++--
>   3 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> index 3174a8dee779..7410e7b93256 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> @@ -1026,7 +1026,7 @@ static void action_ring_cleanup(void *arg)
>   
>   static void pf_gt_migration_check_support(struct xe_gt *gt)
>   {
> -	if (GUC_FIRMWARE_VER(&gt->uc.guc) < MAKE_GUC_VER(70, 54, 0))
> +	if (!GUC_FIRMWARE_VER_AT_LEAST(&gt->uc.guc, 70, 54))
>   		xe_sriov_pf_migration_disable(gt_to_xe(gt), "requires GuC version >= 70.54.0");
>   }
>   
> diff --git a/drivers/gpu/drm/xe/xe_guc.h b/drivers/gpu/drm/xe/xe_guc.h
> index fdb08658d05a..a169f231cbd8 100644
> --- a/drivers/gpu/drm/xe/xe_guc.h
> +++ b/drivers/gpu/drm/xe/xe_guc.h
> @@ -18,10 +18,16 @@
>    */
>   #define MAKE_GUC_VER(maj, min, pat)	(((maj) << 16) | ((min) << 8) | (pat))
>   #define MAKE_GUC_VER_STRUCT(ver)	MAKE_GUC_VER((ver).major, (ver).minor, (ver).patch)
> +#define MAKE_GUC_VER_ARGS(ver...) \
> +	(BUILD_BUG_ON_ZERO(COUNT_ARGS(ver) < 2 || COUNT_ARGS(ver) > 3) + \
> +	 MAKE_GUC_VER(PICK_ARG1(ver), PICK_ARG2(ver), IF_ARGS(PICK_ARG3(ver), 0, PICK_ARG3(ver))))
> +
>   #define GUC_SUBMIT_VER(guc) \
>   	MAKE_GUC_VER_STRUCT((guc)->fw.versions.found[XE_UC_FW_VER_COMPATIBILITY])
>   #define GUC_FIRMWARE_VER(guc) \
>   	MAKE_GUC_VER_STRUCT((guc)->fw.versions.found[XE_UC_FW_VER_RELEASE])
> +#define GUC_FIRMWARE_VER_AT_LEAST(guc, ver...) \
> +	xe_guc_fw_version_at_least((guc), MAKE_GUC_VER_ARGS(ver))
>   
>   struct drm_printer;
>   
> @@ -96,4 +102,19 @@ static inline struct drm_device *guc_to_drm(struct xe_guc *guc)
>   	return &guc_to_xe(guc)->drm;
>   }
>   
> +/**
> + * xe_guc_fw_version_at_least() - Check if GuC is at least of given version.
> + * @guc: the &xe_guc
> + * @ver: the version to check
> + *
> + * The @ver should be prepared using MAKE_GUC_VER(major, minor, patch).
> + *
> + * Return: true if loaded GuC firmware is at least of given version,
> + *         false otherwise.
> + */
> +static inline bool xe_guc_fw_version_at_least(const struct xe_guc *guc, u32 ver)
> +{
> +	return GUC_FIRMWARE_VER(guc) >= ver;
> +}
> +
>   #endif
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index e06c6aa335bf..5feeb91426ee 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -347,10 +347,10 @@ static void guc_waklv_init(struct xe_guc_ads *ads)
>   		guc_waklv_enable(ads, NULL, 0, &offset, &remain,
>   				 GUC_WORKAROUND_KLV_ID_BACK_TO_BACK_RCS_ENGINE_RESET);
>   
> -	if (GUC_FIRMWARE_VER(&gt->uc.guc) >= MAKE_GUC_VER(70, 44, 0) && XE_GT_WA(gt, 16026508708))
> +	if (GUC_FIRMWARE_VER_AT_LEAST(&gt->uc.guc, 70, 44) && XE_GT_WA(gt, 16026508708))
>   		guc_waklv_enable(ads, NULL, 0, &offset, &remain,
>   				 GUC_WA_KLV_RESET_BB_STACK_PTR_ON_VF_SWITCH);
> -	if (GUC_FIRMWARE_VER(&gt->uc.guc) >= MAKE_GUC_VER(70, 47, 0) && XE_GT_WA(gt, 16026007364)) {
> +	if (GUC_FIRMWARE_VER_AT_LEAST(&gt->uc.guc, 70, 47) && XE_GT_WA(gt, 16026007364)) {
>   		u32 data[] = {
>   			0x0,
>   			0xF,