[Intel-gfx] [PATCH] drm/i915/gsc: define gsc fw

Fri Aug 25 17:20:44 UTC 2023

On Fri, Aug 25, 2023 at 09:27:53AM -0700, Daniele Ceraolo Spurio wrote:
> Add FW definition and the matching override modparam.
> 
> The GSC FW has both a release version, based on platform and a rolling
> counter, and a compatibility version, which is the one tracking
> interface changes. Since what we care about is the interface, we use
> the compatibility version in the binary names.
> 
> Same as with the GuC, a major version bump indicate a
> backward-incompatible change, while a minor version bump indicates a
> backward-compatible one, so we use only the former in the file name.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
> Cc: John Harrison <John.C.Harrison at Intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis at intel.com>
> ---
> 
> This patch is already merged in topic/core-for-CI. It was merged there
> because we didn't have a GSC FW ready to ship to linux-firmware, but we
> still wanted to start testing what we had in CI. We finally have a FW
> in flight towards linux-firmware [1], so we can transition this patch
> to drm-intel-gt-next. The patch is unchanged since it was first sent
> and reviewed [2], so I kept the r-b and I'm looking for an ack on the
> move.
> Note that since this patch is already applied, pre-merge CI won't
> correctly run on it (which is not a problem given that the patch is
> already included in all current runs).
> 
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/8705
> [1] https://lists.freedesktop.org/archives/intel-gfx/2023-August/333322.html
> [2] https://patchwork.freedesktop.org/patch/544638/

Thanks for all the info. I agree we are ready to make the move and have
enough time until we get that in linux-firmware.git.

Acked-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

Btw, one last question:
Will we already include this in the new
https://gitlab.freedesktop.org/drm/firmware ?

> 
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 32 ++++++++++++++++++------
>  1 file changed, 24 insertions(+), 8 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 9e833f499ac7..fc0d05d2df59 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -131,6 +131,17 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>  	fw_def(BROXTON,      0, huc_mmp(bxt,  2, 0, 0)) \
>  	fw_def(SKYLAKE,      0, huc_mmp(skl,  2, 0, 0))
>  
> +/*
> + * The GSC FW has multiple version (see intel_gsc_uc.h for details); since what
> + * we care about is the interface, we use the compatibility version in the
> + * binary names.
> + * Same as with the GuC, a major version bump indicate a
> + * backward-incompatible change, while a minor version bump indicates a
> + * backward-compatible one, so we use only the former in the file name.
> + */
> +#define INTEL_GSC_FIRMWARE_DEFS(fw_def, gsc_def) \
> +	fw_def(METEORLAKE,   0, gsc_def(mtl, 1, 0))
> +
>  /*
>   * Set of macros for producing a list of filenames from the above table.
>   */
> @@ -166,6 +177,9 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>  #define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \
>  	__MAKE_UC_FW_PATH_MMP(prefix_, "huc", major_, minor_, patch_)
>  
> +#define MAKE_GSC_FW_PATH(prefix_, major_, minor_) \
> +	__MAKE_UC_FW_PATH_MAJOR(prefix_, "gsc", major_)
> +
>  /*
>   * All blobs need to be declared via MODULE_FIRMWARE().
>   * This first expansion of the table macros is solely to provide
> @@ -176,6 +190,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>  
>  INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR, MAKE_GUC_FW_PATH_MMP)
>  INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK, MAKE_HUC_FW_PATH_MMP, MAKE_HUC_FW_PATH_GSC)
> +INTEL_GSC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GSC_FW_PATH)
>  
>  /*
>   * The next expansion of the table macros (in __uc_fw_auto_select below) provides
> @@ -225,6 +240,10 @@ struct __packed uc_fw_blob {
>  #define HUC_FW_BLOB_GSC(prefix_) \
>  	UC_FW_BLOB_NEW(0, 0, 0, true, MAKE_HUC_FW_PATH_GSC(prefix_))
>  
> +#define GSC_FW_BLOB(prefix_, major_, minor_) \
> +	UC_FW_BLOB_NEW(major_, minor_, 0, true, \
> +		       MAKE_GSC_FW_PATH(prefix_, major_, minor_))
> +
>  struct __packed uc_fw_platform_requirement {
>  	enum intel_platform p;
>  	u8 rev; /* first platform rev using this FW */
> @@ -251,9 +270,14 @@ static const struct uc_fw_platform_requirement blobs_huc[] = {
>  	INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>  };
>  
> +static const struct uc_fw_platform_requirement blobs_gsc[] = {
> +	INTEL_GSC_FIRMWARE_DEFS(MAKE_FW_LIST, GSC_FW_BLOB)
> +};
> +
>  static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>  	[INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>  	[INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
> +	[INTEL_UC_FW_TYPE_GSC] = { blobs_gsc, ARRAY_SIZE(blobs_gsc) },
>  };
>  
>  static void
> @@ -266,14 +290,6 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
>  	int i;
>  	bool found;
>  
> -	/*
> -	 * GSC FW support is still not fully in place, so we're not defining
> -	 * the FW blob yet because we don't want the driver to attempt to load
> -	 * it until we're ready for it.
> -	 */
> -	if (uc_fw->type == INTEL_UC_FW_TYPE_GSC)
> -		return;
> -
>  	/*
>  	 * The only difference between the ADL GuC FWs is the HWConfig support.
>  	 * ADL-N does not support HWConfig, so we should use the same binary as
> -- 
> 2.41.0
> 