[Intel-gfx] [PATCH v2] drm/i915: Tune down init error message due to failure injection

[Chris Wilson]

On Thu, Mar 17, 2016 at 10:53:08PM +0200, Imre Deak wrote:
> On Thu, 2016-03-17 at 20:44 +0000, Chris Wilson wrote:
> > On Thu, Mar 17, 2016 at 09:50:19PM +0200, Imre Deak wrote:
> > > On Thu, 2016-03-17 at 19:41 +0000, Chris Wilson wrote:
> > > > On Thu, Mar 17, 2016 at 06:08:05PM +0200, Imre Deak wrote:
> > > > > On Thu, 2016-03-17 at 15:55 +0000, Chris Wilson wrote:
> > > > > > 80cols rules still apply to messages :)
> > > > > > 
> > > > > > "Device initialization failed (%d). "
> > > > > > "Please file a bug at https://bugs.freedesktop.org/enter_bug.
> > > > > > cgi
> > > > > > "
> > > > > > "against DRI/DRM/Intel providing the dmesg log by booting "
> > > > > > "with drm.debug=0xf\n",
> > > > > 
> > > > > I thought for strings emitted to dmesg it's discouraged, since
> > > > > you
> > > > > can't easily grep then. But I can make the link more precise.
> > > > 
> > > > Hmm, we haven't abided by that. I don't mind really :)
> > > 
> > > Ok, will follow this rule then.
> > > 
> > > > > > I would personally make i915_load_error() a proper function
> > > > > > and
> > > > > > add
> > > > > > the "Please..." output there at a lower logging level than
> > > > > > ERROR.
> > > > > 
> > > > > Using a function is not straightforward since there is no
> > > > > vprintf
> > > > > like
> > > > > interface for DRM_ERROR. But I can move that part of the
> > > > > message to
> > > > > macro.
> > > > 
> > > > In that case, I'm happy enough with ditching the DRM_ERROR here
> > > > and
> > > > go
> > > > with dev_err(). Another nail in the DRM_ERROR coffin.
> > > 
> > > Hm, there doesn't seem to be vprintf like dev_err either. And
> > > dev_dbg
> > > depends on CONFIG_DEBUG while DRM_DEBUG doesn't.
> > 
> > It uses dev_printk_emit(KERN_ERR, dev, fmt, va_list) underneath.
> > 
> > Does that help?
> 
> It uses
> 
> __dev_printk(kern_level, dev, &vaf);
> 
> which is not exported. This one in turn translates to
> 
> dev_printk_emit(KERN_ERR[0] - '0', dev, "%s %s: %pV",
>                 dev_driver_string(dev), dev_name(dev), vaf);
> 
> which we probably don't want to hard-code here.

dev_vprintk_emit() is exported. I am not sure why dev_vprintk() would
not be...

I agree it is a nuisance. So just food for thought.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre