Re: [PATCH v9 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES bitmap to tracefs

From: Jason Baron
Date: Wed Nov 03 2021 - 11:58:25 EST




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?

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,
> [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,
> + [0] = { DRM_DBG_CAT_CORE },
> + [1] = { DRM_DBG_CAT_DRIVER },
> + [2] = { DRM_DBG_CAT_KMS },
> + [3] = { DRM_DBG_CAT_PRIME },
> + [4] = { DRM_DBG_CAT_ATOMIC },
> + [5] = { DRM_DBG_CAT_VBL },
> + [6] = { DRM_DBG_CAT_STATE },
> + [7] = { DRM_DBG_CAT_LEASE },
> + [8] = { DRM_DBG_CAT_DP },
> + [9] = { DRM_DBG_CAT_DRMRES });
> +
> +struct trace_array *trace_arr;
> +#endif
> #endif
>
> void __drm_puts_coredump(struct drm_printer *p, const char *str)
>