>-----Original Message----- >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of >Jani Nikula >Sent: Wednesday, October 9, 2019 11:38 AM >To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx >Cc: Nikula, Jani <jani.nikula@xxxxxxxxx>; Sam Ravnborg <sam@xxxxxxxxxxxx> >Subject: [Intel-gfx] [PATCH 8/8] drm/print: introduce new struct drm_device >based logging macros > >Add new struct drm_device based logging macros modeled after the core >kernel device based logging macros. These would be preferred over the >drm printk and struct device based macros in drm code, where possible. > >We have existing drm specific struct device based logging functions, but >they are too verbose to use for two main reasons: > > * The names are unnecessarily long, for example DRM_DEV_DEBUG_KMS(). > > * The use of struct device over struct drm_device is too generic for > most users, leading to an extra dereference. > >For example: > > DRM_DEV_DEBUG_KMS(drm->dev, "Hello, world\n"); > >vs. > > drm_dbg_kms(drm, "Hello, world\n"); > >It's a matter of taste, but the SHOUTING UPPERCASE has been argued to be >less readable than lowercase. > >Some names are changed from old DRM names to be based on the core >kernel >logging functions. For example, NOTE -> notice, ERROR -> err, DEBUG -> >dbg. > >Due to the conflation of DRM_DEBUG and DRM_DEBUG_DRIVER macro use >(DRM_DEBUG is used widely in drivers though it's supposed to be a core >debugging category), they are named as drm_dbg_core and drm_dbg, >respectively. > >The drm_err and _once/_ratelimited variants no longer include the >function name in order to be able to use the core device based logging >macros. Arguably this is not a significant change; error messages should >not be so common to be only distinguishable by the function name. > >Ratelimited debug logging macros are to be added later. > >Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> >Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > >--- > >With something like this, I think i915 could start migrating to >drm_device based logging. I have a hard time convincing myself or anyone >about migrating to the DRM_DEV_* variants. >--- > include/drm/drm_print.h | 65 >+++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > >diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h >index 085a9685270c..e4040dea0d8c 100644 >--- a/include/drm/drm_print.h >+++ b/include/drm/drm_print.h >@@ -322,6 +322,8 @@ static inline bool drm_debug_enabled(enum >drm_debug_category category) > > /* > * struct device based logging >+ * >+ * Prefer drm_device based logging over device or prink based logging. > */ > > __printf(3, 4) >@@ -417,8 +419,71 @@ void drm_dev_dbg(const struct device *dev, enum >drm_debug_category category, > _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, DRM_UT_PRIME, > \ > fmt, ##__VA_ARGS__) > >+/* >+ * struct drm_device based logging >+ * >+ * Prefer drm_device based logging over device or prink based logging. >+ */ >+ >+/* Helper for struct drm_device based logging. */ >+#define __drm_printk(drm, level, type, fmt, ...) \ >+ dev_##level##type(drm->dev, "[drm] " fmt, ##__VA_ARGS__) In the past, I have been able to do a: #undef pr_fmt #define pr_fmt(fmt) "[myinfo here] " fmt And have the "[myinfo here]" portion show up the output. Is it possible that you might be able to use this instead of "[drm] " fmt? I think that the this will be the same result, but might be more in line with the printk interface? Mike >+ >+ >+#define drm_info(drm, fmt, ...) \ >+ __drm_printk(drm, info,, fmt, ##__VA_ARGS__) >+ >+#define drm_notice(drm, fmt, ...) \ >+ __drm_printk(drm, notice,, fmt, ##__VA_ARGS__) >+ >+#define drm_warn(drm, fmt, ...) \ >+ __drm_printk(drm, warn,, fmt, ##__VA_ARGS__) >+ >+#define drm_err(drm, fmt, ...) \ >+ __drm_printk(drm, err,, "*ERROR* " fmt, ##__VA_ARGS__) >+ >+ >+#define drm_info_once(drm, fmt, ...) \ >+ __drm_printk(drm, info, _once, fmt, ##__VA_ARGS__) >+ >+#define drm_notice_once(drm, fmt, ...) \ >+ __drm_printk(drm, notice, _once, fmt, ##__VA_ARGS__) >+ >+#define drm_warn_once(drm, fmt, ...) \ >+ __drm_printk(drm, warn, _once, fmt, ##__VA_ARGS__) >+ >+#define drm_err_once(drm, fmt, ...) \ >+ __drm_printk(drm, err, _once, "*ERROR* " fmt, ##__VA_ARGS__) >+ >+ >+#define drm_err_ratelimited(drm, fmt, ...) \ >+ __drm_printk(drm, err, _ratelimited, "*ERROR* " fmt, >##__VA_ARGS__) >+ >+ >+#define drm_dbg_core(drm, fmt, ...) \ >+ drm_dev_dbg(drm->dev, DRM_UT_CORE, fmt, ##__VA_ARGS__) >+#define drm_dbg(drm, fmt, ...) > \ >+ drm_dev_dbg(drm->dev, DRM_UT_DRIVER, fmt, ##__VA_ARGS__) >+#define drm_dbg_kms(drm, fmt, ...) \ >+ drm_dev_dbg(drm->dev, DRM_UT_KMS, fmt, ##__VA_ARGS__) >+#define drm_dbg_prime(drm, fmt, ...) > \ >+ drm_dev_dbg(drm->dev, DRM_UT_PRIME, fmt, ##__VA_ARGS__) >+#define drm_dbg_atomic(drm, fmt, ...) > \ >+ drm_dev_dbg(drm->dev, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) >+#define drm_dbg_vbl(drm, fmt, ...) \ >+ drm_dev_dbg(drm->dev, DRM_UT_VBL, fmt, ##__VA_ARGS__) >+#define drm_dbg_state(drm, fmt, ...) \ >+ drm_dev_dbg(drm->dev, DRM_UT_STATE, fmt, ##__VA_ARGS__) >+#define drm_dbg_lease(drm, fmt, ...) \ >+ drm_dev_dbg(drm->dev, DRM_UT_LEASE, fmt, ##__VA_ARGS__) >+#define drm_dbg_dp(drm, fmt, ...) \ >+ drm_dev_dbg(drm->dev, DRM_UT_DP, fmt, ##__VA_ARGS__) >+ >+ > /* > * printk based logging >+ * >+ * Prefer drm_device based logging over device or prink based logging. > */ > > __printf(2, 3) >-- >2.20.1 > >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
