From: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>
To: <John.C.Harrison@Intel.com>, <Intel-GFX@Lists.FreeDesktop.Org>
Cc: DRI-Devel@Lists.FreeDesktop.Org
Subject: Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/uc: Add patch level version number support
Date: Tue, 6 Sep 2022 13:27:47 -0700 [thread overview]
Message-ID: <687bf84c-069e-e5e8-1993-cd6f40ce17fa@intel.com> (raw)
In-Reply-To: <20220827011702.3465334-3-John.C.Harrison@Intel.com>
On 8/26/2022 6:17 PM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> With the move to un-versioned filenames, it becomes more difficult to
> know exactly what version of a given firmware is being used. So add
> the patch level version number to the debugfs output.
>
> Also, support matching by patch level when selecting code paths for
> firmware compatibility. While a patch level change cannot be backwards
> breaking, it is potentially possible that a new feature only works
> from a given patch level onwards (even though it was theoretically
> added in an earlier version that bumped the major or minor version).
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 +++---
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 6 ++--
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 36 ++++++++++++++-----
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 6 ++++
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 8 +++--
> 5 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 04393932623c7..64c4e83153f47 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1868,7 +1868,7 @@ int intel_guc_submission_init(struct intel_guc *guc)
> if (guc->submission_initialized)
> return 0;
>
> - if (guc->fw.file_selected.major_ver < 70) {
> + if (GET_UC_VER(guc) < MAKE_UC_VER(70, 0, 0)) {
> ret = guc_lrc_desc_pool_create_v69(guc);
> if (ret)
> return ret;
> @@ -2303,7 +2303,7 @@ static int register_context(struct intel_context *ce, bool loop)
> GEM_BUG_ON(intel_context_is_child(ce));
> trace_intel_context_register(ce);
>
> - if (guc->fw.file_selected.major_ver >= 70)
> + if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0))
> ret = register_context_v70(guc, ce, loop);
> else
> ret = register_context_v69(guc, ce, loop);
> @@ -2315,7 +2315,7 @@ static int register_context(struct intel_context *ce, bool loop)
> set_context_registered(ce);
> spin_unlock_irqrestore(&ce->guc_state.lock, flags);
>
> - if (guc->fw.file_selected.major_ver >= 70)
> + if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0))
> guc_context_policy_init_v70(ce, loop);
> }
>
> @@ -2921,7 +2921,7 @@ static void __guc_context_set_preemption_timeout(struct intel_guc *guc,
> u16 guc_id,
> u32 preemption_timeout)
> {
> - if (guc->fw.file_selected.major_ver >= 70) {
> + if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) {
> struct context_policy policy;
>
> __guc_context_policy_start_klv(&policy, guc_id);
> @@ -3186,7 +3186,7 @@ static int guc_context_alloc(struct intel_context *ce)
> static void __guc_context_set_prio(struct intel_guc *guc,
> struct intel_context *ce)
> {
> - if (guc->fw.file_selected.major_ver >= 70) {
> + if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) {
> struct context_policy policy;
>
> __guc_context_policy_start_klv(&policy, ce->guc_id.id);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index d965ac4832d60..abf4e142596d0 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -435,9 +435,11 @@ static void print_fw_ver(struct intel_uc *uc, struct intel_uc_fw *fw)
> {
> struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
>
> - drm_info(&i915->drm, "%s firmware %s version %u.%u\n",
> + drm_info(&i915->drm, "%s firmware %s version %u.%u.%u\n",
> intel_uc_fw_type_repr(fw->type), fw->file_selected.path,
> - fw->file_selected.major_ver, fw->file_selected.minor_ver);
> + fw->file_selected.major_ver,
> + fw->file_selected.minor_ver,
> + fw->file_selected.patch_ver);
> }
>
> static int __uc_init_hw(struct intel_uc *uc)
> 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 610f36f1b989a..af425916cdf64 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -447,10 +447,12 @@ static int check_gsc_manifest(const struct firmware *fw,
> struct intel_uc_fw *uc_fw)
> {
> u32 *dw = (u32 *)fw->data;
> - u32 version = dw[HUC_GSC_VERSION_DW];
> + u32 version_hi = dw[HUC_GSC_VERSION_HI_DW];
> + u32 version_lo = dw[HUC_GSC_VERSION_LO_DW];
>
> - uc_fw->file_selected.major_ver = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version);
> - uc_fw->file_selected.minor_ver = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version);
> + uc_fw->file_selected.major_ver = FIELD_GET(HUC_GSC_MAJOR_VER_HI_MASK, version_hi);
> + uc_fw->file_selected.minor_ver = FIELD_GET(HUC_GSC_MINOR_VER_HI_MASK, version_hi);
> + uc_fw->file_selected.patch_ver = FIELD_GET(HUC_GSC_PATCH_VER_LO_MASK, version_lo);
>
> return 0;
> }
> @@ -512,6 +514,8 @@ static int check_ccs_header(struct drm_i915_private *i915,
> css->sw_version);
> uc_fw->file_selected.minor_ver = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
> css->sw_version);
> + uc_fw->file_selected.patch_ver = FIELD_GET(CSS_SW_VERSION_UC_PATCH,
> + css->sw_version);
>
> if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> uc_fw->private_data_size = css->private_data_size;
> @@ -1000,6 +1004,8 @@ size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len)
> */
> void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p)
> {
> + u32 ver_sel, ver_want;
> +
> drm_printf(p, "%s firmware: %s\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path);
> if (uc_fw->file_selected.path != uc_fw->file_wanted.path)
> @@ -1007,13 +1013,25 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p)
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_wanted.path);
> drm_printf(p, "\tstatus: %s\n",
> intel_uc_fw_status_repr(uc_fw->status));
> - if (uc_fw->file_wanted.major_ver)
> - drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n",
> - uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver,
> - uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver);
> + ver_sel = MAKE_UC_VER(uc_fw->file_selected.major_ver,
> + uc_fw->file_selected.minor_ver,
> + uc_fw->file_selected.patch_ver);
> + ver_want = MAKE_UC_VER(uc_fw->file_wanted.major_ver,
> + uc_fw->file_wanted.minor_ver,
> + uc_fw->file_wanted.patch_ver);
> + if (ver_sel < ver_want)
> + drm_printf(p, "\tversion: wanted %u.%u.%u, found %u.%u.%u\n",
> + uc_fw->file_wanted.major_ver,
> + uc_fw->file_wanted.minor_ver,
> + uc_fw->file_wanted.patch_ver,
> + uc_fw->file_selected.major_ver,
> + uc_fw->file_selected.minor_ver,
> + uc_fw->file_selected.patch_ver);
> else
> - drm_printf(p, "\tversion: found %u.%u\n",
> - uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver);
> + drm_printf(p, "\tversion: found %u.%u.%u\n",
> + uc_fw->file_selected.major_ver,
> + uc_fw->file_selected.minor_ver,
> + uc_fw->file_selected.patch_ver);
> drm_printf(p, "\tuCode: %u bytes\n", uc_fw->ucode_size);
> drm_printf(p, "\tRSA: %u bytes\n", uc_fw->rsa_size);
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 344763c942e37..cb586f7df270b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -73,6 +73,7 @@ struct intel_uc_fw_file {
> const char *path;
> u16 major_ver;
> u16 minor_ver;
> + u16 patch_ver;
> };
>
> /*
> @@ -108,6 +109,11 @@ struct intel_uc_fw {
> bool loaded_via_gsc;
> };
>
> +#define MAKE_UC_VER(maj, min, pat) ((pat) | ((min) << 8) | ((maj) << 16))
> +#define GET_UC_VER(uc) (MAKE_UC_VER((uc)->fw.file_selected.major_ver, \
> + (uc)->fw.file_selected.minor_ver, \
> + (uc)->fw.file_selected.patch_ver))
> +
> #ifdef CONFIG_DRM_I915_DEBUG_GUC
> void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> enum intel_uc_fw_status status);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> index b05e0e35b734c..7a411178bdbf2 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> @@ -83,8 +83,10 @@ struct uc_css_header {
> } __packed;
> static_assert(sizeof(struct uc_css_header) == 128);
>
> -#define HUC_GSC_VERSION_DW 44
> -#define HUC_GSC_MAJOR_VER_MASK (0xFF << 0)
> -#define HUC_GSC_MINOR_VER_MASK (0xFF << 16)
> +#define HUC_GSC_VERSION_HI_DW 44
> +#define HUC_GSC_MAJOR_VER_HI_MASK (0xFF << 0)
> +#define HUC_GSC_MINOR_VER_HI_MASK (0xFF << 16)
> +#define HUC_GSC_VERSION_LO_DW 45
> +#define HUC_GSC_PATCH_VER_LO_MASK (0xFF << 0)
>
> #endif /* _INTEL_UC_FW_ABI_H */
next prev parent reply other threads:[~2022-09-06 20:28 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-27 1:16 [Intel-gfx] [PATCH v3 0/3] Drop version numbers from firmware files John.C.Harrison
2022-08-27 1:17 ` [Intel-gfx] [PATCH v3 1/3] drm/i915/uc: Support for version reduced and multiple " John.C.Harrison
2022-08-27 1:17 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/uc: Add patch level version number support John.C.Harrison
2022-09-06 20:27 ` Ceraolo Spurio, Daniele [this message]
2022-08-27 1:17 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/uc: Enable version reduced firmware files for newest platforms John.C.Harrison
2022-09-06 20:29 ` Ceraolo Spurio, Daniele
2022-09-07 16:58 ` Ceraolo Spurio, Daniele
2022-08-27 1:45 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Drop version numbers from firmware files (rev4) Patchwork
2022-08-27 1:45 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-08-27 2:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-08-30 18:30 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=687bf84c-069e-e5e8-1993-cd6f40ce17fa@intel.com \
--to=daniele.ceraolospurio@intel.com \
--cc=DRI-Devel@Lists.FreeDesktop.Org \
--cc=Intel-GFX@Lists.FreeDesktop.Org \
--cc=John.C.Harrison@Intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox