[PATCH v2 4/7] drm/xe/uc: Add support for different firmware files on each GT
Lucas De Marchi
lucas.demarchi at intel.com
Thu Mar 6 02:09:51 UTC 2025
On Mon, Mar 03, 2025 at 11:52:12AM -0800, John.C.Harrison at Intel.com wrote:
>From: John Harrison <John.C.Harrison at Intel.com>
>
>The different GTs on a device can be very different. So add support
>for being able to load a GuC firmware image that is tailored to the
>specific GT type.
Suggest to reword this to take into account that this is not true
for the platforms today, but it may be in future.
>
>Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
>---
> drivers/gpu/drm/xe/xe_uc_fw.c | 67 +++++++++++++++++++++--------------
> 1 file changed, 40 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
>index fb0eda3d5682..748863b75adf 100644
>--- a/drivers/gpu/drm/xe/xe_uc_fw.c
>+++ b/drivers/gpu/drm/xe/xe_uc_fw.c
>@@ -98,6 +98,7 @@ struct uc_fw_entry {
> u16 minor;
> u16 patch;
> bool full_ver_required;
>+ const char *gt_type;
> };
> };
>
>@@ -107,16 +108,16 @@ struct fw_blobs_by_type {
> };
>
> #define XE_GUC_FIRMWARE_DEFS(fw_def, mmp_ver, major_ver) \
>- fw_def(BATTLEMAGE, major_ver(xe, guc, bmg, 70, 29, 2)) \
>- fw_def(LUNARLAKE, major_ver(xe, guc, lnl, 70, 29, 2)) \
>- fw_def(METEORLAKE, major_ver(i915, guc, mtl, 70, 29, 2)) \
>- fw_def(DG2, major_ver(i915, guc, dg2, 70, 29, 2)) \
>- fw_def(DG1, major_ver(i915, guc, dg1, 70, 29, 2)) \
>- fw_def(ALDERLAKE_N, major_ver(i915, guc, tgl, 70, 29, 2)) \
>- fw_def(ALDERLAKE_P, major_ver(i915, guc, adlp, 70, 29, 2)) \
>- fw_def(ALDERLAKE_S, major_ver(i915, guc, tgl, 70, 29, 2)) \
>- fw_def(ROCKETLAKE, major_ver(i915, guc, tgl, 70, 29, 2)) \
>- fw_def(TIGERLAKE, major_ver(i915, guc, tgl, 70, 29, 2))
>+ fw_def(BATTLEMAGE, major_ver(xe, guc, bmg, 70, 29, 2, "")) \
There are a few things I dislike here and I've been pondering if we
merge it as is or if we improve it. I don't want to rush this in for
6.15 as there is no need to.
1) Keeping this ugly empty string repeated for every single platform.
Particularly because it's no descriptive of what it really is. Alas,
adding a "g" later is also a bit cryptic
2) The media type depending on the name like this. Note that the first
field is about the platform match.
3) This hidden behavior coded below that if type **starts with** "g"
it's for main gt, if it starts with "m" it's for media.
4) pahole is not very happy with the new uc_fw_entry. We can do better.
With that in mind, i'd propose this instead, using BMG and LNL just for
example:
#define ANY_GT XE_GT_TYPE_UNINITIALIZED
fw_def(BATTLEMAGE, XE_GT_TYPE_MAIN, major_ver(xe, guc, bmg_g, 70, 29, 2) \
fw_def(BATTLEMAGE, XE_GT_TYPE_MEDIA, major_ver(xe, guc, bmg_m, 70, 29, 1) \
fw_def(LUNARLAKE, ANY_GT, major_ver(xe, guc, lnl, 70, 29, 2) \
>+ fw_def(LUNARLAKE, major_ver(xe, guc, lnl, 70, 29, 2, "")) \
>+ fw_def(METEORLAKE, major_ver(i915, guc, mtl, 70, 29, 2, "")) \
>+ fw_def(DG2, major_ver(i915, guc, dg2, 70, 29, 2, "")) \
>+ fw_def(DG1, major_ver(i915, guc, dg1, 70, 29, 2, "")) \
>+ fw_def(ALDERLAKE_N, major_ver(i915, guc, tgl, 70, 29, 2, "")) \
>+ fw_def(ALDERLAKE_P, major_ver(i915, guc, adlp, 70, 29, 2, "")) \
>+ fw_def(ALDERLAKE_S, major_ver(i915, guc, tgl, 70, 29, 2, "")) \
>+ fw_def(ROCKETLAKE, major_ver(i915, guc, tgl, 70, 29, 2, "")) \
>+ fw_def(TIGERLAKE, major_ver(i915, guc, tgl, 70, 29, 2, ""))
>
> #define XE_HUC_FIRMWARE_DEFS(fw_def, mmp_ver, no_ver) \
> fw_def(BATTLEMAGE, no_ver(xe, huc, bmg)) \
>@@ -133,30 +134,30 @@ struct fw_blobs_by_type {
> fw_def(LUNARLAKE, major_ver(xe, gsc, lnl, 104, 1, 0)) \
> fw_def(METEORLAKE, major_ver(i915, gsc, mtl, 102, 1, 0))
>
>-#define MAKE_FW_PATH(dir__, uc__, shortname__, version__) \
>- __stringify(dir__) "/" __stringify(shortname__) "_" __stringify(uc__) version__ ".bin"
>+#define MAKE_FW_PATH(dir__, uc__, shortname__, type__, version__) \
>+ __stringify(dir__) "/" __stringify(shortname__) type__ "_" __stringify(uc__) version__ ".bin"
>
>-#define fw_filename_mmp_ver(dir_, uc_, shortname_, a, b, c) \
>- MAKE_FW_PATH(dir_, uc_, shortname_, "_" __stringify(a ## . ## b ## . ## c))
>-#define fw_filename_major_ver(dir_, uc_, shortname_, a, b, c) \
>- MAKE_FW_PATH(dir_, uc_, shortname_, "_" __stringify(a))
>+#define fw_filename_mmp_ver(dir_, uc_, shortname_, a, b, c, t) \
>+ MAKE_FW_PATH(dir_, uc_, shortname_, t, "_" __stringify(a ## . ## b ## . ## c))
>+#define fw_filename_major_ver(dir_, uc_, shortname_, a, b, c, t) \
>+ MAKE_FW_PATH(dir_, uc_, shortname_, t, "_" __stringify(a))
> #define fw_filename_no_ver(dir_, uc_, shortname_) \
>- MAKE_FW_PATH(dir_, uc_, shortname_, "")
>+ MAKE_FW_PATH(dir_, uc_, shortname_, "", "")
> #define fw_filename_gsc(dir_, uc_, shortname_, a, b, c) \
>- MAKE_FW_PATH(dir_, uc_, shortname_, "_" __stringify(b))
>-
>-#define uc_fw_entry_mmp_ver(dir_, uc_, shortname_, a, b, c) \
>- { fw_filename_mmp_ver(dir_, uc_, shortname_, a, b, c), \
>- a, b, c, true }
>-#define uc_fw_entry_major_ver(dir_, uc_, shortname_, a, b, c) \
>- { fw_filename_major_ver(dir_, uc_, shortname_, a, b, c), \
>- a, b, c }
>+ MAKE_FW_PATH(dir_, uc_, shortname_, "", "_" __stringify(b))
>+
>+#define uc_fw_entry_mmp_ver(dir_, uc_, shortname_, a, b, c, t) \
>+ { fw_filename_mmp_ver(dir_, uc_, shortname_, a, b, c, t), \
>+ a, b, c, true, t }
>+#define uc_fw_entry_major_ver(dir_, uc_, shortname_, a, b, c, t) \
>+ { fw_filename_major_ver(dir_, uc_, shortname_, a, b, c, t), \
>+ a, b, c, false, t }
> #define uc_fw_entry_no_ver(dir_, uc_, shortname_) \
> { fw_filename_no_ver(dir_, uc_, shortname_), \
>- 0, 0 }
>+ 0, 0, 0, false, "" }
> #define uc_fw_entry_gsc(dir_, uc_, shortname_, a, b, c) \
> { fw_filename_gsc(dir_, uc_, shortname_, a, b, c), \
>- a, b, c }
>+ a, b, c, false, "" }
..
#define XE_UC_FW_ENTRY(platform__, gt_type__, entry__) \
{ \
.platform = XE_ ## platform__, \
.gt_type = gt_type__, \
entry__, \
},
>
> /* All blobs need to be declared via MODULE_FIRMWARE() */
> #define XE_UC_MODULE_FIRMWARE(platform__, fw_filename) \
>@@ -224,6 +225,7 @@ uc_fw_auto_select(struct xe_device *xe, struct xe_uc_fw *uc_fw)
> };
> static const struct uc_fw_entry *entries;
> enum xe_platform p = xe->info.platform;
>+ struct xe_gt *gt = uc_fw_to_gt(uc_fw);
> u32 count;
> int i;
>
>@@ -231,8 +233,19 @@ uc_fw_auto_select(struct xe_device *xe, struct xe_uc_fw *uc_fw)
> entries = blobs_all[uc_fw->type].entries;
> count = blobs_all[uc_fw->type].count;
>
>+ xe_gt_assert(gt, (gt->info.type == XE_GT_TYPE_MAIN) || (gt->info.type == XE_GT_TYPE_MEDIA));
then remove the assert.
>+
> for (i = 0; i < count && p <= entries[i].platform; i++) {
> if (p == entries[i].platform) {
>+ if (entries[i].gt_type[0]) {
>+ if (gt->info.type == XE_GT_TYPE_MAIN &&
>+ (entries[i].gt_type[0] != 'g'))
>+ continue;
>+ if (gt->info.type == XE_GT_TYPE_MEDIA &&
>+ (entries[i].gt_type[0] != 'm'))
>+ continue;
>+ }
>+
and this becomes:
for (i = 0; i < count && p <= entries[i].platform; i++) {
if (p != entries[i].platform)
continue;
if (entries[i].gt_type != ANY_GT &&
gt->info.type != entries[i].gt_type)
continue;
uc_fw->path = entries[i].path;
uc_fw->versions.wanted.major = entries[i].major;
uc_fw->versions.wanted.minor = entries[i].minor;
...
}
Lucas De Marchi
>--
>2.47.0
>
More information about the Intel-xe
mailing list