diff mbox

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

Message ID 1471021254-2563-2-git-send-email-seanpaul@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sean Paul Aug. 12, 2016, 5 p.m. UTC 
This patch consolodates 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>
---
 drivers/gpu/drm/drm_drv.c |  31 +++++------
 include/drm/drmP.h        | 133 ++++++++++++++++++++++++----------------------
 2 files changed, 82 insertions(+), 82 deletions(-)

Comments

Chris Wilson Aug. 12, 2016, 5:13 p.m. UTC | #1 
On Fri, Aug 12, 2016 at 01:00:53PM -0400, Sean Paul wrote:
> This patch consolodates 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.

> +void drm_log(const struct device *dev, const char *level, unsigned int category,
> +	     const char *function_name, const char *prefix,
> +	     const char *format, ...)
>  {
> +	if (dev)
> +		printk("%s%s [" DRM_NAME ":%s]%s %pV", level,
> +		       dev_name(dev), function_name, prefix, &vaf);
> +	else
> +		printk("%s[" DRM_NAME ":%s]%s %pV", level, function_name,
> +		       prefix, &vaf);

My hope was that we would migrate towards dev_printk so that our log
messages would have better conformity, especially between our messages
and those printed by subsystems on our behalf (using our struct device).
-Chris
Sean Paul Aug. 12, 2016, 5:23 p.m. UTC | #2 
On Fri, Aug 12, 2016 at 1:13 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Aug 12, 2016 at 01:00:53PM -0400, Sean Paul wrote:
>> This patch consolodates 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.
>
>> +void drm_log(const struct device *dev, const char *level, unsigned int category,
>> +          const char *function_name, const char *prefix,
>> +          const char *format, ...)
>>  {
>> +     if (dev)
>> +             printk("%s%s [" DRM_NAME ":%s]%s %pV", level,
>> +                    dev_name(dev), function_name, prefix, &vaf);
>> +     else
>> +             printk("%s[" DRM_NAME ":%s]%s %pV", level, function_name,
>> +                    prefix, &vaf);
>
> My hope was that we would migrate towards dev_printk so that our log
> messages would have better conformity, especially between our messages
> and those printed by subsystems on our behalf (using our struct device).

Yep, that makes sense, v2 incoming.

Sean

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 57ce973..e8595f0 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,
+	     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)
+		printk("%s%s [" DRM_NAME ":%s]%s %pV", level,
+		       dev_name(dev), function_name, prefix, &vaf);
+	else
+		printk("%s[" DRM_NAME ":%s]%s %pV", level, function_name,
+		       prefix, &vaf);
 
 	va_end(args);
 }
-EXPORT_SYMBOL(drm_ut_debug_printk);
+EXPORT_SYMBOL(drm_log);
 
 /*
  * DRM Minors
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index f8e87fd..9a6ace2 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -127,6 +127,7 @@  struct dma_buf_attachment;
  * run-time by echoing the debug value in its sysfs node:
  *   # echo 0xf > /sys/module/drm/parameters/debug
  */
+#define DRM_UT_NONE		0x00
 #define DRM_UT_CORE 		0x01
 #define DRM_UT_DRIVER		0x02
 #define DRM_UT_KMS		0x04
@@ -134,11 +135,10 @@  struct dma_buf_attachment;
 #define DRM_UT_ATOMIC		0x10
 #define DRM_UT_VBL		0x20
 
-extern __printf(2, 3)
-void drm_ut_debug_printk(const char *function_name,
-			 const char *format, ...);
-extern __printf(1, 2)
-void drm_err(const char *format, ...);
+extern __printf(6, 7)
+void drm_log(const struct device *dev, const char *level, unsigned int category,
+	     const char *function_name, const char *prefix,
+	     const char *format, ...);
 
 /***********************************************************************/
 /** \name DRM template customization defaults */
@@ -169,8 +169,10 @@  void drm_err(const char *format, ...);
  * \param fmt printf() like format string.
  * \param arg arguments
  */
