drm_debug_enabled() is called a lot to do unlikely bit-tests to control debug printing; this is a good job for dynamic-debug, IFF it is built with JUMP_LABEL.
Enable the use of dynamic-debug to avoid drm_debug_enabled() overheads, opt in with CONFIG_DRM_USE_DYNAMIC_DEBUG=y.
I have this patchset running bare-metal on an i915 laptop & an amdgpu desktop (both as loadable modules).
I booted the amdgpu box with:
BOOT_IMAGE=(hd2,gpt2)/vmlinuz-5.13.0-dd7-13692-g8def25788f56 \ root=UUID=mumble ro \ rootflags=subvol=root00 rhgb \ dynamic_debug.verbose=3 main.dyndbg=+p \ amdgpu.debug=1 amdgpu.test=1 \ "amdgpu.dyndbg=format ^[ +p"
That last line activates ~1700 callsites with a format like '[DML' etc at boot, causing ~76k prdbgs in 409 seconds, before I turned them off with:
echo module amdgpu -p > /proc/dynamic_debug/control
[root@gandalf jimc]# journalctl -b-0 | grep -P '[(DML|VBLANK|SURFACE|BIOS|BANDWIDTH)' | wc 68708 578503 5054437 [root@gandalf jimc]# journalctl -b-0 | grep -P '[(DML|VBLANK|SURFACE|BIOS|BANDWIDTH|\w+)' | wc 76298 661176 6028087
IOW, things appear to hold up under some stress.
this is on top of master @ v5.13-13688-gde5540965853
v1 is here: https://lore.kernel.org/lkml/20201204035318.332419-1-jim.cromie@gmail.com/
Jim Cromie (4): drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro drm: fixup comment spelling drm: RFC add choice to use dynamic debug in drm-debug i915: map gvt pr_debug categories to bits in parameters/debug_gvt
drivers/gpu/drm/Kconfig | 13 ++++ drivers/gpu/drm/drm_print.c | 75 +++++++++++++++++- drivers/gpu/drm/i915/gvt/Makefile | 4 + drivers/gpu/drm/i915/i915_params.c | 76 ++++++++++++++++++ include/drm/drm_print.h | 119 ++++++++++++++++++++--------- 5 files changed, 249 insertions(+), 38 deletions(-)
whitespace only, to diff-minimize a later commit. no functional changes --- include/drm/drm_print.h | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 9b66be54dd16..6419b4e7c5dc 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -524,19 +524,24 @@ void __drm_err(const char *format, ...); #define DRM_DEBUG_DP(fmt, ...) \ __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
-#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ -({ \ - static DEFINE_RATELIMIT_STATE(rs_, DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);\ - const struct drm_device *drm_ = (drm); \ - \ - if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(&rs_)) \ - drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## __VA_ARGS__); \ +#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ +({ \ + static DEFINE_RATELIMIT_STATE(rs_, \ + DEFAULT_RATELIMIT_INTERVAL, \ + DEFAULT_RATELIMIT_BURST); \ + const struct drm_device *drm_ = (drm); \ + \ + if (drm_debug_enabled(DRM_UT_ ## category) \ + && __ratelimit(&rs_)) \ + drm_dev_printk(drm_ ? drm_->dev : NULL, \ + KERN_DEBUG, fmt, ## __VA_ARGS__); \ })
#define drm_dbg_kms_ratelimited(drm, fmt, ...) \ __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__)
-#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__) +#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) \ + drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)
/* * struct drm_device based WARNs
On Sat, 2021-07-10 at 23:49 -0600, Jim Cromie wrote:
whitespace only, to diff-minimize a later commit. no functional changes
[]
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
[]
@@ -524,19 +524,24 @@ void __drm_err(const char *format, ...); #define DRM_DEBUG_DP(fmt, ...) \ __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
-#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ -({ \
- static DEFINE_RATELIMIT_STATE(rs_, DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);\
- const struct drm_device *drm_ = (drm); \
\- if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(&rs_)) \
drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## __VA_ARGS__); \+#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ +({ \
- static DEFINE_RATELIMIT_STATE(rs_, \
DEFAULT_RATELIMIT_INTERVAL, \DEFAULT_RATELIMIT_BURST); \- const struct drm_device *drm_ = (drm); \
\- if (drm_debug_enabled(DRM_UT_ ## category) \
&& __ratelimit(&rs_)) \
Though I don't really see the need for the change, the typical style has the logical continuation at the end of the test.
if (drm_debug_enabled(DRM_UT_ ## category) && \ __ratelimit(&rs_)) \
On Sun, Jul 11, 2021 at 10:17 AM Joe Perches joe@perches.com wrote:
On Sat, 2021-07-10 at 23:49 -0600, Jim Cromie wrote:
whitespace only, to diff-minimize a later commit. no functional changes
[]
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
[]
@@ -524,19 +524,24 @@ void __drm_err(const char *format, ...); #define DRM_DEBUG_DP(fmt, ...) \ __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
-#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ -({ \
static DEFINE_RATELIMIT_STATE(rs_, DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);\const struct drm_device *drm_ = (drm); \\if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(&rs_)) \drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## __VA_ARGS__); \+#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ +({ \
static DEFINE_RATELIMIT_STATE(rs_, \DEFAULT_RATELIMIT_INTERVAL, \DEFAULT_RATELIMIT_BURST); \const struct drm_device *drm_ = (drm); \\if (drm_debug_enabled(DRM_UT_ ## category) \&& __ratelimit(&rs_)) \Though I don't really see the need for the change, the typical style has the logical continuation at the end of the test.
yes I caught that late, old habit, hard to break. That if-statement is altered as part of HEAD+2 drm: RFC add choice to use dynamic debug in drm-debug
which replaces drm_dev_printk with drm_dev_dbg (which calls pr_debug in DRM_USE_DYNAMIC_DEBUG)
if (drm_debug_enabled(DRM_UT_ ## category) && \ __ratelimit(&rs_)) \
s/prink/printk/ - no functional changes --- include/drm/drm_print.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 6419b4e7c5dc..4559583bc88b 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -327,7 +327,7 @@ static inline bool drm_debug_enabled(enum drm_debug_category category) /* * struct device based logging * - * Prefer drm_device based logging over device or prink based logging. + * Prefer drm_device based logging over device or printk based logging. */
__printf(3, 4)
drm's debug system uses distinct categories of debug messages, encoded in an enum (DRM_UT_<CATEGORY>), which are mapped to bits in drm.debug. drm_debug_enabled() does a lot of unlikely bit-mask checks on drm.debug; we can use dynamic debug instead, and get all that static_key/jump_label goodness.
Dynamic debug has no concept of category, but we can map the DRM_UT_* to a set of distinct prefixes; "drm:core:", "drm:kms:" etc, and prepend them to the given formats. Then we can use
`echo module drm format ^drm:core: > control`
to select the whole category with one query. This new prefix changes pr_debug's output, so is user visible, but it seems unlikely to cause trouble for log watchers; they're not relying on the absence of class prefix strings.
This conversion yields ~2100 new callsites on my i7 laptop:
dyndbg: 195 debug prints in module drm_kms_helper dyndbg: 298 debug prints in module drm dyndbg: 1630 debug prints in module i915
CONFIG_DRM_USE_DYNAMIC_DEBUG enables this, and is available if CONFIG_DYNAMIC_DEBUG or CONFIG_DYNAMIC_DEBUG_CORE is chosen, and if CONFIG_JUMP_LABEL is enabled; this because its required to get the promised optimizations.
The indirection/switchover is layered into the macro scheme:
0.A new callback on drm.debug which calls dynamic_debug_exec_queries to map those bits to specific query/commands dynamic_debug_exec_queries("format ^drm:kms: +p", "drm*"); here for POC, this should be in dynamic_debug.c with a MODULE_PARAM_DEBUG_BITMAP(__drm_debug, { "prefix-1", "desc-1" }+)
1.A "converted" or "classy" DRM_UT_* map based on: DRM_UT_* ( symbol => bit-mask ) named it: cDRM_UT_* ( symbol => format-class-prefix-string )
So cDRM_UT_* is either: legacy: cDRM_UT_* <-- DRM_UT_* ( !CONFIG_DRM_USE_DYNAMIC_DEBUG ) enabled: #define cDRM_UT_KMS "drm:kms: " #define cDRM_UT_PRIME "drm:prime: " #define cDRM_UT_ATOMIC "drm:atomic: "
DRM_UT_* are unchanged, since theyre used in drm_debug_enabled() and elsewhere.
2.drm_dev_dbg & drm_debug are renamed (prefixed with '_') old names are now macros, calling either: legacy: -> to renamed fn enabled: -> dev_dbg & pr_debug, with cDRM-prefix prepended format.
3.names in (2) are called from DRM_DEBUG_<Category> and drm_dbg_<Category>. all these macros get "converted" to use cDRM_UT_*, to get right token type.
4.simplification of __DRM_DEFINE_DBG_RATELIMITED macro pass both DRM_UT & cDRM_UT, for drm_debug_enabled & drm_dev_dbg remove DRM_UT_ ## KMS Also reuse gist of: commit 7911902129a8 ("drm/print: Handle potentially NULL drm_devices in drm_dbg_*) --- drivers/gpu/drm/Kconfig | 13 +++++ drivers/gpu/drm/drm_print.c | 75 ++++++++++++++++++++++++-- include/drm/drm_print.h | 104 ++++++++++++++++++++++++++---------- 3 files changed, 159 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 7ff89690a976..e4524ccba040 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -57,6 +57,19 @@ config DRM_DEBUG_MM
If in doubt, say "N".
+config DRM_USE_DYNAMIC_DEBUG + bool "use dynamic debug to implement drm.debug" + default n + depends on DRM + depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE + depends on JUMP_LABEL + help + The drm debug category facility does a lot of unlikely bit-field + tests at runtime; while cheap individually, the cost accumulates. + This option uses dynamic debug facility (if configured and + using jump_label) to avoid those runtime checks, patching + the kernel when those debugs are desired. + config DRM_DEBUG_SELFTEST tristate "kselftests for DRM" depends on DRM diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 111b932cf2a9..e2acdfc7088b 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -52,8 +52,75 @@ 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)"); + +#ifndef CONFIG_DRM_USE_DYNAMIC_DEBUG module_param_named(debug, __drm_debug, int, 0600);
+#else +static char *format_class_prefixes[] = { + cDRM_UT_CORE, + cDRM_UT_DRIVER, + cDRM_UT_KMS, + cDRM_UT_PRIME, + cDRM_UT_ATOMIC, + cDRM_UT_VBL, + cDRM_UT_STATE, + cDRM_UT_LEASE, + cDRM_UT_DP, + cDRM_UT_DRMRES +}; + +#define OUR_QUERY_SIZE 64 /* > strlen "format '^%s' %cp" + longest prefix */ + +static int param_set_dyndbg(const char *instr, const struct kernel_param *kp) +{ + unsigned int val; + unsigned long changes, result; + int rc, chgct = 0, totct = 0, bitpos; + char query[OUR_QUERY_SIZE]; + + rc = kstrtouint(instr, 0, &val); + if (rc) { + pr_err("%s: failed\n", __func__); + return -EINVAL; + } + result = val; + changes = result ^ __drm_debug; + + pr_debug("changes:0x%lx from result:0x%lx\n", changes, result); + + for_each_set_bit(bitpos, &changes, ARRAY_SIZE(format_class_prefixes)) { + + sprintf(query, "format '^%s' %cp", format_class_prefixes[bitpos], + test_bit(bitpos, &result) ? '+' : '-'); + + chgct = dynamic_debug_exec_queries(query, "drm*"); + if (chgct < 0) { + pr_err("%s: exec err:%d on: %s\n", __func__, chgct, query); + continue; + } + pr_debug("change ct:%d on %s\n", chgct, query); + totct += chgct; + } + pr_debug("total changes: %d\n", totct); + __drm_debug = result; + return 0; +} + +static int param_get_dyndbg(char *buffer, const struct kernel_param *kp) +{ + pr_debug("debug-val:0x%x %u\n", __drm_debug, *((unsigned int *)kp->arg)); + return scnprintf(buffer, PAGE_SIZE, "%u\n", + *((unsigned int *)kp->arg)); +} +static const struct kernel_param_ops param_ops_debug = { + .set = param_set_dyndbg, + .get = param_get_dyndbg, +}; +module_param_cb(debug, ¶m_ops_debug, &__drm_debug, 0644); + +#endif /* CONFIG_DRM_USE_DYNAMIC_DEBUG */ + void __drm_puts_coredump(struct drm_printer *p, const char *str) { struct drm_print_iterator *iterator = p->arg; @@ -256,7 +323,7 @@ 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, +void _drm_dev_dbg(const struct device *dev, enum drm_debug_category category, const char *format, ...) { struct va_format vaf; @@ -278,9 +345,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; @@ -297,7 +364,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 4559583bc88b..63dd469d1c68 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>
@@ -319,6 +320,51 @@ enum drm_debug_category { DRM_UT_DRMRES = 0x200, };
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG) + +/* Use legacy drm-debug functions, and drm_debug_enabled(). + * For cDRM_UT_* (converted category), identity map to DRM_UT_* + */ +#define __drm_dbg(cls, fmt, ...) \ + ___drm_dbg(cls, fmt, ##__VA_ARGS__) +#define drm_dev_dbg(dev, cls, fmt, ...) \ + _drm_dev_dbg(dev, cls, fmt, ##__VA_ARGS__) + +#define cDRM_UT_CORE DRM_UT_CORE +#define cDRM_UT_DRIVER DRM_UT_DRIVER +#define cDRM_UT_KMS DRM_UT_KMS +#define cDRM_UT_PRIME DRM_UT_PRIME +#define cDRM_UT_ATOMIC DRM_UT_ATOMIC +#define cDRM_UT_VBL DRM_UT_VBL +#define cDRM_UT_STATE DRM_UT_STATE +#define cDRM_UT_LEASE DRM_UT_LEASE +#define cDRM_UT_DP DRM_UT_DP +#define cDRM_UT_DRMRES DRM_UT_DRMRES + +#else /* !CONFIG_DRM_USE_DYNAMIC_DEBUG */ + +/* use dynamic_debug to avoid drm_debug_enabled(). + * dyndbg has no category, so we prefix the format with a "class" + * string; cDRM_UT_* maps to those class strings + */ +#define __drm_dbg(cls, fmt, ...) \ + pr_debug(cls # fmt, ##__VA_ARGS__) +#define drm_dev_dbg(dev, cls, fmt, ...) \ + dev_dbg(dev, cls # fmt, ##__VA_ARGS__) + +#define cDRM_UT_CORE "drm:core: " +#define cDRM_UT_DRIVER "drm:drvr: " +#define cDRM_UT_KMS "drm:kms: " +#define cDRM_UT_PRIME "drm:prime: " +#define cDRM_UT_ATOMIC "drm:atomic: " +#define cDRM_UT_VBL "drm:vbl: " +#define cDRM_UT_STATE "drm:state: " +#define cDRM_UT_LEASE "drm:lease: " +#define cDRM_UT_DP "drm:dp: " +#define cDRM_UT_DRMRES "drm:res " + +#endif /* !CONFIG_DRM_USE_DYNAMIC_DEBUG */ + static inline bool drm_debug_enabled(enum drm_debug_category category) { return unlikely(__drm_debug & category); @@ -334,7 +380,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, ...);
/** @@ -383,7 +429,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, * @fmt: printf() like format string. */ #define DRM_DEV_DEBUG(dev, fmt, ...) \ - drm_dev_dbg(dev, DRM_UT_CORE, fmt, ##__VA_ARGS__) + drm_dev_dbg(dev, cDRM_UT_CORE, fmt, ##__VA_ARGS__) /** * DRM_DEV_DEBUG_DRIVER() - Debug output for vendor specific part of the driver * @@ -391,7 +437,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, * @fmt: printf() like format string. */ #define DRM_DEV_DEBUG_DRIVER(dev, fmt, ...) \ - drm_dev_dbg(dev, DRM_UT_DRIVER, fmt, ##__VA_ARGS__) + drm_dev_dbg(dev, cDRM_UT_DRIVER, fmt, ##__VA_ARGS__) /** * DRM_DEV_DEBUG_KMS() - Debug output for modesetting code * @@ -399,7 +445,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, * @fmt: printf() like format string. */ #define DRM_DEV_DEBUG_KMS(dev, fmt, ...) \ - drm_dev_dbg(dev, DRM_UT_KMS, fmt, ##__VA_ARGS__) + drm_dev_dbg(dev, cDRM_UT_KMS, fmt, ##__VA_ARGS__)
/* * struct drm_device based logging @@ -443,25 +489,25 @@ 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__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_CORE, fmt, ##__VA_ARGS__) #define drm_dbg(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRIVER, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_DRIVER, fmt, ##__VA_ARGS__) #define drm_dbg_kms(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_KMS, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_KMS, fmt, ##__VA_ARGS__) #define drm_dbg_prime(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_PRIME, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_PRIME, fmt, ##__VA_ARGS__) #define drm_dbg_atomic(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_ATOMIC, fmt, ##__VA_ARGS__) #define drm_dbg_vbl(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_VBL, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_VBL, fmt, ##__VA_ARGS__) #define drm_dbg_state(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_STATE, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_STATE, fmt, ##__VA_ARGS__) #define drm_dbg_lease(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_LEASE, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_LEASE, fmt, ##__VA_ARGS__) #define drm_dbg_dp(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DP, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_DP, fmt, ##__VA_ARGS__) #define drm_dbg_drmres(drm, fmt, ...) \ - drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRMRES, fmt, ##__VA_ARGS__) + drm_dev_dbg((drm) ? (drm)->dev : NULL, cDRM_UT_DRMRES, fmt, ##__VA_ARGS__)
/* @@ -471,7 +517,7 @@ 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, ...);
@@ -500,45 +546,45 @@ void __drm_err(const char *format, ...); #define DRM_ERROR_RATELIMITED(fmt, ...) \ DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
+ #define DRM_DEBUG(fmt, ...) \ - __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__) + __drm_dbg(cDRM_UT_CORE, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_DRIVER(fmt, ...) \ - __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__) + __drm_dbg(cDRM_UT_DRIVER, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_KMS(fmt, ...) \ - __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__) + __drm_dbg(cDRM_UT_KMS, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_PRIME(fmt, ...) \ - __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__) + __drm_dbg(cDRM_UT_PRIME, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_ATOMIC(fmt, ...) \ - __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) + __drm_dbg(cDRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_VBL(fmt, ...) \ - __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__) + __drm_dbg(cDRM_UT_VBL, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_LEASE(fmt, ...) \ - __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__) + __drm_dbg(cDRM_UT_LEASE, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_DP(fmt, ...) \ - __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__) + __drm_dbg(cDRM_UT_DP, fmt, ## __VA_ARGS__)
-#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \ +#define __DRM_DEFINE_DBG_RATELIMITED(cat_legacy, cat_class, drm, fmt, ...)\ ({ \ static DEFINE_RATELIMIT_STATE(rs_, \ DEFAULT_RATELIMIT_INTERVAL, \ DEFAULT_RATELIMIT_BURST); \ const struct drm_device *drm_ = (drm); \ \ - if (drm_debug_enabled(DRM_UT_ ## category) \ - && __ratelimit(&rs_)) \ - drm_dev_printk(drm_ ? drm_->dev : NULL, \ - KERN_DEBUG, fmt, ## __VA_ARGS__); \ + if (drm_debug_enabled(cat_legacy) && __ratelimit(&rs_)) \ + drm_dev_dbg((drm_) ? (drm_)->dev : NULL, \ + cat_class, fmt, ##__VA_ARGS__); \ })
#define drm_dbg_kms_ratelimited(drm, fmt, ...) \ - __DRM_DEFINE_DBG_RATELIMITED(KMS, drm, fmt, ## __VA_ARGS__) + __DRM_DEFINE_DBG_RATELIMITED(DRM_UT_KMS, cDRM_UT_KMS, drm, fmt, ## __VA_ARGS__)
#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) \ drm_dbg_kms_ratelimited(NULL, fmt, ## __VA_ARGS__)
The gvt component of this driver has ~120 pr_debugs, in 9 "classes". Following the interface model of drm.debug, add a parameter to map bits to these classes.
If CONFIG_DRM_USE_DYNAMIC_DEBUG=y (and CONFIG_DYNAMIC_DEBUG_CORE), add -DDYNAMIC_DEBUG_MODULE into Makefile. TBD: maybe add a separate CONFIG_I915_USE_DYNAMIC_DEBUG to more fully optionalize this.
In i915_params.c, add callback to map bits to queries.
TBD: the callback code should probably be moved to lib/dynamic_debug, and given a declarative interface, with implied bit-numbering, something like:
MOD_PARM_BITMAP_DESC(__gvt_debug, "gvt: cmd: ", "command processing" "gvt: core: ", "core help", "gvt: dpy: ", "display help", "gvt: el: ", "help", "gvt: irq: ", "help", "gvt: mm: ", "help", "gvt: mmio: ", "help", "gvt: render: ", "help", "gvt: sched: " "help"); --- drivers/gpu/drm/i915/gvt/Makefile | 4 ++ drivers/gpu/drm/i915/i915_params.c | 76 ++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile index ea8324abc784..846ba73b8de6 100644 --- a/drivers/gpu/drm/i915/gvt/Makefile +++ b/drivers/gpu/drm/i915/gvt/Makefile @@ -7,3 +7,7 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
ccflags-y += -I $(srctree)/$(src) -I $(srctree)/$(src)/$(GVT_DIR)/ i915-y += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE)) + +#ifdef CONFIG_DRM_USE_DYNAMIC_DEBUG +ccflags-y += -DDYNAMIC_DEBUG_MODULE +#endif diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index e07f4cfea63a..e0d13aff5274 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -265,3 +265,79 @@ void i915_params_free(struct i915_params *params) I915_PARAMS_FOR_EACH(FREE); #undef FREE } + +/* POC for callback -> dynamic_debug_exec_queries */ +unsigned long __gvt_debug; +EXPORT_SYMBOL(__gvt_debug); + +static char *format_prefix_classes[] = { + "gvt: cmd: ", + "gvt: core: ", + "gvt: dpy: ", + "gvt: el: ", + "gvt: irq: ", + "gvt: mm: ", + "gvt: mmio: ", + "gvt: render: ", + "gvt: sched: " +}; +#define NUM_CLASSES ARRAY_SIZE(format_prefix_classes) +#define OUR_QUERY_SIZE 128 /* we need about 20 */ + +#include <linux/module.h> + +static int param_set_dyndbg(const char *instr, const struct kernel_param *kp) +{ + unsigned int val; + unsigned long changes, result; + int rc, chgct = 0, totct = 0, bitpos; + char query[OUR_QUERY_SIZE]; + + rc = kstrtouint(instr, 0, &val); + if (rc) { + pr_err("set_dyndbg: failed\n"); + return -EINVAL; + } + result = val; + pr_info("set_dyndbg: result:0x%lx from %s\n", result, instr); + + changes = result ^ __gvt_debug; + + for_each_set_bit(bitpos, &changes, NUM_CLASSES) { + + sprintf(query, "format '^%s' %cp", format_prefix_classes[bitpos], + test_bit(bitpos, &result) ? '+' : '-'); + + chgct = dynamic_debug_exec_queries(query, "i915"); + + pr_info("%d changes on: %s\n", chgct, query); + totct += chgct; + } + pr_info("total changes: %d\n", totct); + __gvt_debug = result; + return 0; +} +static int param_get_dyndbg(char *buffer, const struct kernel_param *kp) +{ + return scnprintf(buffer, PAGE_SIZE, "%u\n", + *((unsigned int *)kp->arg)); +} +static const struct kernel_param_ops param_ops_dyndbg = { + .set = param_set_dyndbg, + .get = param_get_dyndbg, +}; + +#define info_ln(hexi, prefix) "\n\t0x" __stringify(hexi) "\t" prefix + +MODULE_PARM_DESC(debug_gvt, " gvt debug categories:" + info_ln(1, "gvt: cmd:") + info_ln(2, "gvt: core:") + info_ln(4, "gvt: dpy:") + info_ln(8, "gvt: el:") + info_ln(10, "gvt: irq:") + info_ln(20, "gvt: mm:") + info_ln(40, "gvt: mmio:") + info_ln(80, "gvt: render:") + info_ln(100, "gvt: sched:")); + +module_param_cb(debug_gvt, ¶m_ops_dyndbg, &__gvt_debug, 0644);
dri-devel@lists.freedesktop.org
-
Jim Cromie -
jim.cromie＠gmail.com
-
Joe Perches