diff mbox series

[3/9] drm/i915/uc: Unify uC FW selection

Message ID 20190722232048.9970-4-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series uC fw path unification + misc clean-up | expand

Commit Message

Daniele Ceraolo Spurio July 22, 2019, 11:20 p.m. UTC
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.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 109 ++++++----------------
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c | 102 +++++---------------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  |  47 ++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |  27 +++++-
 4 files changed, 121 insertions(+), 164 deletions(-)

Comments

Chris Wilson July 23, 2019, 8:26 a.m. UTC | #1
Quoting Daniele Ceraolo Spurio (2019-07-23 00:20:42)
> +/* must be ordered base on platform + revid, from newer to older */
> +static const struct intel_uc_platform_requirement guc_fw_blobs[] = {
> +       { INTEL_ICELAKE,        0,      &icl_guc_fw_blob },
> +       { INTEL_COFFEELAKE,     0,      &kbl_guc_fw_blob },
> +       { INTEL_GEMINILAKE,     0,      &glk_guc_fw_blob },
> +       { INTEL_KABYLAKE,       0,      &kbl_guc_fw_blob },
> +       { INTEL_BROXTON,        0,      &bxt_guc_fw_blob },
> +       { INTEL_SKYLAKE,        0,      &skl_guc_fw_blob },
> +};

> +/**
> + * intel_uc_fw_select - select the uC firmware to fetch & load
> + * @i915: device private
> + * @uc_fw: uC firmware
> + * @list: list of required firmware for each platform
> + * @length: number of entries in the list
> + *
> + * Select the uC firmware from the provided list based on platform and revid of
> + * the device we're on. If the firmware_path modparam override is set, it takes
> + * precedence over the entries in the list.
> + */
> +void intel_uc_fw_select(struct drm_i915_private *i915,
> +                       struct intel_uc_fw *uc_fw,
> +                       const struct intel_uc_platform_requirement *list,
> +                       unsigned int length)

If this is a list, do we need a length? Is a sentinel not good enough?

> +{
> +       GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED);
> +
> +       if (!HAS_UC(i915)) {
> +               uc_fw->fetch_status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
> +               return;
> +       }

if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) {
	for (i = 1; i < length; i++)
		if (list[i].first_rev <= list[i-1].first_rev) {
			pr_err("...");
			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 {
> +               enum intel_platform p = INTEL_INFO(i915)->platform;
> +               u8 rev = INTEL_REVID(i915);
> +               int i;
> +
> +               for (i = 0; i < length && p <= list[i].p; i++) {
> +                       if (p == list[i].p && rev >= list[i].first_rev) {
> +                               uc_fw->path = list[i].blob->path;
> +                               uc_fw->major_ver_wanted = list[i].blob->major;
> +                               uc_fw->minor_ver_wanted = list[i].blob->minor;
> +                               break;
> +                       }
> +               }
> +       }
> +}
Michal Wajdeczko July 23, 2019, 1:22 p.m. UTC | #2
On Tue, 23 Jul 2019 01:20:42 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> +
> +#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
> +UC_FW_BLOB(prefix_##_guc, major_, minor_, \
> +	   __MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
> +
> +GUC_FW_BLOB(skl, 33, 0, 0);
> +GUC_FW_BLOB(bxt, 33, 0, 0);
> +GUC_FW_BLOB(kbl, 33, 0, 0);
> +GUC_FW_BLOB(glk, 33, 0, 0);
> +GUC_FW_BLOB(icl, 33, 0, 0);
> +
> +/* must be ordered base on platform + revid, from newer to older */
> +static const struct intel_uc_platform_requirement guc_fw_blobs[] = {
> +	{ INTEL_ICELAKE,	0,	&icl_guc_fw_blob },
> +	{ INTEL_COFFEELAKE,	0,	&kbl_guc_fw_blob },
> +	{ INTEL_GEMINILAKE,	0,	&glk_guc_fw_blob },
> +	{ INTEL_KABYLAKE,	0,	&kbl_guc_fw_blob },
> +	{ INTEL_BROXTON,	0,	&bxt_guc_fw_blob },
> +	{ INTEL_SKYLAKE,	0,	&skl_guc_fw_blob },
> +};

Can we avoid pointers to separate blob definitions ?
What about defining each fw in single line like below

#define INTEL_GUC_FIRMWARE_DEFS(fw_def) \
	fw_def(ICELAKE, 0, 33, 0, 0, icl, GUC) \
	fw_def(COFFEELAKE, 0, 33, 0, 0, kbl, GUC) \
	fw_def(GEMINILAKE, 0, 33, 0, 0, glk, GUC) \
	fw_def(KABYLAKE, 0, 33, 0, 0, kbl, GUC) \
	fw_def(BROXTON, 0, 33, 0, 0, bxt, GUC) \
	fw_def(SKYLAKE, 0, 33, 0, 0, skl, GUC) \
	/* end */

with some extra common helpers

#define TO_MODULE_FIRMWARE(_platform, _rev, _major, _minor, _patch,  
_prefix, _builder) \
	MODULE_FIRMWARE(_builder##_FW_PATH_BUILDER(_prefix, _major, _minor,  
_patch));

