Re: [PATCH 1/2] drm/i915/uc: Update MAKE_HUC_FW_PATH macro

[Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>]

On 9/10/19 2:35 PM, Srivatsa, Anusha wrote:
> > -----Original Message-----
> > From: Ceraolo Spurio, Daniele
> > Sent: Tuesday, September 10, 2019 2:05 PM
> > To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; intel-
> > gfx@xxxxxxxxxxxxxxxxxxxxx
> > Subject: Re: [PATCH 1/2] drm/i915/uc: Update MAKE_HUC_FW_PATH macro
> >
> >
> >
> > On 9/9/19 12:28 PM, Anusha Srivatsa wrote:
> > > Update MAKE_HUC_FW_PATH macro to follow the same convention as the
> > > MAKE_GUC_FW_PATH with the separator changing from "_" to "."
> > > and removing "ver".
> > >
> > > The current convention being:
> > > <platform>_<g/h>uc_<major>.<minor>.patch.bin
> > >
> > > Update the versions of huc being loaded of the platforms.
> > >
> > > SKL - v2.0.0
> > > BXT - v2.0.0
> > > KBL - v4.0.0
> > > GLK - v4.0.0
> > > CFL - KBL v4.0.0
> > > ICL - v9.0.0
> > > CML - v4.0.0
> > >
> > > v2: Remove the separator parameter altogether from
> > > __MAKE_UC_FW_PATH.(Daniele)
> > > - Squash all firmware update patches (Daniele)
> > >
> > > Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> > > ---
> > >    drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 25 ++++++++++++------------
> > >    1 file changed, 13 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> > > b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> > > index 296a82603be0..da4bf24368bd 100644
> > > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> > > @@ -40,25 +40,26 @@ void intel_uc_fw_change_status(struct intel_uc_fw
> > *uc_fw,
> > >     */
> > >    #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
> > >    	fw_def(ELKHARTLAKE, 0, guc_def(ehl, 33, 0, 4), huc_def(ehl,  9,  0,    0))
> > \
> > > -	fw_def(ICELAKE,     0, guc_def(icl, 33, 0, 0), huc_def(icl,  8,  4, 3238)) \
> > > -	fw_def(COFFEELAKE,  0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00,
> > 1810)) \
> > > -	fw_def(GEMINILAKE,  0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01,
> > 2893)) \
> > > -	fw_def(KABYLAKE,    0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810))
> > \
> > > -	fw_def(BROXTON,     0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01,  8,
> > 2893)) \
> > > -	fw_def(SKYLAKE,     0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
> > > -
> > > -#define __MAKE_UC_FW_PATH(prefix_, name_, separator_, major_, minor_,
> > > patch_) \
> > > +	fw_def(COFFEELAKE,  0, guc_def(cml, 33, 0, 0), huc_def(cml,  4,  0,    0))
> > \
> > > +	fw_def(ICELAKE,     0, guc_def(icl, 33, 0, 0), huc_def(icl,  9,  0,    0)) \
> > > +	fw_def(COFFEELAKE,  0, guc_def(kbl, 33, 0, 0), huc_def(kbl,  4,  0,    0)) \
> > > +	fw_def(GEMINILAKE,  0, guc_def(glk, 33, 0, 0), huc_def(glk,  4,  0,    0)) \
> > > +	fw_def(KABYLAKE,    0, guc_def(kbl, 33, 0, 0), huc_def(kbl,  4,  0,    0)) \
> > > +	fw_def(BROXTON,     0, guc_def(bxt, 33, 0, 0), huc_def(bxt,  2,  0,    0)) \
> > > +	fw_def(SKYLAKE,     0, guc_def(skl, 33, 0, 0), huc_def(skl,  2,  0,    0))
> >
> > In addition to what Michal pointed out, while we're at it you can also get rid of
> > all the extra spaces in the huc_def() column since we've now standardized the
> > length of the items like we did with GuC. Maybe just leave an extra space for the
> > major, since we're close to hitting v10?
>
> Regarding that, do we still want 4 spaces for HuC build number?
> And 2 spaces for minor?

No, just leave 1 space for minor and build num (like for guc_def) and 2 
for the major so we don't have to move everything when we get HuC v10

Daniele

> Anusha
> > Daniele
> >
> > > +
> > > +#define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
> > >    	"i915/" \
> > >    	__stringify(prefix_) name_ \
> > > -	__stringify(major_) separator_ \
> > > -	__stringify(minor_) separator_ \
> > > +	__stringify(major_) "." \
> > > +	__stringify(minor_) "." \
> > >    	__stringify(patch_) ".bin"
> > >
> > >    #define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \
> > > -	__MAKE_UC_FW_PATH(prefix_, "_guc_", ".", major_, minor_, patch_)
> > > +	__MAKE_UC_FW_PATH(prefix_, "_guc_", major_, minor_, patch_)
> > >
> > >    #define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \
> > > -	__MAKE_UC_FW_PATH(prefix_, "_huc_ver", "_", major_, minor_,
> > bld_num_)
> > > +	__MAKE_UC_FW_PATH(prefix_, "_huc_", major_, minor_, bld_num_)
> > >
> > >    /* All blobs need to be declared via MODULE_FIRMWARE() */
> > >    #define INTEL_UC_MODULE_FW(platform_, revid_, guc_, huc_) \
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx