On Mon, Oct 28, 2019 at 12:38:22PM +0200, Jani Nikula wrote: > 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. Looks reasonable enough to me. For the series Acked-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> One thing that still bugs me about drm_dbg() is that the category check is inside it. So we pay the function call overhead for every debug statement even when debugging is disabled. So I'd also like to see a patch moving the category check back into the macros/static inline (IIRC it was there originally). > --- > 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__) > + > + > +#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 -- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
