[Intel-gfx] [PATCH 3/5] drm/i915/uc: Track patch level versions on reduced version firmware files

Ceraolo Spurio, Daniele daniele.ceraolospurio at intel.com
Tue Apr 18 22:46:26 UTC 2023



On 4/14/2023 5:57 PM, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
>
> When reduced version firmware files were added (matching major
> component being the only strict requirement), the minor version was
> still tracked and a notification reported if it was older. However,
> the patch version should really be tracked as well for the same
> reasons. The KMD can work without the change but if the effort has
> been taken to release a new firmware with the change then there must
> be a valid reason for doing so - important bug fix, security fix, etc.
> And in that case it would be good to alert the user if they are
> missing out on that new fix.
>
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 41 +++++++++++++++++-------
>   1 file changed, 30 insertions(+), 11 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 a82a53dbbc86d..6bb45d6b8da5f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -80,14 +80,14 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>    */
>   #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
>   	fw_def(METEORLAKE,   0, guc_mmp(mtl,  70, 6, 5)) \
> -	fw_def(DG2,          0, guc_maj(dg2,  70, 5)) \
> -	fw_def(ALDERLAKE_P,  0, guc_maj(adlp, 70, 5)) \
> +	fw_def(DG2,          0, guc_maj(dg2,  70, 5, 4)) \
> +	fw_def(ALDERLAKE_P,  0, guc_maj(adlp, 70, 5, 4)) \
>   	fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 70, 1, 1)) \
>   	fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 69, 0, 3)) \
> -	fw_def(ALDERLAKE_S,  0, guc_maj(tgl,  70, 5)) \
> +	fw_def(ALDERLAKE_S,  0, guc_maj(tgl,  70, 5, 4)) \
>   	fw_def(ALDERLAKE_S,  0, guc_mmp(tgl,  70, 1, 1)) \
>   	fw_def(ALDERLAKE_S,  0, guc_mmp(tgl,  69, 0, 3)) \
> -	fw_def(DG1,          0, guc_maj(dg1,  70, 5)) \
> +	fw_def(DG1,          0, guc_maj(dg1,  70, 5, 4)) \
>   	fw_def(ROCKETLAKE,   0, guc_mmp(tgl,  70, 1, 1)) \
>   	fw_def(TIGERLAKE,    0, guc_mmp(tgl,  70, 1, 1)) \
>   	fw_def(JASPERLAKE,   0, guc_mmp(ehl,  70, 1, 1)) \

AFAICS in linux-firmware we don't have any 70.5.4 binaries, the newest 
ones are 70.5.1.

> @@ -141,7 +141,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>   	__stringify(patch_) ".bin"
>   
>   /* Minor for internal driver use, not part of file name */
> -#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_) \
> +#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_) \
>   	__MAKE_UC_FW_PATH_MAJOR(prefix_, "guc", major_)
>   
>   #define MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \
> @@ -197,9 +197,9 @@ struct __packed uc_fw_blob {
>   	{ UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
>   	  .legacy = true }
>   
> -#define GUC_FW_BLOB(prefix_, major_, minor_) \
> -	UC_FW_BLOB_NEW(major_, minor_, 0, false, \
> -		       MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_))
> +#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
> +	UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
> +		       MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
>   
>   #define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
>   	UC_FW_BLOB_OLD(major_, minor_, patch_, \
> @@ -296,6 +296,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
>   		uc_fw->file_wanted.path = blob->path;
>   		uc_fw->file_wanted.ver.major = blob->major;
>   		uc_fw->file_wanted.ver.minor = blob->minor;
> +		uc_fw->file_wanted.ver.patch = blob->patch;
>   		uc_fw->loaded_via_gsc = blob->loaded_via_gsc;
>   		found = true;
>   		break;
> @@ -776,6 +777,17 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>   	if (uc_fw->type == INTEL_UC_FW_TYPE_GUC && !guc_check_version_range(uc_fw))
>   		goto fail;
>   
> +	gt_info(gt, "%s firmware: wanted = %s / %d.%d.%d, got = %s / %d.%d.%d\n",
> +		intel_uc_fw_type_repr(uc_fw->type),
> +		uc_fw->file_wanted.path,
> +		uc_fw->file_wanted.ver.major,
> +		uc_fw->file_wanted.ver.minor,
> +		uc_fw->file_wanted.ver.patch,
> +		uc_fw->file_selected.path,
> +		uc_fw->file_selected.ver.major,
> +		uc_fw->file_selected.ver.minor,
> +		uc_fw->file_selected.ver.patch);

Some of the info here is redundant from print_fw_ver(). Can we have a 
single print with all the info we need?
Something like:

GuC firmware i915/mtl_guc_70.bin (v70.6.5, expected v70.6.4 or newer)

Otherwise, I'd suggest demoting this to gt_dbg to avoid printing the 
same thing twice at info verbosity

Daniele

> +
>   	if (uc_fw->file_wanted.ver.major && uc_fw->file_selected.ver.major) {
>   		/* Check the file's major version was as it claimed */
>   		if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) {
> @@ -790,6 +802,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>   		} else {
>   			if (uc_fw->file_selected.ver.minor < uc_fw->file_wanted.ver.minor)
>   				old_ver = true;
> +			else if ((uc_fw->file_selected.ver.minor == uc_fw->file_wanted.ver.minor) &&
> +				 (uc_fw->file_selected.ver.patch < uc_fw->file_wanted.ver.patch))
> +				old_ver = true;
>   		}
>   	}
>   
> @@ -797,12 +812,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>   		/* Preserve the version that was really wanted */
>   		memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
>   
> -		gt_notice(gt, "%s firmware %s (%d.%d) is recommended, but only %s (%d.%d) was found\n",
> +		gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n",
>   			  intel_uc_fw_type_repr(uc_fw->type),
>   			  uc_fw->file_wanted.path,
> -			  uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor,
> +			  uc_fw->file_wanted.ver.major,
> +			  uc_fw->file_wanted.ver.minor,
> +			  uc_fw->file_wanted.ver.patch,
>   			  uc_fw->file_selected.path,
> -			  uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor);
> +			  uc_fw->file_selected.ver.major,
> +			  uc_fw->file_selected.ver.minor,
> +			  uc_fw->file_selected.ver.patch);
>   		gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n",
>   			INTEL_UC_FIRMWARE_URL);
>   	}



More information about the dri-devel mailing list