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

Submitted by Srivatsa, Anusha on Sept. 9, 2019, 7:28 p.m.

Details

Message ID 20190909192801.3164-2-anusha.srivatsa@intel.com
State New
Headers show
Series "HuC updates" ( rev: 3 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Srivatsa, Anusha Sept. 9, 2019, 7:28 p.m.
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@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 25 ++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

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))
+
+#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_) \

Comments

On Mon, 09 Sep 2019 21:28:00 +0200, Anusha Srivatsa  
<anusha.srivatsa@intel.com> 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".

above commit message (and patch title) is little misleading as updating
a macro is secondary compared to introduced new firmware naming convention

>
> The current convention being:
> <platform>_<g/h>uc_<major>.<minor>.patch.bin
>
> Update the versions of huc being loaded of the platforms.

s/huc/HuC

>
> 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@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  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)) \

this is wrong, CML is CFL rev 5, for reference see [1]

and macro ordering is also wrong (already detected by our code, yeah!):

<3> [321.487555] invalid FW blob order: COFFEELAKE r0 comes before ICELAKE  
r0

and this wrong ordering leads to:

<7> [321.487616] i915 0000:00:02.0: [drm:intel_uc_init_early [i915]]  
enable_guc=2 (guc:no submission:no huc:no)
<6> [321.487618] i915 0000:00:02.0: Incompatible option enable_guc=2 - GuC  
is not supported!
<6> [321.487620] i915 0000:00:02.0: Incompatible option enable_guc=2 - HuC  
is not supported!

see [2] for details

[1] https://patchwork.freedesktop.org/patch/319864/?series=62969&rev=5
[2]  
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14333/fi-cml-u2/igt@i915_module_load@reload-with-fault-injection.html

> +	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))
> +
> +#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_) \
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@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>   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?

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_) \
>
> -----Original Message-----

> From: Ceraolo Spurio, Daniele

> Sent: Tuesday, September 10, 2019 2:05 PM

> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-

> gfx@lists.freedesktop.org

> 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@intel.com>

> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>

> > ---

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

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_) \

> >
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@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> 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@intel.com>
>>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>> ---
>>>    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_) \
>>>