[Intel-xe] [PATCH 2/2] drm/xe/huc: Macro support for loading unversiond HuC

[Matt Roper]

On Mon, Mar 20, 2023 at 04:52:54PM -0700, Anusha Srivatsa wrote:
> Follow the new direction of firmware and add macro
> support for loading unversioned HuC.
> 
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_uc_fw.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> index e2e5f7fbe9db..3c522d970228 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> @@ -50,7 +50,7 @@ static struct xe_device *uc_fw_to_xe(struct xe_uc_fw *uc_fw)
>  	fw_def(DG1,          0, guc_def(dg1,  70, 5, 2)) \
>  	fw_def(TIGERLAKE,    0, guc_def(tgl,  70, 5, 2))
>  
> -#define XE_HUC_FIRMWARE_DEFS(fw_def, huc_def) \
> +#define XE_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_def) \

Why do we still need the old versioned filename?  Can't we get rid of it
completely?  There's no backwards compatibility necessary on Xe yet so
this is our chance to eliminate the version numbers on HuC completely.
It's not clear to me why we're still using huc_def instead of huc_raw
below either.

>  	fw_def(DG1,          0, huc_def(dg1,  7, 9, 3)) \
>  	fw_def(TIGERLAKE,    0, huc_def(tgl,  7, 9, 3))
>  
> @@ -59,6 +59,10 @@ static struct xe_device *uc_fw_to_xe(struct xe_uc_fw *uc_fw)
>  	__stringify(prefix_) "_" name_ "_" \
>  	__stringify(major_) ".bin"
>  
> +#define __MAKE_UC_FW_BLANK_PATH(prefix_, name_) \

Nit:  "UNVERSIONED" may be better than "BLANK."


Matt

> +        "i915/" \
> +        __stringify(prefix_) "_" name_ ".bin"
> +
>  #define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
>          "i915/" \
>         __stringify(prefix_) name_ \
> @@ -72,12 +76,15 @@ static struct xe_device *uc_fw_to_xe(struct xe_uc_fw *uc_fw)
>  #define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \
>  	__MAKE_UC_FW_PATH(prefix_, "_huc_", major_, minor_, bld_num_)
>  
> +#define MAKE_HUC_FW_BLANK_PATH(prefix_) \
> +	__MAKE_UC_FW_BLANK_PATH(prefix_, "huc")
> +
>  /* All blobs need to be declared via MODULE_FIRMWARE() */
>  #define XE_UC_MODULE_FW(platform_, revid_, uc_) \
>  	MODULE_FIRMWARE(uc_);
>  
>  XE_GUC_FIRMWARE_DEFS(XE_UC_MODULE_FW, MAKE_GUC_FW_PATH)
> -XE_HUC_FIRMWARE_DEFS(XE_UC_MODULE_FW, MAKE_HUC_FW_PATH)
> +XE_HUC_FIRMWARE_DEFS(XE_UC_MODULE_FW, MAKE_HUC_FW_BLANK_PATH, MAKE_HUC_FW_PATH)
>  
>  /* The below structs and macros are used to iterate across the list of blobs */
>  struct __packed uc_fw_blob {
> @@ -93,6 +100,9 @@ struct __packed uc_fw_blob {
>  	UC_FW_BLOB(major_, minor_, \
>  		   MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
>  
> +#define HUC_FW_BLOB_BLANK(prefix_) \
> +	UC_FW_BLOB(0, 0, MAKE_HUC_FW_BLANK_PATH(prefix_))
> +
>  #define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \
>  	UC_FW_BLOB(major_, minor_, \
>  		   MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_))
> @@ -122,7 +132,7 @@ uc_fw_auto_select(struct xe_device *xe, struct xe_uc_fw *uc_fw)
>  		XE_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB)
>  	};
>  	static const struct uc_fw_platform_requirement blobs_huc[] = {
> -		XE_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB)
> +		XE_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB_BLANK, HUC_FW_BLOB)
>  	};
>  	static const struct fw_blobs_by_type blobs_all[XE_UC_FW_NUM_TYPES] = {
>  		[XE_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation