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

[Srivatsa, Anusha]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Tuesday, March 21, 2023 8:51 AM
> To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
> Cc: intel-xe at lists.freedesktop.org; De Marchi, Lucas
> <lucas.demarchi at intel.com>
> Subject: Re: [Intel-xe] [PATCH 2/2] drm/xe/huc: Macro support for loading
> unversiond HuC
> 
> 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.

Yes makes sense. Will remove the versioned support.

> >  	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."

Hmmm, if that is going to the norm anyway then  just MAKE_UC_FW_PATH should suffice?

Anusha 
> 
> 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