For starters some fairly benign cleanup, and a proposal for new struct drm_device based drm logging macros analoguous to core kernel struct device based macros.
BR, Jani.
Jani Nikula (8): drm/i915: use drm_debug_enabled() to check for debug categories drm/nouveau: use drm_debug_enabled() to check for debug categories drm/amdgpu: use drm_debug_enabled() to check for debug categories drm/print: rename drm_debug to __drm_debug to discourage use drm/print: underscore prefix functions that should be private to print drm/print: convert debug category macros into an enum drm/print: group logging functions by prink or device based drm/print: introduce new struct drm_device based logging macros
drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 4 +- drivers/gpu/drm/drm_print.c | 18 +- drivers/gpu/drm/i915/display/intel_display.c | 4 +- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_gem.h | 2 +- drivers/gpu/drm/i915/i915_utils.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.h | 4 +- drivers/gpu/drm/nouveau/nouveau_drv.h | 4 +- include/drm/drm_print.h | 304 ++++++++++++------- 11 files changed, 224 insertions(+), 124 deletions(-)
Allow better abstraction of the drm_debug global variable in the future. No functional changes.
Reviewed-by: Eric Engestrom eric@engestrom.ch Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/i915/display/intel_display.c | 4 ++-- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_gem.h | 2 +- drivers/gpu/drm/i915/i915_utils.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 1a533ccdb54f..36d7d11ed2fc 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -12025,7 +12025,7 @@ static void intel_dump_infoframe(struct drm_i915_private *dev_priv, const union hdmi_infoframe *frame) { - if ((drm_debug & DRM_UT_KMS) == 0) + if (!drm_debug_enabled(DRM_UT_KMS)) return;
hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, frame); @@ -12541,7 +12541,7 @@ pipe_config_infoframe_mismatch(struct drm_i915_private *dev_priv, const union hdmi_infoframe *b) { if (fastset) { - if ((drm_debug & DRM_UT_KMS) == 0) + if (!drm_debug_enabled(DRM_UT_KMS)) return;
DRM_DEBUG_KMS("fastset mismatch in %s infoframe\n", name); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0e45c61d7331..e64afc503cb5 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1784,7 +1784,7 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp) { char str[128]; /* FIXME: too big for stack? */
- if ((drm_debug & DRM_UT_KMS) == 0) + if (!drm_debug_enabled(DRM_UT_KMS)) return;
snprintf_int_array(str, sizeof(str), diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f02a34722217..8e2c8e173f7c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1380,7 +1380,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
static void i915_welcome_messages(struct drm_i915_private *dev_priv) { - if (drm_debug & DRM_UT_DRIVER) { + if (drm_debug_enabled(DRM_UT_DRIVER)) { struct drm_printer p = drm_debug_printer("i915 device info:");
drm_printf(&p, "pciid=0x%04x rev=0x%02x platform=%s (subplatform=0x%x) gen=%i\n", diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h index 167a7b56ed5b..a49b39e896b7 100644 --- a/drivers/gpu/drm/i915/i915_gem.h +++ b/drivers/gpu/drm/i915/i915_gem.h @@ -34,7 +34,7 @@ struct drm_i915_private;
#ifdef CONFIG_DRM_I915_DEBUG_GEM
-#define GEM_SHOW_DEBUG() (drm_debug & DRM_UT_DRIVER) +#define GEM_SHOW_DEBUG() drm_debug_enabled(DRM_UT_DRIVER)
#define GEM_BUG_ON(condition) do { if (unlikely((condition))) { \ pr_err("%s:%d GEM_BUG_ON(%s)\n", \ diff --git a/drivers/gpu/drm/i915/i915_utils.c b/drivers/gpu/drm/i915/i915_utils.c index 16acdf7bdbe6..f66540e15793 100644 --- a/drivers/gpu/drm/i915/i915_utils.c +++ b/drivers/gpu/drm/i915/i915_utils.c @@ -23,7 +23,7 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level, struct va_format vaf; va_list args;
- if (is_debug && !(drm_debug & DRM_UT_DRIVER)) + if (is_debug && !drm_debug_enabled(DRM_UT_DRIVER)) return;
va_start(args, fmt); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index bfcf03ab5245..d465c87fa9f3 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5322,7 +5322,7 @@ skl_print_wm_changes(struct intel_atomic_state *state) struct intel_crtc *crtc; int i;
- if ((drm_debug & DRM_UT_KMS) == 0) + if (!drm_debug_enabled(DRM_UT_KMS)) return;
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
Allow better abstraction of the drm_debug global variable in the future. No functional changes.
v2: move unlikely() to drm_debug_enabled()
Cc: Ben Skeggs bskeggs@redhat.com Cc: nouveau@lists.freedesktop.org Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/nouveau/dispnv50/disp.h | 4 ++-- drivers/gpu/drm/nouveau/nouveau_drv.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h b/drivers/gpu/drm/nouveau/dispnv50/disp.h index 7c41b0599d1a..c0a79531b087 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.h +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h @@ -78,14 +78,14 @@ void evo_kick(u32 *, struct nv50_dmac *);
#define evo_mthd(p, m, s) do { \ const u32 _m = (m), _s = (s); \ - if (drm_debug & DRM_UT_KMS) \ + if (drm_debug_enabled(DRM_UT_KMS)) \ pr_err("%04x %d %s\n", _m, _s, __func__); \ *((p)++) = ((_s << 18) | _m); \ } while(0)
#define evo_data(p, d) do { \ const u32 _d = (d); \ - if (drm_debug & DRM_UT_KMS) \ + if (drm_debug_enabled(DRM_UT_KMS)) \ pr_err("\t%08x\n", _d); \ *((p)++) = _d; \ } while(0) diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h index 70f34cacc552..da8c46e09943 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drv.h +++ b/drivers/gpu/drm/nouveau/nouveau_drv.h @@ -248,11 +248,11 @@ void nouveau_drm_device_remove(struct drm_device *dev); #define NV_INFO(drm,f,a...) NV_PRINTK(info, &(drm)->client, f, ##a)
#define NV_DEBUG(drm,f,a...) do { \ - if (unlikely(drm_debug & DRM_UT_DRIVER)) \ + if (drm_debug_enabled(DRM_UT_DRIVER)) \ NV_PRINTK(info, &(drm)->client, f, ##a); \ } while(0) #define NV_ATOMIC(drm,f,a...) do { \ - if (unlikely(drm_debug & DRM_UT_ATOMIC)) \ + if (drm_debug_enabled(DRM_UT_ATOMIC)) \ NV_PRINTK(info, &(drm)->client, f, ##a); \ } while(0)
Allow better abstraction of the drm_debug global variable in the future. No functional changes.
Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: David (ChunMing) Zhou David1.Zhou@amd.com Cc: amd-gfx@lists.freedesktop.org Acked-by: Alex Deucher alexander.deucher@amd.com Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c index c44723c267c9..c902f26cf50d 100644 --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c @@ -234,7 +234,7 @@ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control, DRM_DEBUG_DRIVER("I2C_Transmit(), address = %x, bytes = %d , data: ", (uint16_t)address, numbytes);
- if (drm_debug & DRM_UT_DRIVER) { + if (drm_debug_enabled(DRM_UT_DRIVER)) { print_hex_dump(KERN_INFO, "data: ", DUMP_PREFIX_NONE, 16, 1, data, numbytes, false); } @@ -388,7 +388,7 @@ static uint32_t smu_v11_0_i2c_receive(struct i2c_adapter *control, DRM_DEBUG_DRIVER("I2C_Receive(), address = %x, bytes = %d, data :", (uint16_t)address, bytes_received);
- if (drm_debug & DRM_UT_DRIVER) { + if (drm_debug_enabled(DRM_UT_DRIVER)) { print_hex_dump(KERN_INFO, "data: ", DUMP_PREFIX_NONE, 16, 1, data, bytes_received, false); }
drm_debug_enabled() is the way to check. __drm_debug is now reserved for drm print code only. No functional changes.
v2: Rebase on move unlikely() to drm_debug_enabled()
Acked-by: Alex Deucher alexander.deucher@amd.com Reviewed-by: Eric Engestrom eric@engestrom.ch Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_print.c | 8 ++++---- include/drm/drm_print.h | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 9a25d73c155c..a0d1cde9888a 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -37,11 +37,11 @@ #include <drm/drm_print.h>
/* - * drm_debug: Enable debug output. + * __drm_debug: Enable debug output. * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details. */ -unsigned int drm_debug; -EXPORT_SYMBOL(drm_debug); +unsigned int __drm_debug; +EXPORT_SYMBOL(__drm_debug);
MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n" "\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n" @@ -52,7 +52,7 @@ 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); +module_param_named(debug, __drm_debug, int, 0600);
void __drm_puts_coredump(struct drm_printer *p, const char *str) { diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 5b8049992c24..77fef2c7e312 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -34,7 +34,8 @@
#include <drm/drm.h>
-extern unsigned int drm_debug; +/* Do *not* use outside of drm_print.[ch]! */ +extern unsigned int __drm_debug;
/** * DOC: print @@ -295,7 +296,7 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
static inline bool drm_debug_enabled(unsigned int category) { - return unlikely(drm_debug & category); + return unlikely(__drm_debug & category); }
__printf(3, 4)
We don't want people calling the functions directly. No functional changes.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_print.c | 8 ++++---- include/drm/drm_print.h | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index a0d1cde9888a..703b126dd074 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -280,7 +280,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, } EXPORT_SYMBOL(drm_dev_dbg);
-void drm_dbg(unsigned int category, const char *format, ...) +void __drm_dbg(unsigned int category, const char *format, ...) { struct va_format vaf; va_list args; @@ -297,9 +297,9 @@ void drm_dbg(unsigned int category, const char *format, ...)
va_end(args); } -EXPORT_SYMBOL(drm_dbg); +EXPORT_SYMBOL(__drm_dbg);
-void drm_err(const char *format, ...) +void __drm_err(const char *format, ...) { struct va_format vaf; va_list args; @@ -313,7 +313,7 @@ void drm_err(const char *format, ...)
va_end(args); } -EXPORT_SYMBOL(drm_err); +EXPORT_SYMBOL(__drm_err);
/** * drm_print_regset32 - print the contents of registers to a diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 77fef2c7e312..ce45ec46202a 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -307,9 +307,9 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, const char *format, ...);
__printf(2, 3) -void drm_dbg(unsigned int category, const char *format, ...); +void __drm_dbg(unsigned int category, const char *format, ...); __printf(1, 2) -void drm_err(const char *format, ...); +void __drm_err(const char *format, ...);
/* Macros to make printk easier */
@@ -339,7 +339,7 @@ void drm_err(const char *format, ...); #define DRM_DEV_ERROR(dev, fmt, ...) \ drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__) #define DRM_ERROR(fmt, ...) \ - drm_err(fmt, ##__VA_ARGS__) + __drm_err(fmt, ##__VA_ARGS__)
/** * Rate limited error output. Like DRM_ERROR() but won't flood the log. @@ -380,40 +380,40 @@ void drm_err(const char *format, ...); #define DRM_DEV_DEBUG(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_CORE, fmt, ##__VA_ARGS__) #define DRM_DEBUG(fmt, ...) \ - drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__) + __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_DRIVER(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_DRIVER, fmt, ##__VA_ARGS__) #define DRM_DEBUG_DRIVER(fmt, ...) \ - drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__) + __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_KMS(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_KMS, fmt, ##__VA_ARGS__) #define DRM_DEBUG_KMS(fmt, ...) \ - drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__) + __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_PRIME(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_PRIME, fmt, ##__VA_ARGS__) #define DRM_DEBUG_PRIME(fmt, ...) \ - drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__) + __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) #define DRM_DEBUG_ATOMIC(fmt, ...) \ - drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) + __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_VBL(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_VBL, fmt, ##__VA_ARGS__) #define DRM_DEBUG_VBL(fmt, ...) \ - drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__) + __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_LEASE(fmt, ...) \ - drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__) + __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_DP(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_DP, fmt, ## __VA_ARGS__) #define DRM_DEBUG_DP(fmt, ...) \ - drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__) + __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, category, fmt, ...) \ ({ \
Mostly for improved documentation, convert the debug category macros into an enum. Drop unused DRM_UT_NONE. Document previously undocumented categories.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- drivers/gpu/drm/drm_print.c | 4 +- include/drm/drm_print.h | 101 ++++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 703b126dd074..111b932cf2a9 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -256,7 +256,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, unsigned int category, +void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, const char *format, ...) { struct va_format vaf; @@ -280,7 +280,7 @@ void drm_dev_dbg(const struct device *dev, unsigned int category, } EXPORT_SYMBOL(drm_dev_dbg);
-void __drm_dbg(unsigned int category, const char *format, ...) +void __drm_dbg(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 ce45ec46202a..13f65394376e 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -249,52 +249,73 @@ static inline struct drm_printer drm_err_printer(const char *prefix) return p; }
-/* - * The following categories are defined: - * - * CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c, drm_memory.c, ... - * This is the category used by the DRM_DEBUG() macro. - * - * DRIVER: Used in the vendor specific part of the driver: i915, radeon, ... - * This is the category used by the DRM_DEBUG_DRIVER() macro. - * - * KMS: used in the modesetting code. - * This is the category used by the DRM_DEBUG_KMS() macro. - * - * PRIME: used in the prime code. - * This is the category used by the DRM_DEBUG_PRIME() macro. +/** + * enum drm_debug_category - The DRM debug categories * - * ATOMIC: used in the atomic code. - * This is the category used by the DRM_DEBUG_ATOMIC() macro. + * Each of the DRM debug logging macros use a specific category, and the logging + * is filtered by the drm.debug module parameter. This enum specifies the values + * for the interface. * - * VBL: used for verbose debug message in the vblank code - * This is the category used by the DRM_DEBUG_VBL() macro. + * Each DRM_DEBUG_<CATEGORY> macro logs to DRM_UT_<CATEGORY> category, except + * DRM_DEBUG() logs to DRM_UT_CORE. * - * Enabling verbose debug messages is done through the drm.debug parameter, - * each category being enabled by a bit. + * Enabling verbose debug messages is done through the drm.debug parameter, each + * category being enabled by a bit: * - * drm.debug=0x1 will enable CORE messages - * drm.debug=0x2 will enable DRIVER messages - * drm.debug=0x3 will enable CORE and DRIVER messages - * ... - * drm.debug=0x3f will enable all messages + * - drm.debug=0x1 will enable CORE messages + * - drm.debug=0x2 will enable DRIVER messages + * - drm.debug=0x3 will enable CORE and DRIVER messages + * - ... + * - drm.debug=0x1ff will enable all messages * * An interesting feature is that it's possible to enable verbose logging at - * run-time by echoing the debug value in its sysfs node: + * run-time by echoing the debug value in its sysfs node:: + * * # echo 0xf > /sys/module/drm/parameters/debug + * */ -#define DRM_UT_NONE 0x00 -#define DRM_UT_CORE 0x01 -#define DRM_UT_DRIVER 0x02 -#define DRM_UT_KMS 0x04 -#define DRM_UT_PRIME 0x08 -#define DRM_UT_ATOMIC 0x10 -#define DRM_UT_VBL 0x20 -#define DRM_UT_STATE 0x40 -#define DRM_UT_LEASE 0x80 -#define DRM_UT_DP 0x100 - -static inline bool drm_debug_enabled(unsigned int category) +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_DRIVER: Used in the vendor specific part of the driver: i915, + * radeon, ... macro. + */ + DRM_UT_DRIVER = 0x02, + /** + * @DRM_UT_KMS: Used in the modesetting code. + */ + DRM_UT_KMS = 0x04, + /** + * @DRM_UT_PRIME: Used in the prime code. + */ + DRM_UT_PRIME = 0x08, + /** + * @DRM_UT_ATOMIC: Used in the atomic code. + */ + DRM_UT_ATOMIC = 0x10, + /** + * @DRM_UT_VBL: Used for verbose debug message in the vblank code. + */ + DRM_UT_VBL = 0x20, + /** + * @DRM_UT_STATE: Used for verbose atomic state debugging. + */ + DRM_UT_STATE = 0x40, + /** + * @DRM_UT_LEASE: Used in the lease code. + */ + DRM_UT_LEASE = 0x80, + /** + * @DRM_UT_DP: Used in the DP code. + */ + DRM_UT_DP = 0x100, +}; + +static inline bool drm_debug_enabled(enum drm_debug_category category) { return unlikely(__drm_debug & category); } @@ -303,11 +324,11 @@ __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, unsigned int category, +void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, const char *format, ...);
__printf(2, 3) -void __drm_dbg(unsigned int category, const char *format, ...); +void __drm_dbg(enum drm_debug_category category, const char *format, ...); __printf(1, 2) void __drm_err(const char *format, ...);
In preparation for adding struct drm_device based logging, group the existing functions by prink or struct device based logging. No functional changes.
Signed-off-by: Jani Nikula jani.nikula@intel.com --- include/drm/drm_print.h | 135 ++++++++++++++++++++++------------------ 1 file changed, 74 insertions(+), 61 deletions(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 13f65394376e..085a9685270c 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -320,6 +320,10 @@ static inline bool drm_debug_enabled(enum drm_debug_category category) return unlikely(__drm_debug & category); }
+/* + * struct device based logging + */ + __printf(3, 4) void drm_dev_printk(const struct device *dev, const char *level, const char *format, ...); @@ -327,30 +331,6 @@ __printf(3, 4) void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, const char *format, ...);
-__printf(2, 3) -void __drm_dbg(enum drm_debug_category category, const char *format, ...); -__printf(1, 2) -void __drm_err(const char *format, ...); - -/* Macros to make printk easier */ - -#define _DRM_PRINTK(once, level, fmt, ...) \ - printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__) - -#define DRM_INFO(fmt, ...) \ - _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) -#define DRM_NOTE(fmt, ...) \ - _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) -#define DRM_WARN(fmt, ...) \ - _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) - -#define DRM_INFO_ONCE(fmt, ...) \ - _DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__) -#define DRM_NOTE_ONCE(fmt, ...) \ - _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) -#define DRM_WARN_ONCE(fmt, ...) \ - _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) - /** * Error output. * @@ -359,8 +339,6 @@ void __drm_err(const char *format, ...); */ #define DRM_DEV_ERROR(dev, fmt, ...) \ drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__) -#define DRM_ERROR(fmt, ...) \ - __drm_err(fmt, ##__VA_ARGS__)
/** * Rate limited error output. Like DRM_ERROR() but won't flood the log. @@ -377,10 +355,8 @@ void __drm_err(const char *format, ...); if (__ratelimit(&_rs)) \ DRM_DEV_ERROR(dev, fmt, ##__VA_ARGS__); \ }) -#define DRM_ERROR_RATELIMITED(fmt, ...) \ - DRM_DEV_ERROR_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
-#define DRM_DEV_INFO(dev, fmt, ...) \ +#define DRM_DEV_INFO(dev, fmt, ...) \ drm_dev_printk(dev, KERN_INFO, fmt, ##__VA_ARGS__)
#define DRM_DEV_INFO_ONCE(dev, fmt, ...) \ @@ -400,41 +376,18 @@ void __drm_err(const char *format, ...); */ #define DRM_DEV_DEBUG(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_CORE, fmt, ##__VA_ARGS__) -#define DRM_DEBUG(fmt, ...) \ - __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_DRIVER(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_DRIVER, fmt, ##__VA_ARGS__) -#define DRM_DEBUG_DRIVER(fmt, ...) \ - __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_KMS(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_KMS, fmt, ##__VA_ARGS__) -#define DRM_DEBUG_KMS(fmt, ...) \ - __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_PRIME(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_PRIME, fmt, ##__VA_ARGS__) -#define DRM_DEBUG_PRIME(fmt, ...) \ - __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_ATOMIC(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) -#define DRM_DEBUG_ATOMIC(fmt, ...) \ - __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_VBL(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_VBL, fmt, ##__VA_ARGS__) -#define DRM_DEBUG_VBL(fmt, ...) \ - __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__) - -#define DRM_DEBUG_LEASE(fmt, ...) \ - __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_DP(dev, fmt, ...) \ drm_dev_dbg(dev, DRM_UT_DP, fmt, ## __VA_ARGS__) -#define DRM_DEBUG_DP(fmt, ...) \ - __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, category, fmt, ...) \ ({ \ @@ -454,24 +407,84 @@ void __drm_err(const char *format, ...); #define DRM_DEV_DEBUG_RATELIMITED(dev, fmt, ...) \ _DEV_DRM_DEFINE_DEBUG_RATELIMITED(dev, DRM_UT_CORE, \ fmt, ##__VA_ARGS__) -#define DRM_DEBUG_RATELIMITED(fmt, ...) \ - DRM_DEV_DEBUG_RATELIMITED(NULL, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_DRIVER_RATELIMITED(dev, fmt, ...) \ _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, DRM_UT_DRIVER, \ fmt, ##__VA_ARGS__) -#define DRM_DEBUG_DRIVER_RATELIMITED(fmt, ...) \ - DRM_DEV_DEBUG_DRIVER_RATELIMITED(NULL, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_KMS_RATELIMITED(dev, fmt, ...) \ _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, DRM_UT_KMS, \ fmt, ##__VA_ARGS__) -#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) \ - DRM_DEV_DEBUG_KMS_RATELIMITED(NULL, fmt, ##__VA_ARGS__) - #define DRM_DEV_DEBUG_PRIME_RATELIMITED(dev, fmt, ...) \ _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, DRM_UT_PRIME, \ fmt, ##__VA_ARGS__) + +/* + * printk based logging + */ + +__printf(2, 3) +void __drm_dbg(enum drm_debug_category category, const char *format, ...); +__printf(1, 2) +void __drm_err(const char *format, ...); + +/* Macros to make printk easier */ + +#define _DRM_PRINTK(once, level, fmt, ...) \ + printk##once(KERN_##level "[" DRM_NAME "] " fmt, ##__VA_ARGS__) + +#define DRM_INFO(fmt, ...) \ + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE(fmt, ...) \ + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN(fmt, ...) \ + _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__) + +#define DRM_INFO_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, INFO, fmt, ##__VA_ARGS__) +#define DRM_NOTE_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__) +#define DRM_WARN_ONCE(fmt, ...) \ + _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__) + +#define DRM_ERROR(fmt, ...) \ + __drm_err(fmt, ##__VA_ARGS__) + +#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__) + +#define DRM_DEBUG_DRIVER(fmt, ...) \ + __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_KMS(fmt, ...) \ + __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_PRIME(fmt, ...) \ + __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_ATOMIC(fmt, ...) \ + __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_VBL(fmt, ...) \ + __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_LEASE(fmt, ...) \ + __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_DP(fmt, ...) \ + __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__) + + +#define DRM_DEBUG_RATELIMITED(fmt, ...) \ + DRM_DEV_DEBUG_RATELIMITED(NULL, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_DRIVER_RATELIMITED(fmt, ...) \ + DRM_DEV_DEBUG_DRIVER_RATELIMITED(NULL, fmt, ##__VA_ARGS__) + +#define DRM_DEBUG_KMS_RATELIMITED(fmt, ...) \ + DRM_DEV_DEBUG_KMS_RATELIMITED(NULL, fmt, ##__VA_ARGS__) + #define DRM_DEBUG_PRIME_RATELIMITED(fmt, ...) \ DRM_DEV_DEBUG_PRIME_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
Add new struct drm_device based logging macros modeled after the core kernel device based logging macros. These would be preferred over the drm printk and struct device based macros in drm code, where possible.
We have existing drm specific struct device based logging functions, but they are too verbose to use for two main reasons:
* The names are unnecessarily long, for example DRM_DEV_DEBUG_KMS().
* The use of struct device over struct drm_device is too generic for most users, leading to an extra dereference.
For example:
DRM_DEV_DEBUG_KMS(drm->dev, "Hello, world\n");
vs.
drm_dbg_kms(drm, "Hello, world\n");
It's a matter of taste, but the SHOUTING UPPERCASE has been argued to be less readable than lowercase.
Some names are changed from old DRM names to be based on the core kernel logging functions. For example, NOTE -> notice, ERROR -> err, DEBUG -> dbg.
Due to the conflation of DRM_DEBUG and DRM_DEBUG_DRIVER macro use (DRM_DEBUG is used widely in drivers though it's supposed to be a core debugging category), they are named as drm_dbg_core and drm_dbg, respectively.
The drm_err and _once/_ratelimited variants no longer include the function name in order to be able to use the core device based logging macros. Arguably this is not a significant change; error messages should not be so common to be only distinguishable by the function name.
Ratelimited debug logging macros are to be added later.
Cc: Sam Ravnborg sam@ravnborg.org Signed-off-by: Jani Nikula jani.nikula@intel.com
---
With something like this, I think i915 could start migrating to drm_device based logging. I have a hard time convincing myself or anyone about migrating to the DRM_DEV_* variants. --- include/drm/drm_print.h | 65 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 085a9685270c..e4040dea0d8c 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -322,6 +322,8 @@ 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. */
__printf(3, 4) @@ -417,8 +419,71 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category, _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, DRM_UT_PRIME, \ fmt, ##__VA_ARGS__)
+/* + * struct drm_device based logging + * + * Prefer drm_device based logging over device or prink based logging. + */ + +/* Helper for struct drm_device based logging. */ +#define __drm_printk(drm, level, type, fmt, ...) \ + dev_##level##type(drm->dev, "[drm] " fmt, ##__VA_ARGS__) + + +#define drm_info(drm, fmt, ...) \ + __drm_printk(drm, info,, fmt, ##__VA_ARGS__) + +#define drm_notice(drm, fmt, ...) \ + __drm_printk(drm, notice,, fmt, ##__VA_ARGS__) + +#define drm_warn(drm, fmt, ...) \ + __drm_printk(drm, warn,, fmt, ##__VA_ARGS__) + +#define drm_err(drm, fmt, ...) \ + __drm_printk(drm, err,, "*ERROR* " fmt, ##__VA_ARGS__) + + +#define drm_info_once(drm, fmt, ...) \ + __drm_printk(drm, info, _once, fmt, ##__VA_ARGS__) + +#define drm_notice_once(drm, fmt, ...) \ + __drm_printk(drm, notice, _once, fmt, ##__VA_ARGS__) + +#define drm_warn_once(drm, fmt, ...) \ + __drm_printk(drm, warn, _once, fmt, ##__VA_ARGS__) + +#define drm_err_once(drm, fmt, ...) \ + __drm_printk(drm, err, _once, "*ERROR* " fmt, ##__VA_ARGS__) + + +#define drm_err_ratelimited(drm, fmt, ...) \ + __drm_printk(drm, err, _ratelimited, "*ERROR* " fmt, ##__VA_ARGS__) + + +#define drm_dbg_core(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_CORE, fmt, ##__VA_ARGS__) +#define drm_dbg(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_DRIVER, fmt, ##__VA_ARGS__) +#define drm_dbg_kms(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_KMS, fmt, ##__VA_ARGS__) +#define drm_dbg_prime(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_PRIME, fmt, ##__VA_ARGS__) +#define drm_dbg_atomic(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__) +#define drm_dbg_vbl(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_VBL, fmt, ##__VA_ARGS__) +#define drm_dbg_state(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_STATE, fmt, ##__VA_ARGS__) +#define drm_dbg_lease(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_LEASE, fmt, ##__VA_ARGS__) +#define drm_dbg_dp(drm, fmt, ...) \ + drm_dev_dbg(drm->dev, DRM_UT_DP, fmt, ##__VA_ARGS__) + + /* * printk based logging + * + * Prefer drm_device based logging over device or prink based logging. */
__printf(2, 3)
In the past, I have been able to do a:
#undef pr_fmt #define pr_fmt(fmt) "[myinfo here] " fmt
And have the "[myinfo here]" portion show up the output.
Is it possible that you might be able to use this instead of "[drm] " fmt?
I think that the this will be the same result, but might be more in line with the printk interface?
Mike
On Wed, 09 Oct 2019, "Ruhl, Michael J" michael.j.ruhl@intel.com wrote:
pr_fmt is only used by the pr_<level>() macros in printk.h that use printk. This does not use printk.
BR, Jani.
dri-devel@lists.freedesktop.org
-
Jani Nikula -
Ruhl, Michael J