[Intel-gfx] [RFC v2 0/6] DRM logging tidy

Tvrtko Ursulin tursulin at ursulin.net
Wed Jan 24 16:18:15 UTC 2018


From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

This series tries to solve a few issues in the current DRM logging code to
primarily make it clearer which messages belong to which driver.

Main problem is that currently some logging functions allow individual drivers
to override the log prefix (since they are defined as macros, or static
inlines), while other hardcode the "drm" prefix into them due being situated in
the DRM core modules.

Another thing is that I noticed the DRM_NAME macro which is used for this is
defined in the uAPI header and had a comment which looked outdated.

Therefore I introduce a new define, called, DRM_LOG_NAME, this time defined
internally in the kernel headers and not exported in the uAPI.

I also refactored some logging functions to take this string as a parameter
instead of hardcoding it.

Individual drivers can then override this define to make DRM logging functions
prefix their message with the respective driver prefix.

End result in the case of the i915 driver looks like this:

Old log:

 [drm] Found 128MB of eDRAM
 [drm:skl_enable_dc6 [i915]] Enabling DC6

New log:

 [i915] Found 128MB of eDRAM
 [i915:skl_enable_dc6 [i915]] Enabling DC6

There is some redundancy in the debug logs, as can be seen in the line above,
but this comes from drm_printk printing the calling function name using the %ps
formatting specifier and __builtin_return_address. Theoretically we could remove
the driver prefix from debug messages, but that would make the messages a bit
unusually formatted like:

 [skl_enable_dc6 [i915]] Enabling DC6

Better would probably be:

 [i915:skl_enable_dc6] Enabling DC6

But I do not know how to achieve that apart from changing the drm_printk
wrapper macros to pass in __func__ and drop the __builtin_return_address
business. And that would also mean all DRM drivers would have to start defining
DRM_LOG_NAME or would lose the module owner info. So I opted not to go that
route for now.

Main question is whether people find all this an interesting change or would
prefer to leave the logging prefixes as they were, or perhaps do some heavier
changes to DRM logging.

Cc: dri-devel at lists.freedesktop.org

Tvrtko Ursulin (6):
  drm/armada: Simplify drm_dev_init error log
  drm: Introduce unexported DRM_LOG_NAME define for logging
  drm/i915: Give our log messages our name
  drm: Respect driver set DRM_LOG_NAME in drm_printk
  drm: Respect driver set DRM_LOG_NAME in drm_dev_printk
  drm: Respect driver set DRM_LOG_NAME in drm_info_printer

 drivers/gpu/drm/armada/armada_drv.c  |  3 +-
 drivers/gpu/drm/drm_print.c          | 20 ++++++------
 drivers/gpu/drm/i915/i915_drv.c      |  2 +-
 drivers/gpu/drm/i915/i915_drv.h      |  5 +++
 drivers/gpu/drm/i915/intel_display.c |  3 +-
 include/drm/drm_print.h              | 61 +++++++++++++++++++++---------------
 include/uapi/drm/drm.h               |  2 +-
 7 files changed, 55 insertions(+), 41 deletions(-)

-- 
2.14.1



More information about the Intel-gfx mailing list