Re: [Intel-gfx] [PATCH v7 0/9] dyndbg: drm.debug adaptation

[Ville Syrjälä]

On Thu, Oct 27, 2022 at 09:37:52AM -0600, jim.cromie@xxxxxxxxx wrote:
> On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <jbaron@xxxxxxxxxx> wrote:
> >
> >
> >
> > On 10/21/22 05:18, Jani Nikula wrote:
> > > On Thu, 20 Oct 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> > >> On Sat, Sep 24, 2022 at 03:02:34PM +0200, Greg KH wrote:
> > >>> On Sun, Sep 11, 2022 at 11:28:43PM -0600, Jim Cromie wrote:
> > >>>> hi Greg, Dan, Jason, DRM-folk,
> > >>>>
> > >>>> heres follow-up to V6:
> > >>>>   rebased on driver-core/driver-core-next for -v6 applied bits (thanks)
> > >>>>   rework drm_debug_enabled{_raw,_instrumented,} per Dan.
> > >>>>
> > >>>> It excludes:
> > >>>>   nouveau parts (immature)
> > >>>>   tracefs parts (I missed --to=Steve on v6)
> > >>>>   split _ddebug_site and de-duplicate experiment (way unready)
> > >>>>
> > >>>> IOW, its the remaining commits of V6 on which Dan gave his Reviewed-by.
> > >>>>
> > >>>> If these are good to apply, I'll rebase and repost the rest separately.
> > >>>
> > >>> All now queued up, thanks.
> > >>
> > >> This stuff broke i915 debugs. When I first load i915 no debug prints are
> > >> produced. If I then go fiddle around in /sys/module/drm/parameters/debug
> > >> the debug prints start to suddenly work.
> > >
> > > Wait what? I always assumed the default behaviour would stay the same,
> > > which is usually how we roll. It's a regression in my books. We've got a
> > > CI farm that's not very helpful in terms of dmesg logging right now
> > > because of this.
> > >
> > > BR,
> > > Jani.
> > >
> > >
> >
> > That doesn't sound good - so you are saying that prior to this change some
> > of the drm debugs were default enabled. But now you have to manually enable
> > them?
> >
> > Thanks,
> >
> > -Jason
> 
> 
> Im just seeing this now.
> Any new details ?

No. We just disabled it as BROKEN for now. I was just today thinking
about sending that patch out if no solutin is forthcoming soon since
we need this working before 6.1 is released.

Pretty sure you should see the problem immediately with any driver 
(at least if it's built as a module, didn't try builtin). Or at least
can't think what would make i915 any more special.

> 
> I didnt knowingly change something, but since its apparently happening,
> heres a 1st WAG at a possible cause
> 
> commit ccc2b496324c13e917ef05f563626f4e7826bef1
> Author: Jim Cromie <jim.cromie@xxxxxxxxx>
> Date:   Sun Sep 11 23:28:51 2022 -0600
> 
>     drm_print: prefer bare printk KERN_DEBUG on generic fn
> 
>     drm_print.c calls pr_debug() just once, from __drm_printfn_debug(),
>     which is a generic/service fn.  The callsite is compile-time enabled
>     by DEBUG in both DYNAMIC_DEBUG=y/n builds.
> 
>     For dyndbg builds, reverting this callsite back to bare printk is
>     correcting a few anti-features:
> 
>     1- callsite is generic, serves multiple drm users.
>        it is soft-wired on currently by #define DEBUG
>        could accidentally: #> echo -p > /proc/dynamic_debug/control
> 
>     2- optional "decorations" by dyndbg are unhelpful/misleading here,
>        they describe only the generic site, not end users
> 
>     IOW, 1,2 are unhelpful at best, and possibly confusing.
> 
>     reverting yields a nominal data and text shrink:
> 
>        text    data     bss     dec     hex filename
>      462583   36604   54592 553779   87333 /kernel/drivers/gpu/drm/drm.ko
>      462515   36532   54592 553639   872a7 -dirty/kernel/drivers/gpu/drm/drm.ko
> 
>     Signed-off-by: Jim Cromie <jim.cromie@xxxxxxxxx>
>     Link: https://lore.kernel.org/r/20220912052852.1123868-9-jim.cromie@xxxxxxxxx
>     Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

-- 
Ville Syrjälä
Intel