On Fri, Aug 12, 2016 at 01:30:00PM -0400, Sean Paul wrote:
> This patch consolidates all the various log functions/macros into
> one uber function, drm_log. It also introduces some new DRM_DEV_*
> variants that print the device name to delineate multiple devices
> of the same type.
> 
> Signed-off-by: Sean Paul <seanpaul at chromium.org>
> ---
> 
> Changes in v2:
>       - Use dev_printk for the dev variant (Chris Wilson)
> 
> 
>  drivers/gpu/drm/drm_drv.c |  31 +++++------
>  include/drm/drmP.h        | 133 
> ++++++++++++++++++++++++----------------------
>  2 files changed, 82 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 57ce973..edd3291 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -63,37 +63,30 @@ static struct idr drm_minors_idr;
>  
>  static struct dentry *drm_debugfs_root;
>  
> -void drm_err(const char *format, ...)
> +void drm_log(const struct device *dev, const char *level, unsigned int 
> category,

I would have called this drm_printk() to match the function it wraps.

> +          const char *function_name, const char *prefix,
> +          const char *format, ...)
>  {
>       struct va_format vaf;
>       va_list args;
>  
> -     va_start(args, format);
> -
> -     vaf.fmt = format;
> -     vaf.va = &args;
> -
> -     printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
> -            __builtin_return_address(0), &vaf);
> -
> -     va_end(args);
> -}
> -EXPORT_SYMBOL(drm_err);
> -
> -void drm_ut_debug_printk(const char *function_name, const char *format, ...)
> -{
> -     struct va_format vaf;
> -     va_list args;
> +     if (category != DRM_UT_NONE && !(drm_debug & category))
> +             return;
>  
>       va_start(args, format);
>       vaf.fmt = format;
>       vaf.va = &args;
>  
> -     printk(KERN_DEBUG "[" DRM_NAME ":%s] %pV", function_name, &vaf);
> +     if (dev)
> +             dev_printk(level, dev, "[" DRM_NAME ":%s]%s %pV", function_name,
> +                        prefix, &vaf);
> +     else
> +             printk("%s[" DRM_NAME ":%s]%s %pV", level, function_name,
> +                    prefix, &vaf);

lgtm.

> -#define DRM_ERROR(fmt, ...)                          \
> -     drm_err(fmt, ##__VA_ARGS__)
> +#define DRM_DEV_ERROR(dev, fmt, ...)                                 \
> +     drm_log(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*", fmt,  \
> +             ##__VA_ARGS__)
> +#define DRM_ERROR(fmt, ...) DRM_DEV_ERROR(NULL, fmt, ##__VA_ARGS__)

And these look like a reasonable solution given the constraints.

Out of curiosity, how much did the kernel build grow by adding a NULL
parameter everywhere?

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Reply via email to