Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v9 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE_CATEGORIES
 bitmap to tracefs
To:     Jim Cromie <jim.cromie@gmail.com>, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org, linux@rasmusvillemoes.dk,
        daniel.vetter@ffwll.ch, seanpaul@chromium.org,
        dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
        intel-gvt-dev@lists.freedesktop.org,
        intel-gfx@lists.freedesktop.org
References: <20211027043645.153133-1-jim.cromie@gmail.com>
 <20211027043645.153133-11-jim.cromie@gmail.com>
From:   Jason Baron <jbaron@akamai.com>
Message-ID: <3a55ab9c-8109-8025-21e3-e3635bd891b2@akamai.com>
Date:   Wed, 3 Nov 2021 11:58:10 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <20211027043645.153133-11-jim.cromie@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


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@gmail.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,
>  				[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)
>