On Wed, Nov 3, 2021 at 9:58 AM Jason Baron <jbaron@xxxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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)
