[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJfuBxyMeC5_H-RakOpqH9jDuh07gn4cjCuJ=ebQ8tLQVOTGrg@mail.gmail.com>
Date: Thu, 4 Nov 2021 00:31:58 -0600
From: jim.cromie@...il.com
To: Jason Baron <jbaron@...mai.com>
Cc: Greg KH <gregkh@...uxfoundation.org>,
LKML <linux-kernel@...r.kernel.org>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Sean Paul <seanpaul@...omium.org>,
dri-devel <dri-devel@...ts.freedesktop.org>,
amd-gfx mailing list <amd-gfx@...ts.freedesktop.org>,
intel-gvt-dev@...ts.freedesktop.org,
Intel Graphics Development <intel-gfx@...ts.freedesktop.org>
Subject: Re: [PATCH v9 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES
bitmap to tracefs
On Wed, Nov 3, 2021 at 9:58 AM Jason Baron <jbaron@...mai.com> wrote:
>
>
>
> On 10/27/21 12:36 AM, Jim Cromie wrote:
> > Use new macro to create a sysfs control bitmap knob to control
> > print-to-trace in: /sys/module/drm/parameters/trace
> >
> > todo: reconsider this api, ie a single macro expecting both debug &
> > trace terms (2 each), followed by a single description and the
> > bitmap-spec::
> >
> > Good: declares bitmap once for both interfaces
> >
> > Bad: arg-type/count handling (expecting 4 args) is ugly,
> > especially preceding the bitmap-init var-args.
> >
>
> Hi Jim,
>
> I agree having the bitmap declared twice seems redundant. But I like having fewer args and not necessarily combining the trace/log variants of
> DEBUG_CATEGORIES. hmmm...what if the DEFINE_DYNAMIC_DEBUG_CATEGORIES() took a pointer to the array of struct dyndbg_bitdesc map[] directly as the
> final argument instead of the __VA_ARGS__? Then, we could just declare the map once?
>
indeed. that seems obvious in retrospect,
thanks for the nudge.
also, Im inclined to (uhm, have now done) bikeshed the API in patch 1,
and change _CATEGORIES to something else,
maybe _FMTGRPS
or _BITGRPS < -- this one
ISTM better to be explicit wrt the underlying mechanisms, (least surprise)
let users decide the meaning of "CATEGORIES"
also, HEAD~1 added DEFINE_DYNAMIC_DEBUG_CATEGORIES_FLAGS
which could be used directly for both purposes (after a rename).
TLDR: flags exposes the shared nature of the decorator flags,
the trace and syslog customers of pr_debug traffic should agree on their use.
redoing now...
> Thanks,
>
> -Jason
>
> > Signed-off-by: Jim Cromie <jim.cromie@...il.com>
> > ---
> > drivers/gpu/drm/drm_print.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> > index ce662d0f339b..7b49fbc5e21d 100644
> > --- a/drivers/gpu/drm/drm_print.c
> > +++ b/drivers/gpu/drm/drm_print.c
> > @@ -73,6 +73,25 @@ DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug, __drm_debug,
static mumble-map
> > [7] = { DRM_DBG_CAT_LEASE },
> > [8] = { DRM_DBG_CAT_DP },
> > [9] = { DRM_DBG_CAT_DRMRES });
> > +
> > +#ifdef CONFIG_TRACING
> > +unsigned long __drm_trace;
> > +EXPORT_SYMBOL(__drm_trace);
> > +DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES(trace, __drm_trace,
> > + DRM_DEBUG_DESC,
mumble-map)
Powered by blists - more mailing lists