| Message ID | 20230415005706.4135485-5-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improvements to uc firmare management | expand |
On 4/14/2023 5:57 PM, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > The validation of the firmware table was being done inside the code > for scanning the table for the next available firmware blob. Which is > unnecessary. Potentially, it should be a selftest. But either way, the > first step is pulling it out into a separate function that can be > called just once rather than once per blob attempt per blob type. > > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 164 ++++++++++++++--------- > 1 file changed, 99 insertions(+), 65 deletions(-) > > 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 6bb45d6b8da5f..c589782467265 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -233,20 +233,22 @@ struct fw_blobs_by_type { > u32 count; > }; > > +static const struct uc_fw_platform_requirement blobs_guc[] = { > + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP) > +}; > + > +static const struct uc_fw_platform_requirement blobs_huc[] = { > + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC) > +}; > + > +static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = { > + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) }, > + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) }, > +}; > + > static void > __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) > { > - static const struct uc_fw_platform_requirement blobs_guc[] = { > - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP) > - }; > - static const struct uc_fw_platform_requirement blobs_huc[] = { > - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC) > - }; > - static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = { > - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) }, > - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) }, > - }; > - static bool verified[INTEL_UC_FW_NUM_TYPES]; > const struct uc_fw_platform_requirement *fw_blobs; > enum intel_platform p = INTEL_INFO(i915)->platform; > u32 fw_count; > @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) > continue; > > if (uc_fw->file_selected.path) { > + /* > + * Continuing an earlier search after a found blob failed to load. > + * Once the previously chosen path has been found, clear it out > + * and let the search continue from there. > + */ > if (uc_fw->file_selected.path == blob->path) > uc_fw->file_selected.path = NULL; > > @@ -306,78 +313,103 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) > /* Failed to find a match for the last attempt?! */ > uc_fw->file_selected.path = NULL; > } > +} > > - /* make sure the list is ordered as expected */ > - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified[uc_fw->type]) { > - verified[uc_fw->type] = true; > +static void validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_fw_type type) > +{ > + const struct uc_fw_platform_requirement *fw_blobs; > + u32 fw_count; > + int i; > > - for (i = 1; i < fw_count; i++) { > - /* Next platform is good: */ > - if (fw_blobs[i].p < fw_blobs[i - 1].p) > - continue; > + if (type >= ARRAY_SIZE(blobs_all)) { > + drm_err(&i915->drm, "No blob array for %s\n", intel_uc_fw_type_repr(type)); > + return; > + } > > - /* Next platform revision is good: */ > - if (fw_blobs[i].p == fw_blobs[i - 1].p && > - fw_blobs[i].rev < fw_blobs[i - 1].rev) > - continue; > + fw_blobs = blobs_all[type].blobs; > + fw_count = blobs_all[type].count; > > - /* Platform/revision must be in order: */ > - if (fw_blobs[i].p != fw_blobs[i - 1].p || > - fw_blobs[i].rev != fw_blobs[i - 1].rev) > - goto bad; > + if (!fw_count) > + return; > > - /* Next major version is good: */ > - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) > - continue; > + /* make sure the list is ordered as expected */ > + for (i = 1; i < fw_count; i++) { > + /* Next platform is good: */ > + if (fw_blobs[i].p < fw_blobs[i - 1].p) > + continue; > > - /* New must be before legacy: */ > - if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy) > - goto bad; > + /* Next platform revision is good: */ > + if (fw_blobs[i].p == fw_blobs[i - 1].p && > + fw_blobs[i].rev < fw_blobs[i - 1].rev) > + continue; > > - /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */ > - if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) { > - if (!fw_blobs[i - 1].blob.major) > - continue; > + /* Platform/revision must be in order: */ > + if (fw_blobs[i].p != fw_blobs[i - 1].p || > + fw_blobs[i].rev != fw_blobs[i - 1].rev) > + goto bad; > > - if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major) > - continue; > - } > + /* Next major version is good: */ > + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) > + continue; > > - /* Major versions must be in order: */ > - if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major) > - goto bad; > + /* New must be before legacy: */ > + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy) > + goto bad; > > - /* Next minor version is good: */ > - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor) > + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */ > + if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) { > + if (!fw_blobs[i - 1].blob.major) > continue; > > - /* Minor versions must be in order: */ > - if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor) > - goto bad; > - > - /* Patch versions must be in order: */ > - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch) > + if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major) > continue; > + } > + > + /* Major versions must be in order: */ > + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major) > + goto bad; > + > + /* Next minor version is good: */ > + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor) > + continue; > + > + /* Minor versions must be in order: */ > + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor) > + goto bad; > + > + /* Patch versions must be in order: */ > + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch) > + continue; > > bad: > - drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", > - intel_uc_fw_type_repr(uc_fw->type), > - intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev, > - fw_blobs[i - 1].blob.legacy ? "L" : "v", > - fw_blobs[i - 1].blob.major, > - fw_blobs[i - 1].blob.minor, > - fw_blobs[i - 1].blob.patch, > - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev, > - fw_blobs[i].blob.legacy ? "L" : "v", > - fw_blobs[i].blob.major, > - fw_blobs[i].blob.minor, > - fw_blobs[i].blob.patch); > - > - uc_fw->file_selected.path = NULL; > - } > + drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", > + intel_uc_fw_type_repr(type), > + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev, > + fw_blobs[i - 1].blob.legacy ? "L" : "v", > + fw_blobs[i - 1].blob.major, > + fw_blobs[i - 1].blob.minor, > + fw_blobs[i - 1].blob.patch, > + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev, > + fw_blobs[i].blob.legacy ? "L" : "v", > + fw_blobs[i].blob.major, > + fw_blobs[i].blob.minor, > + fw_blobs[i].blob.patch); Confirming that this big diff was just an indent change was painful :/ > } > } > > +static void validate_fw_table(struct drm_i915_private *i915) > +{ > + enum intel_uc_fw_type type; > + static bool done; > + > + if (!IS_ENABLED(CONFIG_DRM_I915_SELFTEST) || done) > + return; > + done = true; > + > + for (type = 0; type < INTEL_UC_FW_NUM_TYPES; type++) > + validate_fw_table_type(i915, type); > +} > + > static const char *__override_guc_firmware_path(struct drm_i915_private *i915) > { > if (i915->params.enable_guc & ENABLE_GUC_MASK) > @@ -432,6 +464,8 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, > { > struct drm_i915_private *i915 = ____uc_fw_to_gt(uc_fw, type)->i915; > > + validate_fw_table(i915); Personal preference: IMO since we're calling intel_uc_fw_init_early per FW type it would've been cleaner to restrict validate_fw_table() to a single blob type. This would have the negative side effect of having to track the "done" status per FW type, so I can see it's not a clean improvement. Not a blocker. Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > + > /* > * we use FIRMWARE_UNINITIALIZED to detect checks against uc_fw->status > * before we're looked at the HW caps to see if we have uc support
On 4/18/2023 16:14, Ceraolo Spurio, Daniele wrote: > On 4/14/2023 5:57 PM, John.C.Harrison@Intel.com wrote: >> From: John Harrison <John.C.Harrison@Intel.com> >> >> The validation of the firmware table was being done inside the code >> for scanning the table for the next available firmware blob. Which is >> unnecessary. Potentially, it should be a selftest. But either way, the >> first step is pulling it out into a separate function that can be >> called just once rather than once per blob attempt per blob type. >> >> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >> --- >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 164 ++++++++++++++--------- >> 1 file changed, 99 insertions(+), 65 deletions(-) >> >> 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 6bb45d6b8da5f..c589782467265 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> @@ -233,20 +233,22 @@ struct fw_blobs_by_type { >> u32 count; >> }; >> +static const struct uc_fw_platform_requirement blobs_guc[] = { >> + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP) >> +}; >> + >> +static const struct uc_fw_platform_requirement blobs_huc[] = { >> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, >> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC) >> +}; >> + >> +static const struct fw_blobs_by_type >> blobs_all[INTEL_UC_FW_NUM_TYPES] = { >> + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) }, >> + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) }, >> +}; >> + >> static void >> __uc_fw_auto_select(struct drm_i915_private *i915, struct >> intel_uc_fw *uc_fw) >> { >> - static const struct uc_fw_platform_requirement blobs_guc[] = { >> - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, >> GUC_FW_BLOB_MMP) >> - }; >> - static const struct uc_fw_platform_requirement blobs_huc[] = { >> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, >> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC) >> - }; >> - static const struct fw_blobs_by_type >> blobs_all[INTEL_UC_FW_NUM_TYPES] = { >> - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) }, >> - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) }, >> - }; >> - static bool verified[INTEL_UC_FW_NUM_TYPES]; >> const struct uc_fw_platform_requirement *fw_blobs; >> enum intel_platform p = INTEL_INFO(i915)->platform; >> u32 fw_count; >> @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private >> *i915, struct intel_uc_fw *uc_fw) >> continue; >> if (uc_fw->file_selected.path) { >> + /* >> + * Continuing an earlier search after a found blob >> failed to load. >> + * Once the previously chosen path has been found, clear >> it out >> + * and let the search continue from there. >> + */ >> if (uc_fw->file_selected.path == blob->path) >> uc_fw->file_selected.path = NULL; >> @@ -306,78 +313,103 @@ __uc_fw_auto_select(struct drm_i915_private >> *i915, struct intel_uc_fw *uc_fw) >> /* Failed to find a match for the last attempt?! */ >> uc_fw->file_selected.path = NULL; >> } >> +} >> - /* make sure the list is ordered as expected */ >> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && >> !verified[uc_fw->type]) { >> - verified[uc_fw->type] = true; >> +static void validate_fw_table_type(struct drm_i915_private *i915, >> enum intel_uc_fw_type type) >> +{ >> + const struct uc_fw_platform_requirement *fw_blobs; >> + u32 fw_count; >> + int i; >> - for (i = 1; i < fw_count; i++) { >> - /* Next platform is good: */ >> - if (fw_blobs[i].p < fw_blobs[i - 1].p) >> - continue; >> + if (type >= ARRAY_SIZE(blobs_all)) { >> + drm_err(&i915->drm, "No blob array for %s\n", >> intel_uc_fw_type_repr(type)); >> + return; >> + } >> - /* Next platform revision is good: */ >> - if (fw_blobs[i].p == fw_blobs[i - 1].p && >> - fw_blobs[i].rev < fw_blobs[i - 1].rev) >> - continue; >> + fw_blobs = blobs_all[type].blobs; >> + fw_count = blobs_all[type].count; >> - /* Platform/revision must be in order: */ >> - if (fw_blobs[i].p != fw_blobs[i - 1].p || >> - fw_blobs[i].rev != fw_blobs[i - 1].rev) >> - goto bad; >> + if (!fw_count) >> + return; >> - /* Next major version is good: */ >> - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) >> - continue; >> + /* make sure the list is ordered as expected */ >> + for (i = 1; i < fw_count; i++) { >> + /* Next platform is good: */ >> + if (fw_blobs[i].p < fw_blobs[i - 1].p) >> + continue; >> - /* New must be before legacy: */ >> - if (!fw_blobs[i].blob.legacy && fw_blobs[i - >> 1].blob.legacy) >> - goto bad; >> + /* Next platform revision is good: */ >> + if (fw_blobs[i].p == fw_blobs[i - 1].p && >> + fw_blobs[i].rev < fw_blobs[i - 1].rev) >> + continue; >> - /* New to legacy also means 0.0 to X.Y (HuC), or X.0 >> to X.Y (GuC) */ >> - if (fw_blobs[i].blob.legacy && !fw_blobs[i - >> 1].blob.legacy) { >> - if (!fw_blobs[i - 1].blob.major) >> - continue; >> + /* Platform/revision must be in order: */ >> + if (fw_blobs[i].p != fw_blobs[i - 1].p || >> + fw_blobs[i].rev != fw_blobs[i - 1].rev) >> + goto bad; >> - if (fw_blobs[i].blob.major == fw_blobs[i - >> 1].blob.major) >> - continue; >> - } >> + /* Next major version is good: */ >> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) >> + continue; >> - /* Major versions must be in order: */ >> - if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major) >> - goto bad; >> + /* New must be before legacy: */ >> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy) >> + goto bad; >> - /* Next minor version is good: */ >> - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor) >> + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y >> (GuC) */ >> + if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) { >> + if (!fw_blobs[i - 1].blob.major) >> continue; >> - /* Minor versions must be in order: */ >> - if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor) >> - goto bad; >> - >> - /* Patch versions must be in order: */ >> - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch) >> + if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major) >> continue; >> + } >> + >> + /* Major versions must be in order: */ >> + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major) >> + goto bad; >> + >> + /* Next minor version is good: */ >> + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor) >> + continue; >> + >> + /* Minor versions must be in order: */ >> + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor) >> + goto bad; >> + >> + /* Patch versions must be in order: */ >> + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch) >> + continue; >> bad: >> - drm_err(&i915->drm, "Invalid %s blob order: %s r%u >> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", >> - intel_uc_fw_type_repr(uc_fw->type), >> - intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - >> 1].rev, >> - fw_blobs[i - 1].blob.legacy ? "L" : "v", >> - fw_blobs[i - 1].blob.major, >> - fw_blobs[i - 1].blob.minor, >> - fw_blobs[i - 1].blob.patch, >> - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev, >> - fw_blobs[i].blob.legacy ? "L" : "v", >> - fw_blobs[i].blob.major, >> - fw_blobs[i].blob.minor, >> - fw_blobs[i].blob.patch); >> - >> - uc_fw->file_selected.path = NULL; >> - } >> + drm_err(&i915->drm, "Invalid %s blob order: %s r%u >> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", >> + intel_uc_fw_type_repr(type), >> + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - >> 1].rev, >> + fw_blobs[i - 1].blob.legacy ? "L" : "v", >> + fw_blobs[i - 1].blob.major, >> + fw_blobs[i - 1].blob.minor, >> + fw_blobs[i - 1].blob.patch, >> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev, >> + fw_blobs[i].blob.legacy ? "L" : "v", >> + fw_blobs[i].blob.major, >> + fw_blobs[i].blob.minor, >> + fw_blobs[i].blob.patch); > > Confirming that this big diff was just an indent change was painful :/ Yeah, not a lot one can do about that. If you pull the patches to a local tree then you can run 'git show -b'. Can't really post that to patchwork though. > >> } >> } >> +static void validate_fw_table(struct drm_i915_private *i915) >> +{ >> + enum intel_uc_fw_type type; >> + static bool done; >> + >> + if (!IS_ENABLED(CONFIG_DRM_I915_SELFTEST) || done) >> + return; >> + done = true; >> + >> + for (type = 0; type < INTEL_UC_FW_NUM_TYPES; type++) >> + validate_fw_table_type(i915, type); >> +} >> + >> static const char *__override_guc_firmware_path(struct >> drm_i915_private *i915) >> { >> if (i915->params.enable_guc & ENABLE_GUC_MASK) >> @@ -432,6 +464,8 @@ void intel_uc_fw_init_early(struct intel_uc_fw >> *uc_fw, >> { >> struct drm_i915_private *i915 = ____uc_fw_to_gt(uc_fw, >> type)->i915; >> + validate_fw_table(i915); > > Personal preference: IMO since we're calling intel_uc_fw_init_early > per FW type it would've been cleaner to restrict validate_fw_table() > to a single blob type. This would have the negative side effect of > having to track the "done" status per FW type, so I can see it's not a > clean improvement. Not a blocker. That's step 2 - move the validate call out of the per blob type init sequence completely. Either just pushing it further up the call stack or moving it sideways to a selftest. First step was just to separate the code itself out as cleanly as possible. John. > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Daniele > >> + >> /* >> * we use FIRMWARE_UNINITIALIZED to detect checks against >> uc_fw->status >> * before we're looked at the HW caps to see if we have uc support >
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 6bb45d6b8da5f..c589782467265 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -233,20 +233,22 @@ struct fw_blobs_by_type { u32 count; }; +static const struct uc_fw_platform_requirement blobs_guc[] = { + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP) +}; + +static const struct uc_fw_platform_requirement blobs_huc[] = { + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC) +}; + +static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = { + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) }, + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) }, +}; + static void __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) { - static const struct uc_fw_platform_requirement blobs_guc[] = { - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP) - }; - static const struct uc_fw_platform_requirement blobs_huc[] = { - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC) - }; - static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = { - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) }, - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) }, - }; - static bool verified[INTEL_UC_FW_NUM_TYPES]; const struct uc_fw_platform_requirement *fw_blobs; enum intel_platform p = INTEL_INFO(i915)->platform; u32 fw_count; @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) continue; if (uc_fw->file_selected.path) { + /* + * Continuing an earlier search after a found blob failed to load. + * Once the previously chosen path has been found, clear it out + * and let the search continue from there. + */ if (uc_fw->file_selected.path == blob->path) uc_fw->file_selected.path = NULL; @@ -306,78 +313,103 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) /* Failed to find a match for the last attempt?! */ uc_fw->file_selected.path = NULL; } +} - /* make sure the list is ordered as expected */ - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified[uc_fw->type]) { - verified[uc_fw->type] = true; +static void validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_fw_type type) +{ + const struct uc_fw_platform_requirement *fw_blobs; + u32 fw_count; + int i; - for (i = 1; i < fw_count; i++) { - /* Next platform is good: */ - if (fw_blobs[i].p < fw_blobs[i - 1].p) - continue; + if (type >= ARRAY_SIZE(blobs_all)) { + drm_err(&i915->drm, "No blob array for %s\n", intel_uc_fw_type_repr(type)); + return; + } - /* Next platform revision is good: */ - if (fw_blobs[i].p == fw_blobs[i - 1].p && - fw_blobs[i].rev < fw_blobs[i - 1].rev) - continue; + fw_blobs = blobs_all[type].blobs; + fw_count = blobs_all[type].count; - /* Platform/revision must be in order: */ - if (fw_blobs[i].p != fw_blobs[i - 1].p || - fw_blobs[i].rev != fw_blobs[i - 1].rev) - goto bad; + if (!fw_count) + return; - /* Next major version is good: */ - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) - continue; + /* make sure the list is ordered as expected */ + for (i = 1; i < fw_count; i++) { + /* Next platform is good: */ + if (fw_blobs[i].p < fw_blobs[i - 1].p) + continue; - /* New must be before legacy: */ - if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy) - goto bad; + /* Next platform revision is good: */ + if (fw_blobs[i].p == fw_blobs[i - 1].p && + fw_blobs[i].rev < fw_blobs[i - 1].rev) + continue; - /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */ - if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) { - if (!fw_blobs[i - 1].blob.major) - continue; + /* Platform/revision must be in order: */ + if (fw_blobs[i].p != fw_blobs[i - 1].p || + fw_blobs[i].rev != fw_blobs[i - 1].rev) + goto bad; - if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major) - continue; - } + /* Next major version is good: */ + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) + continue; - /* Major versions must be in order: */ - if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major) - goto bad; + /* New must be before legacy: */ + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy) + goto bad; - /* Next minor version is good: */ - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor) + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */ + if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) { + if (!fw_blobs[i - 1].blob.major) continue; - /* Minor versions must be in order: */ - if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor) - goto bad; - - /* Patch versions must be in order: */ - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch) + if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major) continue; + } + + /* Major versions must be in order: */ + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major) + goto bad; + + /* Next minor version is good: */ + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor) + continue; + + /* Minor versions must be in order: */ + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor) + goto bad; + + /* Patch versions must be in order: */ + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch) + continue; bad: - drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", - intel_uc_fw_type_repr(uc_fw->type), - intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev, - fw_blobs[i - 1].blob.legacy ? "L" : "v", - fw_blobs[i - 1].blob.major, - fw_blobs[i - 1].blob.minor, - fw_blobs[i - 1].blob.patch, - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev, - fw_blobs[i].blob.legacy ? "L" : "v", - fw_blobs[i].blob.major, - fw_blobs[i].blob.minor, - fw_blobs[i].blob.patch); - - uc_fw->file_selected.path = NULL; - } + drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", + intel_uc_fw_type_repr(type), + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev, + fw_blobs[i - 1].blob.legacy ? "L" : "v", + fw_blobs[i - 1].blob.major, + fw_blobs[i - 1].blob.minor, + fw_blobs[i - 1].blob.patch, + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev, + fw_blobs[i].blob.legacy ? "L" : "v", + fw_blobs[i].blob.major, + fw_blobs[i].blob.minor, + fw_blobs[i].blob.patch); } } +static void validate_fw_table(struct drm_i915_private *i915) +{ + enum intel_uc_fw_type type; + static bool done; + + if (!IS_ENABLED(CONFIG_DRM_I915_SELFTEST) || done) + return; + done = true; + + for (type = 0; type < INTEL_UC_FW_NUM_TYPES; type++) + validate_fw_table_type(i915, type); +} + static const char *__override_guc_firmware_path(struct drm_i915_private *i915) { if (i915->params.enable_guc & ENABLE_GUC_MASK) @@ -432,6 +464,8 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, { struct drm_i915_private *i915 = ____uc_fw_to_gt(uc_fw, type)->i915; + validate_fw_table(i915); + /* * we use FIRMWARE_UNINITIALIZED to detect checks against uc_fw->status * before we're looked at the HW caps to see if we have uc support