Acked-by: Lyude Paul <lyude@xxxxxxxxxx> 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@xxxxxxxxxxxx> > > Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > > 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