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.
These are also available at:
https://github.com/jimc/linux/releases/tag/dyndbg%2Fdd-drm-class-911
RFC:
DECLARE_DYNDBG_CLASSMAP's interface can be improved: class-names are
currently strings, like "DRM_UT_CORE", which must have corresponding
ENUM symbols defined. It would be better to just pass DRM_UT_CORE,
etc, and stringify the va-args inside the macro while initializing
classnames member. But how ?
Jim Cromie (9):
drm_print: condense enum drm_debug_category
drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
drm_print: interpose drm_*dbg with forwarding macros
drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro
drm-print.h: include dyndbg header
drm-print: add drm_dbg_driver to improve namespace symmetry
drm_print: optimize drm_debug_enabled for jump-label
drm_print: prefer bare printk KERN_DEBUG on generic fn
drm_print: add _ddebug descriptor to drm_*dbg prototypes
drivers/gpu/drm/Kconfig | 12 ++++
drivers/gpu/drm/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +++++
drivers/gpu/drm/display/drm_dp_helper.c | 13 +++++
drivers/gpu/drm/drm_crtc_helper.c | 13 +++++
drivers/gpu/drm/drm_print.c | 48 +++++++++++----
drivers/gpu/drm/i915/i915_params.c | 12 ++++
drivers/gpu/drm/nouveau/nouveau_drm.c | 13 +++++
include/drm/drm_print.h | 78 +++++++++++++++++++------
9 files changed, 174 insertions(+), 31 deletions(-)
--
2.37.3
enum drm_debug_category has 10 categories, but is initialized with
bitmasks which require 10 bits of underlying storage. By using
natural enumeration, and moving the BIT(cat) into drm_debug_enabled(),
the enum fits in 4 bits, allowing the category to be represented
directly in pr_debug callsites, via the ddebug.class_id field.
While this slightly pessimizes the bit-test in drm_debug_enabled(),
using dyndbg with JUMP_LABEL will avoid the function entirely.
NOTE: this change forecloses the possibility of doing:
drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "weird 2-cat experiment")
but thats already strongly implied by the use of the enum itself; its
not a normal enum if it can be 2 values simultaneously.
Signed-off-by: Jim Cromie <[email protected]>
---
include/drm/drm_print.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 22fabdeed297..b3b470440e46 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -279,49 +279,49 @@ enum drm_debug_category {
* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
* drm_memory.c, ...
*/
- DRM_UT_CORE = 0x01,
+ DRM_UT_CORE,
/**
* @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
* radeon, ... macro.
*/
- DRM_UT_DRIVER = 0x02,
+ DRM_UT_DRIVER,
/**
* @DRM_UT_KMS: Used in the modesetting code.
*/
- DRM_UT_KMS = 0x04,
+ DRM_UT_KMS,
/**
* @DRM_UT_PRIME: Used in the prime code.
*/
- DRM_UT_PRIME = 0x08,
+ DRM_UT_PRIME,
/**
* @DRM_UT_ATOMIC: Used in the atomic code.
*/
- DRM_UT_ATOMIC = 0x10,
+ DRM_UT_ATOMIC,
/**
* @DRM_UT_VBL: Used for verbose debug message in the vblank code.
*/
- DRM_UT_VBL = 0x20,
+ DRM_UT_VBL,
/**
* @DRM_UT_STATE: Used for verbose atomic state debugging.
*/
- DRM_UT_STATE = 0x40,
+ DRM_UT_STATE,
/**
* @DRM_UT_LEASE: Used in the lease code.
*/
- DRM_UT_LEASE = 0x80,
+ DRM_UT_LEASE,
/**
* @DRM_UT_DP: Used in the DP code.
*/
- DRM_UT_DP = 0x100,
+ DRM_UT_DP,
/**
* @DRM_UT_DRMRES: Used in the drm managed resources code.
*/
- DRM_UT_DRMRES = 0x200,
+ DRM_UT_DRMRES
};
static inline bool drm_debug_enabled(enum drm_debug_category category)
{
- return unlikely(__drm_debug & category);
+ return unlikely(__drm_debug & BIT(category));
}
/*
--
2.37.3
change drm_dev_dbg & drm_dbg to macros, which forward to the renamed
functions (with __ prefix added).
Those functions sit below the categorized layer of macros implementing
the DRM debug.category API, and implement most of it. These are good
places to insert dynamic-debug jump-label mechanics, which will allow
DRM to avoid the runtime cost of drm_debug_enabled().
no functional changes.
memory cost baseline: (unchanged)
bash-5.1# drms_load
[ 9.220389] dyndbg: 1 debug prints in module drm
[ 9.224426] ACPI: bus type drm_connector registered
[ 9.302192] dyndbg: 2 debug prints in module ttm
[ 9.305033] dyndbg: 8 debug prints in module video
[ 9.627563] dyndbg: 127 debug prints in module i915
[ 9.721505] AMD-Vi: AMD IOMMUv2 functionality not available on this system - This is not a bug.
[ 10.091345] dyndbg: 2196 debug prints in module amdgpu
[ 10.106589] [drm] amdgpu kernel modesetting enabled.
[ 10.107270] amdgpu: CRAT table not found
[ 10.107926] amdgpu: Virtual CRAT table created for CPU
[ 10.108398] amdgpu: Topology: Add CPU node
[ 10.168507] dyndbg: 3 debug prints in module wmi
[ 10.329587] dyndbg: 3 debug prints in module nouveau
Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/drm_print.c | 10 +++++-----
include/drm/drm_print.h | 9 +++++++--
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ec32df35a3e3..29a29949ad0b 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -279,8 +279,8 @@ void drm_dev_printk(const struct device *dev, const char *level,
}
EXPORT_SYMBOL(drm_dev_printk);
-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
- const char *format, ...)
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+ const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -301,9 +301,9 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
va_end(args);
}
-EXPORT_SYMBOL(drm_dev_dbg);
+EXPORT_SYMBOL(__drm_dev_dbg);
-void __drm_dbg(enum drm_debug_category category, const char *format, ...)
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -320,7 +320,7 @@ void __drm_dbg(enum drm_debug_category category, const char *format, ...)
va_end(args);
}
-EXPORT_SYMBOL(__drm_dbg);
+EXPORT_SYMBOL(___drm_dbg);
void __drm_err(const char *format, ...)
{
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 668273e36c2c..c429c258c957 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -335,7 +335,7 @@ __printf(3, 4)
void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...);
__printf(3, 4)
-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
const char *format, ...);
/**
@@ -384,6 +384,9 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
} \
})
+#define drm_dev_dbg(dev, cat, fmt, ...) \
+ __drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+
/**
* DRM_DEV_DEBUG() - Debug output for generic drm code
*
@@ -485,10 +488,12 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
*/
__printf(2, 3)
-void __drm_dbg(enum drm_debug_category category, const char *format, ...);
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
__printf(1, 2)
void __drm_err(const char *format, ...);
+#define __drm_dbg(fmt, ...) ___drm_dbg(fmt, ##__VA_ARGS__)
+
/* Macros to make printk easier */
#define _DRM_PRINTK(once, level, fmt, ...) \
--
2.37.3
For CONFIG_DRM_USE_DYNAMIC_DEBUG=y, wrap __drm_dbg() & __drm_dev_dbg()
in one of dyndbg's Factory macros: _dynamic_func_call_no_desc().
This adds the callsite descriptor into the code, and an entry for each
into /proc/dynamic_debug/control.
#> echo class DRM_UT_ATOMIC +p > /proc/dynamic_debug/control
CONFIG_DRM_USE_DYNAMIC_DEBUG=y/n is configurable because of the .data
footprint cost of per-callsite control; 56 bytes/site * ~2k for i915,
~4k callsites for amdgpu. This is large enough that a kernel builder
might not want it.
Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/Kconfig | 12 ++++++++++++
drivers/gpu/drm/Makefile | 2 ++
include/drm/drm_print.h | 12 ++++++++++++
3 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 6c2256e8474b..2438e0dccfa1 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -50,6 +50,18 @@ config DRM_DEBUG_MM
If in doubt, say "N".
+config DRM_USE_DYNAMIC_DEBUG
+ bool "use dynamic debug to implement drm.debug"
+ default y
+ depends on DRM
+ depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
+ depends on JUMP_LABEL
+ help
+ Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
+ Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
+ bytes per callsite, the .data costs can be substantial, and
+ are therefore configurable.
+
config DRM_DEBUG_SELFTEST
tristate "kselftests for DRM"
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e7af358e6dda..6828197967a6 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -3,6 +3,8 @@
# Makefile for the drm device driver. This driver provides support for the
# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
+CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+
drm-y := drm_aperture.o drm_auth.o drm_cache.o \
drm_file.o drm_gem.o drm_ioctl.o \
drm_drv.o \
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index c429c258c957..2d2cef76b5c1 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -384,8 +384,14 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
} \
})
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
#define drm_dev_dbg(dev, cat, fmt, ...) \
__drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+#else
+#define drm_dev_dbg(dev, cat, fmt, ...) \
+ _dynamic_func_call_no_desc(fmt, __drm_dev_dbg, \
+ dev, cat, fmt, ##__VA_ARGS__)
+#endif
/**
* DRM_DEV_DEBUG() - Debug output for generic drm code
@@ -492,7 +498,13 @@ void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
__printf(1, 2)
void __drm_err(const char *format, ...);
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
#define __drm_dbg(fmt, ...) ___drm_dbg(fmt, ##__VA_ARGS__)
+#else
+#define __drm_dbg(cat, fmt, ...) \
+ _dynamic_func_call_no_desc(fmt, ___drm_dbg, \
+ cat, fmt, ##__VA_ARGS__)
+#endif
/* Macros to make printk easier */
--
2.37.3
When CONFIG_DRM_USE_DYNAMIC_DEBUG=y, the drm.debug API (a macro stack,
calling _+drm_*dbg() eventually) invokes a dyndbg Factory macro to
create a descriptor for each callsite, thus making them individually
>control-able.
In this case, the calls to _drm_*dbg are unreachable unless the
callsite is enabled. So those calls can short-circuit their early
do-nothing returns. Provide and use __drm_debug_enabled(), to do this
when config'd, or the _raw flags-check otherwize.
And since dyndbg is in use, lets also instrument the remaining users
of drm_debug_enabled, by wrapping the _raw in a macro with a:
pr_debug("todo: is this frequent enough to optimize ?\n");
For CONFIG_DRM_USE_DYNAMIC_DEBUG=n, do no site instrumenting at all,
since JUMP_LABEL might be off, and we don't want to make work.
With drm, amdgpu, i915, nouveau loaded, heres remaining uses of
drm_debug_enabled(), which costs ~1.5kb data to control the
pr_debug("todo:..")s.
Some of those uses might be ok to use __drm_debug_enabled() by
inspection, others might warrant conversion to use dyndbg Factory
macros, and that would want callrate data to estimate the savings
possible. TBH, any remaining savings are probably small; drm.debug
covers the vast bulk of the uses. Maybe "vblank" is the exception.
:#> grep todo /proc/dynamic_debug/control | wc
21 168 2357
:#> grep todo /proc/dynamic_debug/control
drivers/gpu/drm/drm_edid_load.c:178 [drm]edid_load =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:410 [drm]drm_crtc_accurate_vblank_count =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:787 [drm]drm_crtc_vblank_helper_get_vblank_timestamp_internal =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:1491 [drm]drm_vblank_restore =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:1433 [drm]drm_vblank_enable =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_plane.c:2168 [drm]drm_mode_setplane =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:1359 [drm_display_helper]drm_dp_mst_wait_tx_reply =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:2864 [drm_display_helper]process_single_tx_qlock =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:2909 [drm_display_helper]drm_dp_queue_down_tx =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:1686 [drm_display_helper]drm_dp_mst_update_slots =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_dp.c:1111 [i915]intel_dp_print_rates =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_backlight.c:5434 [i915]cnp_enable_backlight =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_backlight.c:5459 [i915]intel_backlight_device_register =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_opregion.c:43 [i915]intel_opregion_notify_encoder =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_opregion.c:53 [i915]asle_set_backlight =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_bios.c:1088 [i915]intel_bios_is_dsi_present =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_display_debugfs.c:6153 [i915]i915_drrs_ctl_set =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/intel_pcode.c:26 [i915]snb_pcode_read =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/i915_getparam.c:785 [i915]i915_getparam_ioctl =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c:282 [amdgpu]vcn_v2_5_process_interrupt =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c:433 [amdgpu]vcn_v2_0_process_interrupt =_ "todo: maybe avoid via dyndbg\n"
:#>
Signed-off-by: Jim Cromie <[email protected]>
---
- simplify drm-debug-enabled choices, @DanVet
---
drivers/gpu/drm/drm_print.c | 4 ++--
include/drm/drm_print.h | 21 ++++++++++++++++++++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 29a29949ad0b..cb203d63b286 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -285,7 +285,7 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
struct va_format vaf;
va_list args;
- if (!drm_debug_enabled(category))
+ if (!__drm_debug_enabled(category))
return;
va_start(args, format);
@@ -308,7 +308,7 @@ void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
struct va_format vaf;
va_list args;
- if (!drm_debug_enabled(category))
+ if (!__drm_debug_enabled(category))
return;
va_start(args, format);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index dfdd81c3287c..9af57d3df259 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -321,11 +321,30 @@ enum drm_debug_category {
DRM_UT_DRMRES
};
-static inline bool drm_debug_enabled(enum drm_debug_category category)
+static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
{
return unlikely(__drm_debug & BIT(category));
}
+#define drm_debug_enabled_instrumented(category) \
+ ({ \
+ pr_debug("todo: is this frequent enough to optimize ?\n"); \
+ drm_debug_enabled_raw(category); \
+ })
+
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+/*
+ * the drm.debug API uses dyndbg, so each drm_*dbg macro/callsite gets
+ * a descriptor, and only enabled callsites are reachable. They use
+ * the private macro to avoid re-testing the enable-bit.
+ */
+#define __drm_debug_enabled(category) true
+#define drm_debug_enabled(category) drm_debug_enabled_instrumented(category)
+#else
+#define __drm_debug_enabled(category) drm_debug_enabled_raw(category)
+#define drm_debug_enabled(category) drm_debug_enabled_raw(category)
+#endif
+
/*
* struct device based logging
*
--
2.37.3
lkp robot told me:
>> drivers/gpu/drm/drm_ioc32.c:989:2:
error: call to undeclared function '_dynamic_func_call_cls';
ISO C99 and later do not support implicit function declarations
[-Wimplicit-function-declaration]
DRM_DEBUG("comm=\"%s\", pid=%d, dev=0x%lx, auth=%d, %s\n",
Since that macro is defined in drm_print.h, and under DRM_USE_DYN*=y
configs, invokes dyndbg-factory macros, include dynamic_debug.h from
there too, so that those configs have the definitions of all the
macros in the callchain.
This is done as a separate patch mostly to see how lkp sorts it.
Signed-off-by: Jim Cromie <[email protected]>
---
include/drm/drm_print.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2d2cef76b5c1..f8bb3e7158c6 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -31,6 +31,7 @@
#include <linux/seq_file.h>
#include <linux/device.h>
#include <linux/debugfs.h>
+#include <linux/dynamic_debug.h>
#include <drm/drm.h>
--
2.37.3
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 <[email protected]>
---
drivers/gpu/drm/drm_print.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index cb203d63b286..ec477c44a784 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -23,8 +23,6 @@
* Rob Clark <[email protected]>
*/
-#define DEBUG /* for pr_debug() */
-
#include <linux/stdarg.h>
#include <linux/io.h>
@@ -185,7 +183,8 @@ EXPORT_SYMBOL(__drm_printfn_info);
void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf)
{
- pr_debug("%s %pV", p->prefix, vaf);
+ /* pr_debug callsite decorations are unhelpful here */
+ printk(KERN_DEBUG "%s %pV", p->prefix, vaf);
}
EXPORT_SYMBOL(__drm_printfn_debug);
--
2.37.3
drm_print defines all of these:
drm_dbg_{core,kms,prime,atomic,vbl,lease,_dp,_drmres}
but not drm_dbg_driver itself, since it was the original drm_dbg.
To improve namespace symmetry, change the drm_dbg defn to
drm_dbg_driver, and redef grandfathered name to symmetric one.
This will help with nouveau, which uses its own stack of macros to
construct calls to dev_info, dev_dbg, etc, for which adaptation means
drm_dbg_##driver constructs.
Signed-off-by: Jim Cromie <[email protected]>
---
include/drm/drm_print.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index f8bb3e7158c6..dfdd81c3287c 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -468,7 +468,7 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
#define drm_dbg_core(drm, fmt, ...) \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_CORE, fmt, ##__VA_ARGS__)
-#define drm_dbg(drm, fmt, ...) \
+#define drm_dbg_driver(drm, fmt, ...) \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
#define drm_dbg_kms(drm, fmt, ...) \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_KMS, fmt, ##__VA_ARGS__)
@@ -487,6 +487,7 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
#define drm_dbg_drmres(drm, fmt, ...) \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRMRES, fmt, ##__VA_ARGS__)
+#define drm_dbg(drm, fmt, ...) drm_dbg_driver(drm, fmt, ##__VA_ARGS__)
/*
* printk based logging
--
2.37.3
Use DECLARE_DYNDBG_CLASSMAP across DRM:
- in .c files, since macro defines/initializes a record
- in drivers, $mod_{drv,drm,param}.c
ie where param setup is done, since a classmap is param related
- in drm/drm_print.c
since existing __drm_debug param is defined there,
and we ifdef it, and provide an elaborated alternative.
- in drm_*_helper modules:
dp/drm_dp - 1st item in makefile target
drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
Since these modules all use identical CLASSMAP declarations (ie: names
and .class_id's) they will all respond together to "class DRM_UT_*"
query-commands:
:#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
NOTES:
This changes __drm_debug from int to ulong, so BIT() is usable on it.
DRM's enum drm_debug_category values need to sync with the index of
their respective class-names here. Then .class_id == category, and
dyndbg's class FOO mechanisms will enable drm_dbg(DRM_UT_KMS, ...).
Though DRM needs consistent categories across all modules, thats not
generally needed; modules X and Y could define FOO differently (ie a
different NAME => class_id mapping), changes are made according to
each module's private class-map.
No callsites are actually selected by this patch, since none are
class'd yet.
Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +++++++++++++
drivers/gpu/drm/display/drm_dp_helper.c | 13 ++++++++++++
drivers/gpu/drm/drm_crtc_helper.c | 13 ++++++++++++
drivers/gpu/drm/drm_print.c | 27 +++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_params.c | 12 +++++++++++
drivers/gpu/drm/nouveau/nouveau_drm.c | 13 ++++++++++++
include/drm/drm_print.h | 3 ++-
7 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 429fcdf28836..5f091cb52de2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -38,6 +38,8 @@
#include <linux/mmu_notifier.h>
#include <linux/suspend.h>
#include <linux/cc_platform.h>
+#include <linux/fb.h>
+#include <linux/dynamic_debug.h>
#include "amdgpu.h"
#include "amdgpu_irq.h"
@@ -185,6 +187,18 @@ int amdgpu_vcnfw_log;
static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
struct amdgpu_mgpu_info mgpu_info = {
.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
.delayed_reset_work = __DELAYED_WORK_INITIALIZER(
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index e5bab236b3ae..196dfb1e8d87 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -30,6 +30,7 @@
#include <linux/sched.h>
#include <linux/seq_file.h>
#include <linux/string_helpers.h>
+#include <linux/dynamic_debug.h>
#include <drm/display/drm_dp_helper.h>
#include <drm/display/drm_dp_mst_helper.h>
@@ -40,6 +41,18 @@
#include "drm_dp_helper_internal.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
struct dp_aux_backlight {
struct backlight_device *base;
struct drm_dp_aux *aux;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 8a6d54515f92..a8cee6694cf6 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -32,6 +32,7 @@
#include <linux/export.h>
#include <linux/kernel.h>
#include <linux/moduleparam.h>
+#include <linux/dynamic_debug.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
@@ -51,6 +52,18 @@
#include "drm_crtc_helper_internal.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
/**
* DOC: overview
*
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index f783d4963d4b..ec32df35a3e3 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -40,7 +40,7 @@
* __drm_debug: Enable debug output.
* Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
*/
-unsigned int __drm_debug;
+unsigned long __drm_debug;
EXPORT_SYMBOL(__drm_debug);
MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
@@ -52,7 +52,30 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)\n"
"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
-module_param_named(debug, __drm_debug, int, 0600);
+
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+module_param_named(debug, __drm_debug, ulong, 0600);
+#else
+/* classnames must match vals of enum drm_debug_category */
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
+static struct ddebug_class_param drm_debug_bitmap = {
+ .bits = &__drm_debug,
+ .flags = "p",
+ .map = &drm_debug_classes,
+};
+module_param_cb(debug, ¶m_ops_dyndbg_classes, &drm_debug_bitmap, 0600);
+#endif
void __drm_puts_coredump(struct drm_printer *p, const char *str)
{
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 6fc475a5db61..d1e4d528cb17 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -29,6 +29,18 @@
#include "i915_params.h"
#include "i915_drv.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
#define i915_param_named(name, T, perm, desc) \
module_param_named(name, i915_modparams.name, T, perm); \
MODULE_PARM_DESC(name, desc)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 561309d447e0..fd99ec0f4257 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -28,6 +28,7 @@
#include <linux/pm_runtime.h>
#include <linux/vga_switcheroo.h>
#include <linux/mmu_notifier.h>
+#include <linux/dynamic_debug.h>
#include <drm/drm_aperture.h>
#include <drm/drm_crtc_helper.h>
@@ -70,6 +71,18 @@
#include "nouveau_svm.h"
#include "nouveau_dmem.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
MODULE_PARM_DESC(config, "option string to pass to driver core");
static char *nouveau_config;
module_param_named(config, nouveau_config, charp, 0400);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index b3b470440e46..668273e36c2c 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -35,7 +35,7 @@
#include <drm/drm.h>
/* Do *not* use outside of drm_print.[ch]! */
-extern unsigned int __drm_debug;
+extern unsigned long __drm_debug;
/**
* DOC: print
@@ -275,6 +275,7 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
*
*/
enum drm_debug_category {
+ /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
/**
* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
* drm_memory.c, ...
--
2.37.3
upgrade the callchain to drm_dbg() and drm_dev_dbg(); add a struct
_ddebug ptr parameter to them, and supply that additional param by
replacing the '_no_desc' flavor of dyndbg Factory macro currently used
with the flavor that supplies the descriptor.
NOTES:
The descriptor gives these fns access to the decorator flags, but they
do none of the dynamic-prefixing done by dynamic_emit_prefix(), which
is currently static.
DRM already has conventions for logging/messaging; just tossing
optional decorations on top probably wouldn't help. Instead, existing
flags (or new ones, perhaps 'sd' ala lspci) can be used to make
current message conventions optional. This suggests a new
drmdbg_prefix_emit() to handle prefixing locally.
For CONFIG_DRM_USE_DYNAMIC_DEBUG=N, just pass null descriptor.
desc->class_id is redundant with category parameter, but its
availability is dependent on desc.
Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/drm_print.c | 8 +++++---
include/drm/drm_print.h | 23 ++++++++++++-----------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ec477c44a784..5b93c11895bb 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -29,6 +29,7 @@
#include <linux/moduleparam.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
+#include <linux/dynamic_debug.h>
#include <drm/drm.h>
#include <drm/drm_drv.h>
@@ -278,8 +279,8 @@ void drm_dev_printk(const struct device *dev, const char *level,
}
EXPORT_SYMBOL(drm_dev_printk);
-void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
- const char *format, ...)
+void __drm_dev_dbg(struct _ddebug *desc, const struct device *dev,
+ enum drm_debug_category category, const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -287,6 +288,7 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
if (!__drm_debug_enabled(category))
return;
+ /* we know we are printing for either syslog, tracefs, or both */
va_start(args, format);
vaf.fmt = format;
vaf.va = &args;
@@ -302,7 +304,7 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
}
EXPORT_SYMBOL(__drm_dev_dbg);
-void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
+void ___drm_dbg(struct _ddebug *desc, enum drm_debug_category category, const char *format, ...)
{
struct va_format vaf;
va_list args;
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 9af57d3df259..a44fb7ef257f 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -354,9 +354,10 @@ static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
__printf(3, 4)
void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...);
-__printf(3, 4)
-void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
- const char *format, ...);
+struct _ddebug;
+__printf(4, 5)
+void __drm_dev_dbg(struct _ddebug *desc, const struct device *dev,
+ enum drm_debug_category category, const char *format, ...);
/**
* DRM_DEV_ERROR() - Error output.
@@ -406,11 +407,11 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
#define drm_dev_dbg(dev, cat, fmt, ...) \
- __drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+ __drm_dev_dbg(NULL, dev, cat, fmt, ##__VA_ARGS__)
#else
#define drm_dev_dbg(dev, cat, fmt, ...) \
- _dynamic_func_call_no_desc(fmt, __drm_dev_dbg, \
- dev, cat, fmt, ##__VA_ARGS__)
+ _dynamic_func_call_cls(cat, fmt, __drm_dev_dbg, \
+ dev, cat, fmt, ##__VA_ARGS__)
#endif
/**
@@ -514,17 +515,17 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
* Prefer drm_device based logging over device or prink based logging.
*/
-__printf(2, 3)
-void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
+__printf(3, 4)
+void ___drm_dbg(struct _ddebug *desc, enum drm_debug_category category, const char *format, ...);
__printf(1, 2)
void __drm_err(const char *format, ...);
#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-#define __drm_dbg(fmt, ...) ___drm_dbg(fmt, ##__VA_ARGS__)
+#define __drm_dbg(fmt, ...) ___drm_dbg(NULL, fmt, ##__VA_ARGS__)
#else
#define __drm_dbg(cat, fmt, ...) \
- _dynamic_func_call_no_desc(fmt, ___drm_dbg, \
- cat, fmt, ##__VA_ARGS__)
+ _dynamic_func_call_cls(cat, fmt, ___drm_dbg, \
+ cat, fmt, ##__VA_ARGS__)
#endif
/* Macros to make printk easier */
--
2.37.3
On Sun, 11 Sep 2022, Jim Cromie <[email protected]> wrote:
> Use DECLARE_DYNDBG_CLASSMAP across DRM:
>
> - in .c files, since macro defines/initializes a record
>
> - in drivers, $mod_{drv,drm,param}.c
> ie where param setup is done, since a classmap is param related
>
> - in drm/drm_print.c
> since existing __drm_debug param is defined there,
> and we ifdef it, and provide an elaborated alternative.
>
> - in drm_*_helper modules:
> dp/drm_dp - 1st item in makefile target
> drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
>
> Since these modules all use identical CLASSMAP declarations (ie: names
> and .class_id's) they will all respond together to "class DRM_UT_*"
> query-commands:
>
> :#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
>
> NOTES:
>
> This changes __drm_debug from int to ulong, so BIT() is usable on it.
>
> DRM's enum drm_debug_category values need to sync with the index of
> their respective class-names here. Then .class_id == category, and
> dyndbg's class FOO mechanisms will enable drm_dbg(DRM_UT_KMS, ...).
>
> Though DRM needs consistent categories across all modules, thats not
> generally needed; modules X and Y could define FOO differently (ie a
> different NAME => class_id mapping), changes are made according to
> each module's private class-map.
>
> No callsites are actually selected by this patch, since none are
> class'd yet.
The commit message could start off by saying each module needs to define
DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, ...). That is, IIUC.
Where's DECLARE_DYNDBG_CLASSMAP defined? linux-next? What's it do? What
if multiple modules with that are actually builtin?
The duplication and requirement that they're identical seems like an
error prone combo.
Finally, the choice of placement in e.g. i915_params.c seems completely
arbitrary, and makes you wonder "what here requires this, nothing?".
BR,
Jani.
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +++++++++++++
> drivers/gpu/drm/display/drm_dp_helper.c | 13 ++++++++++++
> drivers/gpu/drm/drm_crtc_helper.c | 13 ++++++++++++
> drivers/gpu/drm/drm_print.c | 27 +++++++++++++++++++++++--
> drivers/gpu/drm/i915/i915_params.c | 12 +++++++++++
> drivers/gpu/drm/nouveau/nouveau_drm.c | 13 ++++++++++++
> include/drm/drm_print.h | 3 ++-
> 7 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 429fcdf28836..5f091cb52de2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -38,6 +38,8 @@
> #include <linux/mmu_notifier.h>
> #include <linux/suspend.h>
> #include <linux/cc_platform.h>
> +#include <linux/fb.h>
> +#include <linux/dynamic_debug.h>
>
> #include "amdgpu.h"
> #include "amdgpu_irq.h"
> @@ -185,6 +187,18 @@ int amdgpu_vcnfw_log;
>
> static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> struct amdgpu_mgpu_info mgpu_info = {
> .mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
> .delayed_reset_work = __DELAYED_WORK_INITIALIZER(
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index e5bab236b3ae..196dfb1e8d87 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -30,6 +30,7 @@
> #include <linux/sched.h>
> #include <linux/seq_file.h>
> #include <linux/string_helpers.h>
> +#include <linux/dynamic_debug.h>
>
> #include <drm/display/drm_dp_helper.h>
> #include <drm/display/drm_dp_mst_helper.h>
> @@ -40,6 +41,18 @@
>
> #include "drm_dp_helper_internal.h"
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> struct dp_aux_backlight {
> struct backlight_device *base;
> struct drm_dp_aux *aux;
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 8a6d54515f92..a8cee6694cf6 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -32,6 +32,7 @@
> #include <linux/export.h>
> #include <linux/kernel.h>
> #include <linux/moduleparam.h>
> +#include <linux/dynamic_debug.h>
>
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> @@ -51,6 +52,18 @@
>
> #include "drm_crtc_helper_internal.h"
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> /**
> * DOC: overview
> *
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index f783d4963d4b..ec32df35a3e3 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -40,7 +40,7 @@
> * __drm_debug: Enable debug output.
> * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
> */
> -unsigned int __drm_debug;
> +unsigned long __drm_debug;
> EXPORT_SYMBOL(__drm_debug);
>
> MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
> @@ -52,7 +52,30 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
> "\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
> "\t\tBit 7 (0x80) will enable LEASE messages (leasing code)\n"
> "\t\tBit 8 (0x100) will enable DP messages (displayport code)");
> -module_param_named(debug, __drm_debug, int, 0600);
> +
> +#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
> +module_param_named(debug, __drm_debug, ulong, 0600);
> +#else
> +/* classnames must match vals of enum drm_debug_category */
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> +static struct ddebug_class_param drm_debug_bitmap = {
> + .bits = &__drm_debug,
> + .flags = "p",
> + .map = &drm_debug_classes,
> +};
> +module_param_cb(debug, ¶m_ops_dyndbg_classes, &drm_debug_bitmap, 0600);
> +#endif
>
> void __drm_puts_coredump(struct drm_printer *p, const char *str)
> {
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 6fc475a5db61..d1e4d528cb17 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -29,6 +29,18 @@
> #include "i915_params.h"
> #include "i915_drv.h"
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> #define i915_param_named(name, T, perm, desc) \
> module_param_named(name, i915_modparams.name, T, perm); \
> MODULE_PARM_DESC(name, desc)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 561309d447e0..fd99ec0f4257 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -28,6 +28,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/vga_switcheroo.h>
> #include <linux/mmu_notifier.h>
> +#include <linux/dynamic_debug.h>
>
> #include <drm/drm_aperture.h>
> #include <drm/drm_crtc_helper.h>
> @@ -70,6 +71,18 @@
> #include "nouveau_svm.h"
> #include "nouveau_dmem.h"
>
> +DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
> + "DRM_UT_CORE",
> + "DRM_UT_DRIVER",
> + "DRM_UT_KMS",
> + "DRM_UT_PRIME",
> + "DRM_UT_ATOMIC",
> + "DRM_UT_VBL",
> + "DRM_UT_STATE",
> + "DRM_UT_LEASE",
> + "DRM_UT_DP",
> + "DRM_UT_DRMRES");
> +
> MODULE_PARM_DESC(config, "option string to pass to driver core");
> static char *nouveau_config;
> module_param_named(config, nouveau_config, charp, 0400);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index b3b470440e46..668273e36c2c 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -35,7 +35,7 @@
> #include <drm/drm.h>
>
> /* Do *not* use outside of drm_print.[ch]! */
> -extern unsigned int __drm_debug;
> +extern unsigned long __drm_debug;
>
> /**
> * DOC: print
> @@ -275,6 +275,7 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
> *
> */
> enum drm_debug_category {
> + /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
> /**
> * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
> * drm_memory.c, ...
--
Jani Nikula, Intel Open Source Graphics Center
On Sun, 11 Sep 2022, Jim Cromie <[email protected]> wrote:
> enum drm_debug_category has 10 categories, but is initialized with
> bitmasks which require 10 bits of underlying storage. By using
> natural enumeration, and moving the BIT(cat) into drm_debug_enabled(),
> the enum fits in 4 bits, allowing the category to be represented
> directly in pr_debug callsites, via the ddebug.class_id field.
>
> While this slightly pessimizes the bit-test in drm_debug_enabled(),
> using dyndbg with JUMP_LABEL will avoid the function entirely.
>
> NOTE: this change forecloses the possibility of doing:
>
> drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "weird 2-cat experiment")
>
> but thats already strongly implied by the use of the enum itself; its
> not a normal enum if it can be 2 values simultaneously.
The drm.debug module parameter values are, arguably, ABI. There are tons
of people, scripts, test environments, documentation, bug reports, etc,
etc, referring to specific drm.debug module parameter values to enable
specific drm debug logging categories.
AFAICT you're not changing any of the values here, but having an enum
without the hard coded values makes it more likely to accidentally
change the category to bit mapping. At the very least deserves a
comment.
BR,
Jani.
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> include/drm/drm_print.h | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 22fabdeed297..b3b470440e46 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -279,49 +279,49 @@ enum drm_debug_category {
> * @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
> * drm_memory.c, ...
> */
> - DRM_UT_CORE = 0x01,
> + DRM_UT_CORE,
> /**
> * @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
> * radeon, ... macro.
> */
> - DRM_UT_DRIVER = 0x02,
> + DRM_UT_DRIVER,
> /**
> * @DRM_UT_KMS: Used in the modesetting code.
> */
> - DRM_UT_KMS = 0x04,
> + DRM_UT_KMS,
> /**
> * @DRM_UT_PRIME: Used in the prime code.
> */
> - DRM_UT_PRIME = 0x08,
> + DRM_UT_PRIME,
> /**
> * @DRM_UT_ATOMIC: Used in the atomic code.
> */
> - DRM_UT_ATOMIC = 0x10,
> + DRM_UT_ATOMIC,
> /**
> * @DRM_UT_VBL: Used for verbose debug message in the vblank code.
> */
> - DRM_UT_VBL = 0x20,
> + DRM_UT_VBL,
> /**
> * @DRM_UT_STATE: Used for verbose atomic state debugging.
> */
> - DRM_UT_STATE = 0x40,
> + DRM_UT_STATE,
> /**
> * @DRM_UT_LEASE: Used in the lease code.
> */
> - DRM_UT_LEASE = 0x80,
> + DRM_UT_LEASE,
> /**
> * @DRM_UT_DP: Used in the DP code.
> */
> - DRM_UT_DP = 0x100,
> + DRM_UT_DP,
> /**
> * @DRM_UT_DRMRES: Used in the drm managed resources code.
> */
> - DRM_UT_DRMRES = 0x200,
> + DRM_UT_DRMRES
> };
>
> static inline bool drm_debug_enabled(enum drm_debug_category category)
> {
> - return unlikely(__drm_debug & category);
> + return unlikely(__drm_debug & BIT(category));
> }
>
> /*
--
Jani Nikula, Intel Open Source Graphics Center
On Mon, Sep 12, 2022 at 4:29 AM Jani Nikula <[email protected]> wrote:
>
> On Sun, 11 Sep 2022, Jim Cromie <[email protected]> wrote:
> > Use DECLARE_DYNDBG_CLASSMAP across DRM:
> >
> > - in .c files, since macro defines/initializes a record
> >
> > - in drivers, $mod_{drv,drm,param}.c
> > ie where param setup is done, since a classmap is param related
> >
> > - in drm/drm_print.c
> > since existing __drm_debug param is defined there,
> > and we ifdef it, and provide an elaborated alternative.
> >
> > - in drm_*_helper modules:
> > dp/drm_dp - 1st item in makefile target
> > drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
> >
> > Since these modules all use identical CLASSMAP declarations (ie: names
> > and .class_id's) they will all respond together to "class DRM_UT_*"
> > query-commands:
>
> The commit message could start off by saying each module needs to define
> DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, ...). That is, IIUC.
>
Yes, I see your point.
All the explanations missing here are in preceding commits,
now in GregKHs driver-core/driver-core-next tree,
so I didnt resend them.
> Where's DECLARE_DYNDBG_CLASSMAP defined? linux-next? What's it do? What
> if multiple modules with that are actually builtin?
The commit that adds the macro is at
https://lore.kernel.org/lkml/[email protected]/
there are many combos of builtin, Ive done at least several:
with caveat that >98% of testing is on virtme (thanks for that tool)
- test_dynamic_debug as module, and builtin.
it has multiple macro uses, showing 1 kind of sharing
- drm as builtin, drivers as modules
that surely pulled in other drm-helpers as builtins
- all loadable modules mostly.
>
> The duplication and requirement that they're identical seems like an
> error prone combo.
I freely acknowledge(d) this is sub-optimal.
There might be a best place for a single declaration that is in-scope
across multiple modules, but I dont know the drm core/driver lifetime
well enough to just drop this into that place.
I may have complicated things by starting with a static struct holding
the classmap, that choice was driven by:
- static declaration into a section solved a problem where the class
definitions
were "registered" (term used in patchset, -v2-3?) too late to be available for
modprobe i915 dyndbg='class DRM_UT_CORE +p'
but worked afterwards
(also true for builtins and boot-time $mod.dyndbg='class notworking +p')
Another subtlety - the "sharing" is due more to: drm_dbg(DRM_UT_*, "")
Im not sure precisely how this might matter.
I also had an "incompleteness" argument circling in my head - something like;
you cant simultaneously allow a drm-wanna-be module to declare "DRM_UT_CORE"
but disallow "DRM_UT_ILL_CONSIDERED". I kind-of stopped there.
Theres also an issue where multiple declarations in a module must
avoid range overlap.
I had no idea how to put that into a BUILD_BUG_ON.
Its done manually, with commentary, in test-dynamic-debug.
Maybe both issues can be improved somewhat by changing the macro
to expect real ENUM symbols, (and stringify _VA_ARGS_ to init the classnames),
not the quoted "DRM_UT_*"s it gets now. That would also obsolete the _base,
since its the value of DRM_UT_CORE (the 1st enum val).
But that still leaves the enum vals enumerated, with possibility of
omission or mixup,
which unlike a spelling error wouldnt get caught, and would be wrong.
I fiddled with the 1st part of that for a while, I lack the macro-fu,
and punted.
Im happy to try an alternative approach, particularly with elaborated
suggestions.
>
> Finally, the choice of placement in e.g. i915_params.c seems completely
> arbitrary, and makes you wonder "what here requires this, nothing?".
acknowledged - I put it there because the access to it is via a parameter,
namely one that already affects it from a distance:
/sys/module/drm/parameters/debug - ie drm.dbg
And its not even i915's parameter.
>
> BR,
> Jani.
>
thanks,
>
> >
> > Signed-off-by: Jim Cromie <[email protected]>
On Mon, Sep 12, 2022 at 4:17 AM Jani Nikula <[email protected]> wrote:
>
> On Sun, 11 Sep 2022, Jim Cromie <[email protected]> wrote:
> > enum drm_debug_category has 10 categories, but is initialized with
> > bitmasks which require 10 bits of underlying storage. By using
> > natural enumeration, and moving the BIT(cat) into drm_debug_enabled(),
> > the enum fits in 4 bits, allowing the category to be represented
> > directly in pr_debug callsites, via the ddebug.class_id field.
> >
> > While this slightly pessimizes the bit-test in drm_debug_enabled(),
> > using dyndbg with JUMP_LABEL will avoid the function entirely.
> >
> > NOTE: this change forecloses the possibility of doing:
> >
> > drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "weird 2-cat experiment")
> >
> > but thats already strongly implied by the use of the enum itself; its
> > not a normal enum if it can be 2 values simultaneously.
>
> The drm.debug module parameter values are, arguably, ABI. There are tons
> of people, scripts, test environments, documentation, bug reports, etc,
> etc, referring to specific drm.debug module parameter values to enable
> specific drm debug logging categories.
>
> AFAICT you're not changing any of the values here, but having an enum
> without the hard coded values makes it more likely to accidentally
> change the category to bit mapping. At the very least deserves a
> comment.
>
hi Jani,
You're correct, this is unchanged :
echo $script_debug_val > /sys/module/drm/parameters/debug
wrt the enum, the next patch adds a comment,
enum drm_debug_category {
+ /* These names must match those in DYNAMIC_DEBUG_CLASSBITS */
/**
* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
But that comment mostly misses the point youre making.
and the specific NAME is stale.
and the s/int/ulong/ __drm_debug should go here, with the use of BIT()
I will fix this and repost.
Is it useful for CI / patchwork / lkp-robot purposes,
to branch-and-rebase onto drm-next/drm-next or drm-tip/drm-tip
(or dated tags on them ) ?
>
> BR,
> Jani.
>
>
thank you
> >
> > Signed-off-by: Jim Cromie <[email protected]>
> > ---
> > inclu
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.
greg k-h
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.
--
Ville Syrj?l?
Intel
On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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.
--
Jani Nikula, Intel Open Source Graphics Center
On 10/21/22 05:18, Jani Nikula wrote:
> On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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
On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
>
>
>
> On 10/21/22 05:18, Jani Nikula wrote:
> > On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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 ?
I didnt knowingly change something, but since its apparently happening,
heres a 1st WAG at a possible cause
commit ccc2b496324c13e917ef05f563626f4e7826bef1
Author: Jim Cromie <[email protected]>
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> >
> >
> >
> > On 10/21/22 05:18, Jani Nikula wrote:
> > > On Thu, 20 Oct 2022, Ville Syrj?l? <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
--
Ville Syrj?l?
Intel
On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
<[email protected]> wrote:
>
> On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> > On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> > >
> > >
> > >
> > > On 10/21/22 05:18, Jani Nikula wrote:
> > > > On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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.
>
So, I should note -
99% of my time & energy on this dyndbg + drm patchset
has been done using virtme,
so my world-view (and dev-hack-test env) has been smaller, simpler
maybe its been fatally simplistic.
ive just rebuilt v6.0 (before the trouble)
and run it thru my virtual home box,
I didnt see any unfamiliar drm-debug output
that I might have inadvertently altered somehow
I have some real HW I can put a reference kernel on,0
to look for the missing output, but its all gonna take some time,
esp to tighten up my dev-test-env
in the meantime, there is:
config DRM_USE_DYNAMIC_DEBUG
bool "use dynamic debug to implement drm.debug"
default y
depends on DRM
depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
depends on JUMP_LABEL
help
Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
bytes per callsite, the .data costs can be substantial, and
are therefore configurable.
Does changing the default fix things for i915 dmesg ?
or is the problem deeper ?
theres also this Makefile addition, which I might have oversimplified
CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
On Thu, Oct 27, 2022 at 01:55:39PM -0600, [email protected] wrote:
> On Thu, Oct 27, 2022 at 9:59 AM Ville Syrj?l?
> <[email protected]> wrote:
> >
> > On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> > > On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> > > >
> > > >
> > > >
> > > > On 10/21/22 05:18, Jani Nikula wrote:
> > > > > On Thu, 20 Oct 2022, Ville Syrj?l? <[email protected]> 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.
> >
>
> So, I should note -
> 99% of my time & energy on this dyndbg + drm patchset
> has been done using virtme,
> so my world-view (and dev-hack-test env) has been smaller, simpler
> maybe its been fatally simplistic.
>
> ive just rebuilt v6.0 (before the trouble)
> and run it thru my virtual home box,
> I didnt see any unfamiliar drm-debug output
> that I might have inadvertently altered somehow
>
> I have some real HW I can put a reference kernel on,0
> to look for the missing output, but its all gonna take some time,
> esp to tighten up my dev-test-env
>
> in the meantime, there is:
>
> config DRM_USE_DYNAMIC_DEBUG
> bool "use dynamic debug to implement drm.debug"
> default y
> depends on DRM
> depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> depends on JUMP_LABEL
> help
> Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> bytes per callsite, the .data costs can be substantial, and
> are therefore configurable.
>
> Does changing the default fix things for i915 dmesg ?
I think we want to mark it BROKEN in addition to make sure no one
enables it by accident. We already got one bug report where I had to
ask the reporter to rebuild their kernel since this had gotten
enabled and we didn't get any debugs from the driver probe.
> or is the problem deeper ?
>
> theres also this Makefile addition, which I might have oversimplified
>
> CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
--
Ville Syrj?l?
Intel
On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
<[email protected]> wrote:
>
> On Thu, Oct 27, 2022 at 01:55:39PM -0600, [email protected] wrote:
> > On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
> > <[email protected]> wrote:
> > >
> > > On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> > > > On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 10/21/22 05:18, Jani Nikula wrote:
> > > > > > On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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.
> > >
> >
> > So, I should note -
> > 99% of my time & energy on this dyndbg + drm patchset
> > has been done using virtme,
> > so my world-view (and dev-hack-test env) has been smaller, simpler
> > maybe its been fatally simplistic.
> >
> > ive just rebuilt v6.0 (before the trouble)
> > and run it thru my virtual home box,
> > I didnt see any unfamiliar drm-debug output
> > that I might have inadvertently altered somehow
> >
> > I have some real HW I can put a reference kernel on,0
> > to look for the missing output, but its all gonna take some time,
> > esp to tighten up my dev-test-env
> >
> > in the meantime, there is:
> >
> > config DRM_USE_DYNAMIC_DEBUG
> > bool "use dynamic debug to implement drm.debug"
> > default y
> > depends on DRM
> > depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> > depends on JUMP_LABEL
> > help
> > Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> > Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> > bytes per callsite, the .data costs can be substantial, and
> > are therefore configurable.
> >
> > Does changing the default fix things for i915 dmesg ?
>
> I think we want to mark it BROKEN in addition to make sure no one
Ok, I get the distinction now.
youre spelling that
depends on BROKEN
I have a notional explanation, and a conflating commit:
can you eliminate
git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
as the cause ?
commit ccc2b496324c13e917ef05f563626f4e7826bef1
Author: Jim Cromie <[email protected]>
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.
This shouldnt have turned off any debug of any kind
(drm.debug nor plain pr_debug)
but that former callsite no longer does the modname:func:line prefixing
that could have been in effect and relied upon (tested for) by your CI
I do need to clarify, I dont know exactly what debug/logging output
is missing such that CI is failing
related,
Ive added DEBUG to test-dynmamic-debug,
to prove the compile-time enablement of pr_debugs.
patch forthcoming, with a couple other fixes.
> enables it by accident. We already got one bug report where I had to
> ask the reporter to rebuild their kernel since this had gotten
> enabled and we didn't get any debugs from the driver probe.
>
> > or is the problem deeper ?
> >
> > theres also this Makefile addition, which I might have oversimplified
> >
> > CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
>
> --
> Ville Syrjälä
> Intel
On Sun, Oct 30, 2022 at 08:42:52AM -0600, [email protected] wrote:
> On Thu, Oct 27, 2022 at 2:10 PM Ville Syrj?l?
> <[email protected]> wrote:
> >
> > On Thu, Oct 27, 2022 at 01:55:39PM -0600, [email protected] wrote:
> > > On Thu, Oct 27, 2022 at 9:59 AM Ville Syrj?l?
> > > <[email protected]> wrote:
> > > >
> > > > On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> > > > > On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 10/21/22 05:18, Jani Nikula wrote:
> > > > > > > On Thu, 20 Oct 2022, Ville Syrj?l? <[email protected]> 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.
> > > >
> > >
> > > So, I should note -
> > > 99% of my time & energy on this dyndbg + drm patchset
> > > has been done using virtme,
> > > so my world-view (and dev-hack-test env) has been smaller, simpler
> > > maybe its been fatally simplistic.
> > >
> > > ive just rebuilt v6.0 (before the trouble)
> > > and run it thru my virtual home box,
> > > I didnt see any unfamiliar drm-debug output
> > > that I might have inadvertently altered somehow
> > >
> > > I have some real HW I can put a reference kernel on,0
> > > to look for the missing output, but its all gonna take some time,
> > > esp to tighten up my dev-test-env
> > >
> > > in the meantime, there is:
> > >
> > > config DRM_USE_DYNAMIC_DEBUG
> > > bool "use dynamic debug to implement drm.debug"
> > > default y
> > > depends on DRM
> > > depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> > > depends on JUMP_LABEL
> > > help
> > > Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> > > Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> > > bytes per callsite, the .data costs can be substantial, and
> > > are therefore configurable.
> > >
> > > Does changing the default fix things for i915 dmesg ?
> >
> > I think we want to mark it BROKEN in addition to make sure no one
>
> Ok, I get the distinction now.
> youre spelling that
> depends on BROKEN
>
> I have a notional explanation, and a conflating commit:
>
> can you eliminate
> git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
>
> as the cause ?
Reverting that doesn't help.
>
>
>
> commit ccc2b496324c13e917ef05f563626f4e7826bef1
> Author: Jim Cromie <[email protected]>
> 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.
>
>
> This shouldnt have turned off any debug of any kind
> (drm.debug nor plain pr_debug)
>
> but that former callsite no longer does the modname:func:line prefixing
> that could have been in effect and relied upon (tested for) by your CI
>
>
> I do need to clarify, I dont know exactly what debug/logging output
> is missing such that CI is failing
CI isn't failing. But any logs it produces are 100% useless,
as are any user reported logs.
The debugs that are missing are anything not coming directly
from drm.ko.
The stuff that I see being printed by i915.ko are drm_info()
and the drm_printer stuff from i915_welcome_messages(). That
also implies that drm_debug_enabled(DRM_UT_DRIVER) does at
least still work correctly.
I suspect that the problem is just that the debug calls
aren't getting patched in when a module loads. And fiddling
with the modparam after the fact does trigger that somehow.
--
Ville Syrj?l?
Intel
On Mon, Oct 31, 2022 at 7:07 AM Ville Syrjälä
<[email protected]> wrote:
>
> On Sun, Oct 30, 2022 at 08:42:52AM -0600, [email protected] wrote:
> > On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
> > <[email protected]> wrote:
> > >
> > > On Thu, Oct 27, 2022 at 01:55:39PM -0600, [email protected] wrote:
> > > > On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
> > > > <[email protected]> wrote:
> > > > >
> > > > > On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> > > > > > On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On 10/21/22 05:18, Jani Nikula wrote:
> > > > > > > > On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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.
> > > > >
> > > >
> > > > So, I should note -
> > > > 99% of my time & energy on this dyndbg + drm patchset
> > > > has been done using virtme,
> > > > so my world-view (and dev-hack-test env) has been smaller, simpler
> > > > maybe its been fatally simplistic.
> > > >
> > > > ive just rebuilt v6.0 (before the trouble)
> > > > and run it thru my virtual home box,
> > > > I didnt see any unfamiliar drm-debug output
> > > > that I might have inadvertently altered somehow
> > > >
> > > > I have some real HW I can put a reference kernel on,0
> > > > to look for the missing output, but its all gonna take some time,
> > > > esp to tighten up my dev-test-env
> > > >
> > > > in the meantime, there is:
> > > >
> > > > config DRM_USE_DYNAMIC_DEBUG
> > > > bool "use dynamic debug to implement drm.debug"
> > > > default y
> > > > depends on DRM
> > > > depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> > > > depends on JUMP_LABEL
> > > > help
> > > > Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> > > > Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> > > > bytes per callsite, the .data costs can be substantial, and
> > > > are therefore configurable.
> > > >
> > > > Does changing the default fix things for i915 dmesg ?
> > >
> > > I think we want to mark it BROKEN in addition to make sure no one
> >
> > Ok, I get the distinction now.
> > youre spelling that
> > depends on BROKEN
> >
> > I have a notional explanation, and a conflating commit:
> >
> > can you eliminate
> > git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
> >
> > as the cause ?
>
> Reverting that doesn't help.
>
thanks for eliminating it.
> >
> > I do need to clarify, I dont know exactly what debug/logging output
> > is missing such that CI is failing
>
> CI isn't failing. But any logs it produces are 100% useless,
> as are any user reported logs.
>
> The debugs that are missing are anything not coming directly
> from drm.ko.
>
> The stuff that I see being printed by i915.ko are drm_info()
> and the drm_printer stuff from i915_welcome_messages(). That
> also implies that drm_debug_enabled(DRM_UT_DRIVER) does at
> least still work correctly.
>
> I suspect that the problem is just that the debug calls
> aren't getting patched in when a module loads. And fiddling
> with the modparam after the fact does trigger that somehow.
>
ok, heres the 'tape' of a virtme boot,
then modprobe going wrong.
[ 1.785873] dyndbg: 2 debug prints in module intel_rapl_msr
[ 2.040598] virtme-init: udev is done
virtme-init: console is ttyS0
> load drm driver
bash-5.2# modprobe i915
> drm module is loaded 1st
[ 6.549451] dyndbg: add-module: drm.302 sites
[ 6.549991] dyndbg: class[0]: module:drm base:0 len:10 ty:0
[ 6.550647] dyndbg: 0: 0 DRM_UT_CORE
[ 6.551097] dyndbg: 1: 1 DRM_UT_DRIVER
[ 6.551531] dyndbg: 2: 2 DRM_UT_KMS
[ 6.551931] dyndbg: 3: 3 DRM_UT_PRIME
[ 6.552402] dyndbg: 4: 4 DRM_UT_ATOMIC
[ 6.552799] dyndbg: 5: 5 DRM_UT_VBL
[ 6.553270] dyndbg: 6: 6 DRM_UT_STATE
[ 6.553634] dyndbg: 7: 7 DRM_UT_LEASE
[ 6.554043] dyndbg: 8: 8 DRM_UT_DP
[ 6.554392] dyndbg: 9: 9 DRM_UT_DRMRES
[ 6.554776] dyndbg: module:drm attached 1 classes
[ 6.555241] dyndbg: 302 debug prints in module drm
> here modprobe reads /etc/modprobe.d/drm-test.conf:
options drm dyndbg="class DRM_UT_CORE +p; class DRM_UT_DRIVER +p"
and dyndbg applies it
[ 6.564284] dyndbg: module: drm dyndbg="class DRM_UT_CORE +p; class
DRM_UT_DRIVER +p"
[ 6.564957] dyndbg: query 0: "class DRM_UT_CORE +p" mod:drm
[ 6.565348] dyndbg: split into words: "class" "DRM_UT_CORE" "+p"
[ 6.565836] dyndbg: op='+'
[ 6.566059] dyndbg: flags=0x1
[ 6.566321] dyndbg: *flagsp=0x1 *maskp=0xffffffff
[ 6.566875] dyndbg: parsed: func="" file="" module="drm" format=""
lineno=0-0 class=DRM_UT_CORE
[ 6.568753] dyndbg: applied: func="" file="" module="drm" format=""
lineno=0-0 class=DRM_UT_CORE
[ 6.569473] dyndbg: query 1: "class DRM_UT_DRIVER +p" mod:drm
[ 6.570139] dyndbg: split into words: "class" "DRM_UT_DRIVER" "+p"
[ 6.570522] dyndbg: op='+'
[ 6.570699] dyndbg: flags=0x1
[ 6.570893] dyndbg: *flagsp=0x1 *maskp=0xffffffff
[ 6.571200] dyndbg: parsed: func="" file="" module="drm" format=""
lineno=0-0 class=DRM_UT_DRIVER
[ 6.571778] dyndbg: no matches for query
[ 6.572031] dyndbg: no-match: func="" file="" module="drm"
format="" lineno=0-0 class=DRM_UT_DRIVER
[ 6.572615] dyndbg: processed 2 queries, with 61 matches, 0 errs
[ 6.573286] ACPI: bus type drm_connector registered
next required module is loaded, but drm.debug isnt propagated.
[ 6.578645] dyndbg: add-module: drm_kms_helper.94 sites
[ 6.579487] dyndbg: class[0]: module:drm_kms_helper base:0 len:10 ty:0
[ 6.580639] dyndbg: 0: 0 DRM_UT_CORE
[ 6.581135] dyndbg: 1: 1 DRM_UT_DRIVER
[ 6.581651] dyndbg: 2: 2 DRM_UT_KMS
[ 6.582178] dyndbg: 3: 3 DRM_UT_PRIME
[ 6.582927] dyndbg: 4: 4 DRM_UT_ATOMIC
[ 6.583627] dyndbg: 5: 5 DRM_UT_VBL
[ 6.584350] dyndbg: 6: 6 DRM_UT_STATE
[ 6.584999] dyndbg: 7: 7 DRM_UT_LEASE
[ 6.585699] dyndbg: 8: 8 DRM_UT_DP
[ 6.586354] dyndbg: 9: 9 DRM_UT_DRMRES
[ 6.587040] dyndbg: module:drm_kms_helper attached 1 classes
[ 6.588103] dyndbg: 94 debug prints in module drm_kms_helper
and so on
[ 6.595628] dyndbg: add-module: drm_display_helper.150 sites
[ 6.596442] dyndbg: class[0]: module:drm_display_helper base:0 len:10 ty:0
[ 6.597453] dyndbg: 0: 0 DRM_UT_CORE
...
[ 6.601678] dyndbg: module:drm_display_helper attached 1 classes
[ 6.602335] dyndbg: 150 debug prints in module drm_display_helper
[ 6.692760] dyndbg: add-module: i915.1657 sites
[ 6.693023] dyndbg: class[0]: module:i915 base:0 len:10 ty:0
[ 6.693323] dyndbg: 0: 0 DRM_UT_CORE
....
[ 6.695220] dyndbg: module:i915 attached 1 classes
[ 6.695463] dyndbg: 1657 debug prints in module i915
bash-5.2#
bash-5.2#
So, what I think I need to add:
ddebug_add_module() scans the module being loaded,
looking for a param thats wired to dyndbg's modparam callback.
Then it calls that callback, with the val of the sysfs-node
(drm.debug in this case), and the module (i915)
the callback will then run the query to enable callsites per drm.debug.
I'll guess the kparams I need to find are in a section somewhere
Anyone want to toss a lawn-dart at the code I need to look at, copy ?
> --
> Ville Syrjälä
> Intel
thanks again
Jim
On 10/31/22 6:11 PM, [email protected] wrote:
> On Mon, Oct 31, 2022 at 7:07 AM Ville Syrjälä
> <[email protected]> wrote:
>> On Sun, Oct 30, 2022 at 08:42:52AM -0600, [email protected] wrote:
>>> On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
>>> <[email protected]> wrote:
>>>> On Thu, Oct 27, 2022 at 01:55:39PM -0600, [email protected] wrote:
>>>>> On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
>>>>> <[email protected]> wrote:
>>>>>> On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
>>>>>>> On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/21/22 05:18, Jani Nikula wrote:
>>>>>>>>> On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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.
>>>>>>
>>>>> So, I should note -
>>>>> 99% of my time & energy on this dyndbg + drm patchset
>>>>> has been done using virtme,
>>>>> so my world-view (and dev-hack-test env) has been smaller, simpler
>>>>> maybe its been fatally simplistic.
>>>>>
>>>>> ive just rebuilt v6.0 (before the trouble)
>>>>> and run it thru my virtual home box,
>>>>> I didnt see any unfamiliar drm-debug output
>>>>> that I might have inadvertently altered somehow
>>>>>
>>>>> I have some real HW I can put a reference kernel on,0
>>>>> to look for the missing output, but its all gonna take some time,
>>>>> esp to tighten up my dev-test-env
>>>>>
>>>>> in the meantime, there is:
>>>>>
>>>>> config DRM_USE_DYNAMIC_DEBUG
>>>>> bool "use dynamic debug to implement drm.debug"
>>>>> default y
>>>>> depends on DRM
>>>>> depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
>>>>> depends on JUMP_LABEL
>>>>> help
>>>>> Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
>>>>> Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
>>>>> bytes per callsite, the .data costs can be substantial, and
>>>>> are therefore configurable.
>>>>>
>>>>> Does changing the default fix things for i915 dmesg ?
>>>> I think we want to mark it BROKEN in addition to make sure no one
>>> Ok, I get the distinction now.
>>> youre spelling that
>>> depends on BROKEN
>>>
>>> I have a notional explanation, and a conflating commit:
>>>
>>> can you eliminate
>>> git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
>>>
>>> as the cause ?
>> Reverting that doesn't help.
>>
> thanks for eliminating it.
>
>>> I do need to clarify, I dont know exactly what debug/logging output
>>> is missing such that CI is failing
>> CI isn't failing. But any logs it produces are 100% useless,
>> as are any user reported logs.
>>
>> The debugs that are missing are anything not coming directly
>> from drm.ko.
>>
>> The stuff that I see being printed by i915.ko are drm_info()
>> and the drm_printer stuff from i915_welcome_messages(). That
>> also implies that drm_debug_enabled(DRM_UT_DRIVER) does at
>> least still work correctly.
>>
>> I suspect that the problem is just that the debug calls
>> aren't getting patched in when a module loads. And fiddling
>> with the modparam after the fact does trigger that somehow.
>>
> ok, heres the 'tape' of a virtme boot,
> then modprobe going wrong.
>
> [ 1.785873] dyndbg: 2 debug prints in module intel_rapl_msr
> [ 2.040598] virtme-init: udev is done
> virtme-init: console is ttyS0
>
>> load drm driver
> bash-5.2# modprobe i915
>
>> drm module is loaded 1st
> [ 6.549451] dyndbg: add-module: drm.302 sites
> [ 6.549991] dyndbg: class[0]: module:drm base:0 len:10 ty:0
> [ 6.550647] dyndbg: 0: 0 DRM_UT_CORE
> [ 6.551097] dyndbg: 1: 1 DRM_UT_DRIVER
> [ 6.551531] dyndbg: 2: 2 DRM_UT_KMS
> [ 6.551931] dyndbg: 3: 3 DRM_UT_PRIME
> [ 6.552402] dyndbg: 4: 4 DRM_UT_ATOMIC
> [ 6.552799] dyndbg: 5: 5 DRM_UT_VBL
> [ 6.553270] dyndbg: 6: 6 DRM_UT_STATE
> [ 6.553634] dyndbg: 7: 7 DRM_UT_LEASE
> [ 6.554043] dyndbg: 8: 8 DRM_UT_DP
> [ 6.554392] dyndbg: 9: 9 DRM_UT_DRMRES
> [ 6.554776] dyndbg: module:drm attached 1 classes
> [ 6.555241] dyndbg: 302 debug prints in module drm
>
>> here modprobe reads /etc/modprobe.d/drm-test.conf:
> options drm dyndbg="class DRM_UT_CORE +p; class DRM_UT_DRIVER +p"
> and dyndbg applies it
Hi,
I'm a bit confused with this. My understanding is that there
is a 'regression' here from how this used to work. But the
'class' keyword is new - are we sure this is the command-line
we are trying to fix?
>
> [ 6.564284] dyndbg: module: drm dyndbg="class DRM_UT_CORE +p; class
> DRM_UT_DRIVER +p"
> [ 6.564957] dyndbg: query 0: "class DRM_UT_CORE +p" mod:drm
> [ 6.565348] dyndbg: split into words: "class" "DRM_UT_CORE" "+p"
> [ 6.565836] dyndbg: op='+'
> [ 6.566059] dyndbg: flags=0x1
> [ 6.566321] dyndbg: *flagsp=0x1 *maskp=0xffffffff
> [ 6.566875] dyndbg: parsed: func="" file="" module="drm" format=""
> lineno=0-0 class=DRM_UT_CORE
> [ 6.568753] dyndbg: applied: func="" file="" module="drm" format=""
> lineno=0-0 class=DRM_UT_CORE
> [ 6.569473] dyndbg: query 1: "class DRM_UT_DRIVER +p" mod:drm
> [ 6.570139] dyndbg: split into words: "class" "DRM_UT_DRIVER" "+p"
> [ 6.570522] dyndbg: op='+'
> [ 6.570699] dyndbg: flags=0x1
> [ 6.570893] dyndbg: *flagsp=0x1 *maskp=0xffffffff
> [ 6.571200] dyndbg: parsed: func="" file="" module="drm" format=""
> lineno=0-0 class=DRM_UT_DRIVER
> [ 6.571778] dyndbg: no matches for query
> [ 6.572031] dyndbg: no-match: func="" file="" module="drm"
> format="" lineno=0-0 class=DRM_UT_DRIVER
> [ 6.572615] dyndbg: processed 2 queries, with 61 matches, 0 errs
> [ 6.573286] ACPI: bus type drm_connector registered
>
> next required module is loaded, but drm.debug isnt propagated.
>
> [ 6.578645] dyndbg: add-module: drm_kms_helper.94 sites
> [ 6.579487] dyndbg: class[0]: module:drm_kms_helper base:0 len:10 ty:0
> [ 6.580639] dyndbg: 0: 0 DRM_UT_CORE
> [ 6.581135] dyndbg: 1: 1 DRM_UT_DRIVER
> [ 6.581651] dyndbg: 2: 2 DRM_UT_KMS
> [ 6.582178] dyndbg: 3: 3 DRM_UT_PRIME
> [ 6.582927] dyndbg: 4: 4 DRM_UT_ATOMIC
> [ 6.583627] dyndbg: 5: 5 DRM_UT_VBL
> [ 6.584350] dyndbg: 6: 6 DRM_UT_STATE
> [ 6.584999] dyndbg: 7: 7 DRM_UT_LEASE
> [ 6.585699] dyndbg: 8: 8 DRM_UT_DP
> [ 6.586354] dyndbg: 9: 9 DRM_UT_DRMRES
> [ 6.587040] dyndbg: module:drm_kms_helper attached 1 classes
> [ 6.588103] dyndbg: 94 debug prints in module drm_kms_helper
>
> and so on
>
> [ 6.595628] dyndbg: add-module: drm_display_helper.150 sites
> [ 6.596442] dyndbg: class[0]: module:drm_display_helper base:0 len:10 ty:0
> [ 6.597453] dyndbg: 0: 0 DRM_UT_CORE
> ...
> [ 6.601678] dyndbg: module:drm_display_helper attached 1 classes
> [ 6.602335] dyndbg: 150 debug prints in module drm_display_helper
>
> [ 6.692760] dyndbg: add-module: i915.1657 sites
> [ 6.693023] dyndbg: class[0]: module:i915 base:0 len:10 ty:0
> [ 6.693323] dyndbg: 0: 0 DRM_UT_CORE
> ....
> [ 6.695220] dyndbg: module:i915 attached 1 classes
> [ 6.695463] dyndbg: 1657 debug prints in module i915
> bash-5.2#
> bash-5.2#
>
>
> So, what I think I need to add:
>
> ddebug_add_module() scans the module being loaded,
> looking for a param thats wired to dyndbg's modparam callback.
> Then it calls that callback, with the val of the sysfs-node
> (drm.debug in this case), and the module (i915)
Ok, I thought the sysfs callbacks only happen when
the sysfs file is written? And thus this works once
when the sysfs file is explicitly written by the user
after boot but not before then?
Thanks,
-Jason
>
> the callback will then run the query to enable callsites per drm.debug.
>
> I'll guess the kparams I need to find are in a section somewhere
> Anyone want to toss a lawn-dart at the code I need to look at, copy ?
>
>> --
>> Ville Syrjälä
>> Intel
> thanks again
> Jim
On Mon, Oct 31, 2022 at 08:20:54PM -0400, Jason Baron wrote:
>
>
> On 10/31/22 6:11 PM, [email protected] wrote:
> > On Mon, Oct 31, 2022 at 7:07 AM Ville Syrj?l?
> > <[email protected]> wrote:
> >> On Sun, Oct 30, 2022 at 08:42:52AM -0600, [email protected] wrote:
> >>> On Thu, Oct 27, 2022 at 2:10 PM Ville Syrj?l?
> >>> <[email protected]> wrote:
> >>>> On Thu, Oct 27, 2022 at 01:55:39PM -0600, [email protected] wrote:
> >>>>> On Thu, Oct 27, 2022 at 9:59 AM Ville Syrj?l?
> >>>>> <[email protected]> wrote:
> >>>>>> On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> >>>>>>> On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 10/21/22 05:18, Jani Nikula wrote:
> >>>>>>>>> On Thu, 20 Oct 2022, Ville Syrj?l? <[email protected]> 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.
> >>>>>>
> >>>>> So, I should note -
> >>>>> 99% of my time & energy on this dyndbg + drm patchset
> >>>>> has been done using virtme,
> >>>>> so my world-view (and dev-hack-test env) has been smaller, simpler
> >>>>> maybe its been fatally simplistic.
> >>>>>
> >>>>> ive just rebuilt v6.0 (before the trouble)
> >>>>> and run it thru my virtual home box,
> >>>>> I didnt see any unfamiliar drm-debug output
> >>>>> that I might have inadvertently altered somehow
> >>>>>
> >>>>> I have some real HW I can put a reference kernel on,0
> >>>>> to look for the missing output, but its all gonna take some time,
> >>>>> esp to tighten up my dev-test-env
> >>>>>
> >>>>> in the meantime, there is:
> >>>>>
> >>>>> config DRM_USE_DYNAMIC_DEBUG
> >>>>> bool "use dynamic debug to implement drm.debug"
> >>>>> default y
> >>>>> depends on DRM
> >>>>> depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> >>>>> depends on JUMP_LABEL
> >>>>> help
> >>>>> Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> >>>>> Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> >>>>> bytes per callsite, the .data costs can be substantial, and
> >>>>> are therefore configurable.
> >>>>>
> >>>>> Does changing the default fix things for i915 dmesg ?
> >>>> I think we want to mark it BROKEN in addition to make sure no one
> >>> Ok, I get the distinction now.
> >>> youre spelling that
> >>> depends on BROKEN
> >>>
> >>> I have a notional explanation, and a conflating commit:
> >>>
> >>> can you eliminate
> >>> git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
> >>>
> >>> as the cause ?
> >> Reverting that doesn't help.
> >>
> > thanks for eliminating it.
> >
> >>> I do need to clarify, I dont know exactly what debug/logging output
> >>> is missing such that CI is failing
> >> CI isn't failing. But any logs it produces are 100% useless,
> >> as are any user reported logs.
> >>
> >> The debugs that are missing are anything not coming directly
> >> from drm.ko.
> >>
> >> The stuff that I see being printed by i915.ko are drm_info()
> >> and the drm_printer stuff from i915_welcome_messages(). That
> >> also implies that drm_debug_enabled(DRM_UT_DRIVER) does at
> >> least still work correctly.
> >>
> >> I suspect that the problem is just that the debug calls
> >> aren't getting patched in when a module loads. And fiddling
> >> with the modparam after the fact does trigger that somehow.
> >>
> > ok, heres the 'tape' of a virtme boot,
> > then modprobe going wrong.
> >
> > [ 1.785873] dyndbg: 2 debug prints in module intel_rapl_msr
> > [ 2.040598] virtme-init: udev is done
> > virtme-init: console is ttyS0
> >
> >> load drm driver
> > bash-5.2# modprobe i915
> >
> >> drm module is loaded 1st
> > [ 6.549451] dyndbg: add-module: drm.302 sites
> > [ 6.549991] dyndbg: class[0]: module:drm base:0 len:10 ty:0
> > [ 6.550647] dyndbg: 0: 0 DRM_UT_CORE
> > [ 6.551097] dyndbg: 1: 1 DRM_UT_DRIVER
> > [ 6.551531] dyndbg: 2: 2 DRM_UT_KMS
> > [ 6.551931] dyndbg: 3: 3 DRM_UT_PRIME
> > [ 6.552402] dyndbg: 4: 4 DRM_UT_ATOMIC
> > [ 6.552799] dyndbg: 5: 5 DRM_UT_VBL
> > [ 6.553270] dyndbg: 6: 6 DRM_UT_STATE
> > [ 6.553634] dyndbg: 7: 7 DRM_UT_LEASE
> > [ 6.554043] dyndbg: 8: 8 DRM_UT_DP
> > [ 6.554392] dyndbg: 9: 9 DRM_UT_DRMRES
> > [ 6.554776] dyndbg: module:drm attached 1 classes
> > [ 6.555241] dyndbg: 302 debug prints in module drm
> >
> >> here modprobe reads /etc/modprobe.d/drm-test.conf:
> > options drm dyndbg="class DRM_UT_CORE +p; class DRM_UT_DRIVER +p"
> > and dyndbg applies it
>
> Hi,
>
> I'm a bit confused with this. My understanding is that there
> is a 'regression' here from how this used to work. But the
> 'class' keyword is new - are we sure this is the command-line
> we are trying to fix?
The thing we need fixed is just the bog standard drm.debug=0xe etc.
--
Ville Syrj?l?
Intel
On Tue, Nov 1, 2022 at 2:52 AM Ville Syrjälä
<[email protected]> wrote:
>
> On Mon, Oct 31, 2022 at 08:20:54PM -0400, Jason Baron wrote:
> >
> >
> > On 10/31/22 6:11 PM, [email protected] wrote:
> > > On Mon, Oct 31, 2022 at 7:07 AM Ville Syrjälä
> > > <[email protected]> wrote:
> > >> On Sun, Oct 30, 2022 at 08:42:52AM -0600, [email protected] wrote:
> > >>> On Thu, Oct 27, 2022 at 2:10 PM Ville Syrjälä
> > >>> <[email protected]> wrote:
> > >>>> On Thu, Oct 27, 2022 at 01:55:39PM -0600, [email protected] wrote:
> > >>>>> On Thu, Oct 27, 2022 at 9:59 AM Ville Syrjälä
> > >>>>> <[email protected]> wrote:
> > >>>>>> On Thu, Oct 27, 2022 at 09:37:52AM -0600, [email protected] wrote:
> > >>>>>>> On Thu, Oct 27, 2022 at 9:08 AM Jason Baron <[email protected]> wrote:
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> On 10/21/22 05:18, Jani Nikula wrote:
> > >>>>>>>>> On Thu, 20 Oct 2022, Ville Syrjälä <[email protected]> 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.
> > >>>>>>
> > >>>>> So, I should note -
> > >>>>> 99% of my time & energy on this dyndbg + drm patchset
> > >>>>> has been done using virtme,
> > >>>>> so my world-view (and dev-hack-test env) has been smaller, simpler
> > >>>>> maybe its been fatally simplistic.
> > >>>>>
> > >>>>> ive just rebuilt v6.0 (before the trouble)
> > >>>>> and run it thru my virtual home box,
> > >>>>> I didnt see any unfamiliar drm-debug output
> > >>>>> that I might have inadvertently altered somehow
> > >>>>>
> > >>>>> I have some real HW I can put a reference kernel on,0
> > >>>>> to look for the missing output, but its all gonna take some time,
> > >>>>> esp to tighten up my dev-test-env
> > >>>>>
> > >>>>> in the meantime, there is:
> > >>>>>
> > >>>>> config DRM_USE_DYNAMIC_DEBUG
> > >>>>> bool "use dynamic debug to implement drm.debug"
> > >>>>> default y
> > >>>>> depends on DRM
> > >>>>> depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
> > >>>>> depends on JUMP_LABEL
> > >>>>> help
> > >>>>> Use dynamic-debug to avoid drm_debug_enabled() runtime overheads.
> > >>>>> Due to callsite counts in DRM drivers (~4k in amdgpu) and 56
> > >>>>> bytes per callsite, the .data costs can be substantial, and
> > >>>>> are therefore configurable.
> > >>>>>
> > >>>>> Does changing the default fix things for i915 dmesg ?
> > >>>> I think we want to mark it BROKEN in addition to make sure no one
> > >>> Ok, I get the distinction now.
> > >>> youre spelling that
> > >>> depends on BROKEN
> > >>>
> > >>> I have a notional explanation, and a conflating commit:
> > >>>
> > >>> can you eliminate
> > >>> git log -p ccc2b496324c13e917ef05f563626f4e7826bef1
> > >>>
> > >>> as the cause ?
> > >> Reverting that doesn't help.
> > >>
> > > thanks for eliminating it.
> > >
> > >>> I do need to clarify, I dont know exactly what debug/logging output
> > >>> is missing such that CI is failing
> > >> CI isn't failing. But any logs it produces are 100% useless,
> > >> as are any user reported logs.
> > >>
> > >> The debugs that are missing are anything not coming directly
> > >> from drm.ko.
> > >>
> > >> The stuff that I see being printed by i915.ko are drm_info()
> > >> and the drm_printer stuff from i915_welcome_messages(). That
> > >> also implies that drm_debug_enabled(DRM_UT_DRIVER) does at
> > >> least still work correctly.
> > >>
> > >> I suspect that the problem is just that the debug calls
> > >> aren't getting patched in when a module loads. And fiddling
> > >> with the modparam after the fact does trigger that somehow.
> > >>
> > > ok, heres the 'tape' of a virtme boot,
> > > then modprobe going wrong.
> > >
> > > [ 1.785873] dyndbg: 2 debug prints in module intel_rapl_msr
> > > [ 2.040598] virtme-init: udev is done
> > > virtme-init: console is ttyS0
> > >
> > >> load drm driver
> > > bash-5.2# modprobe i915
> > >
> > >> drm module is loaded 1st
> > > [ 6.549451] dyndbg: add-module: drm.302 sites
> > > [ 6.549991] dyndbg: class[0]: module:drm base:0 len:10 ty:0
> > > [ 6.550647] dyndbg: 0: 0 DRM_UT_CORE
> > > [ 6.551097] dyndbg: 1: 1 DRM_UT_DRIVER
> > > [ 6.551531] dyndbg: 2: 2 DRM_UT_KMS
> > > [ 6.551931] dyndbg: 3: 3 DRM_UT_PRIME
> > > [ 6.552402] dyndbg: 4: 4 DRM_UT_ATOMIC
> > > [ 6.552799] dyndbg: 5: 5 DRM_UT_VBL
> > > [ 6.553270] dyndbg: 6: 6 DRM_UT_STATE
> > > [ 6.553634] dyndbg: 7: 7 DRM_UT_LEASE
> > > [ 6.554043] dyndbg: 8: 8 DRM_UT_DP
> > > [ 6.554392] dyndbg: 9: 9 DRM_UT_DRMRES
> > > [ 6.554776] dyndbg: module:drm attached 1 classes
> > > [ 6.555241] dyndbg: 302 debug prints in module drm
> > >
> > >> here modprobe reads /etc/modprobe.d/drm-test.conf:
> > > options drm dyndbg="class DRM_UT_CORE +p; class DRM_UT_DRIVER +p"
> > > and dyndbg applies it
> >
> > Hi,
> >
> > I'm a bit confused with this. My understanding is that there
> > is a 'regression' here from how this used to work. But the
> > 'class' keyword is new - are we sure this is the command-line
> > we are trying to fix?
the regression here is that previously, any late changes to drm.debug
are seen by all callers of drm_debug_enabled(), which checks the bits
when called.
Now, the change to drm.debug causes updates to currently loaded modules'
prdbgs. But module loading of dependent modules is incomplete,
and those prdbgs are too late.
>
> The thing we need fixed is just the bog standard drm.debug=0xe etc.
my example was unfortunate, I repeated the test with
options drm debug=0x1f
the results are the same, dyndbgs callback applies the class queries,
but the helper, driver modules arent yet loaded, so the class'd prdbgs
arent there to be enabled.
The Notional fix (Im working on it)
dyndbg gets static list of CLASSMAPS,
ddebug_attach_module_classes() can then distinguish the "owner"
of each classmap (the 1st module declaring it), from the subsequently
loaded modules (which reuse the same previously declared classmap)
as subsequent modules (say drm_kms_helper) refs the DRM_UT_* classmap
(just attached to drm module), ddebug_attach_module_classes() can
invoke the callback to reapply drm.debug to the newly loaded module.
or so I think.
> --
> Ville Syrjälä
> Intel
hi Jason, Greg, DRM-folk,
drm.debug-on-dyndbg has a regression due to a chicken-&-egg problem;
drm.debug is applied to enable dyndbg callsites before the dependent
modules' callsites are available to be enabled.
My "fixes" are unready, so lets just mark it BROKEN for now.
Meanwhile, heres some other fixes, a comment tweak, a proof of
non-bug, an internal simplification, and a cleanup/improvement to the
main macro (API):
Split DECLARE_DYNDBG_CLASSMAP in 1/2; REFERENCE_DYNDBG_CLASSMAP now
refers to a classmap DECLARE'd just once. I think this gives a path
away from the coordination-by-identical-classmaps "feature" that Jani
and others thought was "weird" (my term).
Jim Cromie (7):
drm: mark drm.debug-on-dyndbg as BROKEN for now
drm_print: fixup improve stale comment
test-dyndbg: fixup CLASSMAP usage error
test-dyndbg: show that DEBUG enables prdbgs at compiletime
dyndbg: fix readback value on LEVEL_NAMES interfaces
dyndbg: clone DECLARE_DYNDBG_CLASSMAP to REFERENCE_DYNDBG_CLASSMAP
dyndbg: replace classmap list with a vector
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
drivers/gpu/drm/display/drm_dp_helper.c | 2 +-
drivers/gpu/drm/drm_crtc_helper.c | 2 +-
drivers/gpu/drm/i915/i915_params.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
include/drm/drm_print.h | 5 +-
include/linux/dynamic_debug.h | 10 ++++
lib/dynamic_debug.c | 63 +++++++++++++------------
lib/test_dynamic_debug.c | 4 +-
10 files changed, 57 insertions(+), 36 deletions(-)
--
2.38.1
Since sysfs knobs should generally read-back what was just written
(unless theres a good reason to do otherwise), this result (using
test_dynamic_debug.ko) is suboptimal:
echo L3 > p_level_names
cat p_level_names
4
Fix this with a -1 offset in LEVEL_NAMES readback.
NOTE:
Calling this a BUG is debatable, and the above is slightly inaccurate
wrt "read-back"; whats written is a LEVEL_NAME (a string). Whats read
back is an integer, giving the 'edge' of the 'low-pass-filter'
The actual test looks like:
RTT: L4 -> p_level_names : 4 :: DOING: levels 4-1
[ 17.509594] dyndbg: "L4" > p_level_names:0x4
[ 17.510115] dyndbg: apply: 0x1f to: 0xf
[ 17.510506] dyndbg: query 0: "class L4 +p" mod:*
[ 17.510992] dyndbg: split into words: "class" "L4" "+p"
[ 17.511521] dyndbg: op='+'
[ 17.511811] dyndbg: flags=0x1
[ 17.512127] dyndbg: *flagsp=0x1 *maskp=0xffffffff
[ 17.512604] dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=L4
[ 17.513414] dyndbg: applied: func="" file="" module="" format="" lineno=0-0 class=L4
[ 17.514204] dyndbg: processed 1 queries, with 1 matches, 0 errs
[ 17.514809] dyndbg: bit_4: 1 matches on class: L4 -> 0x1f
[ 17.515355] dyndbg: p_level_names: changed bit-4: "L4" f->1f
[ 17.515933] dyndbg: total matches: 1
crap [[ 5 != 4 ]]
This -1 adjustment just reports the edge consistently with its
input-mapping.
Fixes: b9400852c080 (dyndbg: add drm.debug style (drm/parameters/debug) bitmap support)
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 009f2ead09c1..48ca1387a409 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -794,6 +794,8 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", *dcp->bits);
case DD_CLASS_TYPE_LEVEL_NAMES:
+ return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl - 1);
+
case DD_CLASS_TYPE_LEVEL_NUM:
return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl);
default:
--
2.38.1
more careful reading of test output reveals:
lib/test_dynamic_debug.c:103 [test_dynamic_debug]do_cats =pmf "doing categories\n"
lib/test_dynamic_debug.c:105 [test_dynamic_debug]do_cats =p "LOW msg\n" class:MID
lib/test_dynamic_debug.c:106 [test_dynamic_debug]do_cats =p "MID msg\n" class:HI
lib/test_dynamic_debug.c:107 [test_dynamic_debug]do_cats =_ "HI msg\n" class unknown, _id:13
That last line is wrong, the HI class is declared.
But the enum's 1st val (explicitly initialized) was wrong; it must be
_base, not _base+1 (a DECLARE_DYNDBG_CLASSMAP param). So the last
enumeration exceeded the range of mapped class-id's, which triggered
the "class unknown" report. Basically, I coded in an error, and
forgot to verify it and remove it.
RFC:
This patch fixes a bad usage of DEFINE_DYNDBG_CLASSMAP(), showing that
it is too error-prone. As noted in test-dynamic-debug.c comments:
* Using the CLASSMAP api:
* - classmaps must have corresponding enum
* - enum symbols must match/correlate with class-name strings in the map.
* - base must equal enum's 1st value
* - multiple maps must set their base to share the 0-62 class_id space !!
* (build-bug-on tips welcome)
Those shortcomings could largely be fixed with a __stringify_list
(which doesn't exist) used in DEFINE_DYNAMIC_DEBUG_CLASSMAP(), on
__VA_ARGS__ a 2nd time. Then, DRM would pass DRM_UT_* ; all the
categories, in order, and not their stringifications, which created
all the usage complications above.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/test_dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 8dd250ad022b..a01f0193a419 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -75,7 +75,7 @@ DD_SYS_WRAP(disjoint_bits, p);
DD_SYS_WRAP(disjoint_bits, T);
/* symbolic input, independent bits */
-enum cat_disjoint_names { LOW = 11, MID, HI };
+enum cat_disjoint_names { LOW = 10, MID, HI };
DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
"LOW", "MID", "HI");
DD_SYS_WRAP(disjoint_names, p);
--
2.38.1
On Fri, Nov 11, 2022 at 03:17:08PM -0700, Jim Cromie wrote:
> hi Jason, Greg, DRM-folk,
>
> drm.debug-on-dyndbg has a regression due to a chicken-&-egg problem;
> drm.debug is applied to enable dyndbg callsites before the dependent
> modules' callsites are available to be enabled.
>
> My "fixes" are unready, so lets just mark it BROKEN for now.
>
> Meanwhile, heres some other fixes, a comment tweak, a proof of
> non-bug, an internal simplification, and a cleanup/improvement to the
> main macro (API):
>
> Split DECLARE_DYNDBG_CLASSMAP in 1/2; REFERENCE_DYNDBG_CLASSMAP now
> refers to a classmap DECLARE'd just once. I think this gives a path
> away from the coordination-by-identical-classmaps "feature" that Jani
> and others thought was "weird" (my term).
>
Acked-by: Greg Kroah-Hartman <[email protected]>