diff mbox series

[1/8] drm/print: introduce new struct drm_device based logging macros

Message ID 20191210123050.8799-1-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/8] drm/print: introduce new struct drm_device based logging macros | expand

Commit Message

Jani Nikula Dec. 10, 2019, 12:30 p.m. UTC
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@ravnborg.org>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Sean Paul <sean@poorly.run>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

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(+)

Comments

Jani Nikula Dec. 10, 2019, 12:34 p.m. UTC | #1
On Tue, 10 Dec 2019, Jani Nikula <jani.nikula@intel.com> 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.

As to cover letter, patches 2-8 using the logging macros introduced here
are just the beginning. It's not trivial to write a cocci script to dig
up struct drm_device * where there is none, so much of it may need to be
done manually. But we could start here.

BR,
Jani.
Daniel Vetter Dec. 10, 2019, 10:07 p.m. UTC | #2
On Tue, Dec 10, 2019 at 02:34:33PM +0200, Jani Nikula wrote:
> On Tue, 10 Dec 2019, Jani Nikula <jani.nikula@intel.com> 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.
> 
> As to cover letter, patches 2-8 using the logging macros introduced here
> are just the beginning. It's not trivial to write a cocci script to dig
> up struct drm_device * where there is none, so much of it may need to be
> done manually. But we could start here.

Can you pls do a patch for todo.rst to adjust the existing task to the new
flavour of favorited debug printing? If cocci doesn't work we can at least
throw random interns at this for starter tasks :-)
-Daniel
Sam Ravnborg Dec. 12, 2019, 9:53 p.m. UTC | #3
Hi Jani.

On Tue, Dec 10, 2019 at 02:30:43PM +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@ravnborg.org>
> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Acked-by: Sean Paul <sean@poorly.run>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

To my sensitive eyes the lower case variants are much preferable.

As a follow-up it could be nice to clean up drm_print.h:
- Make it obvious that the old variants are deprecated
- Let the old variants use the new variants - to make it obvious they
  are obsolete wrappers.
- Add some intro that explains for newbies when to use what variant

And then add a todo item - so we can get some janitorials to help with the
conversion to the new varaints.


For logging we have three cases:
- We have a drm_device pointer - nicely covered by this patchset
- We have a device * - what do we do here?
- We have no pointers to device nor drm_device - what do we do here?

Would it be OK to consider drm variants for all the above - so we get
consistent prefix on logging?

Idea:

drm_<level>[_system] - example: drm_info(drm_device *, ..) or drm_info_core(drm_device *, ..)

drm_dev_<level>[_system] - example: drm_dev_info(device *, ..)

drm_pr_<level>[_system] - example: drm_pr_info(..)

level could be info, info_once, info_ratelimited and so on for dbg, err,
notice, warn

With the above I can see we can make a clean shift to drm based logging.
And we do not need to mix different ways to log stuf.

The preferred:
drm_info()
drm_dev_info()
drm_pr_info()

versus:
drm_info()
dev_info()
pr_info()

The patch is OK without the suggested change, but see this as
suggestions for improvements.
So patch as is has my:
Acked-by: Sam Ravnborg <sam@ravnborg.org>


	Sam


> 
> ---
> 
> 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..8f99d389792d 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
Jani Nikula Dec. 13, 2019, 2:41 p.m. UTC | #4
On Thu, 12 Dec 2019, Sam Ravnborg <sam@ravnborg.org> wrote:
> Hi Jani.
>
> On Tue, Dec 10, 2019 at 02:30:43PM +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@ravnborg.org>
>> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Acked-by: Sean Paul <sean@poorly.run>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> To my sensitive eyes the lower case variants are much preferable.
>
> As a follow-up it could be nice to clean up drm_print.h:
> - Make it obvious that the old variants are deprecated
> - Let the old variants use the new variants - to make it obvious they
>   are obsolete wrappers.
> - Add some intro that explains for newbies when to use what variant
>
> And then add a todo item - so we can get some janitorials to help with the
> conversion to the new varaints.
>
>
> For logging we have three cases:
> - We have a drm_device pointer - nicely covered by this patchset
> - We have a device * - what do we do here?
> - We have no pointers to device nor drm_device - what do we do here?
>
> Would it be OK to consider drm variants for all the above - so we get
> consistent prefix on logging?
>
> Idea:
>
> drm_<level>[_system] - example: drm_info(drm_device *, ..) or drm_info_core(drm_device *, ..)
>
> drm_dev_<level>[_system] - example: drm_dev_info(device *, ..)
>
> drm_pr_<level>[_system] - example: drm_pr_info(..)

The main concern I have here is that converting the last two will lead
to a lot of churn with no functional change. And instead of three sets
of functions, we'd actually have five, until all of it is converted.

Also it seems to me adding the last two will validate their widespread
use, while personally I think the focus should be on transforming to use
the struct drm_device based functions introduced in this series.

Maybe we should wait until we have enough code converted to the struct
drm_device based functions, so that the rest could perhaps be converted
en masse using cocci?

Other than that, I could easily get behind your proposal.

> level could be info, info_once, info_ratelimited and so on for dbg, err,
> notice, warn
>
> With the above I can see we can make a clean shift to drm based logging.
> And we do not need to mix different ways to log stuf.
>
> The preferred:
> drm_info()
> drm_dev_info()
> drm_pr_info()
>
> versus:
> drm_info()
> dev_info()
> pr_info()
>
> The patch is OK without the suggested change, but see this as
> suggestions for improvements.
> So patch as is has my:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Thanks, appreciated!

Still, while I'd really just like to merge this now and get done with
it, I think I'd rather have more acks behind this (at the risk of
soliciting bikeshedding...) so nobody feels like I'm sneaking this in.

BR,
Jani.


>
>
> 	Sam
>
>
>> 
>> ---
>> 
>> 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..8f99d389792d 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
Jani Nikula Dec. 17, 2019, 2:45 p.m. UTC | #5
On Tue, 10 Dec 2019, Jani Nikula <jani.nikula@intel.com> 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@ravnborg.org>
> Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Acked-by: Sean Paul <sean@poorly.run>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Pushed this one patch, thanks for the reviews and acks. The rest could
still use review. ;)

BR,
Jani.


>
> ---
>
> 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..8f99d389792d 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)
Sam Ravnborg Dec. 17, 2019, 4:11 p.m. UTC | #6
Hi Jani.

Will you type a todo entry as requested by Daniel?

> Pushed this one patch, thanks for the reviews and acks. The rest could
> still use review. ;)
Will take a look again later tonight.
I recall from last time I looked they were fine but I just never sent
anything to the ML.

	Sam
diff mbox series

Patch

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 085a9685270c..8f99d389792d 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)