#define TO_BLOB_ENTRY(_platform, _rev, _major, _minor, _patch, _prefix,  
_builder) \
{ \
	.platform = INTEL_##_platform, \
	.rev = (_rev), \
	.major = (_major), \
	.minor = (_minor), \
	.patch = (_patch), \
	.blob = _builder##_FW_PATH_BUILDER(_prefix, _major, _minor, _patch), \
},

then we can have immutable

static const struct intel_uc_blob guc_fw_blobs[] = {
INTEL_GUC_FIRMWARE_DEFS(TO_BLOB_ENTRY)
};
INTEL_GUC_FIRMWARE_DEFS(TO_MODULE_FIRMWARE)

(tested locally for feasibility)

Michal
Daniele Ceraolo Spurio July 23, 2019, 3:01 p.m. UTC | #3
On 7/23/2019 6:22 AM, Michal Wajdeczko wrote:
> On Tue, 23 Jul 2019 01:20:42 +0200, Daniele Ceraolo Spurio 
> <daniele.ceraolospurio@intel.com> wrote:
>
>> +
>> +#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
>> +UC_FW_BLOB(prefix_##_guc, major_, minor_, \
>> +       __MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
>> +
>> +GUC_FW_BLOB(skl, 33, 0, 0);
>> +GUC_FW_BLOB(bxt, 33, 0, 0);
>> +GUC_FW_BLOB(kbl, 33, 0, 0);
>> +GUC_FW_BLOB(glk, 33, 0, 0);
>> +GUC_FW_BLOB(icl, 33, 0, 0);
>> +
>> +/* must be ordered base on platform + revid, from newer to older */
>> +static const struct intel_uc_platform_requirement guc_fw_blobs[] = {
>> +    { INTEL_ICELAKE,    0,    &icl_guc_fw_blob },
>> +    { INTEL_COFFEELAKE,    0,    &kbl_guc_fw_blob },
>> +    { INTEL_GEMINILAKE,    0,    &glk_guc_fw_blob },
>> +    { INTEL_KABYLAKE,    0,    &kbl_guc_fw_blob },
>> +    { INTEL_BROXTON,    0,    &bxt_guc_fw_blob },
>> +    { INTEL_SKYLAKE,    0,    &skl_guc_fw_blob },
>> +};
>
> Can we avoid pointers to separate blob definitions ?
> What about defining each fw in single line like below
>
> #define INTEL_GUC_FIRMWARE_DEFS(fw_def) \
>     fw_def(ICELAKE, 0, 33, 0, 0, icl, GUC) \
>     fw_def(COFFEELAKE, 0, 33, 0, 0, kbl, GUC) \
>     fw_def(GEMINILAKE, 0, 33, 0, 0, glk, GUC) \
>     fw_def(KABYLAKE, 0, 33, 0, 0, kbl, GUC) \
>     fw_def(BROXTON, 0, 33, 0, 0, bxt, GUC) \
>     fw_def(SKYLAKE, 0, 33, 0, 0, skl, GUC) \
>     /* end */
>
> with some extra common helpers
>
> #define TO_MODULE_FIRMWARE(_platform, _rev, _major, _minor, _patch, 
> _prefix, _builder) \
>     MODULE_FIRMWARE(_builder##_FW_PATH_BUILDER(_prefix, _major, 
> _minor, _patch));
>
> #define TO_BLOB_ENTRY(_platform, _rev, _major, _minor, _patch, 
> _prefix, _builder) \
> { \
>     .platform = INTEL_##_platform, \
>     .rev = (_rev), \
>     .major = (_major), \
>     .minor = (_minor), \
>     .patch = (_patch), \
>     .blob = _builder##_FW_PATH_BUILDER(_prefix, _major, _minor, 
> _patch), \
> },
>
> then we can have immutable
>
> static const struct intel_uc_blob guc_fw_blobs[] = {
> INTEL_GUC_FIRMWARE_DEFS(TO_BLOB_ENTRY)
> };
> INTEL_GUC_FIRMWARE_DEFS(TO_MODULE_FIRMWARE)
>
> (tested locally for feasibility)
>
> Michal

