* [PATCH v2 0/2] drm: document logging functions
@ 2020-01-02 22:15 Sam Ravnborg
2020-01-02 22:15 ` [PATCH v2 1/2] drm/print: document drm_ " Sam Ravnborg
2020-01-02 22:15 ` [PATCH v2 2/2] drm/print: document DRM_ " Sam Ravnborg
0 siblings, 2 replies; 10+ messages in thread
From: Sam Ravnborg @ 2020-01-02 22:15 UTC (permalink / raw)
To: dri-devel, Jani Nikula, Daniel Vetter
Cc: Joe Perches, Sam Ravnborg, Sean Paul
Add kernel-doc for the drm_ and DRM_ logging
functions.
This is the documentation that I missed when I started to use
the logging functions.
Version 1 of this patchset included drm_ variants of the existing
logging functions - but they are left out for now.
The idea is that we should try to use drm_ logging in favour
of the the variants that take a device *.
This patchset document the existing logging functions with
no functional changes.
And the documentation is properly wired up in drm-internals.rst
Sam
Sam Ravnborg (2):
drm/print: document drm_ logging functions
drm/print: document DRM_ logging functions
Documentation/gpu/drm-internals.rst | 6 ++
include/drm/drm_print.h | 164 +++++++++++++++++++++++++++++++++---
2 files changed, 159 insertions(+), 11 deletions(-)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] drm/print: document drm_ logging functions
2020-01-02 22:15 [PATCH v2 0/2] drm: document logging functions Sam Ravnborg
@ 2020-01-02 22:15 ` Sam Ravnborg
2020-01-07 16:12 ` Daniel Vetter
2020-01-02 22:15 ` [PATCH v2 2/2] drm/print: document DRM_ " Sam Ravnborg
1 sibling, 1 reply; 10+ messages in thread
From: Sam Ravnborg @ 2020-01-02 22:15 UTC (permalink / raw)
To: dri-devel, Jani Nikula, Daniel Vetter
Cc: Joe Perches, Sam Ravnborg, Sean Paul
This is the documentation I have missed when I looked for help
how to do proper logging. Hopefully it can help others.
v2:
- Add parameters to the logging functions in the doc
- Drop notes on other types of logging
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
Documentation/gpu/drm-internals.rst | 6 +++
include/drm/drm_print.h | 80 ++++++++++++++++++++++++++---
2 files changed, 79 insertions(+), 7 deletions(-)
diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index a73320576ca9..c2093611999c 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -164,6 +164,12 @@ File Operations
Misc Utilities
==============
+Logging
+-------
+
+.. kernel-doc:: include/drm/drm_print.h
+ :doc: logging
+
Printer
-------
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 8f99d389792d..89e75eea65d2 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -250,22 +250,42 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
}
/**
- * enum drm_debug_category - The DRM debug categories
+ * DOC: logging
+ *
+ * There is a set of functions/macros available used for logging
+ * in the DRM subsystem.
+ * Using the drm logging function enables that the logging is consistently
+ * prefixed with *[drm]* thus the logging is easy to recognize.
+ *
+ * Example of logging with *[drm]* prefix::
*
- * Each of the DRM debug logging macros use a specific category, and the logging
- * is filtered by the drm.debug module parameter. This enum specifies the values
- * for the interface.
+ * [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
+ * [drm] Driver supports precise vblank timestamp query.
*
- * Each DRM_DEBUG_<CATEGORY> macro logs to DRM_UT_<CATEGORY> category, except
- * DRM_DEBUG() logs to DRM_UT_CORE.
+ *
+ * Each of the debug logging macros use a specific category, and the logging
+ * is filtered by the drm.debug module parameter. The &drm_debug_category enum
+ * specifies the values for the interface.
+ *
+ * Each drm_dbg_<category> macro logs to a DRM_UT_<category> category,
+ * except drm_dbg() that logs to DRM_UT_DRIVER.
*
* Enabling verbose debug messages is done through the drm.debug parameter, each
* category being enabled by a bit:
*
* - drm.debug=0x1 will enable CORE messages
* - drm.debug=0x2 will enable DRIVER messages
+ * - drm.debug=0x4 will enable KMS messages
+ * - drm.debug=0x8 will enable PRIME messages
+ * - drm.debug=0x10 will enable ATOMIC messages
+ * - drm.debug=0x20 will enable VBL messages
+ * - drm.debug=0x40 will enable STATE messages
+ * - drm.debug=0x80 will enable LEASE messages
+ * - drm.debug=0x100 will enable DP messages
+ *
+ * To enable more than one category OR the values - examples:
+ *
* - drm.debug=0x3 will enable CORE and DRIVER messages
- * - ...
* - drm.debug=0x1ff will enable all messages
*
* An interesting feature is that it's possible to enable verbose logging at
@@ -273,6 +293,52 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
*
* # echo 0xf > /sys/module/drm/parameters/debug
*
+ *
+ * When a &drm_device * is available use one of the following logging functions.
+ * The same prototype is shared by all the logging functions
+ * that take a &drm_device * as first argument:
+ *
+ * .. code-block:: c
+ *
+ * void drm_xxx(struct drm_device *, char * fmt, ...)
+ *
+ * DRM/Drivers can use the following functions for logging.
+ *
+ * .. code-block:: none
+ *
+ * # Plain logging
+ * drm_dbg(drm, fmt, ...)
+ * drm_info(drm, fmt, ...)
+ * drm_notice(drm, fmt, ...)
+ * drm_warn(drm, fmt, ...)
+ * drm_err(drm, fmt, ...)
+ *
+ * # Log only once
+ * drm_info_once(drm, fmt, ...)
+ * drm_notice_once(drm, fmt, ...)
+ * drm_warn_once(drm, fmt, ...)
+ * drm_err_once(drm, fmt, ...)
+ *
+ * # Ratelimited - do not flood the logs
+ * drm_err_ratelimited(drm, fmt, ...)
+ *
+ * # Logging with a specific category
+ * drm_dbg_core(drm, fmt, ...)
+ * drm_dbg(drm, fmt, ...) # Uses the DRIVER category
+ * drm_dbg_kms(drm, fmt, ...)
+ * drm_dbg_prime(drm, fmt, ...)
+ * drm_dbg_atomic(drm, fmt, ...)
+ * drm_dbg_vbl(drm, fmt, ...)
+ * drm_dbg_state(drm, fmt, ...)
+ * drm_dbg_lease(drm, fmt, ...)
+ * drm_dbg_dp(drm, fmt, ...)
+ *
+ * See enum &drm_debug_category for a description of the categories.
+ *
+ */
+
+/**
+ * enum drm_debug_category - The DRM debug categories
*/
enum drm_debug_category {
/**
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] drm/print: document DRM_ logging functions
2020-01-02 22:15 [PATCH v2 0/2] drm: document logging functions Sam Ravnborg
2020-01-02 22:15 ` [PATCH v2 1/2] drm/print: document drm_ " Sam Ravnborg
@ 2020-01-02 22:15 ` Sam Ravnborg
2020-01-07 16:08 ` Daniel Vetter
1 sibling, 1 reply; 10+ messages in thread
From: Sam Ravnborg @ 2020-01-02 22:15 UTC (permalink / raw)
To: dri-devel, Jani Nikula, Daniel Vetter
Cc: Joe Perches, Sam Ravnborg, Sean Paul
Document the remaining DRM_ logging functions.
As the logging functions are now all properly
listed drop the few specific kernel-doc markers
so we keep the relevant parts in the documentation.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Sean Paul <sean@poorly.run>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
include/drm/drm_print.h | 84 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 80 insertions(+), 4 deletions(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 89e75eea65d2..abe247199bf7 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -335,6 +335,82 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
*
* See enum &drm_debug_category for a description of the categories.
*
+ * Logging when a &device * is available, but no &drm_device *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * DRM/Drivers can use the following functions for logging when there is a
+ * struct device * available.
+ * The logging functions share the same prototype:
+ *
+ * .. code-block:: c
+ *
+ * void DRM_xxx(struct device *, char * fmt, ...)
+ *
+ * .. code-block:: none
+ *
+ * # Plain logging
+ * DRM_DEV_INFO(dev, fmt, ...)
+ * DRM_DEV_ERROR(dev, fmt, ...)
+ *
+ * # Log only once
+ * DRM_DEV_INFO_ONCE(dev, fmt, ...)
+ *
+ * # Ratelimited - do not flood the logs
+ * DRM_DEV_DEBUG_RATELIMITED(dev, fmt, ...)
+ * DRM_DEV_DEBUG_DRIVER_RATELIMITED(dev, fmt, ...)
+ * DRM_DEV_DEBUG_KMS_RATELIMITED(dev, fmt, ...)
+ * DRM_DEV_DEBUG_PRIME_RATELIMITED(dev, fmt, ...)
+ * DRM_DEV_ERROR_RATELIMITED(dev, fmt, ...)
+ *
+ * # Logging with a specific category
+ * DRM_DEV_DEBUG(dev, fmt, ...) # Logged as CORE
+ * DRM_DEV_DEBUG_DRIVER(dev, fmt, ...)
+ * DRM_DEV_DEBUG_KMS(dev, fmt, ...)
+ * DRM_DEV_DEBUG_PRIME(dev, fmt, ...)
+ * DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...)
+ * DRM_DEV_DEBUG_VBL(dev, fmt, ...)
+ * DRM_DEV_DEBUG_DP(dev, fmt, ...)
+ *
+ * Logging when no &device * nor &drm_device * is available
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * DRM/Drivers can use the following functions for logging when there is no
+ * extra info associated to the logging.
+ * The logging functions share the same prototype:
+ *
+ * .. code-block:: c
+ *
+ * void DRM_xxx(char * fmt, ...)
+ *
+ * .. code-block:: none
+ *
+ * # Plain logging
+ * DRM_INFO(fmt, ...)
+ * DRM_NOTE(fmt, ...)
+ * DRM_WARN(fmt, ...)
+ * DRM_ERROR(fmt, ...)
+ *
+ * # Log only once
+ * DRM_INFO_ONCE(fmt, ...)
+ * DRM_NOTE_ONCE(fmt, ...)
+ * DRM_WARN_ONCE(fmt, ...)
+ *
+ * # Ratelimited - do not flood the logs
+ * DRM_DEBUG_RATELIMITED(fmt, ...)
+ * DRM_DEBUG_DRIVER_RATELIMITED(fmt, ...)
+ * DRM_DEBUG_KMS_RATELIMITED(fmt, ...)
+ * DRM_DEBUG_PRIME_RATELIMITED(fmt, ...)
+ * DRM_ERROR_RATELIMITED(fmt, ...)
+ *
+ * # Logging with a specific category
+ * DRM_DEBUG(fmt, ...) # Logged as CORE
+ * DRM_DEBUG_DRIVER(fmt, ...)
+ * DRM_DEBUG_KMS(fmt, ...)
+ * DRM_DEBUG_PRIME(fmt, ...)
+ * DRM_DEBUG_ATOMIC(fmt, ...)
+ * DRM_DEBUG_VBL(fmt, ...)
+ * DRM_DEBUG_LEASE(fmt, ...)
+ * DRM_DEBUG_DP(fmt, ...)
*/
/**
@@ -399,7 +475,7 @@ __printf(3, 4)
void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
const char *format, ...);
-/**
+/*
* Error output.
*
* @dev: device pointer
@@ -408,7 +484,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
#define DRM_DEV_ERROR(dev, fmt, ...) \
drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
-/**
+/*
* Rate limited error output. Like DRM_ERROR() but won't flood the log.
*
* @dev: device pointer
@@ -436,7 +512,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
} \
})
-/**
+/*
* Debug output.
*
* @dev: device pointer
@@ -466,7 +542,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
drm_dev_dbg(dev, category, fmt, ##__VA_ARGS__); \
})
-/**
+/*
* Rate limited debug output. Like DRM_DEBUG() but won't flood the log.
*
* @dev: device pointer
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/print: document DRM_ logging functions
2020-01-02 22:15 ` [PATCH v2 2/2] drm/print: document DRM_ " Sam Ravnborg
@ 2020-01-07 16:08 ` Daniel Vetter
2020-01-07 18:17 ` Sam Ravnborg
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2020-01-07 16:08 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Jani Nikula, dri-devel, Joe Perches, Sean Paul
On Thu, Jan 02, 2020 at 11:15:19PM +0100, Sam Ravnborg wrote:
> Document the remaining DRM_ logging functions.
> As the logging functions are now all properly
> listed drop the few specific kernel-doc markers
> so we keep the relevant parts in the documentation.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
> include/drm/drm_print.h | 84 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 80 insertions(+), 4 deletions(-)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 89e75eea65d2..abe247199bf7 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -335,6 +335,82 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
> *
> * See enum &drm_debug_category for a description of the categories.
> *
> + * Logging when a &device * is available, but no &drm_device *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * DRM/Drivers can use the following functions for logging when there is a
> + * struct device * available.
> + * The logging functions share the same prototype:
I'd replace the entire block with a "This stuff is deprecated" warning. We
have at least a corresponding todo.rst entry.
-Daniel
> + *
> + * .. code-block:: c
> + *
> + * void DRM_xxx(struct device *, char * fmt, ...)
> + *
> + * .. code-block:: none
> + *
> + * # Plain logging
> + * DRM_DEV_INFO(dev, fmt, ...)
> + * DRM_DEV_ERROR(dev, fmt, ...)
> + *
> + * # Log only once
> + * DRM_DEV_INFO_ONCE(dev, fmt, ...)
> + *
> + * # Ratelimited - do not flood the logs
> + * DRM_DEV_DEBUG_RATELIMITED(dev, fmt, ...)
> + * DRM_DEV_DEBUG_DRIVER_RATELIMITED(dev, fmt, ...)
> + * DRM_DEV_DEBUG_KMS_RATELIMITED(dev, fmt, ...)
> + * DRM_DEV_DEBUG_PRIME_RATELIMITED(dev, fmt, ...)
> + * DRM_DEV_ERROR_RATELIMITED(dev, fmt, ...)
> + *
> + * # Logging with a specific category
> + * DRM_DEV_DEBUG(dev, fmt, ...) # Logged as CORE
> + * DRM_DEV_DEBUG_DRIVER(dev, fmt, ...)
> + * DRM_DEV_DEBUG_KMS(dev, fmt, ...)
> + * DRM_DEV_DEBUG_PRIME(dev, fmt, ...)
> + * DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...)
> + * DRM_DEV_DEBUG_VBL(dev, fmt, ...)
> + * DRM_DEV_DEBUG_DP(dev, fmt, ...)
> + *
> + * Logging when no &device * nor &drm_device * is available
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * DRM/Drivers can use the following functions for logging when there is no
> + * extra info associated to the logging.
> + * The logging functions share the same prototype:
> + *
> + * .. code-block:: c
> + *
> + * void DRM_xxx(char * fmt, ...)
> + *
> + * .. code-block:: none
> + *
> + * # Plain logging
> + * DRM_INFO(fmt, ...)
> + * DRM_NOTE(fmt, ...)
> + * DRM_WARN(fmt, ...)
> + * DRM_ERROR(fmt, ...)
> + *
> + * # Log only once
> + * DRM_INFO_ONCE(fmt, ...)
> + * DRM_NOTE_ONCE(fmt, ...)
> + * DRM_WARN_ONCE(fmt, ...)
> + *
> + * # Ratelimited - do not flood the logs
> + * DRM_DEBUG_RATELIMITED(fmt, ...)
> + * DRM_DEBUG_DRIVER_RATELIMITED(fmt, ...)
> + * DRM_DEBUG_KMS_RATELIMITED(fmt, ...)
> + * DRM_DEBUG_PRIME_RATELIMITED(fmt, ...)
> + * DRM_ERROR_RATELIMITED(fmt, ...)
> + *
> + * # Logging with a specific category
> + * DRM_DEBUG(fmt, ...) # Logged as CORE
> + * DRM_DEBUG_DRIVER(fmt, ...)
> + * DRM_DEBUG_KMS(fmt, ...)
> + * DRM_DEBUG_PRIME(fmt, ...)
> + * DRM_DEBUG_ATOMIC(fmt, ...)
> + * DRM_DEBUG_VBL(fmt, ...)
> + * DRM_DEBUG_LEASE(fmt, ...)
> + * DRM_DEBUG_DP(fmt, ...)
> */
>
> /**
> @@ -399,7 +475,7 @@ __printf(3, 4)
> void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> const char *format, ...);
>
> -/**
> +/*
> * Error output.
> *
> * @dev: device pointer
> @@ -408,7 +484,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> #define DRM_DEV_ERROR(dev, fmt, ...) \
> drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
>
> -/**
> +/*
> * Rate limited error output. Like DRM_ERROR() but won't flood the log.
> *
> * @dev: device pointer
> @@ -436,7 +512,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> } \
> })
>
> -/**
> +/*
> * Debug output.
> *
> * @dev: device pointer
> @@ -466,7 +542,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> drm_dev_dbg(dev, category, fmt, ##__VA_ARGS__); \
> })
>
> -/**
> +/*
> * Rate limited debug output. Like DRM_DEBUG() but won't flood the log.
> *
> * @dev: device pointer
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] drm/print: document drm_ logging functions
2020-01-02 22:15 ` [PATCH v2 1/2] drm/print: document drm_ " Sam Ravnborg
@ 2020-01-07 16:12 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2020-01-07 16:12 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Jani Nikula, dri-devel, Joe Perches, Sean Paul
On Thu, Jan 02, 2020 at 11:15:18PM +0100, Sam Ravnborg wrote:
> This is the documentation I have missed when I looked for help
> how to do proper logging. Hopefully it can help others.
>
> v2:
> - Add parameters to the logging functions in the doc
> - Drop notes on other types of logging
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> ---
> Documentation/gpu/drm-internals.rst | 6 +++
> include/drm/drm_print.h | 80 ++++++++++++++++++++++++++---
> 2 files changed, 79 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
> index a73320576ca9..c2093611999c 100644
> --- a/Documentation/gpu/drm-internals.rst
> +++ b/Documentation/gpu/drm-internals.rst
> @@ -164,6 +164,12 @@ File Operations
> Misc Utilities
> ==============
>
> +Logging
> +-------
> +
> +.. kernel-doc:: include/drm/drm_print.h
> + :doc: logging
> +
> Printer
> -------
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 8f99d389792d..89e75eea65d2 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -250,22 +250,42 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
> }
>
> /**
> - * enum drm_debug_category - The DRM debug categories
> + * DOC: logging
> + *
> + * There is a set of functions/macros available used for logging
> + * in the DRM subsystem.
> + * Using the drm logging function enables that the logging is consistently
> + * prefixed with *[drm]* thus the logging is easy to recognize.
> + *
> + * Example of logging with *[drm]* prefix::
> *
> - * Each of the DRM debug logging macros use a specific category, and the logging
> - * is filtered by the drm.debug module parameter. This enum specifies the values
> - * for the interface.
> + * [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> + * [drm] Driver supports precise vblank timestamp query.
> *
> - * Each DRM_DEBUG_<CATEGORY> macro logs to DRM_UT_<CATEGORY> category, except
> - * DRM_DEBUG() logs to DRM_UT_CORE.
> + *
> + * Each of the debug logging macros use a specific category, and the logging
> + * is filtered by the drm.debug module parameter. The &drm_debug_category enum
> + * specifies the values for the interface.
> + *
> + * Each drm_dbg_<category> macro logs to a DRM_UT_<category> category,
> + * except drm_dbg() that logs to DRM_UT_DRIVER.
> *
> * Enabling verbose debug messages is done through the drm.debug parameter, each
> * category being enabled by a bit:
> *
> * - drm.debug=0x1 will enable CORE messages
> * - drm.debug=0x2 will enable DRIVER messages
> + * - drm.debug=0x4 will enable KMS messages
> + * - drm.debug=0x8 will enable PRIME messages
> + * - drm.debug=0x10 will enable ATOMIC messages
> + * - drm.debug=0x20 will enable VBL messages
> + * - drm.debug=0x40 will enable STATE messages
> + * - drm.debug=0x80 will enable LEASE messages
> + * - drm.debug=0x100 will enable DP messages
> + *
> + * To enable more than one category OR the values - examples:
> + *
> * - drm.debug=0x3 will enable CORE and DRIVER messages
> - * - ...
> * - drm.debug=0x1ff will enable all messages
> *
> * An interesting feature is that it's possible to enable verbose logging at
> @@ -273,6 +293,52 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
> *
> * # echo 0xf > /sys/module/drm/parameters/debug
> *
> + *
> + * When a &drm_device * is available use one of the following logging functions.
> + * The same prototype is shared by all the logging functions
> + * that take a &drm_device * as first argument:
> + *
> + * .. code-block:: c
> + *
> + * void drm_xxx(struct drm_device *, char * fmt, ...)
> + *
> + * DRM/Drivers can use the following functions for logging.
> + *
> + * .. code-block:: none
> + *
> + * # Plain logging
> + * drm_dbg(drm, fmt, ...)
> + * drm_info(drm, fmt, ...)
> + * drm_notice(drm, fmt, ...)
> + * drm_warn(drm, fmt, ...)
> + * drm_err(drm, fmt, ...)
> + *
> + * # Log only once
> + * drm_info_once(drm, fmt, ...)
> + * drm_notice_once(drm, fmt, ...)
> + * drm_warn_once(drm, fmt, ...)
> + * drm_err_once(drm, fmt, ...)
> + *
> + * # Ratelimited - do not flood the logs
> + * drm_err_ratelimited(drm, fmt, ...)
> + *
> + * # Logging with a specific category
> + * drm_dbg_core(drm, fmt, ...)
> + * drm_dbg(drm, fmt, ...) # Uses the DRIVER category
> + * drm_dbg_kms(drm, fmt, ...)
> + * drm_dbg_prime(drm, fmt, ...)
> + * drm_dbg_atomic(drm, fmt, ...)
> + * drm_dbg_vbl(drm, fmt, ...)
> + * drm_dbg_state(drm, fmt, ...)
> + * drm_dbg_lease(drm, fmt, ...)
> + * drm_dbg_dp(drm, fmt, ...)
> + *
> + * See enum &drm_debug_category for a description of the categories.
> + *
> + */
I kinda can't decide between this and just copypasting fairly repetitive
kerneldoc over all the new functions. I think given the long-term idea is
to favour the above functions over all the screaming macros (because of
multi-gpu stuff), I'd go with full kerneldocs for these, plus comments or
a note in the overview doc that everything else is kinda deprecated.
Jani, thoughts?
-Daniel
> +
> +/**
> + * enum drm_debug_category - The DRM debug categories
> */
> enum drm_debug_category {
> /**
> --
> 2.20.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/print: document DRM_ logging functions
2020-01-07 16:08 ` Daniel Vetter
@ 2020-01-07 18:17 ` Sam Ravnborg
2020-01-08 18:49 ` Daniel Vetter
0 siblings, 1 reply; 10+ messages in thread
From: Sam Ravnborg @ 2020-01-07 18:17 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Jani Nikula, Joe Perches, Sean Paul, dri-devel
Hi Daniel.
> > + * Logging when a &device * is available, but no &drm_device *
> > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + * DRM/Drivers can use the following functions for logging when there is a
> > + * struct device * available.
> > + * The logging functions share the same prototype:
>
> I'd replace the entire block with a "This stuff is deprecated" warning. We
> have at least a corresponding todo.rst entry.
We have many situations where no drm_device is available.
At least when you a buried in drm_panel patches.
So it is either DRM_DEV_ERROR() or drv_err().
Which is why I have pushed for nicer drm_ variants of these...
The todo entry only covers the nice new macros that Jani added
where we have a drm_device.
Sam
> -Daniel
>
> > + *
> > + * .. code-block:: c
> > + *
> > + * void DRM_xxx(struct device *, char * fmt, ...)
> > + *
> > + * .. code-block:: none
> > + *
> > + * # Plain logging
> > + * DRM_DEV_INFO(dev, fmt, ...)
> > + * DRM_DEV_ERROR(dev, fmt, ...)
> > + *
> > + * # Log only once
> > + * DRM_DEV_INFO_ONCE(dev, fmt, ...)
> > + *
> > + * # Ratelimited - do not flood the logs
> > + * DRM_DEV_DEBUG_RATELIMITED(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_DRIVER_RATELIMITED(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_KMS_RATELIMITED(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_PRIME_RATELIMITED(dev, fmt, ...)
> > + * DRM_DEV_ERROR_RATELIMITED(dev, fmt, ...)
> > + *
> > + * # Logging with a specific category
> > + * DRM_DEV_DEBUG(dev, fmt, ...) # Logged as CORE
> > + * DRM_DEV_DEBUG_DRIVER(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_KMS(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_PRIME(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_VBL(dev, fmt, ...)
> > + * DRM_DEV_DEBUG_DP(dev, fmt, ...)
> > + *
> > + * Logging when no &device * nor &drm_device * is available
> > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + * DRM/Drivers can use the following functions for logging when there is no
> > + * extra info associated to the logging.
> > + * The logging functions share the same prototype:
> > + *
> > + * .. code-block:: c
> > + *
> > + * void DRM_xxx(char * fmt, ...)
> > + *
> > + * .. code-block:: none
> > + *
> > + * # Plain logging
> > + * DRM_INFO(fmt, ...)
> > + * DRM_NOTE(fmt, ...)
> > + * DRM_WARN(fmt, ...)
> > + * DRM_ERROR(fmt, ...)
> > + *
> > + * # Log only once
> > + * DRM_INFO_ONCE(fmt, ...)
> > + * DRM_NOTE_ONCE(fmt, ...)
> > + * DRM_WARN_ONCE(fmt, ...)
> > + *
> > + * # Ratelimited - do not flood the logs
> > + * DRM_DEBUG_RATELIMITED(fmt, ...)
> > + * DRM_DEBUG_DRIVER_RATELIMITED(fmt, ...)
> > + * DRM_DEBUG_KMS_RATELIMITED(fmt, ...)
> > + * DRM_DEBUG_PRIME_RATELIMITED(fmt, ...)
> > + * DRM_ERROR_RATELIMITED(fmt, ...)
> > + *
> > + * # Logging with a specific category
> > + * DRM_DEBUG(fmt, ...) # Logged as CORE
> > + * DRM_DEBUG_DRIVER(fmt, ...)
> > + * DRM_DEBUG_KMS(fmt, ...)
> > + * DRM_DEBUG_PRIME(fmt, ...)
> > + * DRM_DEBUG_ATOMIC(fmt, ...)
> > + * DRM_DEBUG_VBL(fmt, ...)
> > + * DRM_DEBUG_LEASE(fmt, ...)
> > + * DRM_DEBUG_DP(fmt, ...)
> > */
> >
> > /**
> > @@ -399,7 +475,7 @@ __printf(3, 4)
> > void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > const char *format, ...);
> >
> > -/**
> > +/*
> > * Error output.
> > *
> > * @dev: device pointer
> > @@ -408,7 +484,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > #define DRM_DEV_ERROR(dev, fmt, ...) \
> > drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
> >
> > -/**
> > +/*
> > * Rate limited error output. Like DRM_ERROR() but won't flood the log.
> > *
> > * @dev: device pointer
> > @@ -436,7 +512,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > } \
> > })
> >
> > -/**
> > +/*
> > * Debug output.
> > *
> > * @dev: device pointer
> > @@ -466,7 +542,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > drm_dev_dbg(dev, category, fmt, ##__VA_ARGS__); \
> > })
> >
> > -/**
> > +/*
> > * Rate limited debug output. Like DRM_DEBUG() but won't flood the log.
> > *
> > * @dev: device pointer
> > --
> > 2.20.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/print: document DRM_ logging functions
2020-01-07 18:17 ` Sam Ravnborg
@ 2020-01-08 18:49 ` Daniel Vetter
2020-01-08 20:04 ` Sam Ravnborg
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2020-01-08 18:49 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Jani Nikula, dri-devel, Joe Perches, Sean Paul
On Tue, Jan 07, 2020 at 07:17:52PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
>
> > > + * Logging when a &device * is available, but no &drm_device *
> > > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > + *
> > > + * DRM/Drivers can use the following functions for logging when there is a
> > > + * struct device * available.
> > > + * The logging functions share the same prototype:
> >
> > I'd replace the entire block with a "This stuff is deprecated" warning. We
> > have at least a corresponding todo.rst entry.
>
> We have many situations where no drm_device is available.
> At least when you a buried in drm_panel patches.
>
> So it is either DRM_DEV_ERROR() or drv_err().
> Which is why I have pushed for nicer drm_ variants of these...
Huh, drm_panel indeed has no drm_device. And I guess we don't have a
convenient excuse to add it ...
> The todo entry only covers the nice new macros that Jani added
> where we have a drm_device.
I wonder whether for those cases we shouldn't just directly use the
various dev_* macros?
-Daniel
>
> Sam
>
>
>
> > -Daniel
> >
> > > + *
> > > + * .. code-block:: c
> > > + *
> > > + * void DRM_xxx(struct device *, char * fmt, ...)
> > > + *
> > > + * .. code-block:: none
> > > + *
> > > + * # Plain logging
> > > + * DRM_DEV_INFO(dev, fmt, ...)
> > > + * DRM_DEV_ERROR(dev, fmt, ...)
> > > + *
> > > + * # Log only once
> > > + * DRM_DEV_INFO_ONCE(dev, fmt, ...)
> > > + *
> > > + * # Ratelimited - do not flood the logs
> > > + * DRM_DEV_DEBUG_RATELIMITED(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_DRIVER_RATELIMITED(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_KMS_RATELIMITED(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_PRIME_RATELIMITED(dev, fmt, ...)
> > > + * DRM_DEV_ERROR_RATELIMITED(dev, fmt, ...)
> > > + *
> > > + * # Logging with a specific category
> > > + * DRM_DEV_DEBUG(dev, fmt, ...) # Logged as CORE
> > > + * DRM_DEV_DEBUG_DRIVER(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_KMS(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_PRIME(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_VBL(dev, fmt, ...)
> > > + * DRM_DEV_DEBUG_DP(dev, fmt, ...)
> > > + *
> > > + * Logging when no &device * nor &drm_device * is available
> > > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > + *
> > > + * DRM/Drivers can use the following functions for logging when there is no
> > > + * extra info associated to the logging.
> > > + * The logging functions share the same prototype:
> > > + *
> > > + * .. code-block:: c
> > > + *
> > > + * void DRM_xxx(char * fmt, ...)
> > > + *
> > > + * .. code-block:: none
> > > + *
> > > + * # Plain logging
> > > + * DRM_INFO(fmt, ...)
> > > + * DRM_NOTE(fmt, ...)
> > > + * DRM_WARN(fmt, ...)
> > > + * DRM_ERROR(fmt, ...)
> > > + *
> > > + * # Log only once
> > > + * DRM_INFO_ONCE(fmt, ...)
> > > + * DRM_NOTE_ONCE(fmt, ...)
> > > + * DRM_WARN_ONCE(fmt, ...)
> > > + *
> > > + * # Ratelimited - do not flood the logs
> > > + * DRM_DEBUG_RATELIMITED(fmt, ...)
> > > + * DRM_DEBUG_DRIVER_RATELIMITED(fmt, ...)
> > > + * DRM_DEBUG_KMS_RATELIMITED(fmt, ...)
> > > + * DRM_DEBUG_PRIME_RATELIMITED(fmt, ...)
> > > + * DRM_ERROR_RATELIMITED(fmt, ...)
> > > + *
> > > + * # Logging with a specific category
> > > + * DRM_DEBUG(fmt, ...) # Logged as CORE
> > > + * DRM_DEBUG_DRIVER(fmt, ...)
> > > + * DRM_DEBUG_KMS(fmt, ...)
> > > + * DRM_DEBUG_PRIME(fmt, ...)
> > > + * DRM_DEBUG_ATOMIC(fmt, ...)
> > > + * DRM_DEBUG_VBL(fmt, ...)
> > > + * DRM_DEBUG_LEASE(fmt, ...)
> > > + * DRM_DEBUG_DP(fmt, ...)
> > > */
> > >
> > > /**
> > > @@ -399,7 +475,7 @@ __printf(3, 4)
> > > void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > > const char *format, ...);
> > >
> > > -/**
> > > +/*
> > > * Error output.
> > > *
> > > * @dev: device pointer
> > > @@ -408,7 +484,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > > #define DRM_DEV_ERROR(dev, fmt, ...) \
> > > drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
> > >
> > > -/**
> > > +/*
> > > * Rate limited error output. Like DRM_ERROR() but won't flood the log.
> > > *
> > > * @dev: device pointer
> > > @@ -436,7 +512,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > > } \
> > > })
> > >
> > > -/**
> > > +/*
> > > * Debug output.
> > > *
> > > * @dev: device pointer
> > > @@ -466,7 +542,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > > drm_dev_dbg(dev, category, fmt, ##__VA_ARGS__); \
> > > })
> > >
> > > -/**
> > > +/*
> > > * Rate limited debug output. Like DRM_DEBUG() but won't flood the log.
> > > *
> > > * @dev: device pointer
> > > --
> > > 2.20.1
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/print: document DRM_ logging functions
2020-01-08 18:49 ` Daniel Vetter
@ 2020-01-08 20:04 ` Sam Ravnborg
2020-01-08 20:50 ` Joe Perches
2020-01-12 22:48 ` Daniel Vetter
0 siblings, 2 replies; 10+ messages in thread
From: Sam Ravnborg @ 2020-01-08 20:04 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Jani Nikula, Joe Perches, Sean Paul, dri-devel
Hi Daniel.
> > >
> > > I'd replace the entire block with a "This stuff is deprecated" warning. We
> > > have at least a corresponding todo.rst entry.
> >
> > We have many situations where no drm_device is available.
> > At least when you a buried in drm_panel patches.
> >
> > So it is either DRM_DEV_ERROR() or drv_err().
> > Which is why I have pushed for nicer drm_ variants of these...
>
> Huh, drm_panel indeed has no drm_device. And I guess we don't have a
> convenient excuse to add it ...
>
> > The todo entry only covers the nice new macros that Jani added
> > where we have a drm_device.
>
> I wonder whether for those cases we shouldn't just directly use the
> various dev_* macros?
We would miss the nice [drm] marker in the logging.
So [drm] will be added by the drivers and the core - but not the panels.
That is the only drawback I see right now.
Which was enough justification for me to add the drm_dev_ variants.
Feel free to convince me that this is not justification to add these
variants.
In drm/panel/* there is no use of DRM_DEBUG* - and there is no
reason to introduce the variants we can filer with drm.debug.
There is a single DRM_DEBUG() user, which does not count here.
We could introduce only:
drm_dev_(err|warn|info|debug) - and not the more specialized variants.
Then we avoid that people make shortcuts and use drm_dev_dbg_kms() when
they are supposed to use drm_dbg_kms().
This was one of the very valid argumest against the patch that
introduced all the drm_dev_* variants.
Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/print: document DRM_ logging functions
2020-01-08 20:04 ` Sam Ravnborg
@ 2020-01-08 20:50 ` Joe Perches
2020-01-12 22:48 ` Daniel Vetter
1 sibling, 0 replies; 10+ messages in thread
From: Joe Perches @ 2020-01-08 20:50 UTC (permalink / raw)
To: Sam Ravnborg, Daniel Vetter; +Cc: Jani Nikula, Sean Paul, dri-devel
On Wed, 2020-01-08 at 21:04 +0100, Sam Ravnborg wrote:
> Hi Daniel.
> > > > I'd replace the entire block with a "This stuff is deprecated" warning. We
> > > > have at least a corresponding todo.rst entry.
> > >
> > > We have many situations where no drm_device is available.
> > > At least when you a buried in drm_panel patches.
> > >
> > > So it is either DRM_DEV_ERROR() or drv_err().
> > > Which is why I have pushed for nicer drm_ variants of these...
> >
> > Huh, drm_panel indeed has no drm_device. And I guess we don't have a
> > convenient excuse to add it ...
> >
> > > The todo entry only covers the nice new macros that Jani added
> > > where we have a drm_device.
> >
> > I wonder whether for those cases we shouldn't just directly use the
> > various dev_* macros?
>
> We would miss the nice [drm] marker in the logging.
> So [drm] will be added by the drivers and the core - but not the panels.
> That is the only drawback I see right now.
>
> Which was enough justification for me to add the drm_dev_ variants.
> Feel free to convince me that this is not justification to add these
> variants.
>
> In drm/panel/* there is no use of DRM_DEBUG* - and there is no
> reason to introduce the variants we can filer with drm.debug.
>
> There is a single DRM_DEBUG() user, which does not count here.
>
>
> We could introduce only:
>
> drm_dev_(err|warn|info|debug) - and not the more specialized variants.
>
> Then we avoid that people make shortcuts and use drm_dev_dbg_kms() when
> they are supposed to use drm_dbg_kms().
> This was one of the very valid argumest against the patch that
> introduced all the drm_dev_* variants.
A few of these defines aren't used at all.
$ git grep -P -oh "DRM_DEV_DEBUG[A-Z_]*" | sort | uniq -c | sort -rn
98 DRM_DEV_DEBUG_KMS
38 DRM_DEV_DEBUG_DRIVER
26 DRM_DEV_DEBUG
2 DRM_DEV_DEBUG_RATELIMITED
2 DRM_DEV_DEBUG_PRIME_RATELIMITED
2 DRM_DEV_DEBUG_KMS_RATELIMITED
2 DRM_DEV_DEBUG_DRIVER_RATELIMITED
1 DRM_DEV_DEBUG_VBL
1 DRM_DEV_DEBUG_PRIME
1 DRM_DEV_DEBUG_DP
1 DRM_DEV_DEBUG_ATOMIC
It might be sensible to consolidate these into just 2 calls
and add an appropriate <type> argument.
DRM_DEV_DEBUG(dev, type, fmt, ...)
DRM_DEV_DEBUG_RATELIMITED(dev, type, fmt, ...)
or
drm_dev_dbg(dev, type, fmt, ...)
drm_dev_dbg_ratelimited(dev, type, fmt, ...)
A treewide sed is trivial.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] drm/print: document DRM_ logging functions
2020-01-08 20:04 ` Sam Ravnborg
2020-01-08 20:50 ` Joe Perches
@ 2020-01-12 22:48 ` Daniel Vetter
1 sibling, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2020-01-12 22:48 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Jani Nikula, dri-devel, Joe Perches, Sean Paul
On Wed, Jan 08, 2020 at 09:04:16PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
> > > >
> > > > I'd replace the entire block with a "This stuff is deprecated" warning. We
> > > > have at least a corresponding todo.rst entry.
> > >
> > > We have many situations where no drm_device is available.
> > > At least when you a buried in drm_panel patches.
> > >
> > > So it is either DRM_DEV_ERROR() or drv_err().
> > > Which is why I have pushed for nicer drm_ variants of these...
> >
> > Huh, drm_panel indeed has no drm_device. And I guess we don't have a
> > convenient excuse to add it ...
> >
> > > The todo entry only covers the nice new macros that Jani added
> > > where we have a drm_device.
> >
> > I wonder whether for those cases we shouldn't just directly use the
> > various dev_* macros?
>
> We would miss the nice [drm] marker in the logging.
> So [drm] will be added by the drivers and the core - but not the panels.
> That is the only drawback I see right now.
>
> Which was enough justification for me to add the drm_dev_ variants.
> Feel free to convince me that this is not justification to add these
> variants.
tbh I don't mind, I just don't want to have a massive proliferation of drm
debugging functions, all kinda alike but not the same. If we can settle on
some color choice for this bikeshed and actually reduce the not-longer
favoured version, I'm pretty much ok with anything.
> In drm/panel/* there is no use of DRM_DEBUG* - and there is no
> reason to introduce the variants we can filer with drm.debug.
>
> There is a single DRM_DEBUG() user, which does not count here.
>
>
> We could introduce only:
>
> drm_dev_(err|warn|info|debug) - and not the more specialized variants.
>
> Then we avoid that people make shortcuts and use drm_dev_dbg_kms() when
> they are supposed to use drm_dbg_kms().
> This was one of the very valid argumest against the patch that
> introduced all the drm_dev_* variants.
Hm, if you want something for panels, what about a drm_panel_* set of
functions? Plus ofc patches to just roll them out everywhere (it should be
a simple sed/cocci job, drm/panel/* isn't that big). Some churn, but yay!
for consisteny at least.
If we have another set of generic drm debug/logging functions then I think
consistency is going to take a big toll, and we're already bad with this.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-01-12 22:48 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-02 22:15 [PATCH v2 0/2] drm: document logging functions Sam Ravnborg
2020-01-02 22:15 ` [PATCH v2 1/2] drm/print: document drm_ " Sam Ravnborg
2020-01-07 16:12 ` Daniel Vetter
2020-01-02 22:15 ` [PATCH v2 2/2] drm/print: document DRM_ " Sam Ravnborg
2020-01-07 16:08 ` Daniel Vetter
2020-01-07 18:17 ` Sam Ravnborg
2020-01-08 18:49 ` Daniel Vetter
2020-01-08 20:04 ` Sam Ravnborg
2020-01-08 20:50 ` Joe Perches
2020-01-12 22:48 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.