From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C165C433EF for ; Mon, 6 Sep 2021 17:42:20 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C930660F6B for ; Mon, 6 Sep 2021 17:42:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C930660F6B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D7F1D8993B; Mon, 6 Sep 2021 17:42:15 +0000 (UTC) Received: from mail-ua1-x935.google.com (mail-ua1-x935.google.com [IPv6:2607:f8b0:4864:20::935]) by gabe.freedesktop.org (Postfix) with ESMTPS id 43BEE897AC; Mon, 6 Sep 2021 17:42:14 +0000 (UTC) Received: by mail-ua1-x935.google.com with SMTP id e20so638087uam.11; Mon, 06 Sep 2021 10:42:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xb1RRKBrihRAbWsvTSrPqTS344LUaeD0ur8JecL/ZVs=; b=neYBLjlz1SfLbWGviUoVfiyhOT13XuYZMJaJEf89/UCsP9ym56JeZIn0fm+ISHEPCa iM1W6pAsj/LzxcpAyTQhWe2JytHbPBmAKgTLDpRrTiKwW+Wcni9BiC15bsz/25zT+y76 xfPXgu33X2OC3RJONzxOW7XMOXwQ0lYA0zPzdpFyxZ9h9/2BlhIQjXMH2QbT4EijJLbV bHWRgZmTDt9kNJHHs/s612KHHu4Vq1HiiIYSriB/wU5U3mlivYc2/UstFphZ3+Kj/b/O u2w4V3ehCwxoOiyG5rEJzIdxUogzdrfboGPqdjNaQVve3WuUPWp40PbRZvYm90D9IF4I xfpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xb1RRKBrihRAbWsvTSrPqTS344LUaeD0ur8JecL/ZVs=; b=E3qRxaEVRprOlvbD0hpMGmhIS+kNH5eAbCFSOqEHAPzmXJb6a/J6f0Gh4pXAJ2z8iT lcXF/kPUvP/odxPwLmb6sTiBtwREMwtoksILDmeJctoKhcrDgs/VH8YMQZLQtUAdzWUU o9/vEu6WwWbDU21NveypJ5khLKBdk/rtI7rerOetMr6gYiAXCdm4FEoxz+7zFOleG4BZ O7/+7dscsbU+8FV7OkYN4pZwyhPB1lkeG5GhoEpkmi8q8UMaohFBUOQFsayGnJMS7o3k 0U4TBFaQpoG5yVQ3RMepzkUHsLrUrq9q8tZ8xdGKgg1W7pYpWmGx+goChzVpkRXs55uH j35A== X-Gm-Message-State: AOAM533j11nQ8LW0AWnVTHWqupw6+bhmBsygFpSQ8KxT0e/6NHnR6NiJ geLhIho2EFFJ6Rmab4Yy+vfoCtSEbqAMhm0jvgo= X-Google-Smtp-Source: ABdhPJzaZ5VyPa8E+HWSqH5kvV1sen7OH0Z5KgDumkxGBY7J5H8NfHkNyiwDc4gMa/YR+YvWyz72TmaW2m0zEgPNXGY= X-Received: by 2002:ab0:778c:: with SMTP id x12mr6417234uar.121.1630950132847; Mon, 06 Sep 2021 10:42:12 -0700 (PDT) MIME-Version: 1.0 References: <20210831202133.2165222-1-jim.cromie@gmail.com> <20210831202133.2165222-4-jim.cromie@gmail.com> <9fe5e962-e65e-6844-269a-058cce944a89@linux.intel.com> <2bfbd75c-8f7f-e756-05c3-13dff41264e4@linux.intel.com> In-Reply-To: <2bfbd75c-8f7f-e756-05c3-13dff41264e4@linux.intel.com> From: jim.cromie@gmail.com Date: Mon, 6 Sep 2021 11:41:46 -0600 Message-ID: Subject: Re: [Intel-gfx] [PATCH v7 3/8] i915/gvt: use DEFINE_DYNAMIC_DEBUG_CATEGORIES to create "gvt:core:" etc categories To: Tvrtko Ursulin , Sean Paul , Jason Baron , Daniel Vetter Cc: Greg KH , LKML , dri-devel , amd-gfx mailing list , intel-gvt-dev@lists.freedesktop.org, Intel Graphics Development Content-Type: multipart/alternative; boundary="0000000000005099b905cb572b8f" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" --0000000000005099b905cb572b8f Content-Type: text/plain; charset="UTF-8" On Mon, Sep 6, 2021 at 6:26 AM Tvrtko Ursulin < tvrtko.ursulin@linux.intel.com> wrote: > > > On 03/09/2021 20:22, jim.cromie@gmail.com wrote: > > On Fri, Sep 3, 2021 at 5:07 AM Tvrtko Ursulin > > wrote: > >> > >> > >> On 31/08/2021 21:21, Jim Cromie wrote: > >>> The gvt component of this driver has ~120 pr_debugs, in 9 categories > >>> quite similar to those in DRM. Following the interface model of > >>> drm.debug, add a parameter to map bits to these categorizations. > >>> > >>> DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug, > >>> "dyndbg bitmap desc", > >>> { "gvt:cmd: ", "command processing" }, > >>> v7: > >>> . move ccflags addition up to i915/Makefile from i915/gvt > >>> --- > >>> drivers/gpu/drm/i915/Makefile | 4 ++++ > >>> drivers/gpu/drm/i915/i915_params.c | 35 ++++++++++++++++++++++++++++++ > >> > >> Can this work if put under gvt/ or at least intel_gvt.h|c? I tried this. I moved the code block into gvt/debug.c (new file) added it to Makefile GVT_SOURCES dunno why it wont make. frustratig basic err, Im not seeing. It does seem proper placement, will resolve... > >> > > > > I thought it belonged here more, at least according to the name of the > > config.var > > Hmm bear with me please - the categories this patch creates are intended > to be used explicitly from the GVT "sub-module", or they somehow even > get automatically used with no further intervention to callers required? > 2009 - v5.9.0 the only users were admins reading/echoing /proc/dynamic_debug/control presumably cuz they wanted more info in the logs, episodically. v5.9.0 exported dynamic_debug_exec_queries for in-kernel use, reusing the stringy: echo $query_command > control idiom. My intention was to let in-kernel users roll their own drm.debug type interface, or whatever else they needed. nobodys using it yet. patch 1/8 implements that drm.debug interface. 5/8 is the primary use case 3/8 (this patch) & 4/8 are patches of opportunity, test cases, proof of function/utility. its value as such is easier control of those pr-debugs than given by echo > control Sean Paul seanpaul@chromium.org worked up a patchset to do runtime steering of drm-debug stream, in particular watching for drm:atomic:fail: type activity (a subcategory which doesnt exist yet). 5/8 conflicts with his patchset, I have an rfc approach to that, so his concerns are mine too. note: if this patchset goes in, we dont *really* need the export anymore, since the main use case is covered. We could un-export, and re-add later if its needed for a different use case. Further, it seems likely that the callbacks (refactored) would be a better basis for new in-kernel users. If not that, then full exposure of struct ddebug_query to in-kernel use not quite sure how we got 2 chunks, but theres 1 more q below. On Mon, Sep 6, 2021 at 6:26 AM Tvrtko Ursulin < tvrtko.ursulin@linux.intel.com> wrote: > > On 03/09/2021 20:22, jim.cromie@gmail.com wrote: > > On Fri, Sep 3, 2021 at 5:07 AM Tvrtko Ursulin > > wrote: > >> > >> > >> On 31/08/2021 21:21, Jim Cromie wrote: > >>> The gvt component of this driver has ~120 pr_debugs, in 9 categories > >>> quite similar to those in DRM. Following the interface model of > >>> drm.debug, add a parameter to map bits to these categorizations. > >>> > >>> DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug, > >>> "dyndbg bitmap desc", > >>> { "gvt:cmd: ", "command processing" }, > >>> { "gvt:core: ", "core help" }, > >>> { "gvt:dpy: ", "display help" }, > >>> { "gvt:el: ", "help" }, > >>> { "gvt:irq: ", "help" }, > >>> { "gvt:mm: ", "help" }, > >>> { "gvt:mmio: ", "help" }, > >>> { "gvt:render: ", "help" }, > >>> { "gvt:sched: " "help" }); > >>> > > > > BTW, Ive dropped the help field, its already handled, dont need to > clutter. > > > > > >>> The actual patch has a few details different, cmd_help() macro emits > >>> the initialization construct. > >>> > >>> if CONFIG_DRM_USE_DYNAMIC_DEBUG, then -DDYNAMIC_DEBUG_MODULE is added > >>> cflags, by gvt/Makefile. > >>> > >>> Signed-off-by: Jim Cromie > >>> --- > >>> v5: > >>> . static decl of vector of bit->class descriptors - Emil.V > >>> . relocate gvt-makefile chunk from elsewhere > >>> v7: > >>> . move ccflags addition up to i915/Makefile from i915/gvt > >>> --- > >>> drivers/gpu/drm/i915/Makefile | 4 ++++ > >>> drivers/gpu/drm/i915/i915_params.c | 35 > ++++++++++++++++++++++++++++++ > >> > >> Can this work if put under gvt/ or at least intel_gvt.h|c? > >> > > > > I thought it belonged here more, at least according to the name of the > > config.var > > Hmm bear with me please - the categories this patch creates are intended > to be used explicitly from the GVT "sub-module", or they somehow even > get automatically used with no further intervention to callers required? > > > CONFIG_DRM_USE_DYNAMIC_DEBUG. > > > > I suppose its not a great name, its narrow purpose is to swap > > drm-debug api to use dyndbg. drm-evrything already "uses" > > dyndbg if CONFIG_DYNAMIC_DEBUG=y, those gvt/pr_debugs in particular. > > > > Theres also CONFIG_DYNAMIC_DEBUG_CORE=y, > > which drm basically ignores currently. > > > > So with the name CONFIG_DRM_USE_DYNAMIC_DEBUG > > it seemed proper to arrange for that to be true on DD-CORE=y builds, > > by adding -DDYNAMIC_DEBUG_MODULE > > > > Does that make some sense ? > > How to best resolve the frictions ? > > new CONFIG names ? > > > >>> 2 files changed, 39 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/i915/Makefile > b/drivers/gpu/drm/i915/Makefile > >>> index 4f22cac1c49b..5a4e371a3ec2 100644 > >>> --- a/drivers/gpu/drm/i915/Makefile > >>> +++ b/drivers/gpu/drm/i915/Makefile > >>> @@ -30,6 +30,10 @@ CFLAGS_display/intel_fbdev.o = $(call > cc-disable-warning, override-init) > >>> > >>> subdir-ccflags-y += -I$(srctree)/$(src) > >>> > >>> +#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG > >>> +ccflags-y += -DDYNAMIC_DEBUG_MODULE > >>> +#endif > >> > >> Ignores whether CONFIG_DRM_I915_GVT is enabled or not? > >> > > > > not intentionally. > > I think theres 2 things youre noting: > > > > 1 - make frag into gvt/Makefile > > I had it there earlier, not sure why I moved it up. > > maybe some confusion on proper scope of the flag. > > > > > > 2 - move new declaration code in i915-param.c inside the gvt ifdef > > > > Im good with that. > > I'll probably copy the ifdef wrapper down rather than move the decl up. > > ie: > > > > #if __and(IS_ENABLED(CONFIG_DRM_I915_GVT), > > IS_ENABLED(CONFIG_DRM_USE_DYNAMIC_DEBUG)) > > > > unsigned long __gvt_debug; > > EXPORT_SYMBOL(__gvt_debug); > > > > > >>> + > >>> # Please keep these build lists sorted! > >>> > >>> # core driver code > >>> diff --git a/drivers/gpu/drm/i915/i915_params.c > b/drivers/gpu/drm/i915/i915_params.c > >>> index e07f4cfea63a..e645e149485e 100644 > >>> --- a/drivers/gpu/drm/i915/i915_params.c > >>> +++ b/drivers/gpu/drm/i915/i915_params.c > >>> @@ -265,3 +265,38 @@ void i915_params_free(struct i915_params *params) > >>> + _DD_cat_("gvt:mmio:"), > >>> + _DD_cat_("gvt:render:"), > >>> + _DD_cat_("gvt:sched:")); > >>> + > >>> +#endif > >> > >> So just the foundation - no actual use sites I mean? How would these be > >> used from the code? > >> > > > > there are 120 pr_debug "users" :-) > > > > no users per se, but anyone using drm.debug > > /sys/module/drm/parameters/debug > > might use this too. > > its a bit easier than composing queries for >/proc/dyamic_debug/control > > Same as my previous question, perhaps I am not up to speed with this > yet.. Even if pr_debug is used inside GVT - are the categories and > debug_gvt global as of this patch (or series)? > > they are already global in the sense that if kernel is built with DYNAMIC_DEBUG, the admin can turn those pr_debugs on and off, and change their decorations in the log (mod,func.line). Nor are modules protected from each other; drm-core could use dd-exec-queries to enable/disable pr-debugs in i915 etc This patch just adds a gvt-debug knob like drm-debug. using the existing format prefixes to categorize them. Whether those prefixes should be bent towards consistency with the rest of drm-debug or adapted towards some gvt community need I couldnt say. Its no save-the-world feature, but its pretty cheap. Id expect the same users as those who play with drm.debug, for similar reasons. does this clarify ? > Regards, > > Tvrtko > thanks, Jim --0000000000005099b905cb572b8f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Mon, Sep 6, 2021 at 6:26 AM Tv= rtko Ursulin <tvrtko.u= rsulin@linux.intel.com> wrote:
>
>
> On 03/09/2021= 20:22, jim.cromie@gmail.com wr= ote:
> > On Fri, Sep 3, 2021 at 5:07 AM Tvrtko Ursulin
> >= ; <tvrtko.ursulin@linu= x.intel.com> wrote:
> >>
> >>
> >&g= t; On 31/08/2021 21:21, Jim Cromie wrote:
> >>> The gvt comp= onent of this driver has ~120 pr_debugs, in 9 categories
> >>&g= t; quite similar to those in DRM.=C2=A0 Following the interface model of> >>> drm.debug, add a parameter to map bits to these categori= zations.
> >>>
> >>> DEFINE_DYNAMIC_DEBUG_CAT= EGORIES(debug_gvt, __gvt_debug,
> >>> =C2=A0 =C2=A0 =C2=A0 = =C2=A0"dyndbg bitmap desc",
> >>> =C2=A0 =C2=A0 = =C2=A0 =C2=A0{ "gvt:cmd: ", =C2=A0"command processing" = },

> >>> v7:
> >>> . move ccflags additio= n up to i915/Makefile from i915/gvt
> >>> ---
> >&g= t;> =C2=A0 =C2=A0drivers/gpu/drm/i915/Makefile =C2=A0 =C2=A0 =C2=A0| =C2= =A04 ++++
> >>> =C2=A0 =C2=A0drivers/gpu/drm/i915/i915_param= s.c | 35 ++++++++++++++++++++++++++++++
> >>
> >> C= an this work if put under gvt/ or at least intel_gvt.h|c?

I tried th= is.
I moved the code block into gvt/debug.c (new file)
added it to Ma= kefile GVT_SOURCES
dunno why it wont make.
frustratig basic err, Im n= ot seeing.
It does seem proper placement, will resolve...


>= ; >>
> >
> > I thought it belonged here more, at le= ast according to the name of the
> > config.var
>
> Hm= m bear with me please - the categories this patch creates are intended
&= gt; to be used explicitly from the GVT "sub-module", or they some= how even
> get automatically used with no further intervention to cal= lers required?
>

2009 - v5.9.0 =C2=A0the only users were admin= s reading/echoing /proc/dynamic_debug/control
presumably cuz they wanted= more info in the logs, episodically.
v5.9.0 exported dynamic_debug_exec= _queries for in-kernel use,
reusing the stringy: echo $query_command >= ; control =C2=A0idiom.
My intention was to let in-kernel users roll thei= r own drm.debug type interface,
or whatever else they needed.=C2=A0 nobo= dys using it yet.

patch 1/8 implements that drm.debug interface.
= 5/8 is the primary use case
3/8 (this patch) & 4/8 are patches of op= portunity, test cases, proof of function/utility.
its value as such is e= asier control of those pr-debugs than given by echo > control

Sea= n Paul=C2=A0 seanpaul@chromium.org= worked up a patchset to do runtime steering of drm-debug stream,
=
in particular watching for drm:atomic:fail: type activity (a subcatego= ry which doesnt exist yet).
5/8 conflicts with his patchset, I ha= ve an rfc approach to that, so his concerns are mine too.


note: =C2=A0if this patchset goes in, w= e dont *really* need the export anymore,
since the main use case is cove= red.=C2=A0 We could un-export, and re-add later
if its needed for a diff= erent use case.=C2=A0 Further, it seems likely that the callbacks
(refac= tored) would be a better basis for new in-kernel users.
If not that, the= n full exposure of struct ddebug_query to in-kernel use

<= br>
not quite sure how we got 2 chunks, but theres=C2=A01 more q below= .

O= n Mon, Sep 6, 2021 at 6:26 AM Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:

On 03/09/2021 20:22, jim.cromie@gmail.com wrote:
> On Fri, Sep 3, 2021 at 5:07 AM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>>
>>
>> On 31/08/2021 21:21, Jim Cromie wrote:
>>> The gvt component of this driver has ~120 pr_debugs, in 9 cate= gories
>>> quite similar to those in DRM.=C2=A0 Following the interface m= odel of
>>> drm.debug, add a parameter to map bits to these categorization= s.
>>>
>>> DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug,
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 "dyndbg bitmap desc",
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:cmd: ",=C2=A0 &quo= t;command processing" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:core: ", "cor= e help" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:dpy: ",=C2=A0 &quo= t;display help" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:el: ",=C2=A0 =C2= =A0"help" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:irq: ",=C2=A0 &quo= t;help" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:mm: ",=C2=A0 =C2= =A0"help" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:mmio: ", "hel= p" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:render: ", "h= elp" },
>>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 { "gvt:sched: " "hel= p" });
>>>
>
> BTW, Ive dropped the help field, its already handled, dont need to clu= tter.
>
>
>>> The actual patch has a few details different, cmd_help() macro= emits
>>> the initialization construct.
>>>
>>> if CONFIG_DRM_USE_DYNAMIC_DEBUG, then -DDYNAMIC_DEBUG_MODULE i= s added
>>> cflags, by gvt/Makefile.
>>>
>>> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
>>> ---
>>> v5:
>>> . static decl of vector of bit->class descriptors - Emil.V<= br> >>> . relocate gvt-makefile chunk from elsewhere
>>> v7:
>>> . move ccflags addition up to i915/Makefile from i915/gvt
>>> ---
>>>=C2=A0 =C2=A0 drivers/gpu/drm/i915/Makefile=C2=A0 =C2=A0 =C2=A0= |=C2=A0 4 ++++
>>>=C2=A0 =C2=A0 drivers/gpu/drm/i915/i915_params.c | 35 +++++++++= +++++++++++++++++++++
>>
>> Can this work if put under gvt/ or at least intel_gvt.h|c?
>>
>
> I thought it belonged here more, at least according to the name of the=
> config.var