LGTM, will update. I went with the list because I wanted to ultimately 
unify the GuC and HuC lists into one, but I should be able to set the 
macros appropriately to allow that as the next step.

Daniele
diff mbox series

Patch

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 ddd3b2162528..86df5147fc0a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -31,88 +31,32 @@ 
 #include "intel_guc_fw.h"
 #include "i915_drv.h"
 
-#define __MAKE_GUC_FW_PATH(KEY) \
+#define __MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \
 	"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_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;
-	}
-}
+	__stringify(prefix_) "_guc_" \
+	__stringify(major_) "." \
+	__stringify(minor_) "." \
+	__stringify(patch_) ".bin"
+
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+UC_FW_BLOB(prefix_##_guc, major_, minor_, \
+	   __MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
+
+GUC_FW_BLOB(skl, 33, 0, 0);
+GUC_FW_BLOB(bxt, 33, 0, 0);
+GUC_FW_BLOB(kbl, 33, 0, 0);
+GUC_FW_BLOB(glk, 33, 0, 0);
+GUC_FW_BLOB(icl, 33, 0, 0);
+
+/* must be ordered base on platform + revid, from newer to older */
+static const struct intel_uc_platform_requirement guc_fw_blobs[] = {
+	{ INTEL_ICELAKE,	0,	&icl_guc_fw_blob },
+	{ INTEL_COFFEELAKE,	0,	&kbl_guc_fw_blob },
+	{ INTEL_GEMINILAKE,	0,	&glk_guc_fw_blob },
+	{ INTEL_KABYLAKE,	0,	&kbl_guc_fw_blob },
+	{ INTEL_BROXTON,	0,	&bxt_guc_fw_blob },
+	{ INTEL_SKYLAKE,	0,	&skl_guc_fw_blob },
+};
 
 /**
  * intel_guc_fw_init_early() - initializes GuC firmware struct
@@ -125,7 +69,8 @@  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_select(guc_to_gt(guc)->i915, guc_fw,
+			   guc_fw_blobs, ARRAY_SIZE(guc_fw_blobs));
 }
 
 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 15891cf4bb2c..263977294ef0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -23,90 +23,29 @@ 
  * 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_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;
-	}
-}
+#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \
+UC_FW_BLOB(prefix_##_huc, major_, minor_, \
+	   HUC_FW_PATH(prefix_, major_, minor_, bld_num_))
+
+HUC_FW_BLOB(skl, 01, 07, 1398);
+HUC_FW_BLOB(bxt, 01,  8, 2893);
+HUC_FW_BLOB(kbl, 02, 00, 1810);
+HUC_FW_BLOB(glk, 03, 01, 2893);
+HUC_FW_BLOB(icl,  8,  4, 3238);
+
+/* must be ordered base on platform + revid, from newer to older */
+static const struct intel_uc_platform_requirement huc_fw_blobs[] = {
+	{ INTEL_ICELAKE,	0,	&icl_huc_fw_blob },
+	{ INTEL_COFFEELAKE,	0,	&kbl_huc_fw_blob },
+	{ INTEL_GEMINILAKE,	0,	&glk_huc_fw_blob },
+	{ INTEL_KABYLAKE,	0,	&kbl_huc_fw_blob },
+	{ INTEL_BROXTON,	0,	&bxt_huc_fw_blob },
+	{ INTEL_SKYLAKE,	0,	&skl_huc_fw_blob },
+};
 
 /**
  * intel_huc_fw_init_early() - initializes HuC firmware struct
@@ -119,7 +58,8 @@  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_select(huc_to_gt(huc)->i915, huc_fw,
+			   huc_fw_blobs, ARRAY_SIZE(huc_fw_blobs));
 }
 
 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..faa96748aed3 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,53 @@ 
 #include "intel_uc_fw.h"
 #include "i915_drv.h"
 
+/**
+ * intel_uc_fw_select - select the uC firmware to fetch & load
+ * @i915: device private
+ * @uc_fw: uC firmware
+ * @list: list of required firmware for each platform
+ * @length: number of entries in the list
+ *
+ * Select the uC firmware from the provided list based on platform and revid of
+ * the device we're on. If the firmware_path modparam override is set, it takes
+ * precedence over the entries in the list.
+ */
+void intel_uc_fw_select(struct drm_i915_private *i915,
+			struct intel_uc_fw *uc_fw,
+			const struct intel_uc_platform_requirement *list,
+			unsigned int length)
+{
+	GEM_BUG_ON(uc_fw->fetch_status != INTEL_UC_FIRMWARE_UNINITIALIZED);
+
+	if (!HAS_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 {
+		enum intel_platform p = INTEL_INFO(i915)->platform;
+		u8 rev = INTEL_REVID(i915);
+		int i;
+
+		for (i = 0; i < length && p <= list[i].p; i++) {
+			if (p == list[i].p && rev >= list[i].first_rev) {
+				uc_fw->path = list[i].blob->path;
+				uc_fw->major_ver_wanted = list[i].blob->major;
+				uc_fw->minor_ver_wanted = list[i].blob->minor;
+				break;
+			}
+		}
+	}
+}
+
 /**
  * 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..550b626afb15 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -26,6 +26,7 @@ 
 #define _INTEL_UC_FW_H_
 
 #include <linux/types.h>
+#include "intel_device_info.h"
 #include "i915_gem.h"
 
 struct drm_printer;
@@ -48,6 +49,26 @@  enum intel_uc_fw_type {
 	INTEL_UC_FW_TYPE_HUC
 };
 
+struct __packed intel_uc_fw_blob {
+	u8 major;
+	u8 minor;
+	const char *path;
+};
+
+#define UC_FW_BLOB(prefix_, major_, minor_, path_) \
+static const struct intel_uc_fw_blob prefix_##_fw_blob = { \
+	.major = (major_), \
+	.minor = (minor_), \
+	.path = path_ \
+}; \
+MODULE_FIRMWARE(path_)
+
+struct __packed intel_uc_platform_requirement {
+	enum intel_platform p;
+	u8 first_rev;
+	const struct intel_uc_fw_blob *blob;
+};
+
 /*
  * This structure encapsulates all the data needed during the process
  * of fetching, caching, and loading the firmware image into the uC.
@@ -164,7 +185,11 @@  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_select(struct drm_i915_private *i915,
+			struct intel_uc_fw *uc_fw,
+			const struct intel_uc_platform_requirement *blobs_list,
+			unsigned int length);
+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,