2021-07-11 05:51:34

by Jim Cromie

[permalink] [raw]
Subject: [RFC PATCH v2 0/4] Allow using dyndbg to replace drm_debug_enabled

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/[email protected]/

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(-)

--
2.31.1


2021-07-11 05:51:45

by Jim Cromie

[permalink] [raw]
Subject: [RFC PATCH v2 1/4] drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro

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
--
2.31.1

2021-07-11 05:53:44

by Jim Cromie

[permalink] [raw]
Subject: [RFC PATCH v2 4/4] i915: map gvt pr_debug categories to bits in parameters/debug_gvt

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, &param_ops_dyndbg, &__gvt_debug, 0644);
--
2.31.1

2021-07-11 05:55:19

by Jim Cromie

[permalink] [raw]
Subject: [RFC PATCH v2 3/4] drm: RFC add choice to use dynamic debug in drm-debug

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, &param_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__)
--
2.31.1

2021-07-11 05:55:36

by Jim Cromie

[permalink] [raw]
Subject: [RFC PATCH v2 2/4] drm: fixup comment spelling

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)
--
2.31.1

2021-07-11 16:18:40

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/4] drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro

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_)) \


2021-07-13 01:40:30

by Jim Cromie

[permalink] [raw]
Subject: Re: [RFC PATCH v2 1/4] drm_print.h: rewrap __DRM_DEFINE_DBG_RATELIMITED macro

On Sun, Jul 11, 2021 at 10:17 AM Joe Perches <[email protected]> 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_)) \
>
>