On Mon, 8 Jun 2020 17:05:03 -0400 Sean Paul sean@poorly.run wrote:

From: Sean Paul seanpaul@chromium.org

This patch adds a new module parameter called drm.trace which accepts the same mask as drm.debug. When a debug category is enabled, log messages will be put in a new tracefs instance called drm for consumption.

Using the new tracefs instance will allow distros to enable drm logging in production without impacting performance or spamming the system logs.

Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: David Airlie airlied@gmail.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Pekka Paalanen ppaalanen@gmail.com Cc: Rob Clark robdclark@gmail.com Cc: Steven Rostedt rostedt@goodmis.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Steven Rostedt rostedt@goodmis.org Signed-off-by: Sean Paul seanpaul@chromium.org Link: https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-sean@p... #v1 Link: https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html #v2 Link: https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-sean@p... #v3 Link: https://patchwork.freedesktop.org/patch/msgid/20200114172155.215463-1-sean@p... #v4

Changes in v5:

-Re-write to use trace_array and the tracefs instance support

Documentation/gpu/drm-uapi.rst | 6 + drivers/gpu/drm/drm_drv.c | 3 + drivers/gpu/drm/drm_print.c | 209 ++++++++++++++++++++++++++++----- include/drm/drm_print.h | 63 ++++++++-- 4 files changed, 241 insertions(+), 40 deletions(-)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 56fec6ed1ad8..1c1c4d043f40 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -312,6 +312,12 @@ Debugfs Support .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c :export:

+DRM Tracing +---------------

+.. kernel-doc:: drivers/gpu/drm/drm_print.c

• :doc: DRM Tracing

Sysfs Support

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index bc38322f306e..88404af74c9b 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -1108,12 +1108,15 @@ static void drm_core_exit(void) drm_sysfs_destroy(); idr_destroy(&drm_minors_idr); drm_connector_ida_destroy();

• drm_trace_cleanup();

}

static int __init drm_core_init(void) { int ret;

• drm_trace_init();
• drm_connector_ida_init(); idr_init(&drm_minors_idr);

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 4d984a01b3a3..c4bef38921db 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -31,6 +31,7 @@ #include <linux/moduleparam.h> #include <linux/seq_file.h> #include <linux/slab.h> +#include <linux/trace.h>

#include <drm/drm.h> #include <drm/drm_drv.h> @@ -43,17 +44,34 @@ unsigned int __drm_debug_syslog; EXPORT_SYMBOL(__drm_debug_syslog);

-MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n" -"\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n" -"\t\tBit 1 (0x02) will enable DRIVER messages (drm controller code)\n" -"\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n" -"\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n" -"\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n" -"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n" -"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)\n" -"\t\tBit 8 (0x100) will enable DP messages (displayport code)"); +/*

• • __drm_debug_trace: Enable debug output in drm tracing instance.
• • Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
• */

+unsigned int __drm_debug_trace; +EXPORT_SYMBOL(__drm_debug_trace);

Hi!

Might distributions perhaps want to set a default value for this via Kconfig? Or could setting it via sysfs happen early enough to diagnose e.g. Plymouth problems?

Or maybe there is nothing to see from early boot?

The general usefulness of this feature depends on whether people actually run with it enabled.

+#define DEBUG_PARM_DESC(dst) \ +"Enable debug output to " dst ", where each bit enables a debug category.\n" \ +"\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n" \ +"\t\tBit 1 (0x02) will enable DRIVER messages (drm controller code)\n" \ +"\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n" \ +"\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n" \ +"\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n" \ +"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n" \ +"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)\n" \ +"\t\tBit 8 (0x100) will enable DP messages (displayport code)"

+MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog")); module_param_named(debug, __drm_debug_syslog, int, 0600);

+MODULE_PARM_DESC(trace, DEBUG_PARM_DESC("tracefs")); +module_param_named(trace, __drm_debug_trace, int, 0600);

...

+/**

• • DOC: DRM Tracing
• • *tl;dr* DRM tracing is a lightweight alternative to traditional DRM debug
• • logging.
• • While DRM logging is quite convenient when reproducing a specific issue, it
• • doesn't help when something goes wrong unexpectedly. There are a couple
• • reasons why one does not want to enable DRM logging at all times:
• • 1. We don't want to overwhelm syslog with drm spam, others have to use it too
• • 2. Console logging is slow
• • DRM tracing aims to solve both these problems.
• • To use DRM tracing, set the drm.trace module parameter (via cmdline or sysfs)
• • to a DRM debug category mask (this is a bitmask of &drm_debug_category
• • values):
• • ::
• • eg: echo 0x106 > /sys/module/drm/parameters/trace
• • Once active, all log messages in the specified categories will be written to
• • the DRM trace. Once at capacity, the trace will overwrite old messages with
• • new ones. At any point, one can read the trace file to extract the previous N
• • DRM messages:
• • ::
• • eg: cat /sys/kernel/tracing/instances/drm/trace
• • Considerations

• ---

• • The trace is subsystem wide, so if you have multiple devices active, they
• • will be adding logs to the same trace.
• • The contents of the DRM Trace are **not** considered UABI. **DO NOT depend on
• • the values of these traces in your userspace.** These traces are intended for
• • entertainment purposes only. The contents of these logs carry no warranty,
• • expressed or implied.
• */

Sounds good to me! This part is: Acked-by: Pekka Paalanen pekka.paalanen@collabora.com

Thanks, pq