-#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__)
 
 /**
  * Rate limited error output.  Like DRM_ERROR() but won't flood the log.
@@ -178,21 +180,31 @@  void drm_err(const char *format, ...);
  * \param fmt printf() like format string.
  * \param arg arguments
  */
-#define DRM_ERROR_RATELIMITED(fmt, ...)				\
+#define DRM_DEV_ERROR_RATELIMITED(dev, fmt, ...)			\
 ({									\
 	static DEFINE_RATELIMIT_STATE(_rs,				\
 				      DEFAULT_RATELIMIT_INTERVAL,	\
 				      DEFAULT_RATELIMIT_BURST);		\
 									\
 	if (__ratelimit(&_rs))						\
-		drm_err(fmt, ##__VA_ARGS__);				\
+		DRM_DEV_ERROR(dev, fmt, ##__VA_ARGS__);			\
 })
+#define DRM_ERROR_RATELIMITED(fmt, ...)			\
+	DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
 
-#define DRM_INFO(fmt, ...)				\
-	printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
+#define DRM_DEV_INFO(dev, fmt, ...)			\
+	drm_log(dev, KERN_INFO, DRM_UT_NONE, __func__, "", fmt, ##__VA_ARGS__)
+#define DRM_INFO(fmt, ...) DRM_DEV_INFO(NULL, fmt, ##__VA_ARGS__)
 
-#define DRM_INFO_ONCE(fmt, ...)				\
-	printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
+#define DRM_DEV_INFO_ONCE(dev, fmt, ...)				\
+({									\
+	static bool __print_once __read_mostly;				\
+	if (!__print_once) {						\
+		__print_once = true;					\
+		DRM_DEV_INFO(dev, fmt, ##__VA_ARGS__);			\
+	}								\
+})
+#define DRM_INFO_ONCE(fmt, ...) DRM_DEV_INFO_ONCE(NULL, fmt, ##__VA_ARGS__)
 
 /**
  * Debug output.
@@ -200,52 +212,39 @@  void drm_err(const char *format, ...);
  * \param fmt printf() like format string.
  * \param arg arguments
  */
-#define DRM_DEBUG(fmt, args...)						\
-	do {								\
-		if (unlikely(drm_debug & DRM_UT_CORE))			\
-			drm_ut_debug_printk(__func__, fmt, ##args);	\
-	} while (0)
-
-#define DRM_DEBUG_DRIVER(fmt, args...)					\
-	do {								\
-		if (unlikely(drm_debug & DRM_UT_DRIVER))		\
-			drm_ut_debug_printk(__func__, fmt, ##args);	\
-	} while (0)
-#define DRM_DEBUG_KMS(fmt, args...)					\
-	do {								\
-		if (unlikely(drm_debug & DRM_UT_KMS))			\
-			drm_ut_debug_printk(__func__, fmt, ##args);	\
-	} while (0)
-#define DRM_DEBUG_PRIME(fmt, args...)					\
-	do {								\
-		if (unlikely(drm_debug & DRM_UT_PRIME))			\
-			drm_ut_debug_printk(__func__, fmt, ##args);	\
-	} while (0)
-#define DRM_DEBUG_ATOMIC(fmt, args...)					\
-	do {								\
-		if (unlikely(drm_debug & DRM_UT_ATOMIC))		\
-			drm_ut_debug_printk(__func__, fmt, ##args);	\
-	} while (0)
-#define DRM_DEBUG_VBL(fmt, args...)					\
-	do {								\
-		if (unlikely(drm_debug & DRM_UT_VBL))			\
-			drm_ut_debug_printk(__func__, fmt, ##args);	\
-	} while (0)
-
-#define _DRM_DEFINE_DEBUG_RATELIMITED(level, fmt, args...)		\
-	do {								\
-		if (unlikely(drm_debug & DRM_UT_ ## level)) {		\
-			static DEFINE_RATELIMIT_STATE(			\
-				_rs,					\
-				DEFAULT_RATELIMIT_INTERVAL,		\
-				DEFAULT_RATELIMIT_BURST);		\
-									\
-			if (__ratelimit(&_rs)) {			\
-				drm_ut_debug_printk(__func__, fmt,	\
-						    ##args);		\
-			}						\
-		}							\
-	} while (0)
+#define DRM_DEV_DEBUG(dev, fmt, args...)	\
+	drm_log(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt, ##args)
+#define DRM_DEBUG(fmt, args...)	DRM_DEV_DEBUG(NULL, fmt, ##args)
+
+#define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...)	\
+	drm_log(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "", fmt, ##args)
+#define DRM_DEBUG_DRIVER(fmt, args...) DRM_DEV_DEBUG_DRIVER(NULL, fmt, ##args)
+
+#define DRM_DEV_DEBUG_KMS(dev, fmt, args...)	\
+	drm_log(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt, ##args)
+#define DRM_DEBUG_KMS(fmt, args...) DRM_DEV_DEBUG_KMS(NULL, fmt, ##args)
+
+#define DRM_DEV_DEBUG_PRIME(dev, fmt, args...)	\
+	drm_log(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "", fmt, ##args)
+#define DRM_DEBUG_PRIME(fmt, args...) DRM_DEV_DEBUG_PRIME(NULL, fmt, ##args)
+
+#define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...)	\
+	drm_log(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "", fmt, ##args)
+#define DRM_DEBUG_ATOMIC(fmt, args...) DRM_DEV_DEBUG_ATOMIC(NULL, fmt, ##args)
+
+#define DRM_DEV_DEBUG_VBL(dev, fmt, args...)	\
+	drm_log(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt, ##args)
+#define DRM_DEBUG_VBL(fmt, args...) DRM_DEV_DEBUG_VBL(NULL, fmt, ##args)
+
+#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)	\
+({									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		drm_log(dev, KERN_DEBUG, DRM_UT_ ## level, __func__,	\
+			"", fmt, ##args);				\
+})
 
 /**
  * Rate limited debug output. Like DRM_DEBUG() but won't flood the log.
@@ -253,14 +252,22 @@  void drm_err(const char *format, ...);
  * \param fmt printf() like format string.
  * \param arg arguments
  */
+#define DRM_DEV_DEBUG_RATELIMITED(dev, fmt, args...)			\
+	DEV__DRM_DEFINE_DEBUG_RATELIMITED(dev, CORE, fmt, ##args)
 #define DRM_DEBUG_RATELIMITED(fmt, args...)				\
-	_DRM_DEFINE_DEBUG_RATELIMITED(CORE, fmt, ##args)
+	DRM_DEV_DEBUG_RATELIMITED(NULL, fmt, ##args)
+#define DRM_DEV_DEBUG_DRIVER_RATELIMITED(dev, fmt, args...)		\
+	_DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, DRIVER, fmt, ##args)
 #define DRM_DEBUG_DRIVER_RATELIMITED(fmt, args...)			\
-	_DRM_DEFINE_DEBUG_RATELIMITED(DRIVER, fmt, ##args)
+	DRM_DEV_DEBUG_DRIVER_RATELIMITED(NULL, fmt, ##args)
+#define DRM_DEV_DEBUG_KMS_RATELIMITED(dev, fmt, args...)		\
+	_DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, KMS, fmt, ##args)
 #define DRM_DEBUG_KMS_RATELIMITED(fmt, args...)				\
-	_DRM_DEFINE_DEBUG_RATELIMITED(KMS, fmt, ##args)
+	DRM_DEV_DEBUG_KMS_RATELIMITED(NULL, fmt, ##args)
+#define DRM_DEV_DEBUG_PRIME_RATELIMITED(dev, fmt, args...)		\
+	_DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, PRIME, fmt, ##args)
 #define DRM_DEBUG_PRIME_RATELIMITED(fmt, args...)			\
-	_DRM_DEFINE_DEBUG_RATELIMITED(PRIME, fmt, ##args)
+	DRM_DEV_DEBUG_PRIME_RATELIMITED(NULL, fmt, ##args)
 
 /*@}*/