[Intel-gfx] [PATCH v2 2/8] drm/i915/uc: Unify uC FW selection

Michal Wajdeczko michal.wajdeczko at intel.com
Wed Jul 24 11:31:46 UTC 2019


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

> +			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,


More information about the Intel-gfx mailing list