Re: [Intel-gfx] [PATCH 2/6] drm/i915/gsc: Skip the version check when fetching the GSC FW

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Nov 21, 2022 at 03:16:13PM -0800, Daniele Ceraolo Spurio wrote:
> The current exectation from the FW side is that the driver will query
> the GSC FW version after the FW is loaded, similarly to what the mei
> driver does on DG2. However, we're discussing with the FW team if there
> is a way to extract the version from the bin file before loading, so we
> can keep the code the same as for older FWs.
> 
> Since the GSC FW version is not currently required for functionality and
> is only needed for debug purposes, we can skip the FW version for now at
> fetch time and add it later on when we've agreed on the approach.
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 29 +++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 6 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 5b2e4503aee7..3df52fd59edc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -564,6 +564,26 @@ static int check_ccs_header(struct intel_gt *gt,
>  	return 0;
>  }
>  
> +static int check_fw_header(struct intel_gt *gt,
> +			   const struct firmware *fw,
> +			   struct intel_uc_fw *uc_fw)
> +{
> +	int err = 0;
> +
> +	/* GSC FW version is queried after the FW is loaded */
> +	if (uc_fw->type == INTEL_UC_FW_TYPE_GSC)
> +		return 0;
> +
> +	if (uc_fw->loaded_via_gsc)
> +		err = check_gsc_manifest(fw, uc_fw);
> +	else
> +		err = check_ccs_header(gt, fw, uc_fw);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
>  /**
>   * intel_uc_fw_fetch - fetch uC firmware
>   * @uc_fw: uC firmware
> @@ -633,14 +653,11 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>  	if (err)
>  		goto fail;
>  
> -	if (uc_fw->loaded_via_gsc)
> -		err = check_gsc_manifest(fw, uc_fw);
> -	else
> -		err = check_ccs_header(gt, fw, uc_fw);
> +	err = check_fw_header(gt, fw, uc_fw);
>  	if (err)
>  		goto fail;
>  
> -	if (uc_fw->file_wanted.major_ver) {
> +	if (uc_fw->file_wanted.major_ver && uc_fw->file_selected.major_ver) {
>  		/* Check the file's major version was as it claimed */
>  		if (uc_fw->file_selected.major_ver != uc_fw->file_wanted.major_ver) {
>  			drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> @@ -657,7 +674,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>  		}
>  	}
>  
> -	if (old_ver) {
> +	if (old_ver && uc_fw->file_selected.major_ver) {
>  		/* Preserve the version that was really wanted */
>  		memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
>  
> -- 
> 2.37.3
>