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

[Matt Roper]

On Tue, Mar 21, 2023 at 02:03:49PM -0700, Lucas De Marchi wrote:
> On Tue, Mar 21, 2023 at 08:50:59AM -0700, Matt Roper wrote:
> > 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.
> 
> note the version is saved to uc_fw->major_ver_wanted and
> uc_fw->minor_ver_wanted.

I thought we didn't want to use those for HuC, only for GuC?  Since the
kernel doesn't interact with the firmware at all, there's no need for us
to enforce any kind of minimum version check as far as I can see.  We
don't even care about the major version number, which is why it isn't
included in the filename we load, right?


Matt

> 
> Later when we are parsing the blob we read the version that is encoded
> inside the blob (rather than on the filename).  See xe_uc_fw_init().
> I think the version check is still makes sense:
> major should never change and minor has to be equal on greater than
> the one we first started supporting.
> 
> The version in the filename needs to be removed, however the version in
> the table I believe can stay... it should basically check the minimum
> version required. We could rename s/minor_ver_wanted/minor_ver_minimum/
> to clarify.  And comments on top of the macro definition to document.
> 
> Lucas De Marchi

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