| Message ID | 20230421011525.3282664-7-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improvements to uc firmare management | expand |
On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > If the DEBUG_GEM config option is set then escalate the 'unexpected > firmware version' message from a notice to an error. This will ensure > that the CI system treats such occurences as a failure and logs a bug > about it (or fails the pre-merge testing). > > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 34 ++++++++++++++---------- > 1 file changed, 20 insertions(+), 14 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 bc4011d55667c..c5b21d17ca437 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -17,6 +17,12 @@ > #include "i915_drv.h" > #include "i915_reg.h" > > +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) > +#define UNEXPECTED gt_err > +#else > +#define UNEXPECTED gt_notice I tried to find an alternative word that had the same number of characters as "gt_notice" so that we could avoid the indent in the diff, but nothing came to mind, so: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > +#endif > + > static inline struct intel_gt * > ____uc_fw_to_gt(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type) > { > @@ -828,10 +834,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) > if (uc_fw->file_wanted.ver.major && uc_fw->file_selected.ver.major) { > /* Check the file's major version was as it claimed */ > if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) { > - gt_notice(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, > - uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor, > - uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor); > + UNEXPECTED(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n", > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, > + uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor, > + uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor); > if (!intel_uc_fw_is_overridden(uc_fw)) { > err = -ENOEXEC; > goto fail; > @@ -849,16 +855,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) > /* Preserve the version that was really wanted */ > memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted)); > > - gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n", > - intel_uc_fw_type_repr(uc_fw->type), > - uc_fw->file_wanted.path, > - uc_fw->file_wanted.ver.major, > - uc_fw->file_wanted.ver.minor, > - uc_fw->file_wanted.ver.patch, > - uc_fw->file_selected.path, > - uc_fw->file_selected.ver.major, > - uc_fw->file_selected.ver.minor, > - uc_fw->file_selected.ver.patch); > + UNEXPECTED(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n", > + intel_uc_fw_type_repr(uc_fw->type), > + uc_fw->file_wanted.path, > + uc_fw->file_wanted.ver.major, > + uc_fw->file_wanted.ver.minor, > + uc_fw->file_wanted.ver.patch, > + uc_fw->file_selected.path, > + uc_fw->file_selected.ver.major, > + uc_fw->file_selected.ver.minor, > + uc_fw->file_selected.ver.patch); > gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n", > INTEL_UC_FIRMWARE_URL); > }
On 4/28/2023 17:19, Ceraolo Spurio, Daniele wrote: > On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote: >> From: John Harrison <John.C.Harrison@Intel.com> >> >> If the DEBUG_GEM config option is set then escalate the 'unexpected >> firmware version' message from a notice to an error. This will ensure >> that the CI system treats such occurences as a failure and logs a bug >> about it (or fails the pre-merge testing). >> >> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >> --- >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 34 ++++++++++++++---------- >> 1 file changed, 20 insertions(+), 14 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 bc4011d55667c..c5b21d17ca437 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> @@ -17,6 +17,12 @@ >> #include "i915_drv.h" >> #include "i915_reg.h" >> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) >> +#define UNEXPECTED gt_err >> +#else >> +#define UNEXPECTED gt_notice > > I tried to find an alternative word that had the same number of > characters as "gt_notice" so that we could avoid the indent in the > diff, but nothing came to mind, so: Yup, already went through that! And it didn't seem worth it to abbreviate by dropping a single random vowel. John. > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Daniele > >> +#endif >> + >> static inline struct intel_gt * >> ____uc_fw_to_gt(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type) >> { >> @@ -828,10 +834,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >> if (uc_fw->file_wanted.ver.major && >> uc_fw->file_selected.ver.major) { >> /* Check the file's major version was as it claimed */ >> if (uc_fw->file_selected.ver.major != >> uc_fw->file_wanted.ver.major) { >> - gt_notice(gt, "%s firmware %s: unexpected version: %u.%u >> != %u.%u\n", >> - intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, >> - uc_fw->file_selected.ver.major, >> uc_fw->file_selected.ver.minor, >> - uc_fw->file_wanted.ver.major, >> uc_fw->file_wanted.ver.minor); >> + UNEXPECTED(gt, "%s firmware %s: unexpected version: >> %u.%u != %u.%u\n", >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, >> + uc_fw->file_selected.ver.major, >> uc_fw->file_selected.ver.minor, >> + uc_fw->file_wanted.ver.major, >> uc_fw->file_wanted.ver.minor); >> if (!intel_uc_fw_is_overridden(uc_fw)) { >> err = -ENOEXEC; >> goto fail; >> @@ -849,16 +855,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >> /* Preserve the version that was really wanted */ >> memcpy(&uc_fw->file_wanted, &file_ideal, >> sizeof(uc_fw->file_wanted)); >> - gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, >> but only %s (%d.%d.%d) was found\n", >> - intel_uc_fw_type_repr(uc_fw->type), >> - uc_fw->file_wanted.path, >> - uc_fw->file_wanted.ver.major, >> - uc_fw->file_wanted.ver.minor, >> - uc_fw->file_wanted.ver.patch, >> - uc_fw->file_selected.path, >> - uc_fw->file_selected.ver.major, >> - uc_fw->file_selected.ver.minor, >> - uc_fw->file_selected.ver.patch); >> + UNEXPECTED(gt, "%s firmware %s (%d.%d.%d) is recommended, >> but only %s (%d.%d.%d) was found\n", >> + intel_uc_fw_type_repr(uc_fw->type), >> + uc_fw->file_wanted.path, >> + uc_fw->file_wanted.ver.major, >> + uc_fw->file_wanted.ver.minor, >> + uc_fw->file_wanted.ver.patch, >> + uc_fw->file_selected.path, >> + uc_fw->file_selected.ver.major, >> + uc_fw->file_selected.ver.minor, >> + uc_fw->file_selected.ver.patch); >> gt_info(gt, "Consider updating your linux-firmware pkg or >> downloading from %s\n", >> INTEL_UC_FIRMWARE_URL); >> } >
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 bc4011d55667c..c5b21d17ca437 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -17,6 +17,12 @@ #include "i915_drv.h" #include "i915_reg.h" +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) +#define UNEXPECTED gt_err +#else +#define UNEXPECTED gt_notice +#endif + static inline struct intel_gt * ____uc_fw_to_gt(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type) { @@ -828,10 +834,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) if (uc_fw->file_wanted.ver.major && uc_fw->file_selected.ver.major) { /* Check the file's major version was as it claimed */ if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) { - gt_notice(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, - uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor, - uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor); + UNEXPECTED(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n", + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, + uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor, + uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor); if (!intel_uc_fw_is_overridden(uc_fw)) { err = -ENOEXEC; goto fail; @@ -849,16 +855,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) /* Preserve the version that was really wanted */ memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted)); - gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n", - intel_uc_fw_type_repr(uc_fw->type), - uc_fw->file_wanted.path, - uc_fw->file_wanted.ver.major, - uc_fw->file_wanted.ver.minor, - uc_fw->file_wanted.ver.patch, - uc_fw->file_selected.path, - uc_fw->file_selected.ver.major, - uc_fw->file_selected.ver.minor, - uc_fw->file_selected.ver.patch); + UNEXPECTED(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n", + intel_uc_fw_type_repr(uc_fw->type), + uc_fw->file_wanted.path, + uc_fw->file_wanted.ver.major, + uc_fw->file_wanted.ver.minor, + uc_fw->file_wanted.ver.patch, + uc_fw->file_selected.path, + uc_fw->file_selected.ver.major, + uc_fw->file_selected.ver.minor, + uc_fw->file_selected.ver.patch); gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n", INTEL_UC_FIRMWARE_URL); }