[PATCH v5 13/13] drm/print: Add tracefs support to the drm logging helpers

[Pekka Paalanen]

On Wed, 10 Jun 2020 09:29:37 -0400
Sean Paul <sean at poorly.run> wrote:

> On Wed, Jun 10, 2020 at 3:57 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:
> >
> > On Mon,  8 Jun 2020 17:05:03 -0400
> > Sean Paul <sean at poorly.run> wrote:
> >  
> > > From: Sean Paul <seanpaul at 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 at ffwll.ch>
> > > Cc: David Airlie <airlied at gmail.com>
> > > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > Cc: Pekka Paalanen <ppaalanen at gmail.com>
> > > Cc: Rob Clark <robdclark at gmail.com>
> > > Cc: Steven Rostedt <rostedt at goodmis.org>
> > > Cc: Thomas Zimmermann <tzimmermann at suse.de>
> > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Steven Rostedt <rostedt at goodmis.org>
> > > Signed-off-by: Sean Paul <seanpaul at chromium.org>
> > > Link: https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-sean@poorly.run #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@poorly.run #v3
> > > Link: https://patchwork.freedesktop.org/patch/msgid/20200114172155.215463-1-sean@poorly.run #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(-)
> > >

...

> > >  #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!  
> 
> Hi Pekka,
> Thanks again for the feedback, I'm happy that we seem to be converging!
> 
> >
> > 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.  
> 
> I had assumed that the cmdline argument would be sufficient for
> distros, is Kconfig preferable here? The module parameter has the
> advantage of being runtime configurable and is more in line with
> drm.debug. We can do either in CrOS, so I'm happy to go with crowd
> consensus.

Hi,

I don't know. I did mean only *default* value for the module parameter
via Kconfig. Right now your default value is implicit 0.

But that's just me thinking that making kernel command lines even
longer would be somehow bad. Maybe it's fine?


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200610/2eec3fcc/attachment.sig>