On Fri, 2016-03-18 at 07:58 +0000, Chris Wilson wrote: > On Fri, Mar 18, 2016 at 08:50:37AM +0200, Imre Deak wrote: > > Atm, in case failure injection forces an error the subsequent "*ERROR* > > failed to init modeset" error message will make automated tests (CI) > > report this event as a breakage even though the event is expected. To > > fix this print the error message with debug log level in this case. > > > > While at it print the error message for any init failure and change it > > to > > """ > > Device initialization failed (errno) > > Please file a bug at https://bugs.freedesktop.org/enter_bug.cgi?product=DRI > > against DRM/Intel providing the dmesg log by booting with drm.debug=0xf > > """ > > and export a helper printing error messages using this same format. > > A follow-up patch will convert all uses of DRM_ERROR reporting a user > > facing problem to use this new helper instead. > > > > v2: > > - Include the problematic error message in the commit log, add a > > request to file an fdo bug to the message (Chris) > > v3: > > - Include the new error message too in the commit log, make the > > fdo link more precise and print part of the message with info log > > level (Chris) > > v4: (Chris) > > - Use dev_printk instead of DRM_ERROR/INFO and use NOTICE instead of > > INFO loglevel > > - Export a helper for printing user facing error messages > > v5: > > - Keep the DRM_ERROR message prefix used by piglit-igt/CI to filter > > relevant dmesg lines > > - Use dev_notice(), instead of dev_printk(KERN_NOTICE,...) > > > > CC: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 48 ++++++++++++++++++++++++++++++++++++----- > > drivers/gpu/drm/i915/i915_drv.h | 7 ++++++ > > 2 files changed, 50 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index 68592b0..096dc35 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -66,6 +66,44 @@ bool __i915_inject_load_failure(const char *func, int line) > > return false; > > } > > > > +#define FDO_BUG_URL "https://bugs.freedesktop.org/enter_bug.cgi?product=DRI"; > > +#define FDO_BUG_MSG "Please file a bug at " FDO_BUG_URL " against DRM/Intel " \ > > + "providing the dmesg log by booting with drm.debug=0xf" > > + > > +void > > +__i915_printk(struct drm_i915_private *dev_priv, const char *level, > > + const char *fmt, ...) > > +{ > > static int shown_bug_once; > > > + struct device *dev = dev_priv->dev->dev; > > + bool is_error = level[1] <= KERN_ERR[1]; > > + struct va_format vaf; > > + va_list args; > > + > > + va_start(args, fmt); > > + > > + vaf.fmt = fmt; > > + vaf.va = &args; > > + > > + dev_printk(level, dev, "[" DRM_NAME ":%ps] %pV", > > + __builtin_return_address(0), &vaf); > > Also on my wishlist is to > #undef DRM_NAME > #define DRM_NAME "drm/i915" > in i915_drv.h > > Don't tell me that would break CI! The current regex it uses on all messages with a loglevel <= KERN_NOTICE: (\[drm:|drm_|intel_|i915_) Probably could be changed to 'i915 0000:00:02.0:' if we converted all user facing DRM_ERRORs to use the new helper. Btw, according to your plan the rest non-user facing DRM_ERRORs would be printed with debug log level I assume, so isn't it a problem that they won't be treated as test failures by CI? > > + if (is_error) > > if (is_error && !shown_bug_once) { > > + dev_notice(dev, "%s", FDO_BUG_MSG); > shown_bug_once = 1; > } Ok, will respin it. --Imre > > With just showing the bug once, > Reviwed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
