Quoting Daniele Ceraolo Spurio (2019-07-24 03:21:47)
> Instead of having 2 identical functions for GuC and HuC firmware
> selection, we can unify the selection logic and just use different lists
> based on FW type.
>
> Note that the revid is not relevant for current blobs, but the upcoming
> CML will be identified as CFL rev 5, so by considering the revid we're
> ready for that.
>
> v2: rework blob list defs (Michal), add order check (Chris), fuse GuC
> and HuC lists into one.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> index ff6f7b157ecb..fa2151fa3a13 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
> @@ -23,91 +23,6 @@
> * Note that HuC firmware loading must be done before GuC loading.
> */
>
> -#define SKL_HUC_FW_MAJOR 01
> -#define SKL_HUC_FW_MINOR 07
> -#define SKL_BLD_NUM 1398
> + * List of required GuC and HuC binaries per-platform.
> + * Must be ordered based on platform + revid, from newer to older.
> + */
> +#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
> + fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 8, 4, 3238)) \
✓
> + fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
✓
Oh we still don't have the new version. Hmm, I think you have planned
for it well enough.
> + fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01, 2893)) \
✓
> + fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810)) \
✓
> + fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01, 8, 2893)) \
✓
> + fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
✓
> +#define __MAKE_UC_FW_PATH(prefix_, name_, separator_, major_, minor_, patch_) \
> + "i915/" \
> + __stringify(prefix_) name_ \
> + __stringify(major_) separator_ \
> + __stringify(minor_) separator_ \
> + __stringify(patch_) ".bin"
I still can't believe that encoding version strings into paths is the
best solution but whatever.
> +#define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \
> + __MAKE_UC_FW_PATH(prefix_, "_guc_", ".", major_, minor_, patch_)
> +
> +#define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \
> + __MAKE_UC_FW_PATH(prefix_, "_huc_ver", "_", major_, minor_, bld_num_)
> +
> +/* All blobs need to be declared via MODULE_FIRMWARE() */
> +#define INTEL_UC_MODULE_FW(platform_, revid_, guc_, huc_) \
> + MODULE_FIRMWARE(guc_); \
> + MODULE_FIRMWARE(huc_);
> +
> +INTEL_UC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH, MAKE_HUC_FW_PATH)
Ok. That was fun.
> +
> +/*
> + * The below defs and macros are used to iterate across the list of blobs. See
> + * __uc_fw_select() below for details.
> + */
> +struct __packed intel_uc_fw_blob {
> + u8 major;
> + u8 minor;
> + const char *path;
> +};
> +
> +#define UC_FW_BLOB(major_, minor_, path_) \
> + { .major = major_, .minor = minor_, .path = path_ }
> +
> +#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
> + UC_FW_BLOB(major_, minor_, \
> + MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
> +
> +#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \
> + UC_FW_BLOB(major_, minor_, \
> + MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_))
> +
> +#define MAKE_FW_LIST(platform_, revid_, guc_, huc_) \
> +{ \
> + .p = INTEL_##platform_, \
> + .first_rev = revid_, \
> + .blobs[INTEL_UC_FW_TYPE_GUC] = guc_, \
> + .blobs[INTEL_UC_FW_TYPE_HUC] = huc_, \
> +},
> +
> +static void
> +__uc_fw_select(struct intel_uc_fw *uc_fw, enum intel_platform p, u8 rev)
> +{
> + static const struct __packed {
> + enum intel_platform p;
> + u8 first_rev;
> + const struct intel_uc_fw_blob blobs[INTEL_UC_NUM_TYPES];
> + } fw_blobs[] = {
> + INTEL_UC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, HUC_FW_BLOB)
> + };
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(fw_blobs) && p <= fw_blobs[i].p; i++) {
> + if (p == fw_blobs[i].p && rev >= fw_blobs[i].first_rev) {
> + const struct intel_uc_fw_blob *blob =
> + &fw_blobs[i].blobs[uc_fw->type];
> + uc_fw->path = blob->path;
> + uc_fw->major_ver_wanted = blob->major;
> + uc_fw->minor_ver_wanted = blob->minor;
> + break;
> + }
> + }
> +
> + /* make sure the list is ordered as expected */
> + if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) {
> + for (i = 1; i < ARRAY_SIZE(fw_blobs); i++) {
> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
> + continue;
> +
> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
> + fw_blobs[i].first_rev < fw_blobs[i - 1].first_rev)
> + continue;
> +
> + pr_err("invalid FW blob order: %s r%u comes before %s r%u\n",
> + intel_platform_name(fw_blobs[i - 1].p),
> + fw_blobs[i - 1].first_rev,
> + intel_platform_name(fw_blobs[i].p),
> + fw_blobs[i].first_rev);
> +
> + uc_fw->path = NULL;
> + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> + return;
> + }
> + }
> +}
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
