[PATCH 0/2] DRM - avoid regression in -rc, fix comment

[PATCH 0/2] DRM - avoid regression in -rc, fix comment

[Jim Cromie]

hi DRM-folks,

DRM_USE_DYNAMIC_DEBUG has regression, mark as BROKEN for now.
Also correct a comment.

Jim Cromie (2):
  drm: mark CONFIG_DRM_USE_DYNAMIC_DEBUG as BROKEN for now
  drm_print: fix stale macro-name in comment

 drivers/gpu/drm/Kconfig | 3 ++-
 include/drm/drm_print.h | 5 ++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.38.1

[PATCH 1/2] drm: mark CONFIG_DRM_USE_DYNAMIC_DEBUG as BROKEN for now

[Jim Cromie]

CONFIG_DRM_USE_DYNAMIC_DEBUG=y has a regression, due to a chicken-egg
initialization problem:

1- modprobe i915
   i915 needs drm.ko, which is loaded 1st

2- "modprobe drm drm.debug=0x1ff" (virtual/implied)
   drm.debug is set post-initialization, from boot-args etc

3- `modprobe i915` finishes

W/O drm.debug-on-dyndbg that just works, because drm_dbg* does
drm_debug_enabled() to check __drm_debug & DRM_UT_<CAT> before
printing.

But the whole point of CONFIG_DRM_USE_DYNAMIC_DEBUG is to avoid that
runtime test, by enabling (at end/after module_init) a static-key at
selected callsites in the just-loaded module.

And since drm.ko is loaded before all dependent modules, no other
modules are "just-loaded", and their drm.debug callsites are not
present yet, just those in drm.ko itself.

CC: <ville.syrjala@linux.intel.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
.v2 - default=N, cuz CI tests BROKEN stuff too. @ville.syrjala
---
 drivers/gpu/drm/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 34f5a092c99e..5adc8d5b6a40 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -53,7 +53,8 @@ config DRM_DEBUG_MM
 
 config DRM_USE_DYNAMIC_DEBUG
 	bool "use dynamic debug to implement drm.debug"
-	default y
+	default n
+	depends on BROKEN	# chicken-egg initial enable problem
 	depends on DRM
 	depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
 	depends on JUMP_LABEL
-- 
2.38.1

[PATCH 2/2] drm_print: fix stale macro-name in comment

[Jim Cromie]

Cited commit uses stale macro name, fix this, and explain better.

When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_*
onto BITs in drm.debug.  This still uses enum drm_debug_category, but
it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals.
This requires that the macro args: DRM_UT_* list must be kept in sync
and in order.

Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
. emphasize ABI non-change despite enum val change - Jani Nikula
. reorder to back of patchset to follow API name changes.
---
 include/drm/drm_print.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index a44fb7ef257f..e4c0c7e6d49d 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
  *
  */
 enum drm_debug_category {
-	/* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
+	/*
+	 * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here,
+	 * the enum-values define BIT()s in drm.debug, so are ABI.
+	 */
 	/**
 	 * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
 	 * drm_memory.c, ...
-- 
2.38.1

Re: [PATCH 2/2] drm_print: fix stale macro-name in comment

[Daniel Vetter]

On Mon, Dec 05, 2022 at 09:10:05AM -0700, Jim Cromie wrote:
> Cited commit uses stale macro name, fix this, and explain better.
> 
> When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_*
> onto BITs in drm.debug.  This still uses enum drm_debug_category, but
> it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals.
> This requires that the macro args: DRM_UT_* list must be kept in sync
> and in order.
> 
> Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

What's the status of this series?

Greg, you landed the original entire pile that wasn't quite ready yet? Or
should I apply these two?
-Daniel

> ---
> . emphasize ABI non-change despite enum val change - Jani Nikula
> . reorder to back of patchset to follow API name changes.
> ---
>  include/drm/drm_print.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index a44fb7ef257f..e4c0c7e6d49d 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
>   *
>   */
>  enum drm_debug_category {
> -	/* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
> +	/*
> +	 * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here,
> +	 * the enum-values define BIT()s in drm.debug, so are ABI.
> +	 */
>  	/**
>  	 * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
>  	 * drm_memory.c, ...
> -- 
> 2.38.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/2] drm_print: fix stale macro-name in comment

[jim.cromie]

On Thu, Jan 5, 2023 at 5:46 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Dec 05, 2022 at 09:10:05AM -0700, Jim Cromie wrote:
> > Cited commit uses stale macro name, fix this, and explain better.
> >
> > When DRM_USE_DYNAMIC_DEBUG=y, DYNDBG_CLASSMAP_DEFINE() maps DRM_UT_*
> > onto BITs in drm.debug.  This still uses enum drm_debug_category, but
> > it is somewhat indirect, with the ordered set of DRM_UT_* enum-vals.
> > This requires that the macro args: DRM_UT_* list must be kept in sync
> > and in order.
> >
> > Fixes: f158936b60a7 ("drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.")
> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
>
> What's the status of this series?
>

dead - superseded by
https://patchwork.freedesktop.org/series/111652/
which is still WIP, but improved since that post.
I'll resubmit soon, with same title so patchwork calls it rev 2

> Greg, you landed the original entire pile that wasn't quite ready yet? Or
> should I apply these two?
> -Daniel
>
> > ---
> > . emphasize ABI non-change despite enum val change - Jani Nikula
> > . reorder to back of patchset to follow API name changes.
> > ---
> >  include/drm/drm_print.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> > index a44fb7ef257f..e4c0c7e6d49d 100644
> > --- a/include/drm/drm_print.h
> > +++ b/include/drm/drm_print.h
> > @@ -276,7 +276,10 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
> >   *
> >   */
> >  enum drm_debug_category {
> > -     /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
> > +     /*
> > +      * Keep DYNDBG_CLASSMAP_DEFINE args in sync with changes here,
> > +      * the enum-values define BIT()s in drm.debug, so are ABI.
> > +      */
> >       /**
> >        * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
> >        * drm_memory.c, ...
> > --
> > 2.38.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch