* [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions
@ 2021-09-21 15:28 Douglas Anderson
2021-09-22 7:12 ` Thomas Zimmermann
2021-09-24 21:59 ` Doug Anderson
0 siblings, 2 replies; 6+ messages in thread
From: Douglas Anderson @ 2021-09-21 15:28 UTC (permalink / raw)
To: dri-devel
Cc: sam, daniel.vetter, lyude, jani.nikula, swboyd, airlied,
Douglas Anderson, Daniel Vetter, David Airlie, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, linux-kernel
It's hard for someone (like me) who's not following closely to know
what the suggested best practices are for error printing in DRM
drivers. Add some hints to the header file.
In general, my understanding is that:
* When possible we should be using a `struct drm_device` for logging
and recent patches have tried to make it more possible to access a
relevant `struct drm_device` in more places.
* For most cases when we don't have a `struct drm_device`, we no
longer bother with DRM-specific wrappers on the dev_...() functions
or pr_...() functions and just encourage drivers to use the normal
functions.
* For debug-level functions where we might want filtering based on a
category we'll still have DRM-specific wrappers, but we'll only
support passing a `struct drm_device`, not a `struct
device`. Presumably most of the cases where we want the filtering
are messages that happen while the system is in a normal running
state (AKA not during probe time) and we should have a `struct
drm_device` then. If we absolutely can't get a `struct drm_device`
then these functions begrudgingly accept NULL for the `struct
drm_device` and hopefully the awkwardness of having to manually pass
NULL will keep people from doing this unless absolutely necessary.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
include/drm/drm_print.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 15a089a87c22..22fabdeed297 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -340,6 +340,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
/**
* DRM_DEV_ERROR() - Error output.
*
+ * NOTE: this is deprecated in favor of drm_err() or dev_err().
+ *
* @dev: device pointer
* @fmt: printf() like format string.
*/
@@ -349,6 +351,9 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
/**
* DRM_DEV_ERROR_RATELIMITED() - Rate limited error output.
*
+ * NOTE: this is deprecated in favor of drm_err_ratelimited() or
+ * dev_err_ratelimited().
+ *
* @dev: device pointer
* @fmt: printf() like format string.
*
@@ -364,9 +369,11 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
DRM_DEV_ERROR(dev, fmt, ##__VA_ARGS__); \
})
+/* NOTE: this is deprecated in favor of drm_info() or dev_info(). */
#define DRM_DEV_INFO(dev, fmt, ...) \
drm_dev_printk(dev, KERN_INFO, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_info_once() or dev_info_once(). */
#define DRM_DEV_INFO_ONCE(dev, fmt, ...) \
({ \
static bool __print_once __read_mostly; \
@@ -379,6 +386,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
/**
* DRM_DEV_DEBUG() - Debug output for generic drm code
*
+ * NOTE: this is deprecated in favor of drm_dbg_core().
+ *
* @dev: device pointer
* @fmt: printf() like format string.
*/
@@ -387,6 +396,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
/**
* DRM_DEV_DEBUG_DRIVER() - Debug output for vendor specific part of the driver
*
+ * NOTE: this is deprecated in favor of drm_dbg() or dev_dbg().
+ *
* @dev: device pointer
* @fmt: printf() like format string.
*/
@@ -395,6 +406,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
/**
* DRM_DEV_DEBUG_KMS() - Debug output for modesetting code
*
+ * NOTE: this is deprecated in favor of drm_dbg_kms().
+ *
* @dev: device pointer
* @fmt: printf() like format string.
*/
@@ -480,47 +493,63 @@ void __drm_err(const char *format, ...);
#define _DRM_PRINTK(once, level, fmt, ...) \
printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_info(). */
#define DRM_INFO(fmt, ...) \
_DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_notice(). */
#define DRM_NOTE(fmt, ...) \
_DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_warn(). */
#define DRM_WARN(fmt, ...) \
_DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_info_once(). */
#define DRM_INFO_ONCE(fmt, ...) \
_DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_notice_once(). */
#define DRM_NOTE_ONCE(fmt, ...) \
_DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_warn_once(). */
#define DRM_WARN_ONCE(fmt, ...) \
_DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_err(). */
#define DRM_ERROR(fmt, ...) \
__drm_err(fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of pr_err_ratelimited(). */
#define DRM_ERROR_RATELIMITED(fmt, ...) \
DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_core(NULL, ...). */
#define DRM_DEBUG(fmt, ...) \
__drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */
#define DRM_DEBUG_DRIVER(fmt, ...) \
__drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_kms(NULL, ...). */
#define DRM_DEBUG_KMS(fmt, ...) \
__drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_prime(NULL, ...). */
#define DRM_DEBUG_PRIME(fmt, ...) \
__drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_atomic(NULL, ...). */
#define DRM_DEBUG_ATOMIC(fmt, ...) \
__drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_vbl(NULL, ...). */
#define DRM_DEBUG_VBL(fmt, ...) \
__drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_lease(NULL, ...). */
#define DRM_DEBUG_LEASE(fmt, ...) \
__drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_dp(NULL, ...). */
#define DRM_DEBUG_DP(fmt, ...) \
__drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
@@ -536,6 +565,7 @@ void __drm_err(const char *format, ...);
#define drm_dbg_kms_ratelimited(drm, fmt, ...) \
__DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
+/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL, ...). */
#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)
/*
--
2.33.0.464.g1972c5931b-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions
2021-09-21 15:28 [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions Douglas Anderson
@ 2021-09-22 7:12 ` Thomas Zimmermann
2021-09-24 18:16 ` Lyude Paul
2021-09-24 21:59 ` Doug Anderson
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Zimmermann @ 2021-09-22 7:12 UTC (permalink / raw)
To: Douglas Anderson, dri-devel
Cc: sam, daniel.vetter, lyude, jani.nikula, swboyd, airlied,
Daniel Vetter, David Airlie, Maarten Lankhorst, Maxime Ripard,
linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 7488 bytes --]
Hi
Am 21.09.21 um 17:28 schrieb Douglas Anderson:
> It's hard for someone (like me) who's not following closely to know
> what the suggested best practices are for error printing in DRM
> drivers. Add some hints to the header file.
>
> In general, my understanding is that:
> * When possible we should be using a `struct drm_device` for logging
> and recent patches have tried to make it more possible to access a
> relevant `struct drm_device` in more places.
> * For most cases when we don't have a `struct drm_device`, we no
> longer bother with DRM-specific wrappers on the dev_...() functions
> or pr_...() functions and just encourage drivers to use the normal
> functions.
> * For debug-level functions where we might want filtering based on a
> category we'll still have DRM-specific wrappers, but we'll only
> support passing a `struct drm_device`, not a `struct
> device`. Presumably most of the cases where we want the filtering
> are messages that happen while the system is in a normal running
> state (AKA not during probe time) and we should have a `struct
> drm_device` then. If we absolutely can't get a `struct drm_device`
> then these functions begrudgingly accept NULL for the `struct
> drm_device` and hopefully the awkwardness of having to manually pass
> NULL will keep people from doing this unless absolutely necessary.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Thanks a lot.
> ---
>
> include/drm/drm_print.h | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 15a089a87c22..22fabdeed297 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -340,6 +340,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> /**
> * DRM_DEV_ERROR() - Error output.
> *
> + * NOTE: this is deprecated in favor of drm_err() or dev_err().
> + *
> * @dev: device pointer
> * @fmt: printf() like format string.
> */
> @@ -349,6 +351,9 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> /**
> * DRM_DEV_ERROR_RATELIMITED() - Rate limited error output.
> *
> + * NOTE: this is deprecated in favor of drm_err_ratelimited() or
> + * dev_err_ratelimited().
> + *
> * @dev: device pointer
> * @fmt: printf() like format string.
> *
> @@ -364,9 +369,11 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> DRM_DEV_ERROR(dev, fmt, ##__VA_ARGS__); \
> })
>
> +/* NOTE: this is deprecated in favor of drm_info() or dev_info(). */
> #define DRM_DEV_INFO(dev, fmt, ...) \
> drm_dev_printk(dev, KERN_INFO, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_info_once() or dev_info_once(). */
> #define DRM_DEV_INFO_ONCE(dev, fmt, ...) \
> ({ \
> static bool __print_once __read_mostly; \
> @@ -379,6 +386,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> /**
> * DRM_DEV_DEBUG() - Debug output for generic drm code
> *
> + * NOTE: this is deprecated in favor of drm_dbg_core().
> + *
> * @dev: device pointer
> * @fmt: printf() like format string.
> */
> @@ -387,6 +396,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> /**
> * DRM_DEV_DEBUG_DRIVER() - Debug output for vendor specific part of the driver
> *
> + * NOTE: this is deprecated in favor of drm_dbg() or dev_dbg().
> + *
> * @dev: device pointer
> * @fmt: printf() like format string.
> */
> @@ -395,6 +406,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> /**
> * DRM_DEV_DEBUG_KMS() - Debug output for modesetting code
> *
> + * NOTE: this is deprecated in favor of drm_dbg_kms().
> + *
> * @dev: device pointer
> * @fmt: printf() like format string.
> */
> @@ -480,47 +493,63 @@ void __drm_err(const char *format, ...);
> #define _DRM_PRINTK(once, level, fmt, ...) \
> printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of pr_info(). */
> #define DRM_INFO(fmt, ...) \
> _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__)
> +/* NOTE: this is deprecated in favor of pr_notice(). */
> #define DRM_NOTE(fmt, ...) \
> _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__)
> +/* NOTE: this is deprecated in favor of pr_warn(). */
> #define DRM_WARN(fmt, ...) \
> _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of pr_info_once(). */
> #define DRM_INFO_ONCE(fmt, ...) \
> _DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__)
> +/* NOTE: this is deprecated in favor of pr_notice_once(). */
> #define DRM_NOTE_ONCE(fmt, ...) \
> _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
> +/* NOTE: this is deprecated in favor of pr_warn_once(). */
> #define DRM_WARN_ONCE(fmt, ...) \
> _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of pr_err(). */
> #define DRM_ERROR(fmt, ...) \
> __drm_err(fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of pr_err_ratelimited(). */
> #define DRM_ERROR_RATELIMITED(fmt, ...) \
> DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_core(NULL, ...). */
> #define DRM_DEBUG(fmt, ...) \
> __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */
> #define DRM_DEBUG_DRIVER(fmt, ...) \
> __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_kms(NULL, ...). */
> #define DRM_DEBUG_KMS(fmt, ...) \
> __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_prime(NULL, ...). */
> #define DRM_DEBUG_PRIME(fmt, ...) \
> __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_atomic(NULL, ...). */
> #define DRM_DEBUG_ATOMIC(fmt, ...) \
> __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_vbl(NULL, ...). */
> #define DRM_DEBUG_VBL(fmt, ...) \
> __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_lease(NULL, ...). */
> #define DRM_DEBUG_LEASE(fmt, ...) \
> __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_dp(NULL, ...). */
> #define DRM_DEBUG_DP(fmt, ...) \
> __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
>
> @@ -536,6 +565,7 @@ void __drm_err(const char *format, ...);
> #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
> __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
>
> +/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL, ...). */
> #define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)
>
> /*
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions
2021-09-22 7:12 ` Thomas Zimmermann
@ 2021-09-24 18:16 ` Lyude Paul
0 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2021-09-24 18:16 UTC (permalink / raw)
To: Thomas Zimmermann, Douglas Anderson, dri-devel
Cc: sam, daniel.vetter, jani.nikula, swboyd, airlied, Daniel Vetter,
David Airlie, Maarten Lankhorst, Maxime Ripard, linux-kernel
Acked-by: Lyude Paul <lyude@redhat.com>
On Wed, 2021-09-22 at 09:12 +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 21.09.21 um 17:28 schrieb Douglas Anderson:
> > It's hard for someone (like me) who's not following closely to know
> > what the suggested best practices are for error printing in DRM
> > drivers. Add some hints to the header file.
> >
> > In general, my understanding is that:
> > * When possible we should be using a `struct drm_device` for logging
> > and recent patches have tried to make it more possible to access a
> > relevant `struct drm_device` in more places.
> > * For most cases when we don't have a `struct drm_device`, we no
> > longer bother with DRM-specific wrappers on the dev_...() functions
> > or pr_...() functions and just encourage drivers to use the normal
> > functions.
> > * For debug-level functions where we might want filtering based on a
> > category we'll still have DRM-specific wrappers, but we'll only
> > support passing a `struct drm_device`, not a `struct
> > device`. Presumably most of the cases where we want the filtering
> > are messages that happen while the system is in a normal running
> > state (AKA not during probe time) and we should have a `struct
> > drm_device` then. If we absolutely can't get a `struct drm_device`
> > then these functions begrudgingly accept NULL for the `struct
> > drm_device` and hopefully the awkwardness of having to manually pass
> > NULL will keep people from doing this unless absolutely necessary.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> Thanks a lot.
>
> > ---
> >
> > include/drm/drm_print.h | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> > index 15a089a87c22..22fabdeed297 100644
> > --- a/include/drm/drm_print.h
> > +++ b/include/drm/drm_print.h
> > @@ -340,6 +340,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_ERROR() - Error output.
> > *
> > + * NOTE: this is deprecated in favor of drm_err() or dev_err().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -349,6 +351,9 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_ERROR_RATELIMITED() - Rate limited error output.
> > *
> > + * NOTE: this is deprecated in favor of drm_err_ratelimited() or
> > + * dev_err_ratelimited().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > *
> > @@ -364,9 +369,11 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > DRM_DEV_ERROR(dev, fmt, ##__VA_ARGS__); \
> > })
> >
> > +/* NOTE: this is deprecated in favor of drm_info() or dev_info(). */
> > #define DRM_DEV_INFO(dev, fmt, ...) \
> > drm_dev_printk(dev, KERN_INFO, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_info_once() or
> > dev_info_once(). */
> > #define DRM_DEV_INFO_ONCE(dev, fmt, ...) \
> > ({ \
> > static bool __print_once __read_mostly; \
> > @@ -379,6 +386,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_DEBUG() - Debug output for generic drm code
> > *
> > + * NOTE: this is deprecated in favor of drm_dbg_core().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -387,6 +396,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_DEBUG_DRIVER() - Debug output for vendor specific part of the
> > driver
> > *
> > + * NOTE: this is deprecated in favor of drm_dbg() or dev_dbg().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -395,6 +406,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_DEBUG_KMS() - Debug output for modesetting code
> > *
> > + * NOTE: this is deprecated in favor of drm_dbg_kms().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -480,47 +493,63 @@ void __drm_err(const char *format, ...);
> > #define _DRM_PRINTK(once, level, fmt, ...) \
> > printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_info(). */
> > #define DRM_INFO(fmt, ...) \
> > _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_notice(). */
> > #define DRM_NOTE(fmt, ...) \
> > _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_warn(). */
> > #define DRM_WARN(fmt, ...) \
> > _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_info_once(). */
> > #define DRM_INFO_ONCE(fmt,
> > ...) \
> > _DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_notice_once(). */
> > #define DRM_NOTE_ONCE(fmt,
> > ...) \
> > _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_warn_once(). */
> > #define DRM_WARN_ONCE(fmt,
> > ...) \
> > _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_err(). */
> > #define DRM_ERROR(fmt, ...) \
> > __drm_err(fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_err_ratelimited(). */
> > #define DRM_ERROR_RATELIMITED(fmt,
> > ...) \
> > DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_core(NULL, ...). */
> > #define DRM_DEBUG(fmt, ...) \
> > __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */
> > #define DRM_DEBUG_DRIVER(fmt, ...) \
> > __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_kms(NULL, ...). */
> > #define DRM_DEBUG_KMS(fmt,
> > ...) \
> > __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_prime(NULL, ...). */
> > #define DRM_DEBUG_PRIME(fmt, ...) \
> > __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_atomic(NULL, ...). */
> > #define DRM_DEBUG_ATOMIC(fmt, ...) \
> > __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_vbl(NULL, ...). */
> > #define DRM_DEBUG_VBL(fmt,
> > ...) \
> > __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_lease(NULL, ...). */
> > #define DRM_DEBUG_LEASE(fmt, ...) \
> > __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_dp(NULL, ...). */
> > #define DRM_DEBUG_DP(fmt,
> > ...) \
> > __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
> >
> > @@ -536,6 +565,7 @@ void __drm_err(const char *format, ...);
> > #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
> > __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL,
> > ...). */
> > #define DRM_DEBUG_KMS_RATELIMITED(fmt, ...)
> > drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)
> >
> > /*
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions
@ 2021-09-24 18:16 ` Lyude Paul
0 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2021-09-24 18:16 UTC (permalink / raw)
To: Thomas Zimmermann, Douglas Anderson, dri-devel
Cc: sam, daniel.vetter, jani.nikula, swboyd, airlied, Daniel Vetter,
David Airlie, Maarten Lankhorst, Maxime Ripard, linux-kernel
Acked-by: Lyude Paul <lyude@redhat.com>
On Wed, 2021-09-22 at 09:12 +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 21.09.21 um 17:28 schrieb Douglas Anderson:
> > It's hard for someone (like me) who's not following closely to know
> > what the suggested best practices are for error printing in DRM
> > drivers. Add some hints to the header file.
> >
> > In general, my understanding is that:
> > * When possible we should be using a `struct drm_device` for logging
> > and recent patches have tried to make it more possible to access a
> > relevant `struct drm_device` in more places.
> > * For most cases when we don't have a `struct drm_device`, we no
> > longer bother with DRM-specific wrappers on the dev_...() functions
> > or pr_...() functions and just encourage drivers to use the normal
> > functions.
> > * For debug-level functions where we might want filtering based on a
> > category we'll still have DRM-specific wrappers, but we'll only
> > support passing a `struct drm_device`, not a `struct
> > device`. Presumably most of the cases where we want the filtering
> > are messages that happen while the system is in a normal running
> > state (AKA not during probe time) and we should have a `struct
> > drm_device` then. If we absolutely can't get a `struct drm_device`
> > then these functions begrudgingly accept NULL for the `struct
> > drm_device` and hopefully the awkwardness of having to manually pass
> > NULL will keep people from doing this unless absolutely necessary.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> Thanks a lot.
>
> > ---
> >
> > include/drm/drm_print.h | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> > index 15a089a87c22..22fabdeed297 100644
> > --- a/include/drm/drm_print.h
> > +++ b/include/drm/drm_print.h
> > @@ -340,6 +340,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_ERROR() - Error output.
> > *
> > + * NOTE: this is deprecated in favor of drm_err() or dev_err().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -349,6 +351,9 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_ERROR_RATELIMITED() - Rate limited error output.
> > *
> > + * NOTE: this is deprecated in favor of drm_err_ratelimited() or
> > + * dev_err_ratelimited().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > *
> > @@ -364,9 +369,11 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > DRM_DEV_ERROR(dev, fmt, ##__VA_ARGS__); \
> > })
> >
> > +/* NOTE: this is deprecated in favor of drm_info() or dev_info(). */
> > #define DRM_DEV_INFO(dev, fmt, ...) \
> > drm_dev_printk(dev, KERN_INFO, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_info_once() or
> > dev_info_once(). */
> > #define DRM_DEV_INFO_ONCE(dev, fmt, ...) \
> > ({ \
> > static bool __print_once __read_mostly; \
> > @@ -379,6 +386,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_DEBUG() - Debug output for generic drm code
> > *
> > + * NOTE: this is deprecated in favor of drm_dbg_core().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -387,6 +396,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_DEBUG_DRIVER() - Debug output for vendor specific part of the
> > driver
> > *
> > + * NOTE: this is deprecated in favor of drm_dbg() or dev_dbg().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -395,6 +406,8 @@ void drm_dev_dbg(const struct device *dev, enum
> > drm_debug_category category,
> > /**
> > * DRM_DEV_DEBUG_KMS() - Debug output for modesetting code
> > *
> > + * NOTE: this is deprecated in favor of drm_dbg_kms().
> > + *
> > * @dev: device pointer
> > * @fmt: printf() like format string.
> > */
> > @@ -480,47 +493,63 @@ void __drm_err(const char *format, ...);
> > #define _DRM_PRINTK(once, level, fmt, ...) \
> > printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_info(). */
> > #define DRM_INFO(fmt, ...) \
> > _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_notice(). */
> > #define DRM_NOTE(fmt, ...) \
> > _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_warn(). */
> > #define DRM_WARN(fmt, ...) \
> > _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_info_once(). */
> > #define DRM_INFO_ONCE(fmt,
> > ...) \
> > _DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_notice_once(). */
> > #define DRM_NOTE_ONCE(fmt,
> > ...) \
> > _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
> > +/* NOTE: this is deprecated in favor of pr_warn_once(). */
> > #define DRM_WARN_ONCE(fmt,
> > ...) \
> > _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_err(). */
> > #define DRM_ERROR(fmt, ...) \
> > __drm_err(fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of pr_err_ratelimited(). */
> > #define DRM_ERROR_RATELIMITED(fmt,
> > ...) \
> > DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_core(NULL, ...). */
> > #define DRM_DEBUG(fmt, ...) \
> > __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */
> > #define DRM_DEBUG_DRIVER(fmt, ...) \
> > __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_kms(NULL, ...). */
> > #define DRM_DEBUG_KMS(fmt,
> > ...) \
> > __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_prime(NULL, ...). */
> > #define DRM_DEBUG_PRIME(fmt, ...) \
> > __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_atomic(NULL, ...). */
> > #define DRM_DEBUG_ATOMIC(fmt, ...) \
> > __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_vbl(NULL, ...). */
> > #define DRM_DEBUG_VBL(fmt,
> > ...) \
> > __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_lease(NULL, ...). */
> > #define DRM_DEBUG_LEASE(fmt, ...) \
> > __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_dp(NULL, ...). */
> > #define DRM_DEBUG_DP(fmt,
> > ...) \
> > __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
> >
> > @@ -536,6 +565,7 @@ void __drm_err(const char *format, ...);
> > #define drm_dbg_kms_ratelimited(drm, fmt, ...) \
> > __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
> >
> > +/* NOTE: this is deprecated in favor of drm_dbg_kms_ratelimited(NULL,
> > ...). */
> > #define DRM_DEBUG_KMS_RATELIMITED(fmt, ...)
> > drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)
> >
> > /*
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions
2021-09-21 15:28 [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions Douglas Anderson
@ 2021-09-24 21:59 ` Doug Anderson
2021-09-24 21:59 ` Doug Anderson
1 sibling, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2021-09-24 21:59 UTC (permalink / raw)
To: dri-devel
Cc: Sam Ravnborg, Daniel Vetter, Lyude Paul, Jani Nikula,
Stephen Boyd, David Airlie, Daniel Vetter, David Airlie,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, LKML
Hi,
On Tue, Sep 21, 2021 at 8:28 AM Douglas Anderson <dianders@chromium.org> wrote:
>
> It's hard for someone (like me) who's not following closely to know
> what the suggested best practices are for error printing in DRM
> drivers. Add some hints to the header file.
>
> In general, my understanding is that:
> * When possible we should be using a `struct drm_device` for logging
> and recent patches have tried to make it more possible to access a
> relevant `struct drm_device` in more places.
> * For most cases when we don't have a `struct drm_device`, we no
> longer bother with DRM-specific wrappers on the dev_...() functions
> or pr_...() functions and just encourage drivers to use the normal
> functions.
> * For debug-level functions where we might want filtering based on a
> category we'll still have DRM-specific wrappers, but we'll only
> support passing a `struct drm_device`, not a `struct
> device`. Presumably most of the cases where we want the filtering
> are messages that happen while the system is in a normal running
> state (AKA not during probe time) and we should have a `struct
> drm_device` then. If we absolutely can't get a `struct drm_device`
> then these functions begrudgingly accept NULL for the `struct
> drm_device` and hopefully the awkwardness of having to manually pass
> NULL will keep people from doing this unless absolutely necessary.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> include/drm/drm_print.h | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
Landed in drm-misc-next:
306589856399 drm/print: Add deprecation notes to DRM_...() functions
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions
@ 2021-09-24 21:59 ` Doug Anderson
0 siblings, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2021-09-24 21:59 UTC (permalink / raw)
To: dri-devel
Cc: Sam Ravnborg, Daniel Vetter, Lyude Paul, Jani Nikula,
Stephen Boyd, David Airlie, Daniel Vetter, David Airlie,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, LKML
Hi,
On Tue, Sep 21, 2021 at 8:28 AM Douglas Anderson <dianders@chromium.org> wrote:
>
> It's hard for someone (like me) who's not following closely to know
> what the suggested best practices are for error printing in DRM
> drivers. Add some hints to the header file.
>
> In general, my understanding is that:
> * When possible we should be using a `struct drm_device` for logging
> and recent patches have tried to make it more possible to access a
> relevant `struct drm_device` in more places.
> * For most cases when we don't have a `struct drm_device`, we no
> longer bother with DRM-specific wrappers on the dev_...() functions
> or pr_...() functions and just encourage drivers to use the normal
> functions.
> * For debug-level functions where we might want filtering based on a
> category we'll still have DRM-specific wrappers, but we'll only
> support passing a `struct drm_device`, not a `struct
> device`. Presumably most of the cases where we want the filtering
> are messages that happen while the system is in a normal running
> state (AKA not during probe time) and we should have a `struct
> drm_device` then. If we absolutely can't get a `struct drm_device`
> then these functions begrudgingly accept NULL for the `struct
> drm_device` and hopefully the awkwardness of having to manually pass
> NULL will keep people from doing this unless absolutely necessary.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> include/drm/drm_print.h | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
Landed in drm-misc-next:
306589856399 drm/print: Add deprecation notes to DRM_...() functions
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-24 21:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21 15:28 [RFC PATCH] drm/print: Add deprecation notes to DRM_...() functions Douglas Anderson
2021-09-22 7:12 ` Thomas Zimmermann
2021-09-24 18:16 ` Lyude Paul
2021-09-24 18:16 ` Lyude Paul
2021-09-24 21:59 ` Doug Anderson
2021-09-24 21:59 ` Doug Anderson
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.