[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