Re: [PATCH] drm/i915: Introduce i915_dbg macro

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2016-01-25 at 18:57 +0000, Dave Gordon wrote:
> On 25/01/16 18:17, Daniel Vetter wrote:
> > On Fri, Jan 22, 2016 at 05:54:15PM +0530, akash.goel@intel.com
> > wrote:
> > > From: Akash Goel <akash.goel@intel.com>
> > > 
> > > Added a new macro i915_dbg, which is a wrapper over dev_dbg
> > > macro.
> > > dev_dbg allows use of dynamic debug framework, so offers a number
> > > of advantages over DRM_DEBUG to debug user space startup issues.
> > > Like provides more fine grain control by allowing to
> > > enable/disable
> > > certain debug messages of interest on the fly, also allows
> > > filtering
> > > of debug messages based on pid.
> > > 
> > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/i915_drv.h | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index bc7164f..749513f 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2456,6 +2456,7 @@ struct drm_i915_cmd_table {
> > >   		BUILD_BUG(); \
> > >   	__p; \
> > >   })
> > > +#define i915_dbg(DEV, args...)	dev_dbg(__I915__(DEV)-
> > > >dev->dev, ##args)
> 
> I915_DBG(...) ?
> 
> It's conventional that macros should be UPPERCASE.
> 
> Especially when some config options may mean that the code
> disappears 
> entirely, so you have to be sure not to use arguments with side-
> effects!
> 

Slight correction here (for future), from Kernel Coding Style
documentation;

"CAPITALIZED macro names are appreciated but macros resembling
functions may be named in lower case."

And looking at "include/linux/device.h", dev_dbg definition is a macro
too, like almost all the printing functions. I'd rather see it as
i915_dbg. Arguments with side effects can be handled nicely as can be
seen.

We really should increase the priority of modernizing the debugging
infrastructure for i915 (and as a dependency for DRM as Daniel hoped).

Regards, Joonas

> .Dave.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx