[Intel-gfx] [PATCH v2] drm/i915/guc: Consolidate firmware major-minor to one place

[Antoine, Peter]

Should we not get the HuC patches merged before we amend them?

Peter.

-----Original Message-----
From: Gordon, David S 
Sent: Wednesday, August 10, 2016 4:25 PM
To: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>; Intel-gfx at lists.freedesktop.org
Cc: Ursulin, Tvrtko <tvrtko.ursulin at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>; Antoine, Peter <peter.antoine at intel.com>; Thierry, Michel <michel.thierry at intel.com>
Subject: Re: [PATCH v2] drm/i915/guc: Consolidate firmware major-minor to one place

On 10/08/16 16:16, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Currently to change the firmware one has to update the exported module 
> firmware string and the major-minor versions used for verification 
> after load. Consolidate that to a single place defining correct major 
> and minor versions per platform.
>
> v2: Rebased for KBL.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Dave Gordon <david.s.gordon at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Peter Antoine <peter.antoine at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Reviewed-by: Dave Gordon <david.s.gordon at intel.com> (v1)

You can carry the R-b over to v2 too :)
But wouldn't it be even nicer to unify with the HuC and DMC loaders too?

.Dave.

> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 30 
> +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 3763e30cc165..bfcf6b5d29ad 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -59,13 +59,25 @@
>   *
>   */
>
> -#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6_1.bin"
> +#define SKL_FW_MAJOR 6
> +#define SKL_FW_MINOR 1
> +
> +#define BXT_FW_MAJOR 8
> +#define BXT_FW_MINOR 7
> +
> +#define KBL_FW_MAJOR 9
> +#define KBL_FW_MINOR 14
> +
> +#define GUC_FW_PATH(platform, major, minor) \
> +       "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin"
> +
> +#define I915_SKL_GUC_UCODE GUC_FW_PATH(skl, SKL_FW_MAJOR, 
> +SKL_FW_MINOR)
>  MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
>
> -#define I915_BXT_GUC_UCODE "i915/bxt_guc_ver8_7.bin"
> +#define I915_BXT_GUC_UCODE GUC_FW_PATH(bxt, BXT_FW_MAJOR, 
> +BXT_FW_MINOR)
>  MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
>
> -#define I915_KBL_GUC_UCODE "i915/kbl_guc_ver9_14.bin"
> +#define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, 
> +KBL_FW_MINOR)
>  MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
>
>  /* User-friendly representation of an enum */ @@ -697,16 +709,16 @@ 
> void intel_guc_init(struct drm_device *dev)
>  		fw_path = NULL;
>  	} else if (IS_SKYLAKE(dev)) {
>  		fw_path = I915_SKL_GUC_UCODE;
> -		guc_fw->guc_fw_major_wanted = 6;
> -		guc_fw->guc_fw_minor_wanted = 1;
> +		guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR;
> +		guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR;
>  	} else if (IS_BROXTON(dev)) {
>  		fw_path = I915_BXT_GUC_UCODE;
> -		guc_fw->guc_fw_major_wanted = 8;
> -		guc_fw->guc_fw_minor_wanted = 7;
> +		guc_fw->guc_fw_major_wanted = BXT_FW_MAJOR;
> +		guc_fw->guc_fw_minor_wanted = BXT_FW_MINOR;
>  	} else if (IS_KABYLAKE(dev)) {
>  		fw_path = I915_KBL_GUC_UCODE;
> -		guc_fw->guc_fw_major_wanted = 9;
> -		guc_fw->guc_fw_minor_wanted = 14;
> +		guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR;
> +		guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR;
>  	} else {
>  		fw_path = "";	/* unknown device */
>  	}
>