[Intel-gfx] [RFC 0/5] DRM logging tidy

[Robert Bragg]

On Tue, Dec 6, 2016 at 6:57 PM, Tvrtko Ursulin <tursulin at ursulin.net> wrote:

> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> I wasn't here at the beginnings of DRM so I might have gotten this wrong,
> however the existance of DRM_NAME suggested to me that the intention was to
> allow individual drivers to override it and get appropriate prefixes in
> their
> log messages.
>
> I can't see that any driver is using it like that but I still thought it
> would
> be neat to do that. That way we could have our log messages look more
> obviously ours. For example after this series we have:
>
>  [i915] Memory usable by graphics device = 4096M
>  [i915] VT-d active for gfx access
>  [i915] Replacing VGA console driver
>  [i915] ACPI BIOS requests an excessive sleep of 20000 ms, using 1500 ms
> instead
>  [i915] Finished loading DMC firmware i915/skl_dmc_ver1_26.bin (v1.26)
>  [i915] Disabling framebuffer compression (FBC) to prevent screen flicker
> with VT-d enabled
>  [i915] GuC firmware load skipped
>  [i915] Initialized i915 1.6.0 20161205 for 0000:00:02.0 on minor 0
>  [i915] DRM_I915_DEBUG enabled
>  [i915] DRM_I915_DEBUG_GEM enabled
>  [i915] RC6 on
>
> Previously all that was prefixed with "[drm]" which was OK but I think the
> above is even better.
>
> Also to consider is that recent drm_printk work has removed (it hardcoded)
> DRM_NAME from DRM_ERROR and DRM_DEBUG macros, while leaving it with the
> rest
> (DRM_INFO, NOTE and WARNING) creating a bit of a inconsistency.
>

I wonder if I can maybe fold some of this idea into my related DRM_DEBUG
[RFC] sent out recently:
https://lists.freedesktop.org/archives/dri-devel/2016-December/126094.html

Instead of using DRM_NAME, I've experimented with updating my changes
adding support for dynamic debug to add a prefix based on
module_name(THIS_MODULE) for a similar result

One thing to consider here is that with the addition of dynamic debug
support this prefix arguably becomes redundant because the
dynamic_debug/control interface lets you choose to add a module name or
function prefix to messages, e.g. like:

echo "module i915 +mfp" > dynamic_debug/control

I've ignored the redundancy because my change still allows enabling
messages with the drm.drm_debug parameter and in that case the prefix is
still useful.

Br,
- Robert



> This series also makes all the logging macros use drm_printk, but also
> makes DRM_NAME passed in from the macro wrappers in all cases. So drivers
> can override it regardless of the log level.
>
> And finally, the series also removes a bit of redundant data from the debug
> messages effectively converting this:
>
>  [drm:edp_panel_off [i915]] Wait for panel power off time
>
> Into this:
>
>  [edp_panel_off [i915]] Wait for panel power off time
>
> Which still has all the data in it.
>
> Tvrtko Ursulin (5):
>   drm/i915: Give our log messages our name
>   drm: Respect driver set DRM_NAME in drm_printk
>   drm: Respect driver set DRM_NAME in drm_dev_printk
>   drm: Use drm_printk for all logging macros
>   drm: Do not log driver prefix in debug messages
>
>  drivers/gpu/drm/drm_drv.c       | 39 +++++++++++------
>  drivers/gpu/drm/i915/i915_drv.c |  3 +-
>  include/drm/drmP.h              | 94 ++++++++++++++++++++++++------
> -----------
>  include/drm/drm_drv.h           | 11 ++---
>  include/uapi/drm/i915_drm.h     |  3 ++
>  5 files changed, 92 insertions(+), 58 deletions(-)
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20161207/4fb87cc8/attachment.html>