Hmm bear with me please - the categories this patch creates are intended to be used explicitly from the GVT "sub-module", or they somehow = even
get automatically used with no further intervention to callers required?
> CONFIG_DRM_USE_DYNAMIC_DEBUG.
>
> I suppose its not a great name, its narrow purpose is to swap
> drm-debug api to use dyndbg.=C2=A0 =C2=A0drm-evrything already "u= ses"
> dyndbg if CONFIG_DYNAMIC_DEBUG=3Dy, those gvt/pr_debugs in particular.=
>
> Theres also CONFIG_DYNAMIC_DEBUG_CORE=3Dy,
> which drm basically ignores currently.
>
> So with the name CONFIG_DRM_USE_DYNAMIC_DEBUG
> it seemed proper to arrange for that=C2=A0 to be true on DD-CORE=3Dy b= uilds,
> by adding -DDYNAMIC_DEBUG_MODULE
>
> Does that make some sense ?
> How to best resolve the frictions ?
> new CONFIG names ?
>
>>>=C2=A0 =C2=A0 2 files changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i= 915/Makefile
>>> index 4f22cac1c49b..5a4e371a3ec2 100644
>>> --- a/drivers/gpu/drm/i915/Makefile
>>> +++ b/drivers/gpu/drm/i915/Makefile
>>> @@ -30,6 +30,10 @@ CFLAGS_display/intel_fbdev.o =3D $(call cc-= disable-warning, override-init)
>>>
>>>=C2=A0 =C2=A0 subdir-ccflags-y +=3D -I$(srctree)/$(src)
>>>
>>> +#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG
>>> +ccflags-y +=3D -DDYNAMIC_DEBUG_MODULE
>>> +#endif
>>
>> Ignores whether CONFIG_DRM_I915_GVT is enabled or not?
>>
>
> not intentionally.
> I think theres 2 things youre noting:
>
> 1 - make frag into gvt/Makefile
> I had it there earlier, not sure why I moved it up.
> maybe some confusion on proper scope of the flag.
>
>
> 2 - move new declaration code in i915-param.c inside the gvt ifdef
>
> Im good with that.
> I'll probably copy the ifdef wrapper down rather than move the dec= l up.
> ie:
>
> #if __and(IS_ENABLED(CONFIG_DRM_I915_GVT),
>=C2=A0 =C2=A0 IS_ENABLED(CONFIG_DRM_USE_DYNAMIC_DEBUG))
>
> unsigned long __gvt_debug;
> EXPORT_SYMBOL(__gvt_debug);
>
>
>>> +
>>>=C2=A0 =C2=A0 # Please keep these build lists sorted!
>>>
>>>=C2=A0 =C2=A0 # core driver code
>>> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/= drm/i915/i915_params.c
>>> index e07f4cfea63a..e645e149485e 100644
>>> --- a/drivers/gpu/drm/i915/i915_params.c
>>> +++ b/drivers/gpu/drm/i915/i915_params.c
>>> @@ -265,3 +265,38 @@ void i915_params_free(struct i915_params = *params)
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0_DD_cat_("gvt:m= mio:"),
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0_DD_cat_("gvt:render:&quo= t;),
>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0_DD_cat_("gvt:sched:"= ;));
>>> +
>>> +#endif
>>
>> So just the foundation - no actual use sites I mean? How would the= se be
>> used from the code?
>>
>
> there are 120 pr_debug "users" :-)
>
> no users per se, but anyone using drm.debug
> /sys/module/drm/parameters/debug
> might use this too.
> its a bit easier than composing queries for >/proc/dyamic_debug/con= trol

Same as my previous question, perhaps I am not up to speed with this
yet.. Even if pr_debug is used inside GVT - are the categories and
debug_gvt global as of this patch (or series)?


they are already global in the sense t= hat if kernel is built with DYNAMIC_DEBUG,
the admin can turn tho= se pr_debugs on and off,=C2=A0and change their decorations in=C2=A0the=C2= =A0log (mod,func.line).
Nor are modules protected from each other= ; drm-core could use dd-exec-queries to enable/disable
pr-debugs = in i915 etc

This patch just adds a gvt-debug knob = like drm-debug. using the existing format prefixes to categorize them.
Whether those prefixes should be bent towards consistency with the re= st of drm-debug
or adapted towards some gvt community need I coul= dnt say.

Its no save-the-world feature, but its pr= etty cheap.

Id expect the same users as those who = play with drm.debug, for similar reasons.

does thi= s clarify ?
=C2=A0
Regards,

Tvrtko

thanks,
Jim=C2=A0
--0000000000005099b905cb572b8f--