public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tursulin@ursulin.net>
To: Intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Subject: [RFC v2 0/6] DRM logging tidy
Date: Wed, 24 Jan 2018 16:18:15 +0000	[thread overview]
Message-ID: <20180124161821.28780-1-tvrtko.ursulin@linux.intel.com> (raw)

From: Tvrtko Ursulin <tvrtko.ursulin@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@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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

             reply	other threads:[~2018-01-24 16:18 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-24 16:18 Tvrtko Ursulin [this message]
2018-01-24 16:18 ` [RFC 1/6] drm/armada: Simplify drm_dev_init error log Tvrtko Ursulin
2018-01-24 17:44   ` Russell King - ARM Linux
2018-01-24 16:18 ` [RFC 2/6] drm: Introduce unexported DRM_LOG_NAME define for logging Tvrtko Ursulin
2018-01-24 16:18 ` [RFC 3/6] drm/i915: Give our log messages our name Tvrtko Ursulin
2018-01-26 13:10   ` [Intel-gfx] " Michal Wajdeczko
2018-01-30 10:53     ` Tvrtko Ursulin
2018-01-24 16:18 ` [RFC 4/6] drm: Respect driver set DRM_LOG_NAME in drm_printk Tvrtko Ursulin
2018-01-24 16:18 ` [RFC 5/6] drm: Respect driver set DRM_LOG_NAME in drm_dev_printk Tvrtko Ursulin
2018-01-24 16:18 ` [RFC 6/6] drm: Respect driver set DRM_LOG_NAME in drm_info_printer Tvrtko Ursulin
2018-01-24 16:23 ` [Intel-gfx] [RFC v2 0/6] DRM logging tidy Chris Wilson
2018-01-24 16:48   ` Tvrtko Ursulin
2018-01-25 11:32     ` Jani Nikula
2018-01-25 13:38       ` [Intel-gfx] " Tvrtko Ursulin
2018-01-24 16:42 ` ✓ Fi.CI.BAT: success for DRM logging tidy (rev2) Patchwork
2018-01-24 20:12 ` ✗ Fi.CI.IGT: warning " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180124161821.28780-1-tvrtko.ursulin@linux.intel.com \
    --to=tursulin@ursulin.net \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox