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 2/3] drm/i915/uc: More refactoring of UC version numbers
Date: Tue, 22 Nov 2022 15:17:35 -0800 [thread overview]
Message-ID: <dc7236e1-e6ec-0ffc-4e7d-9126fc436bf3@intel.com> (raw)
In-Reply-To: <20221122200915.680629-3-John.C.Harrison@Intel.com>
On 11/22/2022 12:09 PM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> As a precursor to a coming change (for adding a GuC submission API
> version), abstract the UC version number into its own private
> structure separate to the firmware filename.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Daniele
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 6 +-
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 76 +++++++++++-------------
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 15 +++--
> 3 files changed, 48 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 1d28286e6f066..e6edad6f8f9dd 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -437,9 +437,9 @@ static void print_fw_ver(struct intel_uc *uc, struct intel_uc_fw *fw)
>
> 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.patch_ver);
> + fw->file_selected.ver.major,
> + fw->file_selected.ver.minor,
> + fw->file_selected.ver.patch);
> }
>
> 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 774c3d84a4243..5e2ee1ac89514 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -278,8 +278,8 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
>
> uc_fw->file_selected.path = blob->path;
> uc_fw->file_wanted.path = blob->path;
> - uc_fw->file_wanted.major_ver = blob->major;
> - uc_fw->file_wanted.minor_ver = blob->minor;
> + uc_fw->file_wanted.ver.major = blob->major;
> + uc_fw->file_wanted.ver.minor = blob->minor;
> uc_fw->loaded_via_gsc = blob->loaded_via_gsc;
> found = true;
> break;
> @@ -438,28 +438,28 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e)
> uc_fw->user_overridden = user;
> } else if (i915_inject_probe_error(i915, e)) {
> /* require next major version */
> - uc_fw->file_wanted.major_ver += 1;
> - uc_fw->file_wanted.minor_ver = 0;
> + uc_fw->file_wanted.ver.major += 1;
> + uc_fw->file_wanted.ver.minor = 0;
> uc_fw->user_overridden = user;
> } else if (i915_inject_probe_error(i915, e)) {
> /* require next minor version */
> - uc_fw->file_wanted.minor_ver += 1;
> + uc_fw->file_wanted.ver.minor += 1;
> uc_fw->user_overridden = user;
> - } else if (uc_fw->file_wanted.major_ver &&
> + } else if (uc_fw->file_wanted.ver.major &&
> i915_inject_probe_error(i915, e)) {
> /* require prev major version */
> - uc_fw->file_wanted.major_ver -= 1;
> - uc_fw->file_wanted.minor_ver = 0;
> + uc_fw->file_wanted.ver.major -= 1;
> + uc_fw->file_wanted.ver.minor = 0;
> uc_fw->user_overridden = user;
> - } else if (uc_fw->file_wanted.minor_ver &&
> + } else if (uc_fw->file_wanted.ver.minor &&
> i915_inject_probe_error(i915, e)) {
> /* require prev minor version - hey, this should work! */
> - uc_fw->file_wanted.minor_ver -= 1;
> + uc_fw->file_wanted.ver.minor -= 1;
> uc_fw->user_overridden = user;
> } else if (user && i915_inject_probe_error(i915, e)) {
> /* officially unsupported platform */
> - uc_fw->file_wanted.major_ver = 0;
> - uc_fw->file_wanted.minor_ver = 0;
> + uc_fw->file_wanted.ver.major = 0;
> + uc_fw->file_wanted.ver.minor = 0;
> uc_fw->user_overridden = true;
> }
> }
> @@ -471,9 +471,9 @@ static int check_gsc_manifest(const struct firmware *fw,
> 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_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);
> + uc_fw->file_selected.ver.major = FIELD_GET(HUC_GSC_MAJOR_VER_HI_MASK, version_hi);
> + uc_fw->file_selected.ver.minor = FIELD_GET(HUC_GSC_MINOR_VER_HI_MASK, version_hi);
> + uc_fw->file_selected.ver.patch = FIELD_GET(HUC_GSC_PATCH_VER_LO_MASK, version_lo);
>
> return 0;
> }
> @@ -532,11 +532,11 @@ static int check_ccs_header(struct intel_gt *gt,
> }
>
> /* Get version numbers from the CSS header */
> - uc_fw->file_selected.major_ver = FIELD_GET(CSS_SW_VERSION_UC_MAJOR,
> + uc_fw->file_selected.ver.major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR,
> css->sw_version);
> - uc_fw->file_selected.minor_ver = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
> + uc_fw->file_selected.ver.minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
> css->sw_version);
> - uc_fw->file_selected.patch_ver = FIELD_GET(CSS_SW_VERSION_UC_PATCH,
> + uc_fw->file_selected.ver.patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH,
> css->sw_version);
>
> if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> @@ -621,19 +621,19 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> if (err)
> goto fail;
>
> - if (uc_fw->file_wanted.major_ver) {
> + if (uc_fw->file_wanted.ver.major) {
> /* Check the file's major version was as it claimed */
> - if (uc_fw->file_selected.major_ver != uc_fw->file_wanted.major_ver) {
> + if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) {
> drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
> - uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver,
> - uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver);
> + uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor,
> + uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor);
> if (!intel_uc_fw_is_overridden(uc_fw)) {
> err = -ENOEXEC;
> goto fail;
> }
> } else {
> - if (uc_fw->file_selected.minor_ver < uc_fw->file_wanted.minor_ver)
> + if (uc_fw->file_selected.ver.minor < uc_fw->file_wanted.ver.minor)
> old_ver = true;
> }
> }
> @@ -646,9 +646,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> "%s firmware %s (%d.%d) is recommended, but only %s (%d.%d) was found\n",
> intel_uc_fw_type_repr(uc_fw->type),
> uc_fw->file_wanted.path,
> - uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver,
> + uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor,
> uc_fw->file_selected.path,
> - uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver);
> + uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor);
> drm_info(&i915->drm,
> "Consider updating your linux-firmware pkg or downloading from %s\n",
> INTEL_UC_FIRMWARE_URL);
> @@ -1063,25 +1063,21 @@ 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));
> - 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);
> + ver_sel = MAKE_UC_VER_STRUCT(uc_fw->file_selected.ver);
> + ver_want = MAKE_UC_VER_STRUCT(uc_fw->file_wanted.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);
> + uc_fw->file_wanted.ver.major,
> + uc_fw->file_wanted.ver.minor,
> + uc_fw->file_wanted.ver.patch,
> + uc_fw->file_selected.ver.major,
> + uc_fw->file_selected.ver.minor,
> + uc_fw->file_selected.ver.patch);
> else
> 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);
> + uc_fw->file_selected.ver.major,
> + uc_fw->file_selected.ver.minor,
> + uc_fw->file_selected.ver.patch);
> 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 bc898ba5355dc..6501d6f1fbdff 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -65,6 +65,12 @@ enum intel_uc_fw_type {
> };
> #define INTEL_UC_FW_NUM_TYPES 2
>
> +struct intel_uc_fw_ver {
> + u16 major;
> + u16 minor;
> + u16 patch;
> +};
> +
> /*
> * The firmware build process will generate a version header file with major and
> * minor version defined. The versions are built into CSS header of firmware.
> @@ -72,9 +78,7 @@ enum intel_uc_fw_type {
> */
> struct intel_uc_fw_file {
> const char *path;
> - u16 major_ver;
> - u16 minor_ver;
> - u16 patch_ver;
> + struct intel_uc_fw_ver ver;
> };
>
> /*
> @@ -111,9 +115,8 @@ struct intel_uc_fw {
> };
>
> #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))
> +#define MAKE_UC_VER_STRUCT(ver) MAKE_UC_VER((ver).major, (ver).minor, (ver).patch)
> +#define GET_UC_VER(uc) (MAKE_UC_VER_STRUCT((uc)->fw.file_selected.ver))
>
> /*
> * When we load the uC binaries, we pin them in a reserved section at the top of
next prev parent reply other threads:[~2022-11-22 23:17 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-22 20:09 [Intel-gfx] [PATCH 0/3] More GuC firmware version improvements John.C.Harrison
2022-11-22 20:09 ` [Intel-gfx] [PATCH 1/3] drm/i915/uc: Rationalise delimiters in filename macros John.C.Harrison
2022-11-22 23:15 ` Ceraolo Spurio, Daniele
2022-11-22 20:09 ` [Intel-gfx] [PATCH 2/3] drm/i915/uc: More refactoring of UC version numbers John.C.Harrison
2022-11-22 23:17 ` Ceraolo Spurio, Daniele [this message]
2022-11-22 20:09 ` [Intel-gfx] [PATCH 3/3] drm/i915/guc: Use GuC submission API version number John.C.Harrison
2022-11-22 23:38 ` Ceraolo Spurio, Daniele
2022-11-22 21:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for More GuC firmware version improvements Patchwork
2022-11-22 21:52 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-11-22 22:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-11-23 13: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=dc7236e1-e6ec-0ffc-4e7d-9126fc436bf3@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