Re: [PATCH v2 1/2] drm: Introduce DRM_DEV_* log messages

From: Sean Paul <seanpaul@chromium.org>
To: "Chris Wilson" <chris@chris-wilson.co.uk>,
	"Sean Paul" <seanpaul@chromium.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	linux-rockchip@lists.infradead.org, 姚智情 <mark.yao@rock-chips.com>
Subject: Re: [PATCH v2 1/2] drm: Introduce DRM_DEV_* log messages
Date: Fri, 12 Aug 2016 15:04:37 -0400	[thread overview]
Message-ID: <CAOw6vb+J-XaKGkEVokhzch7Qi=ZHKbKOoaVn_WKW1j9O7KAGwA@mail.gmail.com> (raw)
In-Reply-To: <20160812183938.GH15011@nuc-i3427.alporthouse.com>

On Fri, Aug 12, 2016 at 2:39 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 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@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?


uncompressed vmlinux is 8286 bytes larger on my i915 build, 34832 on exynos

bzImage on i915 build is 2912 bytes larger and zImage on exynos is
6752 bytes larger

Sean

>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel