Re: [PATCH v1 0/19] drm/panel: drmP.h removal and DRM_DEV*
From: Joe Perches
Date: Fri Feb 01 2019 - 20:32:03 EST
On Fri, 2019-02-01 at 14:37 +0100, Sam Ravnborg wrote:
> Hi Thierry.
>
> > I'm slightly on the fence about this patch.
>
> Please ignore patch 3-19, there is no consensus on the logging changes.
> We do not want to apply these and then have to redo parts/all of
> it later.
>
> But the first two patches has not seen any feedback yet:
>
> [PATCH v1 01/19] drm/panel: drop drmP.h usage
> [PATCH v1 02/19] drm/panel: panel-innolux: drop unused variable
>
> Please consider these, or maybe wait a little to see if someone
> find time to review.
>
> I can resend the patches if this is preferred.
My preference would also convert all the
DRM_DEV_<level> uses to drm_dev_<level> eventually.
Also, the macros themselves could change to use a
more consistent mechanism.
This would make the drm logging mechanisms more like
other logging mechanisms used in the kernel.
Something like:
---
Subject: [PATCH] drm: Improve and standardize logging functions
Use a more typical logging style.
Add and use drm_printk and drm_dev_printk functions that include the
test for KERN_ERR use where '*ERROR*' is added to logging output.
Remove the slightly unusual _DRM_PRINTK macro and use the new drm_printk
function instead.
---
drivers/gpu/drm/drm_print.c | 67 +++++++++++++++++++++++++++++----------------
include/drm/drm_print.h | 51 ++++++++++++++++++++--------------
2 files changed, 74 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 0e7fc3e7dfb4..4e3ae7b5cce1 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -174,22 +174,59 @@ void drm_printf(struct drm_printer *p, const char *f, ...)
}
EXPORT_SYMBOL(drm_printf);
-void drm_dev_printk(const struct device *dev, const char *level,
- const char *format, ...)
+void drm_printk(const char *format, ...)
{
struct va_format vaf;
va_list args;
+ char lvl[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = KERN_DEFAULT;
+ int level = printk_get_level(format);
+ size_t size = printk_skip_level(format) - format;
+
+ if (size) {
+ memcpy(lvl, format, size);
+ lvl[size] = '\0';
+ }
va_start(args, format);
- vaf.fmt = format;
+ vaf.fmt = format + size;
+ vaf.va = &args;
+
+ printk("%s" "[" DRM_NAME ":%ps] %s%pV",
+ lvl, __builtin_return_address(0),
+ level == LOGLEVEL_ERR ? "*ERROR* " : "",
+ &vaf);
+
+ va_end(args);
+}
+EXPORT_SYMBOL(drm_printk);
+
+void drm_dev_printk(const struct device *dev, const char *format, ...)
+{
+ struct va_format vaf;
+ va_list args;
+ char lvl[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = KERN_DEFAULT;
+ int level = printk_get_level(format);
+ size_t size = printk_skip_level(format) - format;
+
+ if (size) {
+ memcpy(lvl, format, size);
+ lvl[size] = '\0';
+ }
+
+ va_start(args, format);
+ vaf.fmt = format + size;
vaf.va = &args;
if (dev)
- dev_printk(level, dev, "[" DRM_NAME ":%ps] %pV",
- __builtin_return_address(0), &vaf);
+ dev_printk(lvl, dev, "[" DRM_NAME ":%ps] %s%pV",
+ __builtin_return_address(0),
+ level == LOGLEVEL_ERR ? "*ERROR* " : "",
+ &vaf);
else
- printk("%s" "[" DRM_NAME ":%ps] %pV",
- level, __builtin_return_address(0), &vaf);
+ printk("%s" "[" DRM_NAME ":%ps] %s%pV",
+ lvl, __builtin_return_address(0),
+ level == LOGLEVEL_ERR ? "*ERROR* " : "",
+ &vaf);
va_end(args);
}
@@ -237,19 +274,3 @@ void drm_dbg(unsigned int category, const char *format, ...)
va_end(args);
}
EXPORT_SYMBOL(drm_dbg);
-
-void drm_err(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);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index afbc3beef089..9d9fd10e6ff8 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -268,9 +268,8 @@ static inline struct drm_printer drm_debug_printer(const char *prefix)
#define DRM_UT_LEASE 0x80
#define DRM_UT_DP 0x100
-__printf(3, 4)
-void drm_dev_printk(const struct device *dev, const char *level,
- const char *format, ...);
+__printf(2, 3)
+void drm_dev_printk(const struct device *dev, const char *format, ...);
__printf(3, 4)
void drm_dev_dbg(const struct device *dev, unsigned int category,
const char *format, ...);
@@ -278,26 +277,38 @@ void drm_dev_dbg(const struct device *dev, unsigned int category,
__printf(2, 3)
void drm_dbg(unsigned int category, const char *format, ...);
__printf(1, 2)
-void drm_err(const char *format, ...);
+void drm_printk(const char *format, ...);
/* Macros to make printk easier */
-#define _DRM_PRINTK(once, level, fmt, ...) \
- printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
-
-#define DRM_INFO(fmt, ...) \
- _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__)
-#define DRM_NOTE(fmt, ...) \
- _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__)
+#define DRM_ERROR(fmt, ...) \
+ drm_printk(KERN_ERR fmt, ##__VA_ARGS__)
+#define drm_err DRM_ERROR
#define DRM_WARN(fmt, ...) \
- _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
+ drm_printk(KERN_WARNING fmt, ##__VA_ARGS__)
+#define DRM_NOTE(fmt, ...) \
+ drm_printk(KERN_NOTICE fmt, ##__VA_ARGS__)
+#define DRM_INFO(fmt, ...) \
+ drm_printk(KERN_INFO fmt, ##__VA_ARGS__)
+
+#define drm_printk_once(fmt, ...) \
+({ \
+ static bool __print_once __read_mostly; \
+ bool __ret_print_once = !__print_once; \
+ \
+ if (!__print_once) { \
+ __print_once = true; \
+ drm_printk(fmt, ##__VA_ARGS__); \
+ } \
+ unlikely(__ret_print_once); \
+})
-#define DRM_INFO_ONCE(fmt, ...) \
- _DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__)
-#define DRM_NOTE_ONCE(fmt, ...) \
- _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
#define DRM_WARN_ONCE(fmt, ...) \
- _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
+ drm_printk_once(KERN_WARNING fmt, ##__VA_ARGS__)
+#define DRM_NOTE_ONCE(fmt, ...) \
+ drm_printk_once(KERN_NOTICE fmt, ##__VA_ARGS__)
+#define DRM_INFO_ONCE(fmt, ...) \
+ drm_printk_once(KERN_INFO fmt, ##__VA_ARGS__)
/**
* Error output.
@@ -306,9 +317,7 @@ void drm_err(const char *format, ...);
* @fmt: printf() like format string.
*/
#define DRM_DEV_ERROR(dev, fmt, ...) \
- drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
-#define DRM_ERROR(fmt, ...) \
- drm_err(fmt, ##__VA_ARGS__)
+ drm_dev_printk(dev, KERN_ERR fmt, ##__VA_ARGS__)
/**
* Rate limited error output. Like DRM_ERROR() but won't flood the log.
@@ -329,7 +338,7 @@ void drm_err(const char *format, ...);
DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
#define DRM_DEV_INFO(dev, fmt, ...) \
- drm_dev_printk(dev, KERN_INFO, fmt, ##__VA_ARGS__)
+ drm_dev_printk(dev, KERN_INFO fmt, ##__VA_ARGS__)
#define DRM_DEV_INFO_ONCE(dev, fmt, ...) \
({ \