| Message ID | 20190724022153.8927-3-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | uC fw path unification + misc clean-up | expand |
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@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > 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@chris-wilson.co.uk> -Chris
On Wed, 24 Jul 2019 04:21:47 +0200, Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> wrote: > 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@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 86 +----------- > drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 88 +----------- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 156 ++++++++++++++++++++++ > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 29 ++-- > 4 files changed, 167 insertions(+), 192 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c > b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c > index 87169e826747..a027deb80330 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c > @@ -31,89 +31,6 @@ > #include "intel_guc_fw.h" > #include "i915_drv.h" > -#define __MAKE_GUC_FW_PATH(KEY) \ > - "i915/" \ > - __stringify(KEY##_GUC_FW_PREFIX) "_guc_" \ > - __stringify(KEY##_GUC_FW_MAJOR) "." \ > - __stringify(KEY##_GUC_FW_MINOR) "." \ > - __stringify(KEY##_GUC_FW_PATCH) ".bin" > - > -#define SKL_GUC_FW_PREFIX skl > -#define SKL_GUC_FW_MAJOR 33 > -#define SKL_GUC_FW_MINOR 0 > -#define SKL_GUC_FW_PATCH 0 > -#define SKL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(SKL) > -MODULE_FIRMWARE(SKL_GUC_FIRMWARE_PATH); > - > -#define BXT_GUC_FW_PREFIX bxt > -#define BXT_GUC_FW_MAJOR 33 > -#define BXT_GUC_FW_MINOR 0 > -#define BXT_GUC_FW_PATCH 0 > -#define BXT_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(BXT) > -MODULE_FIRMWARE(BXT_GUC_FIRMWARE_PATH); > - > -#define KBL_GUC_FW_PREFIX kbl > -#define KBL_GUC_FW_MAJOR 33 > -#define KBL_GUC_FW_MINOR 0 > -#define KBL_GUC_FW_PATCH 0 > -#define KBL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(KBL) > -MODULE_FIRMWARE(KBL_GUC_FIRMWARE_PATH); > - > -#define GLK_GUC_FW_PREFIX glk > -#define GLK_GUC_FW_MAJOR 33 > -#define GLK_GUC_FW_MINOR 0 > -#define GLK_GUC_FW_PATCH 0 > -#define GLK_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(GLK) > -MODULE_FIRMWARE(GLK_GUC_FIRMWARE_PATH); > - > -#define ICL_GUC_FW_PREFIX icl > -#define ICL_GUC_FW_MAJOR 33 > -#define ICL_GUC_FW_MINOR 0 > -#define ICL_GUC_FW_PATCH 0 > -#define ICL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(ICL) > -MODULE_FIRMWARE(ICL_GUC_FIRMWARE_PATH); > - > -static void guc_fw_select(struct intel_uc_fw *guc_fw) > -{ > - struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); > - struct drm_i915_private *i915 = guc_to_gt(guc)->i915; > - > - GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC); > - > - if (!HAS_GT_UC(i915)) { > - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; > - return; > - } > - > - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; > - > - if (i915_modparams.guc_firmware_path) { > - guc_fw->path = i915_modparams.guc_firmware_path; > - guc_fw->major_ver_wanted = 0; > - guc_fw->minor_ver_wanted = 0; > - } else if (IS_ICELAKE(i915)) { > - guc_fw->path = ICL_GUC_FIRMWARE_PATH; > - guc_fw->major_ver_wanted = ICL_GUC_FW_MAJOR; > - guc_fw->minor_ver_wanted = ICL_GUC_FW_MINOR; > - } else if (IS_GEMINILAKE(i915)) { > - guc_fw->path = GLK_GUC_FIRMWARE_PATH; > - guc_fw->major_ver_wanted = GLK_GUC_FW_MAJOR; > - guc_fw->minor_ver_wanted = GLK_GUC_FW_MINOR; > - } else if (IS_KABYLAKE(i915) || IS_COFFEELAKE(i915)) { > - guc_fw->path = KBL_GUC_FIRMWARE_PATH; > - guc_fw->major_ver_wanted = KBL_GUC_FW_MAJOR; > - guc_fw->minor_ver_wanted = KBL_GUC_FW_MINOR; > - } else if (IS_BROXTON(i915)) { > - guc_fw->path = BXT_GUC_FIRMWARE_PATH; > - guc_fw->major_ver_wanted = BXT_GUC_FW_MAJOR; > - guc_fw->minor_ver_wanted = BXT_GUC_FW_MINOR; > - } else if (IS_SKYLAKE(i915)) { > - guc_fw->path = SKL_GUC_FIRMWARE_PATH; > - guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR; > - guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR; > - } > -} > - > /** > * intel_guc_fw_init_early() - initializes GuC firmware struct > * @guc: intel_guc struct > @@ -124,8 +41,7 @@ void intel_guc_fw_init_early(struct intel_guc *guc) > { > struct intel_uc_fw *guc_fw = &guc->fw; > - intel_uc_fw_init_early(guc_fw, INTEL_UC_FW_TYPE_GUC); > - guc_fw_select(guc_fw); > + intel_uc_fw_init_early(guc_to_gt(guc)->i915, guc_fw, > INTEL_UC_FW_TYPE_GUC); > } > static void guc_prepare_xfer(struct intel_guc *guc) > 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 BXT_HUC_FW_MAJOR 01 > -#define BXT_HUC_FW_MINOR 8 > -#define BXT_BLD_NUM 2893 > - > -#define SKL_HUC_FW_MAJOR 01 > -#define SKL_HUC_FW_MINOR 07 > -#define SKL_BLD_NUM 1398 > - > -#define KBL_HUC_FW_MAJOR 02 > -#define KBL_HUC_FW_MINOR 00 > -#define KBL_BLD_NUM 1810 > - > -#define GLK_HUC_FW_MAJOR 03 > -#define GLK_HUC_FW_MINOR 01 > -#define GLK_BLD_NUM 2893 > - > -#define ICL_HUC_FW_MAJOR 8 > -#define ICL_HUC_FW_MINOR 4 > -#define ICL_BLD_NUM 3238 > - > -#define HUC_FW_PATH(platform, major, minor, bld_num) \ > - "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \ > - __stringify(minor) "_" __stringify(bld_num) ".bin" > - > -#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \ > - SKL_HUC_FW_MINOR, SKL_BLD_NUM) > -MODULE_FIRMWARE(I915_SKL_HUC_UCODE); > - > -#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \ > - BXT_HUC_FW_MINOR, BXT_BLD_NUM) > -MODULE_FIRMWARE(I915_BXT_HUC_UCODE); > - > -#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \ > - KBL_HUC_FW_MINOR, KBL_BLD_NUM) > -MODULE_FIRMWARE(I915_KBL_HUC_UCODE); > - > -#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \ > - GLK_HUC_FW_MINOR, GLK_BLD_NUM) > -MODULE_FIRMWARE(I915_GLK_HUC_UCODE); > - > -#define I915_ICL_HUC_UCODE HUC_FW_PATH(icl, ICL_HUC_FW_MAJOR, \ > - ICL_HUC_FW_MINOR, ICL_BLD_NUM) > -MODULE_FIRMWARE(I915_ICL_HUC_UCODE); > - > -static void huc_fw_select(struct intel_uc_fw *huc_fw) > -{ > - struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw); > - struct drm_i915_private *dev_priv = huc_to_gt(huc)->i915; > - > - GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC); > - > - if (!HAS_GT_UC(dev_priv)) { > - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; > - return; > - } > - > - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; > - > - if (i915_modparams.huc_firmware_path) { > - huc_fw->path = i915_modparams.huc_firmware_path; > - huc_fw->major_ver_wanted = 0; > - huc_fw->minor_ver_wanted = 0; > - } else if (IS_SKYLAKE(dev_priv)) { > - huc_fw->path = I915_SKL_HUC_UCODE; > - huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR; > - huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR; > - } else if (IS_BROXTON(dev_priv)) { > - huc_fw->path = I915_BXT_HUC_UCODE; > - huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR; > - huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR; > - } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) { > - huc_fw->path = I915_KBL_HUC_UCODE; > - huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; > - huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; > - } else if (IS_GEMINILAKE(dev_priv)) { > - huc_fw->path = I915_GLK_HUC_UCODE; > - huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR; > - huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR; > - } else if (IS_ICELAKE(dev_priv)) { > - huc_fw->path = I915_ICL_HUC_UCODE; > - huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR; > - huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR; > - } > -} > - > /** > * intel_huc_fw_init_early() - initializes HuC firmware struct > * @huc: intel_huc struct > @@ -118,8 +33,7 @@ void intel_huc_fw_init_early(struct intel_huc *huc) > { > struct intel_uc_fw *huc_fw = &huc->fw; > - intel_uc_fw_init_early(huc_fw, INTEL_UC_FW_TYPE_HUC); > - huc_fw_select(huc_fw); > + intel_uc_fw_init_early(huc_to_gt(huc)->i915, huc_fw, > INTEL_UC_FW_TYPE_HUC); > } > static void huc_xfer_rsa(struct intel_huc *huc) > 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 8ce7210907c0..48100dff466d 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -29,6 +29,162 @@ > #include "intel_uc_fw.h" > #include "i915_drv.h" > +/* > + * 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)) \ > + 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" > + > +#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) > + > +/* > + * 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; HuC firmware is using 16 bits for major/minor but I guess we have some time until we get to version 255 > + 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_, \ > +}, nit: unnamed struct on which above macro operates is defined inside function below - either keep this macro private to the function or move struct definition outside function, btw, above you've already provided definition for struct intel_uc_fw_blob > + > +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; nit: using full "platform" word instead of "p" will not hurt > + u8 first_rev; nit: maybe just "rev", from comment above we know that this is first one > + 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; if we reorder this selftest with actual search then maybe we can skip resetting path (btw, major/minor are untouched) and return > + return; maybe for full selftest we should scan whole table instead of returning on first mismatch ? > + } > + } > +} > + > +static void > +uc_fw_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) > +{ > + GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED); > + > + if (!HAS_GT_UC(i915)) { > + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; maybe we should check this in 'init_early' before calling 'select' function ? > + return; > + } > + > + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; > + > + if (unlikely(i915_modparams.guc_firmware_path && > + uc_fw->type == INTEL_UC_FW_TYPE_GUC)) > + uc_fw->path = i915_modparams.guc_firmware_path; > + else if (unlikely(i915_modparams.huc_firmware_path && > + uc_fw->type == INTEL_UC_FW_TYPE_HUC)) > + uc_fw->path = i915_modparams.huc_firmware_path; > + else > + __uc_fw_select(uc_fw, INTEL_INFO(i915)->platform, INTEL_REVID(i915)); if we don't select anything (no override, no hardcoded path) then we will stay with fetch_status = NOT_STARTED, but I think for such case we should use NOT_SUPPORTED > +} > + > +/** > + * intel_uc_fw_init_early - initialize the uC object and select the > firmware > + * @i915: device private > + * @uc_fw: uC firmware > + * @type: type of uC > + * > + * Initialize the state of our uC object and relevant tracking and > select the > + * firmware to fetch and load. > + */ > +void intel_uc_fw_init_early(struct drm_i915_private *i915, > + struct intel_uc_fw *uc_fw, > + enum intel_uc_fw_type type) void intel_uc_fw_init_early( struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type, struct drm_i915_private *i915) to use correct object/subject ordering > +{ > + /* > + * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status > + * before we're looked at the HW caps to see if we have uc support > + */ > + BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); > + > + uc_fw->path = NULL; > + uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED; > + uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; > + uc_fw->type = type; > + > + uc_fw_select(i915, uc_fw); maybe: BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); GEM_BUG_ON(uc_fw->fetch_status); GEM_BUG_ON(uc_fw->load_status); GEM_BUG_ON(uc_fw->path); uc_fw->type = type; if (HAS_GT_UC(i915) && !__uc_fw_override(uc_fw)) __uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform, INTEL_REVID(i915)); if (uc_fw->path) { uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; } else { uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; } > +} > + > /** > * intel_uc_fw_fetch - fetch uC firmware > * > 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 833d04d06576..c2868ef15eda 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h > @@ -44,8 +44,9 @@ enum intel_uc_fw_status { > }; > enum intel_uc_fw_type { > - INTEL_UC_FW_TYPE_GUC, > - INTEL_UC_FW_TYPE_HUC > + INTEL_UC_FW_TYPE_GUC = 0, > + INTEL_UC_FW_TYPE_HUC, > + INTEL_UC_NUM_TYPES this is bad idea, as now we have NUM_TYPES as valid fw type #define INTEL_UC_NUM_TYPES 2 > }; > /* > @@ -105,24 +106,9 @@ static inline const char > *intel_uc_fw_type_repr(enum intel_uc_fw_type type) > return "GuC"; > case INTEL_UC_FW_TYPE_HUC: > return "HuC"; > + default: > + return "uC"; > } > - return "uC"; > -} > - > -static inline > -void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, > - enum intel_uc_fw_type type) > -{ > - /* > - * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status > - * before we're looked at the HW caps to see if we have uc support > - */ > - BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); > - > - uc_fw->path = NULL; > - uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED; > - uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; > - uc_fw->type = type; > } > static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw) > @@ -164,7 +150,10 @@ static inline u32 > intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw) > return uc_fw->header_size + uc_fw->ucode_size; > } > -void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, > +void intel_uc_fw_init_early(struct drm_i915_private *i915, > + struct intel_uc_fw *uc_fw, > + enum intel_uc_fw_type type); > +void intel_uc_fw_fetch(struct drm_i915_private *i915, > struct intel_uc_fw *uc_fw); > void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw); > int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
On 7/24/2019 4:31 AM, Michal Wajdeczko wrote: > On Wed, 24 Jul 2019 04:21:47 +0200, Daniele Ceraolo Spurio > <daniele.ceraolospurio@intel.com> wrote: > >> 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@intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> --- >> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 86 +----------- >> drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 88 +----------- >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 156 ++++++++++++++++++++++ >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 29 ++-- >> 4 files changed, 167 insertions(+), 192 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c >> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c >> index 87169e826747..a027deb80330 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c >> @@ -31,89 +31,6 @@ >> #include "intel_guc_fw.h" >> #include "i915_drv.h" >> -#define __MAKE_GUC_FW_PATH(KEY) \ >> - "i915/" \ >> - __stringify(KEY##_GUC_FW_PREFIX) "_guc_" \ >> - __stringify(KEY##_GUC_FW_MAJOR) "." \ >> - __stringify(KEY##_GUC_FW_MINOR) "." \ >> - __stringify(KEY##_GUC_FW_PATCH) ".bin" >> - >> -#define SKL_GUC_FW_PREFIX skl >> -#define SKL_GUC_FW_MAJOR 33 >> -#define SKL_GUC_FW_MINOR 0 >> -#define SKL_GUC_FW_PATCH 0 >> -#define SKL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(SKL) >> -MODULE_FIRMWARE(SKL_GUC_FIRMWARE_PATH); >> - >> -#define BXT_GUC_FW_PREFIX bxt >> -#define BXT_GUC_FW_MAJOR 33 >> -#define BXT_GUC_FW_MINOR 0 >> -#define BXT_GUC_FW_PATCH 0 >> -#define BXT_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(BXT) >> -MODULE_FIRMWARE(BXT_GUC_FIRMWARE_PATH); >> - >> -#define KBL_GUC_FW_PREFIX kbl >> -#define KBL_GUC_FW_MAJOR 33 >> -#define KBL_GUC_FW_MINOR 0 >> -#define KBL_GUC_FW_PATCH 0 >> -#define KBL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(KBL) >> -MODULE_FIRMWARE(KBL_GUC_FIRMWARE_PATH); >> - >> -#define GLK_GUC_FW_PREFIX glk >> -#define GLK_GUC_FW_MAJOR 33 >> -#define GLK_GUC_FW_MINOR 0 >> -#define GLK_GUC_FW_PATCH 0 >> -#define GLK_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(GLK) >> -MODULE_FIRMWARE(GLK_GUC_FIRMWARE_PATH); >> - >> -#define ICL_GUC_FW_PREFIX icl >> -#define ICL_GUC_FW_MAJOR 33 >> -#define ICL_GUC_FW_MINOR 0 >> -#define ICL_GUC_FW_PATCH 0 >> -#define ICL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(ICL) >> -MODULE_FIRMWARE(ICL_GUC_FIRMWARE_PATH); >> - >> -static void guc_fw_select(struct intel_uc_fw *guc_fw) >> -{ >> - struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); >> - struct drm_i915_private *i915 = guc_to_gt(guc)->i915; >> - >> - GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC); >> - >> - if (!HAS_GT_UC(i915)) { >> - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; >> - return; >> - } >> - >> - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; >> - >> - if (i915_modparams.guc_firmware_path) { >> - guc_fw->path = i915_modparams.guc_firmware_path; >> - guc_fw->major_ver_wanted = 0; >> - guc_fw->minor_ver_wanted = 0; >> - } else if (IS_ICELAKE(i915)) { >> - guc_fw->path = ICL_GUC_FIRMWARE_PATH; >> - guc_fw->major_ver_wanted = ICL_GUC_FW_MAJOR; >> - guc_fw->minor_ver_wanted = ICL_GUC_FW_MINOR; >> - } else if (IS_GEMINILAKE(i915)) { >> - guc_fw->path = GLK_GUC_FIRMWARE_PATH; >> - guc_fw->major_ver_wanted = GLK_GUC_FW_MAJOR; >> - guc_fw->minor_ver_wanted = GLK_GUC_FW_MINOR; >> - } else if (IS_KABYLAKE(i915) || IS_COFFEELAKE(i915)) { >> - guc_fw->path = KBL_GUC_FIRMWARE_PATH; >> - guc_fw->major_ver_wanted = KBL_GUC_FW_MAJOR; >> - guc_fw->minor_ver_wanted = KBL_GUC_FW_MINOR; >> - } else if (IS_BROXTON(i915)) { >> - guc_fw->path = BXT_GUC_FIRMWARE_PATH; >> - guc_fw->major_ver_wanted = BXT_GUC_FW_MAJOR; >> - guc_fw->minor_ver_wanted = BXT_GUC_FW_MINOR; >> - } else if (IS_SKYLAKE(i915)) { >> - guc_fw->path = SKL_GUC_FIRMWARE_PATH; >> - guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR; >> - guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR; >> - } >> -} >> - >> /** >> * intel_guc_fw_init_early() - initializes GuC firmware struct >> * @guc: intel_guc struct >> @@ -124,8 +41,7 @@ void intel_guc_fw_init_early(struct intel_guc *guc) >> { >> struct intel_uc_fw *guc_fw = &guc->fw; >> - intel_uc_fw_init_early(guc_fw, INTEL_UC_FW_TYPE_GUC); >> - guc_fw_select(guc_fw); >> + intel_uc_fw_init_early(guc_to_gt(guc)->i915, guc_fw, >> INTEL_UC_FW_TYPE_GUC); >> } >> static void guc_prepare_xfer(struct intel_guc *guc) >> 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 BXT_HUC_FW_MAJOR 01 >> -#define BXT_HUC_FW_MINOR 8 >> -#define BXT_BLD_NUM 2893 >> - >> -#define SKL_HUC_FW_MAJOR 01 >> -#define SKL_HUC_FW_MINOR 07 >> -#define SKL_BLD_NUM 1398 >> - >> -#define KBL_HUC_FW_MAJOR 02 >> -#define KBL_HUC_FW_MINOR 00 >> -#define KBL_BLD_NUM 1810 >> - >> -#define GLK_HUC_FW_MAJOR 03 >> -#define GLK_HUC_FW_MINOR 01 >> -#define GLK_BLD_NUM 2893 >> - >> -#define ICL_HUC_FW_MAJOR 8 >> -#define ICL_HUC_FW_MINOR 4 >> -#define ICL_BLD_NUM 3238 >> - >> -#define HUC_FW_PATH(platform, major, minor, bld_num) \ >> - "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \ >> - __stringify(minor) "_" __stringify(bld_num) ".bin" >> - >> -#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \ >> - SKL_HUC_FW_MINOR, SKL_BLD_NUM) >> -MODULE_FIRMWARE(I915_SKL_HUC_UCODE); >> - >> -#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \ >> - BXT_HUC_FW_MINOR, BXT_BLD_NUM) >> -MODULE_FIRMWARE(I915_BXT_HUC_UCODE); >> - >> -#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \ >> - KBL_HUC_FW_MINOR, KBL_BLD_NUM) >> -MODULE_FIRMWARE(I915_KBL_HUC_UCODE); >> - >> -#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \ >> - GLK_HUC_FW_MINOR, GLK_BLD_NUM) >> -MODULE_FIRMWARE(I915_GLK_HUC_UCODE); >> - >> -#define I915_ICL_HUC_UCODE HUC_FW_PATH(icl, ICL_HUC_FW_MAJOR, \ >> - ICL_HUC_FW_MINOR, ICL_BLD_NUM) >> -MODULE_FIRMWARE(I915_ICL_HUC_UCODE); >> - >> -static void huc_fw_select(struct intel_uc_fw *huc_fw) >> -{ >> - struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw); >> - struct drm_i915_private *dev_priv = huc_to_gt(huc)->i915; >> - >> - GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC); >> - >> - if (!HAS_GT_UC(dev_priv)) { >> - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; >> - return; >> - } >> - >> - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; >> - >> - if (i915_modparams.huc_firmware_path) { >> - huc_fw->path = i915_modparams.huc_firmware_path; >> - huc_fw->major_ver_wanted = 0; >> - huc_fw->minor_ver_wanted = 0; >> - } else if (IS_SKYLAKE(dev_priv)) { >> - huc_fw->path = I915_SKL_HUC_UCODE; >> - huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR; >> - huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR; >> - } else if (IS_BROXTON(dev_priv)) { >> - huc_fw->path = I915_BXT_HUC_UCODE; >> - huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR; >> - huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR; >> - } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) { >> - huc_fw->path = I915_KBL_HUC_UCODE; >> - huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; >> - huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; >> - } else if (IS_GEMINILAKE(dev_priv)) { >> - huc_fw->path = I915_GLK_HUC_UCODE; >> - huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR; >> - huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR; >> - } else if (IS_ICELAKE(dev_priv)) { >> - huc_fw->path = I915_ICL_HUC_UCODE; >> - huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR; >> - huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR; >> - } >> -} >> - >> /** >> * intel_huc_fw_init_early() - initializes HuC firmware struct >> * @huc: intel_huc struct >> @@ -118,8 +33,7 @@ void intel_huc_fw_init_early(struct intel_huc *huc) >> { >> struct intel_uc_fw *huc_fw = &huc->fw; >> - intel_uc_fw_init_early(huc_fw, INTEL_UC_FW_TYPE_HUC); >> - huc_fw_select(huc_fw); >> + intel_uc_fw_init_early(huc_to_gt(huc)->i915, huc_fw, >> INTEL_UC_FW_TYPE_HUC); >> } >> static void huc_xfer_rsa(struct intel_huc *huc) >> 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 8ce7210907c0..48100dff466d 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> @@ -29,6 +29,162 @@ >> #include "intel_uc_fw.h" >> #include "i915_drv.h" >> +/* >> + * 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)) \ >> + 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" >> + >> +#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) >> + >> +/* >> + * 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; > > HuC firmware is using 16 bits for major/minor but I guess we have > some time until we get to version 255 > >> + 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_, \ >> +}, > > nit: unnamed struct on which above macro operates is defined inside > function below - either keep this macro private to the function or > move struct definition outside function, > > btw, above you've already provided definition for struct intel_uc_fw_blob > >> + >> +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; > > nit: using full "platform" word instead of "p" will not hurt > >> + u8 first_rev; > > nit: maybe just "rev", from comment above we know that this is first one > >> + 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; > > if we reorder this selftest with actual search then maybe we can skip > resetting path (btw, major/minor are untouched) and return > Actually better to always scan the whole table (as you mentioned below) >> + return; > > maybe for full selftest we should scan whole table instead of returning > on first mismatch ? > >> + } >> + } >> +} >> + >> +static void >> +uc_fw_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) >> +{ >> + GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED); >> + >> + if (!HAS_GT_UC(i915)) { >> + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; > > maybe we should check this in 'init_early' before calling 'select' > function ? > >> + return; >> + } >> + >> + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; >> + >> + if (unlikely(i915_modparams.guc_firmware_path && >> + uc_fw->type == INTEL_UC_FW_TYPE_GUC)) >> + uc_fw->path = i915_modparams.guc_firmware_path; >> + else if (unlikely(i915_modparams.huc_firmware_path && >> + uc_fw->type == INTEL_UC_FW_TYPE_HUC)) >> + uc_fw->path = i915_modparams.huc_firmware_path; >> + else >> + __uc_fw_select(uc_fw, INTEL_INFO(i915)->platform, >> INTEL_REVID(i915)); > > if we don't select anything (no override, no hardcoded path) then we > will stay with fetch_status = NOT_STARTED, but I think for such case > we should use NOT_SUPPORTED I'm not changing any of the status flow in this patch ;) Anyway, I agree with you, but we can't do that at the moment as we still need a way to differentiate between no uC HW (NOT_SUPPORTED) and HW available but no FW (NOT_STARTED/SELECTION_DONE && path == NULL) for the error logging in sanitize options. As we've discussed offline, Once we've cleaned up the sanitizing we can make those 2 states converge into one. Alternatively, we can just drop the log difference since the user won't really care why the support is not there and a developer should be able to quickly spot the reason. Thoughts? > >> +} >> + >> +/** >> + * intel_uc_fw_init_early - initialize the uC object and select the >> firmware >> + * @i915: device private >> + * @uc_fw: uC firmware >> + * @type: type of uC >> + * >> + * Initialize the state of our uC object and relevant tracking and >> select the >> + * firmware to fetch and load. >> + */ >> +void intel_uc_fw_init_early(struct drm_i915_private *i915, >> + struct intel_uc_fw *uc_fw, >> + enum intel_uc_fw_type type) > > void intel_uc_fw_init_early( > struct intel_uc_fw *uc_fw, > enum intel_uc_fw_type type, > struct drm_i915_private *i915) > > to use correct object/subject ordering > Will flip. I had it like this to match the intel_uc_fw_fetch function. >> +{ >> + /* >> + * we use FIRMWARE_UNINITIALIZED to detect checks against >> fetch_status >> + * before we're looked at the HW caps to see if we have uc support >> + */ >> + BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); >> + >> + uc_fw->path = NULL; >> + uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED; >> + uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; >> + uc_fw->type = type; >> + >> + uc_fw_select(i915, uc_fw); > > maybe: > BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); > GEM_BUG_ON(uc_fw->fetch_status); > GEM_BUG_ON(uc_fw->load_status); > GEM_BUG_ON(uc_fw->path); > > uc_fw->type = type; > > if (HAS_GT_UC(i915) && !__uc_fw_override(uc_fw)) > __uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform, > INTEL_REVID(i915)); > > if (uc_fw->path) { > uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; > uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; > } else { > uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; > uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; > } I can't do it like this for the logging reasons mentioned above, but I can rework it as: if (!HAS_GT_UC(i915)) { uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; return; } if(!__uc_fw_override(uc_fw)) __uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform, INTEL_REVID(i915)); uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; return; >> +} >> + >> /** >> * intel_uc_fw_fetch - fetch uC firmware >> * >> 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 833d04d06576..c2868ef15eda 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >> @@ -44,8 +44,9 @@ enum intel_uc_fw_status { >> }; >> enum intel_uc_fw_type { >> - INTEL_UC_FW_TYPE_GUC, >> - INTEL_UC_FW_TYPE_HUC >> + INTEL_UC_FW_TYPE_GUC = 0, >> + INTEL_UC_FW_TYPE_HUC, >> + INTEL_UC_NUM_TYPES > > this is bad idea, as now we have NUM_TYPES as valid fw type > #define INTEL_UC_NUM_TYPES 2 > ok Daniele >> }; >> /* >> @@ -105,24 +106,9 @@ static inline const char >> *intel_uc_fw_type_repr(enum intel_uc_fw_type type) >> return "GuC"; >> case INTEL_UC_FW_TYPE_HUC: >> return "HuC"; >> + default: >> + return "uC"; >> } >> - return "uC"; >> -} >> - >> -static inline >> -void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, >> - enum intel_uc_fw_type type) >> -{ >> - /* >> - * we use FIRMWARE_UNINITIALIZED to detect checks against >> fetch_status >> - * before we're looked at the HW caps to see if we have uc support >> - */ >> - BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); >> - >> - uc_fw->path = NULL; >> - uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED; >> - uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; >> - uc_fw->type = type; >> } >> static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw) >> @@ -164,7 +150,10 @@ static inline u32 >> intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw) >> return uc_fw->header_size + uc_fw->ucode_size; >> } >> -void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, >> +void intel_uc_fw_init_early(struct drm_i915_private *i915, >> + struct intel_uc_fw *uc_fw, >> + enum intel_uc_fw_type type); >> +void intel_uc_fw_fetch(struct drm_i915_private *i915, >> struct intel_uc_fw *uc_fw); >> void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw); >> int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c index 87169e826747..a027deb80330 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c @@ -31,89 +31,6 @@ #include "intel_guc_fw.h" #include "i915_drv.h" -#define __MAKE_GUC_FW_PATH(KEY) \ - "i915/" \ - __stringify(KEY##_GUC_FW_PREFIX) "_guc_" \ - __stringify(KEY##_GUC_FW_MAJOR) "." \ - __stringify(KEY##_GUC_FW_MINOR) "." \ - __stringify(KEY##_GUC_FW_PATCH) ".bin" - -#define SKL_GUC_FW_PREFIX skl -#define SKL_GUC_FW_MAJOR 33 -#define SKL_GUC_FW_MINOR 0 -#define SKL_GUC_FW_PATCH 0 -#define SKL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(SKL) -MODULE_FIRMWARE(SKL_GUC_FIRMWARE_PATH); - -#define BXT_GUC_FW_PREFIX bxt -#define BXT_GUC_FW_MAJOR 33 -#define BXT_GUC_FW_MINOR 0 -#define BXT_GUC_FW_PATCH 0 -#define BXT_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(BXT) -MODULE_FIRMWARE(BXT_GUC_FIRMWARE_PATH); - -#define KBL_GUC_FW_PREFIX kbl -#define KBL_GUC_FW_MAJOR 33 -#define KBL_GUC_FW_MINOR 0 -#define KBL_GUC_FW_PATCH 0 -#define KBL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(KBL) -MODULE_FIRMWARE(KBL_GUC_FIRMWARE_PATH); - -#define GLK_GUC_FW_PREFIX glk -#define GLK_GUC_FW_MAJOR 33 -#define GLK_GUC_FW_MINOR 0 -#define GLK_GUC_FW_PATCH 0 -#define GLK_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(GLK) -MODULE_FIRMWARE(GLK_GUC_FIRMWARE_PATH); - -#define ICL_GUC_FW_PREFIX icl -#define ICL_GUC_FW_MAJOR 33 -#define ICL_GUC_FW_MINOR 0 -#define ICL_GUC_FW_PATCH 0 -#define ICL_GUC_FIRMWARE_PATH __MAKE_GUC_FW_PATH(ICL) -MODULE_FIRMWARE(ICL_GUC_FIRMWARE_PATH); - -static void guc_fw_select(struct intel_uc_fw *guc_fw) -{ - struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); - struct drm_i915_private *i915 = guc_to_gt(guc)->i915; - - GEM_BUG_ON(guc_fw->type != INTEL_UC_FW_TYPE_GUC); - - if (!HAS_GT_UC(i915)) { - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; - return; - } - - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; - - if (i915_modparams.guc_firmware_path) { - guc_fw->path = i915_modparams.guc_firmware_path; - guc_fw->major_ver_wanted = 0; - guc_fw->minor_ver_wanted = 0; - } else if (IS_ICELAKE(i915)) { - guc_fw->path = ICL_GUC_FIRMWARE_PATH; - guc_fw->major_ver_wanted = ICL_GUC_FW_MAJOR; - guc_fw->minor_ver_wanted = ICL_GUC_FW_MINOR; - } else if (IS_GEMINILAKE(i915)) { - guc_fw->path = GLK_GUC_FIRMWARE_PATH; - guc_fw->major_ver_wanted = GLK_GUC_FW_MAJOR; - guc_fw->minor_ver_wanted = GLK_GUC_FW_MINOR; - } else if (IS_KABYLAKE(i915) || IS_COFFEELAKE(i915)) { - guc_fw->path = KBL_GUC_FIRMWARE_PATH; - guc_fw->major_ver_wanted = KBL_GUC_FW_MAJOR; - guc_fw->minor_ver_wanted = KBL_GUC_FW_MINOR; - } else if (IS_BROXTON(i915)) { - guc_fw->path = BXT_GUC_FIRMWARE_PATH; - guc_fw->major_ver_wanted = BXT_GUC_FW_MAJOR; - guc_fw->minor_ver_wanted = BXT_GUC_FW_MINOR; - } else if (IS_SKYLAKE(i915)) { - guc_fw->path = SKL_GUC_FIRMWARE_PATH; - guc_fw->major_ver_wanted = SKL_GUC_FW_MAJOR; - guc_fw->minor_ver_wanted = SKL_GUC_FW_MINOR; - } -} - /** * intel_guc_fw_init_early() - initializes GuC firmware struct * @guc: intel_guc struct @@ -124,8 +41,7 @@ void intel_guc_fw_init_early(struct intel_guc *guc) { struct intel_uc_fw *guc_fw = &guc->fw; - intel_uc_fw_init_early(guc_fw, INTEL_UC_FW_TYPE_GUC); - guc_fw_select(guc_fw); + intel_uc_fw_init_early(guc_to_gt(guc)->i915, guc_fw, INTEL_UC_FW_TYPE_GUC); } static void guc_prepare_xfer(struct intel_guc *guc) 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 BXT_HUC_FW_MAJOR 01 -#define BXT_HUC_FW_MINOR 8 -#define BXT_BLD_NUM 2893 - -#define SKL_HUC_FW_MAJOR 01 -#define SKL_HUC_FW_MINOR 07 -#define SKL_BLD_NUM 1398 - -#define KBL_HUC_FW_MAJOR 02 -#define KBL_HUC_FW_MINOR 00 -#define KBL_BLD_NUM 1810 - -#define GLK_HUC_FW_MAJOR 03 -#define GLK_HUC_FW_MINOR 01 -#define GLK_BLD_NUM 2893 - -#define ICL_HUC_FW_MAJOR 8 -#define ICL_HUC_FW_MINOR 4 -#define ICL_BLD_NUM 3238 - -#define HUC_FW_PATH(platform, major, minor, bld_num) \ - "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \ - __stringify(minor) "_" __stringify(bld_num) ".bin" - -#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \ - SKL_HUC_FW_MINOR, SKL_BLD_NUM) -MODULE_FIRMWARE(I915_SKL_HUC_UCODE); - -#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \ - BXT_HUC_FW_MINOR, BXT_BLD_NUM) -MODULE_FIRMWARE(I915_BXT_HUC_UCODE); - -#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \ - KBL_HUC_FW_MINOR, KBL_BLD_NUM) -MODULE_FIRMWARE(I915_KBL_HUC_UCODE); - -#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \ - GLK_HUC_FW_MINOR, GLK_BLD_NUM) -MODULE_FIRMWARE(I915_GLK_HUC_UCODE); - -#define I915_ICL_HUC_UCODE HUC_FW_PATH(icl, ICL_HUC_FW_MAJOR, \ - ICL_HUC_FW_MINOR, ICL_BLD_NUM) -MODULE_FIRMWARE(I915_ICL_HUC_UCODE); - -static void huc_fw_select(struct intel_uc_fw *huc_fw) -{ - struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw); - struct drm_i915_private *dev_priv = huc_to_gt(huc)->i915; - - GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC); - - if (!HAS_GT_UC(dev_priv)) { - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; - return; - } - - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; - - if (i915_modparams.huc_firmware_path) { - huc_fw->path = i915_modparams.huc_firmware_path; - huc_fw->major_ver_wanted = 0; - huc_fw->minor_ver_wanted = 0; - } else if (IS_SKYLAKE(dev_priv)) { - huc_fw->path = I915_SKL_HUC_UCODE; - huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR; - } else if (IS_BROXTON(dev_priv)) { - huc_fw->path = I915_BXT_HUC_UCODE; - huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR; - } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) { - huc_fw->path = I915_KBL_HUC_UCODE; - huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; - } else if (IS_GEMINILAKE(dev_priv)) { - huc_fw->path = I915_GLK_HUC_UCODE; - huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR; - } else if (IS_ICELAKE(dev_priv)) { - huc_fw->path = I915_ICL_HUC_UCODE; - huc_fw->major_ver_wanted = ICL_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = ICL_HUC_FW_MINOR; - } -} - /** * intel_huc_fw_init_early() - initializes HuC firmware struct * @huc: intel_huc struct @@ -118,8 +33,7 @@ void intel_huc_fw_init_early(struct intel_huc *huc) { struct intel_uc_fw *huc_fw = &huc->fw; - intel_uc_fw_init_early(huc_fw, INTEL_UC_FW_TYPE_HUC); - huc_fw_select(huc_fw); + intel_uc_fw_init_early(huc_to_gt(huc)->i915, huc_fw, INTEL_UC_FW_TYPE_HUC); } static void huc_xfer_rsa(struct intel_huc *huc) 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 8ce7210907c0..48100dff466d 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -29,6 +29,162 @@ #include "intel_uc_fw.h" #include "i915_drv.h" +/* + * 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)) \ + 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" + +#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) + +/* + * 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; + } + } +} + +static void +uc_fw_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) +{ + GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED); + + if (!HAS_GT_UC(i915)) { + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED; + return; + } + + uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_STARTED; + + if (unlikely(i915_modparams.guc_firmware_path && + uc_fw->type == INTEL_UC_FW_TYPE_GUC)) + uc_fw->path = i915_modparams.guc_firmware_path; + else if (unlikely(i915_modparams.huc_firmware_path && + uc_fw->type == INTEL_UC_FW_TYPE_HUC)) + uc_fw->path = i915_modparams.huc_firmware_path; + else + __uc_fw_select(uc_fw, INTEL_INFO(i915)->platform, INTEL_REVID(i915)); +} + +/** + * intel_uc_fw_init_early - initialize the uC object and select the firmware + * @i915: device private + * @uc_fw: uC firmware + * @type: type of uC + * + * Initialize the state of our uC object and relevant tracking and select the + * firmware to fetch and load. + */ +void intel_uc_fw_init_early(struct drm_i915_private *i915, + struct intel_uc_fw *uc_fw, + enum intel_uc_fw_type type) +{ + /* + * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status + * before we're looked at the HW caps to see if we have uc support + */ + BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); + + uc_fw->path = NULL; + uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED; + uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; + uc_fw->type = type; + + uc_fw_select(i915, uc_fw); +} + /** * intel_uc_fw_fetch - fetch uC firmware * 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 833d04d06576..c2868ef15eda 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h @@ -44,8 +44,9 @@ enum intel_uc_fw_status { }; enum intel_uc_fw_type { - INTEL_UC_FW_TYPE_GUC, - INTEL_UC_FW_TYPE_HUC + INTEL_UC_FW_TYPE_GUC = 0, + INTEL_UC_FW_TYPE_HUC, + INTEL_UC_NUM_TYPES }; /* @@ -105,24 +106,9 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type) return "GuC"; case INTEL_UC_FW_TYPE_HUC: return "HuC"; + default: + return "uC"; } - return "uC"; -} - -static inline -void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, - enum intel_uc_fw_type type) -{ - /* - * we use FIRMWARE_UNINITIALIZED to detect checks against fetch_status - * before we're looked at the HW caps to see if we have uc support - */ - BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); - - uc_fw->path = NULL; - uc_fw->fetch_status = INTEL_UC_FIRMWARE_UNINITIALIZED; - uc_fw->load_status = INTEL_UC_FIRMWARE_NOT_STARTED; - uc_fw->type = type; } static inline bool intel_uc_fw_is_selected(struct intel_uc_fw *uc_fw) @@ -164,7 +150,10 @@ static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw) return uc_fw->header_size + uc_fw->ucode_size; } -void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, +void intel_uc_fw_init_early(struct drm_i915_private *i915, + struct intel_uc_fw *uc_fw, + enum intel_uc_fw_type type); +void intel_uc_fw_fetch(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw); void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw); int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
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@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 86 +----------- drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 88 +----------- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 156 ++++++++++++++++++++++ drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 29 ++-- 4 files changed, 167 insertions(+), 192 deletions(-)