[RFC 0/5] DRM logging tidy

[RFC 0/5] DRM logging tidy

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I wasn't here at the beginnings of DRM so I might have gotten this wrong,
however the existance of DRM_NAME suggested to me that the intention was to
allow individual drivers to override it and get appropriate prefixes in their
log messages.

I can't see that any driver is using it like that but I still thought it would
be neat to do that. That way we could have our log messages look more
obviously ours. For example after this series we have:

 [i915] Memory usable by graphics device = 4096M
 [i915] VT-d active for gfx access
 [i915] Replacing VGA console driver
 [i915] ACPI BIOS requests an excessive sleep of 20000 ms, using 1500 ms instead
 [i915] Finished loading DMC firmware i915/skl_dmc_ver1_26.bin (v1.26)
 [i915] Disabling framebuffer compression (FBC) to prevent screen flicker with VT-d enabled
 [i915] GuC firmware load skipped
 [i915] Initialized i915 1.6.0 20161205 for 0000:00:02.0 on minor 0
 [i915] DRM_I915_DEBUG enabled
 [i915] DRM_I915_DEBUG_GEM enabled
 [i915] RC6 on

Previously all that was prefixed with "[drm]" which was OK but I think the
above is even better.

Also to consider is that recent drm_printk work has removed (it hardcoded)
DRM_NAME from DRM_ERROR and DRM_DEBUG macros, while leaving it with the rest
(DRM_INFO, NOTE and WARNING) creating a bit of a inconsistency.

This series also makes all the logging macros use drm_printk, but also
makes DRM_NAME passed in from the macro wrappers in all cases. So drivers
can override it regardless of the log level.

And finally, the series also removes a bit of redundant data from the debug
messages effectively converting this:

 [drm:edp_panel_off [i915]] Wait for panel power off time

Into this:

 [edp_panel_off [i915]] Wait for panel power off time

Which still has all the data in it.

Tvrtko Ursulin (5):
  drm/i915: Give our log messages our name
  drm: Respect driver set DRM_NAME in drm_printk
  drm: Respect driver set DRM_NAME in drm_dev_printk
  drm: Use drm_printk for all logging macros
  drm: Do not log driver prefix in debug messages

 drivers/gpu/drm/drm_drv.c       | 39 +++++++++++------
 drivers/gpu/drm/i915/i915_drv.c |  3 +-
 include/drm/drmP.h              | 94 ++++++++++++++++++++++++-----------------
 include/drm/drm_drv.h           | 11 ++---
 include/uapi/drm/i915_drm.h     |  3 ++
 5 files changed, 92 insertions(+), 58 deletions(-)

-- 
2.7.4

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

[RFC 1/5] drm/i915: Give our log messages our name

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Define DRM_NAME to i915 so that the log messages we output change from:

 [drm] RC6 on

to:

 [i915] RC6 on

Since I wasn't around in the beginning of DRM I wonder whether that
was the intended purpose of DRM_NAME or if it was something else.

Today it looks a bit messy with it being used by both macros and core
DRM helpers, like drm_dev_printk and drm_printk, meaning that this
change only affects some and not all log messages.

Following patches will try to correct that as well.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 include/uapi/drm/i915_drm.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index da32c2f6c3f9..b777a6b7b3dc 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -29,6 +29,9 @@
 
 #include "drm.h"
 
+#undef DRM_NAME
+#define DRM_NAME "i915"
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
-- 
2.7.4

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

[RFC 2/5] drm: Respect driver set DRM_NAME in drm_printk

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Instead of logging with the DRM core name, pass in the name explicitly
in order to allow individual drivers to have all their log messages
prefixed with their own name.

For instance i915, instead of:

 [drm:edp_panel_off [i915]] Wait for panel power off time

would now log debug messages as:

 [i915:edp_panel_off [i915]] Wait for panel power off time

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/drm_drv.c |  6 +++---
 include/drm/drmP.h        | 14 +++++++-------
 include/drm/drm_drv.h     |  4 ++--
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index f74b7d06ec01..f6b674b03db9 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -92,7 +92,7 @@ void drm_dev_printk(const struct device *dev, const char *level,
 EXPORT_SYMBOL(drm_dev_printk);
 
 void drm_printk(const char *level, unsigned int category,
-		const char *format, ...)
+		const char *driver, const char *format, ...)
 {
 	struct va_format vaf;
 	va_list args;
@@ -104,8 +104,8 @@ void drm_printk(const char *level, unsigned int category,
 	vaf.fmt = format;
 	vaf.va = &args;
 
-	printk("%s" "[" DRM_NAME ":%ps]%s %pV",
-	       level, __builtin_return_address(0),
+	printk("%s[%s:%ps]%s %pV",
+	       level, driver, __builtin_return_address(0),
 	       strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
 
 	va_end(args);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index a9cfd33c7b1a..eedfbe51795a 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -176,7 +176,7 @@ struct dma_buf_attachment;
 	drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
 		       fmt, ##__VA_ARGS__)
 #define DRM_ERROR(fmt, ...)						\
-	drm_printk(KERN_ERR, DRM_UT_NONE, fmt,	##__VA_ARGS__)
+	drm_printk(KERN_ERR, DRM_UT_NONE, DRM_NAME, fmt, ##__VA_ARGS__)
 
 /**
  * Rate limited error output.  Like DRM_ERROR() but won't flood the log.
@@ -219,37 +219,37 @@ struct dma_buf_attachment;
 	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt,	\
 		       ##args)
 #define DRM_DEBUG(fmt, ...)						\
-	drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
+	drm_printk(KERN_DEBUG, DRM_UT_CORE, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...)				\
 	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "",	\
 		       fmt, ##args)
 #define DRM_DEBUG_DRIVER(fmt, ...)					\
-	drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
+	drm_printk(KERN_DEBUG, DRM_UT_DRIVER, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_KMS(dev, fmt, args...)				\
 	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt,	\
 		       ##args)
 #define DRM_DEBUG_KMS(fmt, ...)					\
-	drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
+	drm_printk(KERN_DEBUG, DRM_UT_KMS, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_PRIME(dev, fmt, args...)				\
 	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "",	\
 		       fmt, ##args)
 #define DRM_DEBUG_PRIME(fmt, ...)					\
-	drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
+	drm_printk(KERN_DEBUG, DRM_UT_PRIME, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...)				\
 	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "",	\
 		       fmt, ##args)
 #define DRM_DEBUG_ATOMIC(fmt, ...)					\
-	drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
+	drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_VBL(dev, fmt, args...)				\
 	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt,	\
 		       ##args)
 #define DRM_DEBUG_VBL(fmt, ...)					\
-	drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
+	drm_printk(KERN_DEBUG, DRM_UT_VBL, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)	\
 ({									\
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index c4fc49583dc0..0ad6cde5757b 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -411,9 +411,9 @@ extern __printf(6, 7)
 void drm_dev_printk(const struct device *dev, const char *level,
 		    unsigned int category, const char *function_name,
 		    const char *prefix, const char *format, ...);
-extern __printf(3, 4)
+extern __printf(4, 5)
 void drm_printk(const char *level, unsigned int category,
-		const char *format, ...);
+		const char *driver, const char *format, ...);
 extern unsigned int drm_debug;
 
 int drm_dev_init(struct drm_device *dev,
-- 
2.7.4

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

[RFC 3/5] drm: Respect driver set DRM_NAME in drm_dev_printk

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Same as the previous patch did for drm_printk, allow for the
logging macros to pass in the driver set DRM_NAME.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/drm_drv.c       | 14 +++++++-------
 drivers/gpu/drm/i915/i915_drv.c |  3 ++-
 include/drm/drmP.h              | 36 ++++++++++++++++++------------------
 include/drm/drm_drv.h           |  7 ++++---
 4 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index f6b674b03db9..a2f4eb4509b9 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -65,11 +65,10 @@ static struct idr drm_minors_idr;
 
 static struct dentry *drm_debugfs_root;
 
-#define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
-
 void drm_dev_printk(const struct device *dev, const char *level,
-		    unsigned int category, const char *function_name,
-		    const char *prefix, const char *format, ...)
+		    unsigned int category, const char *driver,
+		    const char *function_name, const char *prefix,
+		    const char *format, ...)
 {
 	struct va_format vaf;
 	va_list args;
@@ -82,10 +81,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
 	vaf.va = &args;
 
 	if (dev)
-		dev_printk(level, dev, DRM_PRINTK_FMT, function_name, prefix,
-			   &vaf);
+		dev_printk(level, dev, "[%s:%ps]%s %pV",
+			   driver, function_name, prefix, &vaf);
 	else
-		printk("%s" DRM_PRINTK_FMT, level, function_name, prefix, &vaf);
+		printk("%s[%s:%ps]%s %pV",
+		       level, driver, function_name, prefix, &vaf);
 
 	va_end(args);
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 78b2d03bf808..77d82fce371d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -101,7 +101,8 @@ i915_load_error(struct drm_i915_private *dev_priv, const char *fmt, ...)
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	drm_dev_printk(kdev, level, DRM_UT_DRIVER, __func__, "", fmt, &vaf);
+	drm_dev_printk(kdev, level, DRM_UT_DRIVER, DRM_NAME, __func__, "",
+		       fmt, &vaf);
 
 	va_end(args);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index eedfbe51795a..8661045ffaf1 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -173,8 +173,8 @@ struct dma_buf_attachment;
  * \param arg arguments
  */
 #define DRM_DEV_ERROR(dev, fmt, ...)					\
-	drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
-		       fmt, ##__VA_ARGS__)
+	drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, DRM_NAME, __func__,	\
+		       " *ERROR*", fmt, ##__VA_ARGS__)
 #define DRM_ERROR(fmt, ...)						\
 	drm_printk(KERN_ERR, DRM_UT_NONE, DRM_NAME, fmt, ##__VA_ARGS__)
 
@@ -197,8 +197,8 @@ struct dma_buf_attachment;
 	DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_INFO(dev, fmt, ...)					\
-	drm_dev_printk(dev, KERN_INFO, DRM_UT_NONE, __func__, "", fmt,	\
-		       ##__VA_ARGS__)
+	drm_dev_printk(dev, KERN_INFO, DRM_UT_NONE, DRM_NAME, __func__,	\
+		       "", fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_INFO_ONCE(dev, fmt, ...)				\
 ({									\
@@ -216,38 +216,38 @@ struct dma_buf_attachment;
  * \param arg arguments
  */
 #define DRM_DEV_DEBUG(dev, fmt, args...)				\
-	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt,	\
-		       ##args)
+	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, DRM_NAME, __func__,\
+		       "", fmt, ##args)
 #define DRM_DEBUG(fmt, ...)						\
 	drm_printk(KERN_DEBUG, DRM_UT_CORE, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...)				\
-	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "",	\
-		       fmt, ##args)
+	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, DRM_NAME,	\
+		       __func__, "", fmt, ##args)
 #define DRM_DEBUG_DRIVER(fmt, ...)					\
 	drm_printk(KERN_DEBUG, DRM_UT_DRIVER, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_KMS(dev, fmt, args...)				\
-	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt,	\
-		       ##args)
-#define DRM_DEBUG_KMS(fmt, ...)					\
+	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, DRM_NAME, __func__,	\
+		       "", fmt, ##args)
+#define DRM_DEBUG_KMS(fmt, ...)						\
 	drm_printk(KERN_DEBUG, DRM_UT_KMS, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_PRIME(dev, fmt, args...)				\
-	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "",	\
-		       fmt, ##args)
+	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, DRM_NAME,		\
+		       __func__, "", fmt, ##args)
 #define DRM_DEBUG_PRIME(fmt, ...)					\
 	drm_printk(KERN_DEBUG, DRM_UT_PRIME, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...)				\
-	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "",	\
-		       fmt, ##args)
+	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, DRM_NAME,	\
+		       __func__, "", fmt, ##args)
 #define DRM_DEBUG_ATOMIC(fmt, ...)					\
 	drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_DEV_DEBUG_VBL(dev, fmt, args...)				\
-	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt,	\
-		       ##args)
+	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, DRM_NAME, __func__,	\
+		       "", fmt, ##args)
 #define DRM_DEBUG_VBL(fmt, ...)					\
 	drm_printk(KERN_DEBUG, DRM_UT_VBL, DRM_NAME, fmt, ##__VA_ARGS__)
 
@@ -258,7 +258,7 @@ struct dma_buf_attachment;
 				      DEFAULT_RATELIMIT_BURST);		\
 	if (__ratelimit(&_rs))						\
 		drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ ## level,	\
-			       __func__, "", fmt, ##args);		\
+			       DRM_NAME, __func__, "", fmt, ##args);	\
 })
 
 /**
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 0ad6cde5757b..8299c13004df 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -407,10 +407,11 @@ struct drm_driver {
 	struct list_head legacy_dev_list;
 };
 
-extern __printf(6, 7)
+extern __printf(7, 8)
 void drm_dev_printk(const struct device *dev, const char *level,
-		    unsigned int category, const char *function_name,
-		    const char *prefix, const char *format, ...);
+		    unsigned int category, const char *driver,
+		    const char *function_name, const char *prefix,
+		    const char *format, ...);
 extern __printf(4, 5)
 void drm_printk(const char *level, unsigned int category,
 		const char *driver, const char *format, ...);
-- 
2.7.4

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

[RFC 4/5] drm: Use drm_printk for all logging macros

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Eliminate _DRM_PRINTK macro and use drm_printk for all log levels.

This required drm_printk to vary the verbosity level of the logged
metadata depending on the log level.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 31 ++++++++++++++++++++++---------
 include/drm/drmP.h        | 44 ++++++++++++++++++++++++++++++--------------
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a2f4eb4509b9..bda4639c73e4 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -80,12 +80,21 @@ void drm_dev_printk(const struct device *dev, const char *level,
 	vaf.fmt = format;
 	vaf.va = &args;
 
-	if (dev)
-		dev_printk(level, dev, "[%s:%ps]%s %pV",
-			   driver, function_name, prefix, &vaf);
-	else
-		printk("%s[%s:%ps]%s %pV",
-		       level, driver, function_name, prefix, &vaf);
+	if (level[1] != KERN_DEBUG[1]) {
+		if (dev)
+			dev_printk(level, dev, "[%s]%s %pV",
+				   driver, prefix, &vaf);
+		else
+			printk("%s[%s]%s %pV",
+			       level, driver, prefix, &vaf);
+	} else {
+		if (dev)
+			dev_printk(level, dev, "[%s:%ps]%s %pV",
+				   driver, function_name, prefix, &vaf);
+		else
+			printk("%s[%s:%ps]%s %pV",
+			       level, driver, function_name, prefix, &vaf);
+	}
 
 	va_end(args);
 }
@@ -104,9 +113,13 @@ void drm_printk(const char *level, unsigned int category,
 	vaf.fmt = format;
 	vaf.va = &args;
 
-	printk("%s[%s:%ps]%s %pV",
-	       level, driver, __builtin_return_address(0),
-	       strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
+	if (level[1] != KERN_DEBUG[1])
+		printk("%s[%s]%s %pV",
+		       level, driver,
+		       strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
+	else
+		printk("%s[%s:%ps] %pV",
+		       level, driver, __builtin_return_address(0), &vaf);
 
 	va_end(args);
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8661045ffaf1..b8f9dcd49b36 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -146,25 +146,41 @@ struct dma_buf_attachment;
 /** \name Macros to make printk easier */
 /*@{*/
 
-#define _DRM_PRINTK(once, level, fmt, ...)				\
-	do {								\
-		printk##once(KERN_##level "[" DRM_NAME "] " fmt,	\
-			     ##__VA_ARGS__);				\
-	} while (0)
-
 #define DRM_INFO(fmt, ...)						\
-	_DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__)
-#define DRM_NOTE(fmt, ...)						\
-	_DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__)
-#define DRM_WARN(fmt, ...)						\
-	_DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
+	drm_printk(KERN_INFO, DRM_UT_NONE, DRM_NAME, fmt, ##__VA_ARGS__)
 
 #define DRM_INFO_ONCE(fmt, ...)						\
-	_DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__)
+({									\
+	static bool __print_once __read_mostly;				\
+	if (!__print_once) {						\
+		__print_once = true;					\
+		DRM_INFO(fmt, ##__VA_ARGS__);				\
+	}								\
+})
+
+#define DRM_NOTE(fmt, ...)						\
+	drm_printk(KERN_NOTICE, DRM_UT_NONE, DRM_NAME, fmt, ##__VA_ARGS__)
+
 #define DRM_NOTE_ONCE(fmt, ...)						\
-	_DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
+({									\
+	static bool __print_once __read_mostly;				\
+	if (!__print_once) {						\
+		__print_once = true;					\
+		DRM_NOTE(fmt, ##__VA_ARGS__);				\
+	}								\
+})
+
+#define DRM_WARN(fmt, ...)						\
+	drm_printk(KERN_WARNING, DRM_UT_NONE, DRM_NAME, fmt, ##__VA_ARGS__)
+
 #define DRM_WARN_ONCE(fmt, ...)						\
-	_DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
+({									\
+	static bool __print_once __read_mostly;				\
+	if (!__print_once) {						\
+		__print_once = true;					\
+		DRM_WARN(fmt, ##__VA_ARGS__);				\
+	}								\
+})
 
 /**
  * Error output.
-- 
2.7.4

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

[RFC 5/5] drm: Do not log driver prefix in debug messages

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Driver prefix is a bit redundant in debug messages. If we choose
not to log it we change debug messages which used to look like this:

 [i915:edp_panel_off [i915]] Wait for panel power off time

to this:

 [edp_panel_off [i915]] Wait for panel power off time

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index bda4639c73e4..498beaf8ee8b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -118,8 +118,8 @@ void drm_printk(const char *level, unsigned int category,
 		       level, driver,
 		       strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
 	else
-		printk("%s[%s:%ps] %pV",
-		       level, driver, __builtin_return_address(0), &vaf);
+		printk("%s[%ps] %pV",
+		       level, __builtin_return_address(0), &vaf);
 
 	va_end(args);
 }
-- 
2.7.4

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

✗ Fi.CI.BAT: failure for DRM logging tidy

[Patchwork]

== Series Details ==

Series: DRM logging tidy
URL   : https://patchwork.freedesktop.org/series/16439/
State : failure

== Summary ==

  CC [M]  drivers/gpu/drm/i915/intel_renderstate_gen6.o
  CC [M]  drivers/gpu/drm/i915/i915_guc_submission.o
  CC [M]  drivers/gpu/drm/i915/intel_renderstate_gen7.o
  LD      drivers/mmc/built-in.o
  CC [M]  drivers/gpu/drm/i915/intel_renderstate_gen8.o
  CC [M]  drivers/gpu/drm/i915/intel_audio.o
  CC [M]  drivers/gpu/drm/i915/intel_renderstate_gen9.o
cc1: all warnings being treated as errors
  LD      drivers/pnp/pnpacpi/built-in.o
  LD      kernel/built-in.o
  GEN     lib/crc32table.h
  CC [M]  drivers/gpu/drm/i915/intel_atomic.o
  CC [M]  drivers/gpu/drm/i915/intel_atomic_plane.o
  CC [M]  drivers/gpu/drm/i915/intel_bios.o
  LD      drivers/pnp/built-in.o
scripts/Makefile.build:293: recipe for target 'drivers/gpu/drm/i915/i915_drv.o' failed
make[4]: *** [drivers/gpu/drm/i915/i915_drv.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  CC      lib/crc32.o
  CC [M]  drivers/gpu/drm/i915/intel_color.o
  CC [M]  drivers/gpu/drm/i915/intel_dpio_phy.o
  CC [M]  drivers/gpu/drm/i915/intel_display.o
  LD [M]  drivers/misc/mei/mei-me.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD      drivers/misc/built-in.o
  LD      drivers/usb/storage/usb-storage.o
  LD      drivers/usb/storage/built-in.o
  LD [M]  drivers/usb/serial/usbserial.o
  LD      drivers/acpi/acpica/acpi.o
  LD      drivers/iommu/built-in.o
  LD [M]  sound/pci/hda/snd-hda-codec-generic.o
  LD      drivers/pci/pcie/aer/aerdriver.o
  LD      drivers/spi/built-in.o
  LD      sound/pci/built-in.o
  LD      drivers/pci/pcie/aer/built-in.o
  LD      drivers/pci/pcie/built-in.o
  LD      drivers/acpi/acpica/built-in.o
  LD      drivers/thermal/thermal_sys.o
  LD      drivers/tty/serial/8250/8250.o
  LD      drivers/gpu/drm/drm.o
  LD      drivers/acpi/built-in.o
  LD      sound/built-in.o
  LD      drivers/thermal/built-in.o
  LD      drivers/video/fbdev/core/fb.o
  LD      net/ipv6/ipv6.o
  LD      drivers/video/fbdev/core/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
  LD      lib/raid6/raid6_pq.o
  LD      drivers/usb/gadget/libcomposite.o
  LD      drivers/video/fbdev/built-in.o
  LD      lib/raid6/built-in.o
  LD      net/ipv6/built-in.o
  LD      drivers/scsi/scsi_mod.o
  LD      drivers/pci/built-in.o
  LD      drivers/tty/serial/8250/8250_base.o
  LD      drivers/tty/serial/8250/built-in.o
  LD      drivers/tty/serial/built-in.o
  LD      drivers/usb/gadget/udc/udc-core.o
  LD      fs/btrfs/btrfs.o
  LD      drivers/usb/gadget/udc/built-in.o
  LD      drivers/usb/gadget/built-in.o
  LD      fs/btrfs/built-in.o
  LD      net/ipv4/built-in.o
  LD      drivers/video/console/built-in.o
  LD      drivers/video/built-in.o
  CC      arch/x86/kernel/cpu/capflags.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD      drivers/usb/core/usbcore.o
  LD      arch/x86/kernel/cpu/built-in.o
  LD      arch/x86/kernel/built-in.o
  LD      drivers/usb/core/built-in.o
  LD      drivers/scsi/sd_mod.o
  LD      drivers/scsi/built-in.o
  LD      arch/x86/built-in.o
  LD      drivers/md/md-mod.o
  LD      drivers/md/built-in.o
  AR      lib/lib.a
  EXPORTS lib/lib-ksyms.o
  LD      lib/built-in.o
  LD      drivers/usb/host/xhci-hcd.o
  LD      net/core/built-in.o
  LD      net/built-in.o
  LD      drivers/tty/vt/built-in.o
  LD      drivers/tty/built-in.o
  LD      drivers/usb/host/built-in.o
  LD      drivers/usb/built-in.o
  LD      fs/ext4/ext4.o
  LD      fs/ext4/built-in.o
  LD      fs/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
scripts/Makefile.build:544: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:544: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:544: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:988: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [RFC 5/5] drm: Do not log driver prefix in debug messages

[Gustavo Padovan]

Hi Tvrtko,

2016-12-06 Tvrtko Ursulin <tursulin@ursulin.net>:

> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Driver prefix is a bit redundant in debug messages. If we choose
> not to log it we change debug messages which used to look like this:
> 
>  [i915:edp_panel_off [i915]] Wait for panel power off time
> 
> to this:
> 
>  [edp_panel_off [i915]] Wait for panel power off time

Why not remove the second mention to the driver's name instead.

[i915:edp_panel_off] looks more logic to me.

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

Re: [Intel-gfx] [RFC 5/5] drm: Do not log driver prefix in debug messages

[Tvrtko Ursulin]

Hi,

On 06/12/2016 19:49, Gustavo Padovan wrote:
> Hi Tvrtko,
>
> 2016-12-06 Tvrtko Ursulin <tursulin@ursulin.net>:
>
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Driver prefix is a bit redundant in debug messages. If we choose
>> not to log it we change debug messages which used to look like this:
>>
>>  [i915:edp_panel_off [i915]] Wait for panel power off time
>>
>> to this:
>>
>>  [edp_panel_off [i915]] Wait for panel power off time
>
> Why not remove the second mention to the driver's name instead.
>
> [i915:edp_panel_off] looks more logic to me.

Would possibly be more readable yes, but the whole string comes from %ps 
AFAICS so I don't think we could split it.

Regards,

Tvrtko

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 0/5] DRM logging tidy

[Robert Bragg]

[-- Attachment #1.1: Type: text/plain, Size: 3599 bytes --]

On Tue, Dec 6, 2016 at 6:57 PM, Tvrtko Ursulin <tursulin@ursulin.net> wrote:

> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> I wasn't here at the beginnings of DRM so I might have gotten this wrong,
> however the existance of DRM_NAME suggested to me that the intention was to
> allow individual drivers to override it and get appropriate prefixes in
> their
> log messages.
>
> I can't see that any driver is using it like that but I still thought it
> would
> be neat to do that. That way we could have our log messages look more
> obviously ours. For example after this series we have:
>
>  [i915] Memory usable by graphics device = 4096M
>  [i915] VT-d active for gfx access
>  [i915] Replacing VGA console driver
>  [i915] ACPI BIOS requests an excessive sleep of 20000 ms, using 1500 ms
> instead
>  [i915] Finished loading DMC firmware i915/skl_dmc_ver1_26.bin (v1.26)
>  [i915] Disabling framebuffer compression (FBC) to prevent screen flicker
> with VT-d enabled
>  [i915] GuC firmware load skipped
>  [i915] Initialized i915 1.6.0 20161205 for 0000:00:02.0 on minor 0
>  [i915] DRM_I915_DEBUG enabled
>  [i915] DRM_I915_DEBUG_GEM enabled
>  [i915] RC6 on
>
> Previously all that was prefixed with "[drm]" which was OK but I think the
> above is even better.
>
> Also to consider is that recent drm_printk work has removed (it hardcoded)
> DRM_NAME from DRM_ERROR and DRM_DEBUG macros, while leaving it with the
> rest
> (DRM_INFO, NOTE and WARNING) creating a bit of a inconsistency.
>

I wonder if I can maybe fold some of this idea into my related DRM_DEBUG
[RFC] sent out recently:
https://lists.freedesktop.org/archives/dri-devel/2016-December/126094.html

Instead of using DRM_NAME, I've experimented with updating my changes
adding support for dynamic debug to add a prefix based on
module_name(THIS_MODULE) for a similar result

One thing to consider here is that with the addition of dynamic debug
support this prefix arguably becomes redundant because the
dynamic_debug/control interface lets you choose to add a module name or
function prefix to messages, e.g. like:

echo "module i915 +mfp" > dynamic_debug/control

I've ignored the redundancy because my change still allows enabling
messages with the drm.drm_debug parameter and in that case the prefix is
still useful.

Br,
- Robert



> This series also makes all the logging macros use drm_printk, but also
> makes DRM_NAME passed in from the macro wrappers in all cases. So drivers
> can override it regardless of the log level.
>
> And finally, the series also removes a bit of redundant data from the debug
> messages effectively converting this:
>
>  [drm:edp_panel_off [i915]] Wait for panel power off time
>
> Into this:
>
>  [edp_panel_off [i915]] Wait for panel power off time
>
> Which still has all the data in it.
>
> Tvrtko Ursulin (5):
>   drm/i915: Give our log messages our name
>   drm: Respect driver set DRM_NAME in drm_printk
>   drm: Respect driver set DRM_NAME in drm_dev_printk
>   drm: Use drm_printk for all logging macros
>   drm: Do not log driver prefix in debug messages
>
>  drivers/gpu/drm/drm_drv.c       | 39 +++++++++++------
>  drivers/gpu/drm/i915/i915_drv.c |  3 +-
>  include/drm/drmP.h              | 94 ++++++++++++++++++++++++------
> -----------
>  include/drm/drm_drv.h           | 11 ++---
>  include/uapi/drm/i915_drm.h     |  3 ++
>  5 files changed, 92 insertions(+), 58 deletions(-)
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 4890 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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