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
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 <[email protected]>
---
. 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
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: <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
---
.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
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 <[email protected]>
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
On Thu, Jan 5, 2023 at 5:46 AM Daniel Vetter <[email protected]> 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 <[email protected]>
>
> 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