| Message ID | 20190108101145.6701-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Downgrade scare message for unknwown HuC firmware | expand |
typo in title On Tue, 08 Jan 2019 11:11:45 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > If we haven't shipped and enabled firmware for a particular platform, > there is nothing the user can do about it. Don't scare the user with an > unactionable, unindentifiable warning! Did you mean: unidentifiable ? > > <6> [310.769452] i915 0000:00:02.0: GuC: No firmware known for this > platform! > <4> [310.769458] [drm] HuC: No firmware known for this platform! > > Unify both GuC/HuC messages to include the device for which we lack the > firmware, and provide the platform name as an aide-memoire. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_guc_fw.c | 4 +--- > drivers/gpu/drm/i915/intel_huc_fw.c | 3 +-- > drivers/gpu/drm/i915/intel_uc_fw.c | 8 ++++++++ > drivers/gpu/drm/i915/intel_uc_fw.h | 2 ++ > 4 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c > b/drivers/gpu/drm/i915/intel_guc_fw.c > index 4b437e05e2cd..b8364bd8ccf8 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fw.c > +++ b/drivers/gpu/drm/i915/intel_guc_fw.c > @@ -78,9 +78,7 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) > guc_fw->major_ver_wanted = KBL_FW_MAJOR; > guc_fw->minor_ver_wanted = KBL_FW_MINOR; > } else { > - dev_info(dev_priv->drm.dev, > - "%s: No firmware known for this platform!\n", > - intel_uc_fw_type_repr(guc_fw->type)); > + intel_uc_fw_unknown(dev_priv, guc_fw); Maybe instead of adding extra function that have to be explicitly called, just move message to intel_uc_fw_fetch() where we already check for fw path: if (!uc_fw->path) { dev_info(i915->drm.dev, "No %s firmware was defined for %s!\n", intel_uc_fw_type_repr(uc_fw->type), intel_platform_name(INTEL_INFO(i915)->platform)); return; } > } > } > diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c > b/drivers/gpu/drm/i915/intel_huc_fw.c > index 9612227b3c44..ba584321554c 100644 > --- a/drivers/gpu/drm/i915/intel_huc_fw.c > +++ b/drivers/gpu/drm/i915/intel_huc_fw.c > @@ -77,8 +77,7 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) > huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; > huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; > } else { > - DRM_WARN("%s: No firmware known for this platform!\n", > - intel_uc_fw_type_repr(huc_fw->type)); > + intel_uc_fw_unknown(dev_priv, huc_fw); > } > } > diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c > b/drivers/gpu/drm/i915/intel_uc_fw.c > index fd496416087c..7673e3a3d237 100644 > --- a/drivers/gpu/drm/i915/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/intel_uc_fw.c > @@ -310,3 +310,11 @@ void intel_uc_fw_dump(const struct intel_uc_fw > *uc_fw, struct drm_printer *p) > drm_printf(p, "\tRSA: offset %u, size %u\n", > uc_fw->rsa_offset, uc_fw->rsa_size); > } > + > +void intel_uc_fw_unknown(struct drm_i915_private *i915, struct > intel_uc_fw *fw) > +{ > + dev_info(i915->drm.dev, > + "%s: No firmware known for %s!\n", [nitpick] Maybe "No %s firmware known for %s!\n" or "No %s firmware was defined for %s!\n" to get nicer: i915 0000:00:02.0: No GuC firmware known for XXX! i915 0000:00:02.0: No GuC firmware was defined for XXX! > + intel_uc_fw_type_repr(fw->type), > + intel_platform_name(INTEL_INFO(i915)->platform)); > +} > diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h > b/drivers/gpu/drm/i915/intel_uc_fw.h > index 0e3bd580e267..4367f3d28c41 100644 > --- a/drivers/gpu/drm/i915/intel_uc_fw.h > +++ b/drivers/gpu/drm/i915/intel_uc_fw.h > @@ -150,4 +150,6 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, > void intel_uc_fw_fini(struct intel_uc_fw *uc_fw); > void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct > drm_printer *p); > +void intel_uc_fw_unknown(struct drm_i915_private *i915, struct > intel_uc_fw *fw); > + > #endif Thanks, Michal
Quoting Michal Wajdeczko (2019-01-08 12:48:24) > typo in title > > On Tue, 08 Jan 2019 11:11:45 +0100, Chris Wilson > <chris@chris-wilson.co.uk> wrote: > > > If we haven't shipped and enabled firmware for a particular platform, > > there is nothing the user can do about it. Don't scare the user with an > > unactionable, unindentifiable warning! > > Did you mean: unidentifiable ? Unindentable! > > > > > <6> [310.769452] i915 0000:00:02.0: GuC: No firmware known for this > > platform! > > <4> [310.769458] [drm] HuC: No firmware known for this platform! > > > > Unify both GuC/HuC messages to include the device for which we lack the > > firmware, and provide the platform name as an aide-memoire. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/intel_guc_fw.c | 4 +--- > > drivers/gpu/drm/i915/intel_huc_fw.c | 3 +-- > > drivers/gpu/drm/i915/intel_uc_fw.c | 8 ++++++++ > > drivers/gpu/drm/i915/intel_uc_fw.h | 2 ++ > > 4 files changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c > > b/drivers/gpu/drm/i915/intel_guc_fw.c > > index 4b437e05e2cd..b8364bd8ccf8 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_fw.c > > +++ b/drivers/gpu/drm/i915/intel_guc_fw.c > > @@ -78,9 +78,7 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) > > guc_fw->major_ver_wanted = KBL_FW_MAJOR; > > guc_fw->minor_ver_wanted = KBL_FW_MINOR; > > } else { > > - dev_info(dev_priv->drm.dev, > > - "%s: No firmware known for this platform!\n", > > - intel_uc_fw_type_repr(guc_fw->type)); > > + intel_uc_fw_unknown(dev_priv, guc_fw); > > Maybe instead of adding extra function that have to be explicitly called, > just move message to intel_uc_fw_fetch() where we already check for fw > path: If that suits; I presumed there was something inherent with why it was there :) -Chris
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c index 4b437e05e2cd..b8364bd8ccf8 100644 --- a/drivers/gpu/drm/i915/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/intel_guc_fw.c @@ -78,9 +78,7 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) guc_fw->major_ver_wanted = KBL_FW_MAJOR; guc_fw->minor_ver_wanted = KBL_FW_MINOR; } else { - dev_info(dev_priv->drm.dev, - "%s: No firmware known for this platform!\n", - intel_uc_fw_type_repr(guc_fw->type)); + intel_uc_fw_unknown(dev_priv, guc_fw); } } diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c b/drivers/gpu/drm/i915/intel_huc_fw.c index 9612227b3c44..ba584321554c 100644 --- a/drivers/gpu/drm/i915/intel_huc_fw.c +++ b/drivers/gpu/drm/i915/intel_huc_fw.c @@ -77,8 +77,7 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; } else { - DRM_WARN("%s: No firmware known for this platform!\n", - intel_uc_fw_type_repr(huc_fw->type)); + intel_uc_fw_unknown(dev_priv, huc_fw); } } diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c index fd496416087c..7673e3a3d237 100644 --- a/drivers/gpu/drm/i915/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/intel_uc_fw.c @@ -310,3 +310,11 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p) drm_printf(p, "\tRSA: offset %u, size %u\n", uc_fw->rsa_offset, uc_fw->rsa_size); } + +void intel_uc_fw_unknown(struct drm_i915_private *i915, struct intel_uc_fw *fw) +{ + dev_info(i915->drm.dev, + "%s: No firmware known for %s!\n", + intel_uc_fw_type_repr(fw->type), + intel_platform_name(INTEL_INFO(i915)->platform)); +} diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h index 0e3bd580e267..4367f3d28c41 100644 --- a/drivers/gpu/drm/i915/intel_uc_fw.h +++ b/drivers/gpu/drm/i915/intel_uc_fw.h @@ -150,4 +150,6 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, void intel_uc_fw_fini(struct intel_uc_fw *uc_fw); void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p); +void intel_uc_fw_unknown(struct drm_i915_private *i915, struct intel_uc_fw *fw); + #endif
If we haven't shipped and enabled firmware for a particular platform, there is nothing the user can do about it. Don't scare the user with an unactionable, unindentifiable warning! <6> [310.769452] i915 0000:00:02.0: GuC: No firmware known for this platform! <4> [310.769458] [drm] HuC: No firmware known for this platform! Unify both GuC/HuC messages to include the device for which we lack the firmware, and provide the platform name as an aide-memoire. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_guc_fw.c | 4 +--- drivers/gpu/drm/i915/intel_huc_fw.c | 3 +-- drivers/gpu/drm/i915/intel_uc_fw.c | 8 ++++++++ drivers/gpu/drm/i915/intel_uc_fw.h | 2 ++ 4 files changed, 12 insertions(+), 5 deletions(-)