[Intel-gfx] [PATCH v2 2/8] drm/i915/uc: Unify uC FW selection
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Jul 24 16:28:06 UTC 2019
On 7/24/2019 4:31 AM, Michal Wajdeczko wrote:
> On Wed, 24 Jul 2019 04:21:47 +0200, Daniele Ceraolo Spurio
> <daniele.ceraolospurio at 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 at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> Cc: Chris Wilson <chris at 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,
More information about the Intel-gfx
mailing list