[PATCH 00/10] drm: drm debug and error logging improvements

[PATCH 00/10] drm: drm debug and error logging improvements

[Jani Nikula]

This is resend and more patches on top of [1]. I don't think I've
changed anything since then.

BR,
Jani.

[1] https://patchwork.freedesktop.org/series/126873/


Jani Nikula (10):
  drm/print: make drm_err_printer() device specific by using drm_err()
  drm/print: move enum drm_debug_category etc. earlier in drm_print.h
  drm/print: add drm_dbg_printer() for drm device specific printer
  drm/dp_mst: switch from drm_debug_printer() to device specific
    drm_dbg_printer()
  drm/mode: switch from drm_debug_printer() to device specific
    drm_dbg_printer()
  drm/dp: switch drm_dp_vsc_sdp_log() to struct drm_printer
  drm/i915: switch from drm_debug_printer() to device specific
    drm_dbg_printer()
  drm/i915: use drm_printf() with the drm_err_printer intead of pr_err()
  drm/xe: switch from drm_debug_printer() to device specific
    drm_dbg_printer()
  drm: remove drm_debug_printer in favor of drm_dbg_printer

 drivers/gpu/drm/display/drm_dp_helper.c       |  17 +-
 drivers/gpu/drm/display/drm_dp_mst_topology.c |  23 +-
 drivers/gpu/drm/drm_mode_config.c             |   2 +-
 drivers/gpu/drm/drm_modeset_lock.c            |   2 +-
 drivers/gpu/drm/drm_print.c                   |  29 ++-
 .../drm/i915/display/intel_crtc_state_dump.c  |   5 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  27 ++-
 .../drm/i915/display/intel_display_driver.c   |   3 +-
 .../gpu/drm/i915/gt/intel_engine_heartbeat.c  |   3 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   |   3 +-
 drivers/gpu/drm/i915/gt/selftest_context.c    |   3 +-
 .../drm/i915/gt/selftest_engine_heartbeat.c   |  10 +-
 drivers/gpu/drm/i915/i915_driver.c            |   3 +-
 drivers/gpu/drm/i915/selftests/i915_active.c  |   8 +-
 drivers/gpu/drm/ttm/ttm_bo.c                  |   2 +-
 drivers/gpu/drm/xe/xe_gt.c                    |   2 +-
 drivers/gpu/drm/xe/xe_gt_topology.c           |   4 +-
 drivers/gpu/drm/xe/xe_reg_sr.c                |   2 +-
 include/drm/display/drm_dp_helper.h           |   3 +-
 include/drm/drm_print.h                       | 220 +++++++++---------
 21 files changed, 206 insertions(+), 168 deletions(-)

-- 
2.39.2

[PATCH 01/10] drm/print: make drm_err_printer() device specific by using drm_err()

[Jani Nikula]

With few users for drm_err_printer(), it's still feasible to convert it
to be device specific. Use drm_err() under the hood.

While at it, make the prefix optional.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_print.c                         |  7 ++++++-
 drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c |  4 ++--
 drivers/gpu/drm/i915/selftests/i915_active.c        |  4 ++--
 include/drm/drm_print.h                             | 11 ++++++++---
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 5b93c11895bb..91dbcdeaad3f 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -191,7 +191,12 @@ EXPORT_SYMBOL(__drm_printfn_debug);
 
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
 {
-	pr_err("*ERROR* %s %pV", p->prefix, vaf);
+	struct drm_device *drm = p->arg;
+
+	if (p->prefix)
+		drm_err(drm, "%s %pV", p->prefix, vaf);
+	else
+		drm_err(drm, "%pV", vaf);
 }
 EXPORT_SYMBOL(__drm_printfn_err);
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index bc441ce7b380..be827318275c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -122,7 +122,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
 	GEM_BUG_ON(!llist_empty(&engine->barrier_tasks));
 
 	if (engine_sync_barrier(engine)) {
-		struct drm_printer m = drm_err_printer("pulse");
+		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
 
 		pr_err("%s: no heartbeat pulse?\n", engine->name);
 		intel_engine_dump(engine, &m, "%s", engine->name);
@@ -136,7 +136,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
 	pulse_unlock_wait(p); /* synchronize with the retirement callback */
 
 	if (!i915_active_is_idle(&p->active)) {
-		struct drm_printer m = drm_err_printer("pulse");
+		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
 
 		pr_err("%s: heartbeat pulse did not flush idle tasks\n",
 		       engine->name);
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
index b61fe850e924..8886752ade63 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -156,7 +156,7 @@ static int live_active_wait(void *arg)
 
 	__i915_active_wait(&active->base, TASK_UNINTERRUPTIBLE);
 	if (!READ_ONCE(active->retired)) {
-		struct drm_printer p = drm_err_printer(__func__);
+		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
 		pr_err("i915_active not retired after waiting!\n");
 		i915_active_print(&active->base, &p);
@@ -189,7 +189,7 @@ static int live_active_retire(void *arg)
 		err = -EIO;
 
 	if (!READ_ONCE(active->retired)) {
-		struct drm_printer p = drm_err_printer(__func__);
+		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
 		pr_err("i915_active not retired after flushing!\n");
 		i915_active_print(&active->base, &p);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 5ed26a702e3e..1040213d02a4 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -35,6 +35,8 @@
 
 #include <drm/drm.h>
 
+struct drm_device;
+
 /* Do *not* use outside of drm_print.[ch]! */
 extern unsigned long __drm_debug;
 
@@ -235,16 +237,19 @@ static inline struct drm_printer drm_debug_printer(const char *prefix)
 }
 
 /**
- * drm_err_printer - construct a &drm_printer that outputs to pr_err()
- * @prefix: debug output prefix
+ * drm_err_printer - construct a &drm_printer that outputs to drm_err()
+ * @drm: the &struct drm_device pointer
+ * @prefix: debug output prefix, or NULL for no prefix
  *
  * RETURNS:
  * The &drm_printer object
  */
-static inline struct drm_printer drm_err_printer(const char *prefix)
+static inline struct drm_printer drm_err_printer(struct drm_device *drm,
+						 const char *prefix)
 {
 	struct drm_printer p = {
 		.printfn = __drm_printfn_err,
+		.arg = drm,
 		.prefix = prefix
 	};
 	return p;
-- 
2.39.2

[PATCH 02/10] drm/print: move enum drm_debug_category etc. earlier in drm_print.h

[Jani Nikula]

Avoid forward declarations in subsequent changes, but separate this
movement to an independent change.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 include/drm/drm_print.h | 190 ++++++++++++++++++++--------------------
 1 file changed, 95 insertions(+), 95 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 1040213d02a4..b8b4cb9bb878 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -69,6 +69,101 @@ extern unsigned long __drm_debug;
  *     }
  */
 
+/**
+ * enum drm_debug_category - The DRM debug categories
+ *
+ * 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.
+ *
+ * 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:
+ *
+ *  - 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::
+ *
+ *   # echo 0xf > /sys/module/drm/parameters/debug
+ *
+ */
+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, ...
+	 */
+	DRM_UT_CORE,
+	/**
+	 * @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
+	 * radeon, ... macro.
+	 */
+	DRM_UT_DRIVER,
+	/**
+	 * @DRM_UT_KMS: Used in the modesetting code.
+	 */
+	DRM_UT_KMS,
+	/**
+	 * @DRM_UT_PRIME: Used in the prime code.
+	 */
+	DRM_UT_PRIME,
+	/**
+	 * @DRM_UT_ATOMIC: Used in the atomic code.
+	 */
+	DRM_UT_ATOMIC,
+	/**
+	 * @DRM_UT_VBL: Used for verbose debug message in the vblank code.
+	 */
+	DRM_UT_VBL,
+	/**
+	 * @DRM_UT_STATE: Used for verbose atomic state debugging.
+	 */
+	DRM_UT_STATE,
+	/**
+	 * @DRM_UT_LEASE: Used in the lease code.
+	 */
+	DRM_UT_LEASE,
+	/**
+	 * @DRM_UT_DP: Used in the DP code.
+	 */
+	DRM_UT_DP,
+	/**
+	 * @DRM_UT_DRMRES: Used in the drm managed resources code.
+	 */
+	DRM_UT_DRMRES
+};
+
+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 drm_printer - drm output "stream"
  *
@@ -255,101 +350,6 @@ static inline struct drm_printer drm_err_printer(struct drm_device *drm,
 	return p;
 }
 
-/**
- * enum drm_debug_category - The DRM debug categories
- *
- * 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.
- *
- * 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:
- *
- *  - 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::
- *
- *   # echo 0xf > /sys/module/drm/parameters/debug
- *
- */
-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, ...
-	 */
-	DRM_UT_CORE,
-	/**
-	 * @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
-	 * radeon, ... macro.
-	 */
-	DRM_UT_DRIVER,
-	/**
-	 * @DRM_UT_KMS: Used in the modesetting code.
-	 */
-	DRM_UT_KMS,
-	/**
-	 * @DRM_UT_PRIME: Used in the prime code.
-	 */
-	DRM_UT_PRIME,
-	/**
-	 * @DRM_UT_ATOMIC: Used in the atomic code.
-	 */
-	DRM_UT_ATOMIC,
-	/**
-	 * @DRM_UT_VBL: Used for verbose debug message in the vblank code.
-	 */
-	DRM_UT_VBL,
-	/**
-	 * @DRM_UT_STATE: Used for verbose atomic state debugging.
-	 */
-	DRM_UT_STATE,
-	/**
-	 * @DRM_UT_LEASE: Used in the lease code.
-	 */
-	DRM_UT_LEASE,
-	/**
-	 * @DRM_UT_DP: Used in the DP code.
-	 */
-	DRM_UT_DP,
-	/**
-	 * @DRM_UT_DRMRES: Used in the drm managed resources code.
-	 */
-	DRM_UT_DRMRES
-};
-
-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.39.2

[PATCH 03/10] drm/print: add drm_dbg_printer() for drm device specific printer

[Jani Nikula]

We've lacked a device specific debug printer. Add one. Take category
into account too.

__builtin_return_address(0) is inaccurate here, so don't use it. If
necessary, we can later pass __func__ to drm_dbg_printer() by wrapping
it inside a macro.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_print.c | 21 +++++++++++++++++++++
 include/drm/drm_print.h     | 24 ++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 91dbcdeaad3f..673b29c732ea 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -189,6 +189,27 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf)
 }
 EXPORT_SYMBOL(__drm_printfn_debug);
 
+void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf)
+{
+	const struct drm_device *drm = p->arg;
+	const struct device *dev = drm ? drm->dev : NULL;
+	enum drm_debug_category category = p->category;
+	const char *prefix = p->prefix ?: "";
+	const char *prefix_pad = p->prefix ? " " : "";
+
+	if (!__drm_debug_enabled(category))
+		return;
+
+	/* Note: __builtin_return_address(0) is useless here. */
+	if (dev)
+		dev_printk(KERN_DEBUG, dev, "[" DRM_NAME "]%s%s %pV",
+			   prefix_pad, prefix, vaf);
+	else
+		printk(KERN_DEBUG "[" DRM_NAME "]%s%s %pV",
+		       prefix_pad, prefix, vaf);
+}
+EXPORT_SYMBOL(__drm_printfn_dbg);
+
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
 {
 	struct drm_device *drm = p->arg;
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index b8b4cb9bb878..27e23c06dad4 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -176,6 +176,7 @@ struct drm_printer {
 	void (*puts)(struct drm_printer *p, const char *str);
 	void *arg;
 	const char *prefix;
+	enum drm_debug_category category;
 };
 
 void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf);
@@ -184,6 +185,7 @@ void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf);
 void __drm_puts_seq_file(struct drm_printer *p, const char *str);
 void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
+void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
 
 __printf(2, 3)
@@ -331,6 +333,28 @@ static inline struct drm_printer drm_debug_printer(const char *prefix)
 	return p;
 }
 
+/**
+ * drm_dbg_printer - construct a &drm_printer for drm device specific output
+ * @drm: the &struct drm_device pointer, or NULL
+ * @category: the debug category to use
+ * @prefix: debug output prefix, or NULL for no prefix
+ *
+ * RETURNS:
+ * The &drm_printer object
+ */
+static inline struct drm_printer drm_dbg_printer(struct drm_device *drm,
+						 enum drm_debug_category category,
+						 const char *prefix)
+{
+	struct drm_printer p = {
+		.printfn = __drm_printfn_dbg,
+		.arg = drm,
+		.prefix = prefix,
+		.category = category,
+	};
+	return p;
+}
+
 /**
  * drm_err_printer - construct a &drm_printer that outputs to drm_err()
  * @drm: the &struct drm_device pointer
-- 
2.39.2

[PATCH 04/10] drm/dp_mst: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Jani Nikula]

Prefer the device specific debug printer.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/display/drm_dp_mst_topology.c | 23 +++++++++++--------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 8ca01a6bf645..fba6e37b051b 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -1306,7 +1306,8 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
 	}
 out:
 	if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) {
-		struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+		struct drm_printer p = drm_dbg_printer(mgr->dev, DRM_UT_DP,
+						       DBG_PREFIX);
 
 		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
 	}
@@ -1593,10 +1594,11 @@ topology_ref_type_to_str(enum drm_dp_mst_topology_ref_type type)
 }
 
 static void
-__dump_topology_ref_history(struct drm_dp_mst_topology_ref_history *history,
+__dump_topology_ref_history(struct drm_device *drm,
+			    struct drm_dp_mst_topology_ref_history *history,
 			    void *ptr, const char *type_str)
 {
-	struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+	struct drm_printer p = drm_dbg_printer(drm, DRM_UT_DP, DBG_PREFIX);
 	char *buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	int i;
 
@@ -1638,15 +1640,15 @@ __dump_topology_ref_history(struct drm_dp_mst_topology_ref_history *history,
 static __always_inline void
 drm_dp_mst_dump_mstb_topology_history(struct drm_dp_mst_branch *mstb)
 {
-	__dump_topology_ref_history(&mstb->topology_ref_history, mstb,
-				    "MSTB");
+	__dump_topology_ref_history(mstb->mgr->dev, &mstb->topology_ref_history,
+				    mstb, "MSTB");
 }
 
 static __always_inline void
 drm_dp_mst_dump_port_topology_history(struct drm_dp_mst_port *port)
 {
-	__dump_topology_ref_history(&port->topology_ref_history, port,
-				    "Port");
+	__dump_topology_ref_history(port->mgr->dev, &port->topology_ref_history,
+				    port, "Port");
 }
 
 static __always_inline void
@@ -2824,7 +2826,9 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr,
 	ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
 	if (ret) {
 		if (drm_debug_enabled(DRM_UT_DP)) {
-			struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+			struct drm_printer p = drm_dbg_printer(mgr->dev,
+							       DRM_UT_DP,
+							       DBG_PREFIX);
 
 			drm_printf(&p, "sideband msg failed to send\n");
 			drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
@@ -2869,7 +2873,8 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr,
 	list_add_tail(&txmsg->next, &mgr->tx_msg_downq);
 
 	if (drm_debug_enabled(DRM_UT_DP)) {
-		struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+		struct drm_printer p = drm_dbg_printer(mgr->dev, DRM_UT_DP,
+						       DBG_PREFIX);
 
 		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
 	}
-- 
2.39.2

[PATCH 05/10] drm/mode: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Jani Nikula]

Prefer the device specific debug printer.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_mode_config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 8525ef851540..48fd2d67f352 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -544,7 +544,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	 */
 	WARN_ON(!list_empty(&dev->mode_config.fb_list));
 	list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
-		struct drm_printer p = drm_debug_printer("[leaked fb]");
+		struct drm_printer p = drm_dbg_printer(dev, DRM_UT_KMS, "[leaked fb]");
 
 		drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
 		drm_framebuffer_print_info(&p, 1, fb);
-- 
2.39.2

[PATCH 06/10] drm/dp: switch drm_dp_vsc_sdp_log() to struct drm_printer

[Jani Nikula]

Use the existing drm printer infrastructure instead of local macros.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c       | 17 +++++-------
 .../drm/i915/display/intel_crtc_state_dump.c  |  5 ++--
 drivers/gpu/drm/i915/display/intel_display.c  | 27 +++++++++----------
 include/drm/display/drm_dp_helper.h           |  3 +--
 4 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index d72b6f9a352c..1cf51a748022 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2898,22 +2898,19 @@ static const char *dp_content_type_get_name(enum dp_content_type content_type)
 	}
 }
 
-void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
-			const struct drm_dp_vsc_sdp *vsc)
+void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp *vsc)
 {
-#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
-	DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
+	drm_printf(p, "DP SDP: VSC, revision %u, length %u\n",
 		   vsc->revision, vsc->length);
-	DP_SDP_LOG("    pixelformat: %s\n",
+	drm_printf(p, "    pixelformat: %s\n",
 		   dp_pixelformat_get_name(vsc->pixelformat));
-	DP_SDP_LOG("    colorimetry: %s\n",
+	drm_printf(p, "    colorimetry: %s\n",
 		   dp_colorimetry_get_name(vsc->pixelformat, vsc->colorimetry));
-	DP_SDP_LOG("    bpc: %u\n", vsc->bpc);
-	DP_SDP_LOG("    dynamic range: %s\n",
+	drm_printf(p, "    bpc: %u\n", vsc->bpc);
+	drm_printf(p, "    dynamic range: %s\n",
 		   dp_dynamic_range_get_name(vsc->dynamic_range));
-	DP_SDP_LOG("    content type: %s\n",
+	drm_printf(p, "    content type: %s\n",
 		   dp_content_type_get_name(vsc->content_type));
-#undef DP_SDP_LOG
 }
 EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
 
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 49fd100ec98a..4bcf446c75f4 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -55,10 +55,9 @@ static void
 intel_dump_dp_vsc_sdp(struct drm_i915_private *i915,
 		      const struct drm_dp_vsc_sdp *vsc)
 {
-	if (!drm_debug_enabled(DRM_UT_KMS))
-		return;
+	struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
 
-	drm_dp_vsc_sdp_log(KERN_DEBUG, i915->drm.dev, vsc);
+	drm_dp_vsc_sdp_log(&p, vsc);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a92e959c8ac7..e1573e0a2661 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4813,28 +4813,27 @@ pipe_config_infoframe_mismatch(struct drm_i915_private *dev_priv,
 }
 
 static void
-pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private *dev_priv,
+pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private *i915,
 				bool fastset, const char *name,
 				const struct drm_dp_vsc_sdp *a,
 				const struct drm_dp_vsc_sdp *b)
 {
+	struct drm_printer p;
+
 	if (fastset) {
-		if (!drm_debug_enabled(DRM_UT_KMS))
-			return;
+		p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
 
-		drm_dbg_kms(&dev_priv->drm,
-			    "fastset requirement not met in %s dp sdp\n", name);
-		drm_dbg_kms(&dev_priv->drm, "expected:\n");
-		drm_dp_vsc_sdp_log(KERN_DEBUG, dev_priv->drm.dev, a);
-		drm_dbg_kms(&dev_priv->drm, "found:\n");
-		drm_dp_vsc_sdp_log(KERN_DEBUG, dev_priv->drm.dev, b);
+		drm_printf(&p, "fastset requirement not met in %s dp sdp\n", name);
 	} else {
-		drm_err(&dev_priv->drm, "mismatch in %s dp sdp\n", name);
-		drm_err(&dev_priv->drm, "expected:\n");
-		drm_dp_vsc_sdp_log(KERN_ERR, dev_priv->drm.dev, a);
-		drm_err(&dev_priv->drm, "found:\n");
-		drm_dp_vsc_sdp_log(KERN_ERR, dev_priv->drm.dev, b);
+		p = drm_err_printer(&i915->drm, NULL);
+
+		drm_printf(&p, "mismatch in %s dp sdp\n", name);
 	}
+
+	drm_printf(&p, "expected:\n");
+	drm_dp_vsc_sdp_log(&p, a);
+	drm_printf(&p, "found:\n");
+	drm_dp_vsc_sdp_log(&p, b);
 }
 
 /* Returns the length up to and including the last differing byte */
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 863b2e7add29..d02014a87f12 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -98,8 +98,7 @@ struct drm_dp_vsc_sdp {
 	enum dp_content_type content_type;
 };
 
-void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
-			const struct drm_dp_vsc_sdp *vsc);
+void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp *vsc);
 
 int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
 
-- 
2.39.2

[PATCH 07/10] drm/i915: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Jani Nikula]

Prefer the device specific debug printer.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_driver.c | 3 ++-
 drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c    | 3 ++-
 drivers/gpu/drm/i915/gt/intel_reset.c               | 3 ++-
 drivers/gpu/drm/i915/gt/intel_workarounds.c         | 3 ++-
 drivers/gpu/drm/i915/gt/selftest_context.c          | 3 ++-
 drivers/gpu/drm/i915/i915_driver.c                  | 3 ++-
 6 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index ecf9cb74734b..e733f7c4c6c6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -518,7 +518,8 @@ int intel_display_driver_probe(struct drm_i915_private *i915)
 
 void intel_display_driver_register(struct drm_i915_private *i915)
 {
-	struct drm_printer p = drm_debug_printer("i915 display info:");
+	struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS,
+					       "i915 display info:");
 
 	if (!HAS_DISPLAY(i915))
 		return;
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 1a8e2b7db013..0f6406f0cca0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -96,7 +96,8 @@ static void heartbeat_commit(struct i915_request *rq,
 static void show_heartbeat(const struct i915_request *rq,
 			   struct intel_engine_cs *engine)
 {
-	struct drm_printer p = drm_debug_printer("heartbeat");
+	struct drm_printer p = drm_dbg_printer(&rq->i915->drm, DRM_UT_DRIVER,
+					       "heartbeat");
 
 	if (!rq) {
 		intel_engine_dump(engine, &p,
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 6801f8b95c53..c8e9aa41fdea 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1015,7 +1015,8 @@ void intel_gt_set_wedged(struct intel_gt *gt)
 	mutex_lock(&gt->reset.mutex);
 
 	if (GEM_SHOW_DEBUG()) {
-		struct drm_printer p = drm_debug_printer(__func__);
+		struct drm_printer p = drm_dbg_printer(&gt->i915->drm,
+						       DRM_UT_DRIVER, __func__);
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 270b56fc85e2..e6f48aa967c2 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1236,7 +1236,8 @@ static void __set_mcr_steering(struct i915_wa_list *wal,
 
 static void debug_dump_steering(struct intel_gt *gt)
 {
-	struct drm_printer p = drm_debug_printer("MCR Steering:");
+	struct drm_printer p = drm_dbg_printer(&gt->i915->drm, DRM_UT_DRIVER,
+					       "MCR Steering:");
 
 	if (drm_debug_enabled(DRM_UT_DRIVER))
 		intel_gt_mcr_report_steering(&p, gt, false);
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index 47070cba7eb1..12eca750f7d0 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -285,7 +285,8 @@ static int __live_active_context(struct intel_engine_cs *engine)
 	intel_engine_pm_flush(engine);
 
 	if (intel_engine_pm_is_awake(engine)) {
-		struct drm_printer p = drm_debug_printer(__func__);
+		struct drm_printer p = drm_dbg_printer(&engine->i915->drm,
+						       DRM_UT_DRIVER, __func__);
 
 		intel_engine_dump(engine, &p,
 				  "%s is still awake:%d after idle-barriers\n",
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index a951050f6a75..9ee902d5b72c 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -681,7 +681,8 @@ i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
 static void i915_welcome_messages(struct drm_i915_private *dev_priv)
 {
 	if (drm_debug_enabled(DRM_UT_DRIVER)) {
-		struct drm_printer p = drm_debug_printer("i915 device info:");
+		struct drm_printer p = drm_dbg_printer(&dev_priv->drm, DRM_UT_DRIVER,
+						       "device info:");
 		struct intel_gt *gt;
 		unsigned int i;
 
-- 
2.39.2

[PATCH 08/10] drm/i915: use drm_printf() with the drm_err_printer intead of pr_err()

[Jani Nikula]

There's already a related drm_printer. Use it to preserve the context
instead of a separate pr_err().

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c | 6 +++---
 drivers/gpu/drm/i915/selftests/i915_active.c        | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
index be827318275c..ef014df4c4fc 100644
--- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
@@ -124,7 +124,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
 	if (engine_sync_barrier(engine)) {
 		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
 
-		pr_err("%s: no heartbeat pulse?\n", engine->name);
+		drm_printf(&m, "%s: no heartbeat pulse?\n", engine->name);
 		intel_engine_dump(engine, &m, "%s", engine->name);
 
 		err = -ETIME;
@@ -138,8 +138,8 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
 	if (!i915_active_is_idle(&p->active)) {
 		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
 
-		pr_err("%s: heartbeat pulse did not flush idle tasks\n",
-		       engine->name);
+		drm_printf(&m, "%s: heartbeat pulse did not flush idle tasks\n",
+			   engine->name);
 		i915_active_print(&p->active, &m);
 
 		err = -EINVAL;
diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
index 8886752ade63..0d89d70b9c36 100644
--- a/drivers/gpu/drm/i915/selftests/i915_active.c
+++ b/drivers/gpu/drm/i915/selftests/i915_active.c
@@ -158,7 +158,7 @@ static int live_active_wait(void *arg)
 	if (!READ_ONCE(active->retired)) {
 		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
-		pr_err("i915_active not retired after waiting!\n");
+		drm_printf(&p, "i915_active not retired after waiting!\n");
 		i915_active_print(&active->base, &p);
 
 		err = -EINVAL;
@@ -191,7 +191,7 @@ static int live_active_retire(void *arg)
 	if (!READ_ONCE(active->retired)) {
 		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
 
-		pr_err("i915_active not retired after flushing!\n");
+		drm_printf(&p, "i915_active not retired after flushing!\n");
 		i915_active_print(&active->base, &p);
 
 		err = -EINVAL;
-- 
2.39.2

[PATCH 09/10] drm/xe: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Jani Nikula]

Prefer the device specific debug printer.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/xe/xe_gt.c          | 2 +-
 drivers/gpu/drm/xe/xe_gt_topology.c | 4 +++-
 drivers/gpu/drm/xe/xe_reg_sr.c      | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index 0f2258dc4a00..16481f9b3125 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -327,7 +327,7 @@ static void dump_pat_on_error(struct xe_gt *gt)
 	char prefix[32];
 
 	snprintf(prefix, sizeof(prefix), "[GT%u Error]", gt->info.id);
-	p = drm_debug_printer(prefix);
+	p = drm_dbg_printer(&gt_to_xe(gt)->drm, DRM_UT_DRIVER, prefix);
 
 	xe_pat_dump(gt, &p);
 }
diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
index a8d7f272c30a..5dc62fe1be49 100644
--- a/drivers/gpu/drm/xe/xe_gt_topology.c
+++ b/drivers/gpu/drm/xe/xe_gt_topology.c
@@ -84,7 +84,7 @@ void
 xe_gt_topology_init(struct xe_gt *gt)
 {
 	struct xe_device *xe = gt_to_xe(gt);
-	struct drm_printer p = drm_debug_printer("GT topology");
+	struct drm_printer p;
 	int num_geometry_regs, num_compute_regs;
 
 	get_num_dss_regs(xe, &num_geometry_regs, &num_compute_regs);
@@ -107,6 +107,8 @@ xe_gt_topology_init(struct xe_gt *gt)
 		      XE2_GT_COMPUTE_DSS_2);
 	load_eu_mask(gt, gt->fuse_topo.eu_mask_per_dss);
 
+	p = drm_dbg_printer(&gt_to_xe(gt)->drm, DRM_UT_DRIVER, "GT topology");
+
 	xe_gt_topology_dump(gt, &p);
 }
 
diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
index 87adefb56024..440ac572f6e5 100644
--- a/drivers/gpu/drm/xe/xe_reg_sr.c
+++ b/drivers/gpu/drm/xe/xe_reg_sr.c
@@ -231,7 +231,7 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
 	if (err)
 		goto err_force_wake;
 
-	p = drm_debug_printer(KBUILD_MODNAME);
+	p = drm_dbg_printer(&xe->drm, DRM_UT_DRIVER, NULL);
 	xa_for_each(&sr->xa, reg, entry) {
 		if (slot == RING_MAX_NONPRIV_SLOTS) {
 			xe_gt_err(gt,
-- 
2.39.2

[PATCH 10/10] drm: remove drm_debug_printer in favor of drm_dbg_printer

[Jani Nikula]

Convert the remaining drm_debug_printer users over to drm_dbg_printer,
as it can handle the cases without struct drm_device pointer, and also
provides drm debug category and prefix support. Remove drm_debug_printer
altogether.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_modeset_lock.c |  2 +-
 drivers/gpu/drm/drm_print.c        |  7 -------
 drivers/gpu/drm/ttm/ttm_bo.c       |  2 +-
 include/drm/drm_print.h            | 17 -----------------
 4 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 918065982db4..7694b85e75e3 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -91,7 +91,7 @@ static noinline depot_stack_handle_t __drm_stack_depot_save(void)
 
 static void __drm_stack_depot_print(depot_stack_handle_t stack_depot)
 {
-	struct drm_printer p = drm_debug_printer("drm_modeset_lock");
+	struct drm_printer p = drm_dbg_printer(NULL, DRM_UT_KMS, "drm_modeset_lock");
 	unsigned long *entries;
 	unsigned int nr_entries;
 	char *buf;
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 673b29c732ea..699b7dbffd7b 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -182,13 +182,6 @@ void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf)
 }
 EXPORT_SYMBOL(__drm_printfn_info);
 
-void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf)
-{
-	/* pr_debug callsite decorations are unhelpful here */
-	printk(KERN_DEBUG "%s %pV", p->prefix, vaf);
-}
-EXPORT_SYMBOL(__drm_printfn_debug);
-
 void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf)
 {
 	const struct drm_device *drm = p->arg;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index edf10618fe2b..30e1fe864826 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -49,7 +49,7 @@
 static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
 					struct ttm_placement *placement)
 {
-	struct drm_printer p = drm_debug_printer(TTM_PFX);
+	struct drm_printer p = drm_dbg_printer(NULL, DRM_UT_CORE, TTM_PFX);
 	struct ttm_resource_manager *man;
 	int i, mem_type;
 
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 27e23c06dad4..9cc473e5d353 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -184,7 +184,6 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str);
 void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf);
 void __drm_puts_seq_file(struct drm_printer *p, const char *str);
 void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
-void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
 
@@ -317,22 +316,6 @@ static inline struct drm_printer drm_info_printer(struct device *dev)
 	return p;
 }
 
-/**
- * drm_debug_printer - construct a &drm_printer that outputs to pr_debug()
- * @prefix: debug output prefix
- *
- * RETURNS:
- * The &drm_printer object
- */
-static inline struct drm_printer drm_debug_printer(const char *prefix)
-{
-	struct drm_printer p = {
-		.printfn = __drm_printfn_debug,
-		.prefix = prefix
-	};
-	return p;
-}
-
 /**
  * drm_dbg_printer - construct a &drm_printer for drm device specific output
  * @drm: the &struct drm_device pointer, or NULL
-- 
2.39.2

Re: [PATCH 01/10] drm/print: make drm_err_printer() device specific by using drm_err()

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> With few users for drm_err_printer(), it's still feasible to convert it
> to be device specific. Use drm_err() under the hood.
> 
> While at it, make the prefix optional.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_print.c                         |  7 ++++++-
>  drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c |  4 ++--
>  drivers/gpu/drm/i915/selftests/i915_active.c        |  4 ++--
>  include/drm/drm_print.h                             | 11 ++++++++---
>  4 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 5b93c11895bb..91dbcdeaad3f 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -191,7 +191,12 @@ EXPORT_SYMBOL(__drm_printfn_debug);
>  
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
>  {
> -	pr_err("*ERROR* %s %pV", p->prefix, vaf);
> +	struct drm_device *drm = p->arg;
> +
> +	if (p->prefix)
> +		drm_err(drm, "%s %pV", p->prefix, vaf);
> +	else
> +		drm_err(drm, "%pV", vaf);
>  }
>  EXPORT_SYMBOL(__drm_printfn_err);
>  
> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> index bc441ce7b380..be827318275c 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> @@ -122,7 +122,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
>  	GEM_BUG_ON(!llist_empty(&engine->barrier_tasks));
>  
>  	if (engine_sync_barrier(engine)) {
> -		struct drm_printer m = drm_err_printer("pulse");
> +		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
>  
>  		pr_err("%s: no heartbeat pulse?\n", engine->name);
>  		intel_engine_dump(engine, &m, "%s", engine->name);
> @@ -136,7 +136,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
>  	pulse_unlock_wait(p); /* synchronize with the retirement callback */
>  
>  	if (!i915_active_is_idle(&p->active)) {
> -		struct drm_printer m = drm_err_printer("pulse");
> +		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
>  
>  		pr_err("%s: heartbeat pulse did not flush idle tasks\n",
>  		       engine->name);
> diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
> index b61fe850e924..8886752ade63 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_active.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_active.c
> @@ -156,7 +156,7 @@ static int live_active_wait(void *arg)
>  
>  	__i915_active_wait(&active->base, TASK_UNINTERRUPTIBLE);
>  	if (!READ_ONCE(active->retired)) {
> -		struct drm_printer p = drm_err_printer(__func__);
> +		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
>  
>  		pr_err("i915_active not retired after waiting!\n");
>  		i915_active_print(&active->base, &p);
> @@ -189,7 +189,7 @@ static int live_active_retire(void *arg)
>  		err = -EIO;
>  
>  	if (!READ_ONCE(active->retired)) {
> -		struct drm_printer p = drm_err_printer(__func__);
> +		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
>  
>  		pr_err("i915_active not retired after flushing!\n");
>  		i915_active_print(&active->base, &p);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 5ed26a702e3e..1040213d02a4 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -35,6 +35,8 @@
>  
>  #include <drm/drm.h>
>  
> +struct drm_device;
> +
>  /* Do *not* use outside of drm_print.[ch]! */
>  extern unsigned long __drm_debug;
>  
> @@ -235,16 +237,19 @@ static inline struct drm_printer drm_debug_printer(const char *prefix)
>  }
>  
>  /**
> - * drm_err_printer - construct a &drm_printer that outputs to pr_err()
> - * @prefix: debug output prefix
> + * drm_err_printer - construct a &drm_printer that outputs to drm_err()
> + * @drm: the &struct drm_device pointer
> + * @prefix: debug output prefix, or NULL for no prefix
>   *
>   * RETURNS:
>   * The &drm_printer object
>   */
> -static inline struct drm_printer drm_err_printer(const char *prefix)
> +static inline struct drm_printer drm_err_printer(struct drm_device *drm,
> +						 const char *prefix)
>  {
>  	struct drm_printer p = {
>  		.printfn = __drm_printfn_err,
> +		.arg = drm,
>  		.prefix = prefix
>  	};
>  	return p;

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 02/10] drm/print: move enum drm_debug_category etc. earlier in drm_print.h

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> Avoid forward declarations in subsequent changes, but separate this
> movement to an independent change.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  include/drm/drm_print.h | 190 ++++++++++++++++++++--------------------
>  1 file changed, 95 insertions(+), 95 deletions(-)
> 
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 1040213d02a4..b8b4cb9bb878 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -69,6 +69,101 @@ extern unsigned long __drm_debug;
>   *     }
>   */
>  
> +/**
> + * enum drm_debug_category - The DRM debug categories
> + *
> + * 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.
> + *
> + * 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:
> + *
> + *  - 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::
> + *
> + *   # echo 0xf > /sys/module/drm/parameters/debug
> + *
> + */
> +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, ...
> +	 */
> +	DRM_UT_CORE,
> +	/**
> +	 * @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
> +	 * radeon, ... macro.
> +	 */
> +	DRM_UT_DRIVER,
> +	/**
> +	 * @DRM_UT_KMS: Used in the modesetting code.
> +	 */
> +	DRM_UT_KMS,
> +	/**
> +	 * @DRM_UT_PRIME: Used in the prime code.
> +	 */
> +	DRM_UT_PRIME,
> +	/**
> +	 * @DRM_UT_ATOMIC: Used in the atomic code.
> +	 */
> +	DRM_UT_ATOMIC,
> +	/**
> +	 * @DRM_UT_VBL: Used for verbose debug message in the vblank code.
> +	 */
> +	DRM_UT_VBL,
> +	/**
> +	 * @DRM_UT_STATE: Used for verbose atomic state debugging.
> +	 */
> +	DRM_UT_STATE,
> +	/**
> +	 * @DRM_UT_LEASE: Used in the lease code.
> +	 */
> +	DRM_UT_LEASE,
> +	/**
> +	 * @DRM_UT_DP: Used in the DP code.
> +	 */
> +	DRM_UT_DP,
> +	/**
> +	 * @DRM_UT_DRMRES: Used in the drm managed resources code.
> +	 */
> +	DRM_UT_DRMRES
> +};
> +
> +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 drm_printer - drm output "stream"
>   *
> @@ -255,101 +350,6 @@ static inline struct drm_printer drm_err_printer(struct drm_device *drm,
>  	return p;
>  }
>  
> -/**
> - * enum drm_debug_category - The DRM debug categories
> - *
> - * 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.
> - *
> - * 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:
> - *
> - *  - 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::
> - *
> - *   # echo 0xf > /sys/module/drm/parameters/debug
> - *
> - */
> -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, ...
> -	 */
> -	DRM_UT_CORE,
> -	/**
> -	 * @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
> -	 * radeon, ... macro.
> -	 */
> -	DRM_UT_DRIVER,
> -	/**
> -	 * @DRM_UT_KMS: Used in the modesetting code.
> -	 */
> -	DRM_UT_KMS,
> -	/**
> -	 * @DRM_UT_PRIME: Used in the prime code.
> -	 */
> -	DRM_UT_PRIME,
> -	/**
> -	 * @DRM_UT_ATOMIC: Used in the atomic code.
> -	 */
> -	DRM_UT_ATOMIC,
> -	/**
> -	 * @DRM_UT_VBL: Used for verbose debug message in the vblank code.
> -	 */
> -	DRM_UT_VBL,
> -	/**
> -	 * @DRM_UT_STATE: Used for verbose atomic state debugging.
> -	 */
> -	DRM_UT_STATE,
> -	/**
> -	 * @DRM_UT_LEASE: Used in the lease code.
> -	 */
> -	DRM_UT_LEASE,
> -	/**
> -	 * @DRM_UT_DP: Used in the DP code.
> -	 */
> -	DRM_UT_DP,
> -	/**
> -	 * @DRM_UT_DRMRES: Used in the drm managed resources code.
> -	 */
> -	DRM_UT_DRMRES
> -};
> -
> -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
>   *

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 03/10] drm/print: add drm_dbg_printer() for drm device specific printer

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> We've lacked a device specific debug printer. Add one. Take category
> into account too.
> 
> __builtin_return_address(0) is inaccurate here, so don't use it. If
> necessary, we can later pass __func__ to drm_dbg_printer() by wrapping
> it inside a macro.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_print.c | 21 +++++++++++++++++++++
>  include/drm/drm_print.h     | 24 ++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 91dbcdeaad3f..673b29c732ea 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -189,6 +189,27 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf)
>  }
>  EXPORT_SYMBOL(__drm_printfn_debug);
>  
> +void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf)
> +{
> +	const struct drm_device *drm = p->arg;
> +	const struct device *dev = drm ? drm->dev : NULL;
> +	enum drm_debug_category category = p->category;
> +	const char *prefix = p->prefix ?: "";
> +	const char *prefix_pad = p->prefix ? " " : "";
> +
> +	if (!__drm_debug_enabled(category))
> +		return;
> +
> +	/* Note: __builtin_return_address(0) is useless here. */
> +	if (dev)
> +		dev_printk(KERN_DEBUG, dev, "[" DRM_NAME "]%s%s %pV",
> +			   prefix_pad, prefix, vaf);
> +	else
> +		printk(KERN_DEBUG "[" DRM_NAME "]%s%s %pV",
> +		       prefix_pad, prefix, vaf);
> +}
> +EXPORT_SYMBOL(__drm_printfn_dbg);
> +
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
>  {
>  	struct drm_device *drm = p->arg;
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index b8b4cb9bb878..27e23c06dad4 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -176,6 +176,7 @@ struct drm_printer {
>  	void (*puts)(struct drm_printer *p, const char *str);
>  	void *arg;
>  	const char *prefix;
> +	enum drm_debug_category category;
>  };
>  
>  void __drm_printfn_coredump(struct drm_printer *p, struct va_format *vaf);
> @@ -184,6 +185,7 @@ void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf);
>  void __drm_puts_seq_file(struct drm_printer *p, const char *str);
>  void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
> +void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf);

I was wondering why you had both _debug() and _dbg() functions here,
but I see your goal is to remove _debug() at the end (done in the last
patch in the series).


>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
>  
>  __printf(2, 3)
> @@ -331,6 +333,28 @@ static inline struct drm_printer drm_debug_printer(const char *prefix)
>  	return p;
>  }
>  
> +/**
> + * drm_dbg_printer - construct a &drm_printer for drm device specific output
> + * @drm: the &struct drm_device pointer, or NULL
> + * @category: the debug category to use
> + * @prefix: debug output prefix, or NULL for no prefix
> + *
> + * RETURNS:
> + * The &drm_printer object
> + */
> +static inline struct drm_printer drm_dbg_printer(struct drm_device *drm,
> +						 enum drm_debug_category category,
> +						 const char *prefix)
> +{
> +	struct drm_printer p = {
> +		.printfn = __drm_printfn_dbg,
> +		.arg = drm,
> +		.prefix = prefix,
> +		.category = category,
> +	};
> +	return p;
> +}
> +
>  /**
>   * drm_err_printer - construct a &drm_printer that outputs to drm_err()
>   * @drm: the &struct drm_device pointer

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 04/10] drm/dp_mst: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> Prefer the device specific debug printer.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 23 +++++++++++--------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 8ca01a6bf645..fba6e37b051b 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -1306,7 +1306,8 @@ static int drm_dp_mst_wait_tx_reply(struct drm_dp_mst_branch *mstb,
>  	}
>  out:
>  	if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) {
> -		struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> +		struct drm_printer p = drm_dbg_printer(mgr->dev, DRM_UT_DP,
> +						       DBG_PREFIX);
>  
>  		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
>  	}
> @@ -1593,10 +1594,11 @@ topology_ref_type_to_str(enum drm_dp_mst_topology_ref_type type)
>  }
>  
>  static void
> -__dump_topology_ref_history(struct drm_dp_mst_topology_ref_history *history,
> +__dump_topology_ref_history(struct drm_device *drm,
> +			    struct drm_dp_mst_topology_ref_history *history,
>  			    void *ptr, const char *type_str)
>  {
> -	struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> +	struct drm_printer p = drm_dbg_printer(drm, DRM_UT_DP, DBG_PREFIX);
>  	char *buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>  	int i;
>  
> @@ -1638,15 +1640,15 @@ __dump_topology_ref_history(struct drm_dp_mst_topology_ref_history *history,
>  static __always_inline void
>  drm_dp_mst_dump_mstb_topology_history(struct drm_dp_mst_branch *mstb)
>  {
> -	__dump_topology_ref_history(&mstb->topology_ref_history, mstb,
> -				    "MSTB");
> +	__dump_topology_ref_history(mstb->mgr->dev, &mstb->topology_ref_history,
> +				    mstb, "MSTB");
>  }
>  
>  static __always_inline void
>  drm_dp_mst_dump_port_topology_history(struct drm_dp_mst_port *port)
>  {
> -	__dump_topology_ref_history(&port->topology_ref_history, port,
> -				    "Port");
> +	__dump_topology_ref_history(port->mgr->dev, &port->topology_ref_history,
> +				    port, "Port");
>  }
>  
>  static __always_inline void
> @@ -2824,7 +2826,9 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr,
>  	ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
>  	if (ret) {
>  		if (drm_debug_enabled(DRM_UT_DP)) {
> -			struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> +			struct drm_printer p = drm_dbg_printer(mgr->dev,
> +							       DRM_UT_DP,
> +							       DBG_PREFIX);
>  
>  			drm_printf(&p, "sideband msg failed to send\n");
>  			drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> @@ -2869,7 +2873,8 @@ static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr,
>  	list_add_tail(&txmsg->next, &mgr->tx_msg_downq);
>  
>  	if (drm_debug_enabled(DRM_UT_DP)) {
> -		struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> +		struct drm_printer p = drm_dbg_printer(mgr->dev, DRM_UT_DP,
> +						       DBG_PREFIX);
>  
>  		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
>  	}

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 05/10] drm/mode: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> Prefer the device specific debug printer.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_mode_config.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 8525ef851540..48fd2d67f352 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -544,7 +544,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
>  	 */
>  	WARN_ON(!list_empty(&dev->mode_config.fb_list));
>  	list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
> -		struct drm_printer p = drm_debug_printer("[leaked fb]");
> +		struct drm_printer p = drm_dbg_printer(dev, DRM_UT_KMS, "[leaked fb]");
>  
>  		drm_printf(&p, "framebuffer[%u]:\n", fb->base.id);
>  		drm_framebuffer_print_info(&p, 1, fb);

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 06/10] drm/dp: switch drm_dp_vsc_sdp_log() to struct drm_printer

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> Use the existing drm printer infrastructure instead of local macros.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c       | 17 +++++-------
>  .../drm/i915/display/intel_crtc_state_dump.c  |  5 ++--
>  drivers/gpu/drm/i915/display/intel_display.c  | 27 +++++++++----------
>  include/drm/display/drm_dp_helper.h           |  3 +--
>  4 files changed, 23 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index d72b6f9a352c..1cf51a748022 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2898,22 +2898,19 @@ static const char *dp_content_type_get_name(enum dp_content_type content_type)
>  	}
>  }
>  
> -void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
> -			const struct drm_dp_vsc_sdp *vsc)
> +void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp *vsc)
>  {
> -#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
> -	DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
> +	drm_printf(p, "DP SDP: VSC, revision %u, length %u\n",
>  		   vsc->revision, vsc->length);
> -	DP_SDP_LOG("    pixelformat: %s\n",
> +	drm_printf(p, "    pixelformat: %s\n",
>  		   dp_pixelformat_get_name(vsc->pixelformat));
> -	DP_SDP_LOG("    colorimetry: %s\n",
> +	drm_printf(p, "    colorimetry: %s\n",
>  		   dp_colorimetry_get_name(vsc->pixelformat, vsc->colorimetry));
> -	DP_SDP_LOG("    bpc: %u\n", vsc->bpc);
> -	DP_SDP_LOG("    dynamic range: %s\n",
> +	drm_printf(p, "    bpc: %u\n", vsc->bpc);
> +	drm_printf(p, "    dynamic range: %s\n",
>  		   dp_dynamic_range_get_name(vsc->dynamic_range));
> -	DP_SDP_LOG("    content type: %s\n",
> +	drm_printf(p, "    content type: %s\n",
>  		   dp_content_type_get_name(vsc->content_type));
> -#undef DP_SDP_LOG
>  }
>  EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index 49fd100ec98a..4bcf446c75f4 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -55,10 +55,9 @@ static void
>  intel_dump_dp_vsc_sdp(struct drm_i915_private *i915,
>  		      const struct drm_dp_vsc_sdp *vsc)
>  {
> -	if (!drm_debug_enabled(DRM_UT_KMS))
> -		return;
> +	struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
>  
> -	drm_dp_vsc_sdp_log(KERN_DEBUG, i915->drm.dev, vsc);
> +	drm_dp_vsc_sdp_log(&p, vsc);
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a92e959c8ac7..e1573e0a2661 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4813,28 +4813,27 @@ pipe_config_infoframe_mismatch(struct drm_i915_private *dev_priv,
>  }
>  
>  static void
> -pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private *dev_priv,
> +pipe_config_dp_vsc_sdp_mismatch(struct drm_i915_private *i915,
>  				bool fastset, const char *name,
>  				const struct drm_dp_vsc_sdp *a,
>  				const struct drm_dp_vsc_sdp *b)
>  {
> +	struct drm_printer p;
> +
>  	if (fastset) {
> -		if (!drm_debug_enabled(DRM_UT_KMS))
> -			return;
> +		p = drm_dbg_printer(&i915->drm, DRM_UT_KMS, NULL);
>  
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "fastset requirement not met in %s dp sdp\n", name);
> -		drm_dbg_kms(&dev_priv->drm, "expected:\n");
> -		drm_dp_vsc_sdp_log(KERN_DEBUG, dev_priv->drm.dev, a);
> -		drm_dbg_kms(&dev_priv->drm, "found:\n");
> -		drm_dp_vsc_sdp_log(KERN_DEBUG, dev_priv->drm.dev, b);
> +		drm_printf(&p, "fastset requirement not met in %s dp sdp\n", name);
>  	} else {
> -		drm_err(&dev_priv->drm, "mismatch in %s dp sdp\n", name);
> -		drm_err(&dev_priv->drm, "expected:\n");
> -		drm_dp_vsc_sdp_log(KERN_ERR, dev_priv->drm.dev, a);
> -		drm_err(&dev_priv->drm, "found:\n");
> -		drm_dp_vsc_sdp_log(KERN_ERR, dev_priv->drm.dev, b);
> +		p = drm_err_printer(&i915->drm, NULL);
> +
> +		drm_printf(&p, "mismatch in %s dp sdp\n", name);
>  	}
> +
> +	drm_printf(&p, "expected:\n");
> +	drm_dp_vsc_sdp_log(&p, a);
> +	drm_printf(&p, "found:\n");
> +	drm_dp_vsc_sdp_log(&p, b);
>  }
>  
>  /* Returns the length up to and including the last differing byte */
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index 863b2e7add29..d02014a87f12 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -98,8 +98,7 @@ struct drm_dp_vsc_sdp {
>  	enum dp_content_type content_type;
>  };
>  
> -void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
> -			const struct drm_dp_vsc_sdp *vsc);
> +void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp *vsc);
>  
>  int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
>  

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 07/10] drm/i915: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> Prefer the device specific debug printer.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_driver.c | 3 ++-
>  drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c    | 3 ++-
>  drivers/gpu/drm/i915/gt/intel_reset.c               | 3 ++-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c         | 3 ++-
>  drivers/gpu/drm/i915/gt/selftest_context.c          | 3 ++-
>  drivers/gpu/drm/i915/i915_driver.c                  | 3 ++-
>  6 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index ecf9cb74734b..e733f7c4c6c6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -518,7 +518,8 @@ int intel_display_driver_probe(struct drm_i915_private *i915)
>  
>  void intel_display_driver_register(struct drm_i915_private *i915)
>  {
> -	struct drm_printer p = drm_debug_printer("i915 display info:");
> +	struct drm_printer p = drm_dbg_printer(&i915->drm, DRM_UT_KMS,
> +					       "i915 display info:");
>  
>  	if (!HAS_DISPLAY(i915))
>  		return;
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> index 1a8e2b7db013..0f6406f0cca0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
> @@ -96,7 +96,8 @@ static void heartbeat_commit(struct i915_request *rq,
>  static void show_heartbeat(const struct i915_request *rq,
>  			   struct intel_engine_cs *engine)
>  {
> -	struct drm_printer p = drm_debug_printer("heartbeat");
> +	struct drm_printer p = drm_dbg_printer(&rq->i915->drm, DRM_UT_DRIVER,
> +					       "heartbeat");
>  
>  	if (!rq) {
>  		intel_engine_dump(engine, &p,
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index 6801f8b95c53..c8e9aa41fdea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -1015,7 +1015,8 @@ void intel_gt_set_wedged(struct intel_gt *gt)
>  	mutex_lock(&gt->reset.mutex);
>  
>  	if (GEM_SHOW_DEBUG()) {
> -		struct drm_printer p = drm_debug_printer(__func__);
> +		struct drm_printer p = drm_dbg_printer(&gt->i915->drm,
> +						       DRM_UT_DRIVER, __func__);
>  		struct intel_engine_cs *engine;
>  		enum intel_engine_id id;
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 270b56fc85e2..e6f48aa967c2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1236,7 +1236,8 @@ static void __set_mcr_steering(struct i915_wa_list *wal,
>  
>  static void debug_dump_steering(struct intel_gt *gt)
>  {
> -	struct drm_printer p = drm_debug_printer("MCR Steering:");
> +	struct drm_printer p = drm_dbg_printer(&gt->i915->drm, DRM_UT_DRIVER,
> +					       "MCR Steering:");
>  
>  	if (drm_debug_enabled(DRM_UT_DRIVER))
>  		intel_gt_mcr_report_steering(&p, gt, false);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> index 47070cba7eb1..12eca750f7d0 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_context.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> @@ -285,7 +285,8 @@ static int __live_active_context(struct intel_engine_cs *engine)
>  	intel_engine_pm_flush(engine);
>  
>  	if (intel_engine_pm_is_awake(engine)) {
> -		struct drm_printer p = drm_debug_printer(__func__);
> +		struct drm_printer p = drm_dbg_printer(&engine->i915->drm,
> +						       DRM_UT_DRIVER, __func__);
>  
>  		intel_engine_dump(engine, &p,
>  				  "%s is still awake:%d after idle-barriers\n",
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index a951050f6a75..9ee902d5b72c 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -681,7 +681,8 @@ i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
>  static void i915_welcome_messages(struct drm_i915_private *dev_priv)
>  {
>  	if (drm_debug_enabled(DRM_UT_DRIVER)) {
> -		struct drm_printer p = drm_debug_printer("i915 device info:");
> +		struct drm_printer p = drm_dbg_printer(&dev_priv->drm, DRM_UT_DRIVER,
> +						       "device info:");
>  		struct intel_gt *gt;
>  		unsigned int i;
>  

Okay, this are all pretty much the same pattern, so I should have given
a single r-b for the whole series, but now I have only a few left,
so...

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 08/10] drm/i915: use drm_printf() with the drm_err_printer intead of pr_err()

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> There's already a related drm_printer. Use it to preserve the context
> instead of a separate pr_err().
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c | 6 +++---
>  drivers/gpu/drm/i915/selftests/i915_active.c        | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> index be827318275c..ef014df4c4fc 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_engine_heartbeat.c
> @@ -124,7 +124,7 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
>  	if (engine_sync_barrier(engine)) {
>  		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
>  
> -		pr_err("%s: no heartbeat pulse?\n", engine->name);
> +		drm_printf(&m, "%s: no heartbeat pulse?\n", engine->name);
>  		intel_engine_dump(engine, &m, "%s", engine->name);
>  
>  		err = -ETIME;
> @@ -138,8 +138,8 @@ static int __live_idle_pulse(struct intel_engine_cs *engine,
>  	if (!i915_active_is_idle(&p->active)) {
>  		struct drm_printer m = drm_err_printer(&engine->i915->drm, "pulse");
>  
> -		pr_err("%s: heartbeat pulse did not flush idle tasks\n",
> -		       engine->name);
> +		drm_printf(&m, "%s: heartbeat pulse did not flush idle tasks\n",
> +			   engine->name);
>  		i915_active_print(&p->active, &m);
>  
>  		err = -EINVAL;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c b/drivers/gpu/drm/i915/selftests/i915_active.c
> index 8886752ade63..0d89d70b9c36 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_active.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_active.c
> @@ -158,7 +158,7 @@ static int live_active_wait(void *arg)
>  	if (!READ_ONCE(active->retired)) {
>  		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
>  
> -		pr_err("i915_active not retired after waiting!\n");
> +		drm_printf(&p, "i915_active not retired after waiting!\n");
>  		i915_active_print(&active->base, &p);
>  
>  		err = -EINVAL;
> @@ -191,7 +191,7 @@ static int live_active_retire(void *arg)
>  	if (!READ_ONCE(active->retired)) {
>  		struct drm_printer p = drm_err_printer(&i915->drm, __func__);
>  
> -		pr_err("i915_active not retired after flushing!\n");
> +		drm_printf(&p, "i915_active not retired after flushing!\n");
>  		i915_active_print(&active->base, &p);
>  
>  		err = -EINVAL;

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 09/10] drm/xe: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> Prefer the device specific debug printer.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt.c          | 2 +-
>  drivers/gpu/drm/xe/xe_gt_topology.c | 4 +++-
>  drivers/gpu/drm/xe/xe_reg_sr.c      | 2 +-
>  3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 0f2258dc4a00..16481f9b3125 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -327,7 +327,7 @@ static void dump_pat_on_error(struct xe_gt *gt)
>  	char prefix[32];
>  
>  	snprintf(prefix, sizeof(prefix), "[GT%u Error]", gt->info.id);
> -	p = drm_debug_printer(prefix);
> +	p = drm_dbg_printer(&gt_to_xe(gt)->drm, DRM_UT_DRIVER, prefix);
>  
>  	xe_pat_dump(gt, &p);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
> index a8d7f272c30a..5dc62fe1be49 100644
> --- a/drivers/gpu/drm/xe/xe_gt_topology.c
> +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
> @@ -84,7 +84,7 @@ void
>  xe_gt_topology_init(struct xe_gt *gt)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
> -	struct drm_printer p = drm_debug_printer("GT topology");
> +	struct drm_printer p;
>  	int num_geometry_regs, num_compute_regs;
>  
>  	get_num_dss_regs(xe, &num_geometry_regs, &num_compute_regs);
> @@ -107,6 +107,8 @@ xe_gt_topology_init(struct xe_gt *gt)
>  		      XE2_GT_COMPUTE_DSS_2);
>  	load_eu_mask(gt, gt->fuse_topo.eu_mask_per_dss);
>  
> +	p = drm_dbg_printer(&gt_to_xe(gt)->drm, DRM_UT_DRIVER, "GT topology");
> +

I'm not sure there was a specific reason to move the initialization out
of the declaration, but it's probably a bit cleaner and more aligned
with other places...


>  	xe_gt_topology_dump(gt, &p);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
> index 87adefb56024..440ac572f6e5 100644
> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
> @@ -231,7 +231,7 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
>  	if (err)
>  		goto err_force_wake;
>  
> -	p = drm_debug_printer(KBUILD_MODNAME);
> +	p = drm_dbg_printer(&xe->drm, DRM_UT_DRIVER, NULL);
>  	xa_for_each(&sr->xa, reg, entry) {
>  		if (slot == RING_MAX_NONPRIV_SLOTS) {
>  			xe_gt_err(gt,


Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 10/10] drm: remove drm_debug_printer in favor of drm_dbg_printer

[Luca Coelho]

On Tue, 2024-01-16 at 15:07 +0200, Jani Nikula wrote:
> Convert the remaining drm_debug_printer users over to drm_dbg_printer,
> as it can handle the cases without struct drm_device pointer, and also
> provides drm debug category and prefix support. Remove drm_debug_printer
> altogether.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_modeset_lock.c |  2 +-
>  drivers/gpu/drm/drm_print.c        |  7 -------
>  drivers/gpu/drm/ttm/ttm_bo.c       |  2 +-
>  include/drm/drm_print.h            | 17 -----------------
>  4 files changed, 2 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
> index 918065982db4..7694b85e75e3 100644
> --- a/drivers/gpu/drm/drm_modeset_lock.c
> +++ b/drivers/gpu/drm/drm_modeset_lock.c
> @@ -91,7 +91,7 @@ static noinline depot_stack_handle_t __drm_stack_depot_save(void)
>  
>  static void __drm_stack_depot_print(depot_stack_handle_t stack_depot)
>  {
> -	struct drm_printer p = drm_debug_printer("drm_modeset_lock");
> +	struct drm_printer p = drm_dbg_printer(NULL, DRM_UT_KMS, "drm_modeset_lock");
>  	unsigned long *entries;
>  	unsigned int nr_entries;
>  	char *buf;
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 673b29c732ea..699b7dbffd7b 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -182,13 +182,6 @@ void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf)
>  }
>  EXPORT_SYMBOL(__drm_printfn_info);
>  
> -void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf)
> -{
> -	/* pr_debug callsite decorations are unhelpful here */
> -	printk(KERN_DEBUG "%s %pV", p->prefix, vaf);
> -}
> -EXPORT_SYMBOL(__drm_printfn_debug);
> -
>  void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf)
>  {
>  	const struct drm_device *drm = p->arg;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index edf10618fe2b..30e1fe864826 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -49,7 +49,7 @@
>  static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
>  					struct ttm_placement *placement)
>  {
> -	struct drm_printer p = drm_debug_printer(TTM_PFX);
> +	struct drm_printer p = drm_dbg_printer(NULL, DRM_UT_CORE, TTM_PFX);
>  	struct ttm_resource_manager *man;
>  	int i, mem_type;
>  
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 27e23c06dad4..9cc473e5d353 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -184,7 +184,6 @@ void __drm_puts_coredump(struct drm_printer *p, const char *str);
>  void __drm_printfn_seq_file(struct drm_printer *p, struct va_format *vaf);
>  void __drm_puts_seq_file(struct drm_printer *p, const char *str);
>  void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf);
> -void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
>  
> @@ -317,22 +316,6 @@ static inline struct drm_printer drm_info_printer(struct device *dev)
>  	return p;
>  }
>  
> -/**
> - * drm_debug_printer - construct a &drm_printer that outputs to pr_debug()
> - * @prefix: debug output prefix
> - *
> - * RETURNS:
> - * The &drm_printer object
> - */
> -static inline struct drm_printer drm_debug_printer(const char *prefix)
> -{
> -	struct drm_printer p = {
> -		.printfn = __drm_printfn_debug,
> -		.prefix = prefix
> -	};
> -	return p;
> -}
> -
>  /**
>   * drm_dbg_printer - construct a &drm_printer for drm device specific output
>   * @drm: the &struct drm_device pointer, or NULL

Why did you choose to make the last changes together with the patch
that removed the old version entirely? Was it because you didn't to
have any commit where the old version is defined but not used?

In any case, this is fine with me, so:

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 00/10] drm: drm debug and error logging improvements

[Jani Nikula]

On Tue, 16 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> This is resend and more patches on top of [1]. I don't think I've
> changed anything since then.

Hi drm-misc maintainers -

I've got R-b from Luca, but given there's no comments outside of Intel,
are you okay with me merging this to drm-misc?

BR,
Jani.


>
> BR,
> Jani.
>
> [1] https://patchwork.freedesktop.org/series/126873/
>
>
> Jani Nikula (10):
>   drm/print: make drm_err_printer() device specific by using drm_err()
>   drm/print: move enum drm_debug_category etc. earlier in drm_print.h
>   drm/print: add drm_dbg_printer() for drm device specific printer
>   drm/dp_mst: switch from drm_debug_printer() to device specific
>     drm_dbg_printer()
>   drm/mode: switch from drm_debug_printer() to device specific
>     drm_dbg_printer()
>   drm/dp: switch drm_dp_vsc_sdp_log() to struct drm_printer
>   drm/i915: switch from drm_debug_printer() to device specific
>     drm_dbg_printer()
>   drm/i915: use drm_printf() with the drm_err_printer intead of pr_err()
>   drm/xe: switch from drm_debug_printer() to device specific
>     drm_dbg_printer()
>   drm: remove drm_debug_printer in favor of drm_dbg_printer
>
>  drivers/gpu/drm/display/drm_dp_helper.c       |  17 +-
>  drivers/gpu/drm/display/drm_dp_mst_topology.c |  23 +-
>  drivers/gpu/drm/drm_mode_config.c             |   2 +-
>  drivers/gpu/drm/drm_modeset_lock.c            |   2 +-
>  drivers/gpu/drm/drm_print.c                   |  29 ++-
>  .../drm/i915/display/intel_crtc_state_dump.c  |   5 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |  27 ++-
>  .../drm/i915/display/intel_display_driver.c   |   3 +-
>  .../gpu/drm/i915/gt/intel_engine_heartbeat.c  |   3 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c         |   3 +-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c   |   3 +-
>  drivers/gpu/drm/i915/gt/selftest_context.c    |   3 +-
>  .../drm/i915/gt/selftest_engine_heartbeat.c   |  10 +-
>  drivers/gpu/drm/i915/i915_driver.c            |   3 +-
>  drivers/gpu/drm/i915/selftests/i915_active.c  |   8 +-
>  drivers/gpu/drm/ttm/ttm_bo.c                  |   2 +-
>  drivers/gpu/drm/xe/xe_gt.c                    |   2 +-
>  drivers/gpu/drm/xe/xe_gt_topology.c           |   4 +-
>  drivers/gpu/drm/xe/xe_reg_sr.c                |   2 +-
>  include/drm/display/drm_dp_helper.h           |   3 +-
>  include/drm/drm_print.h                       | 220 +++++++++---------
>  21 files changed, 206 insertions(+), 168 deletions(-)

-- 
Jani Nikula, Intel

Re: [PATCH 09/10] drm/xe: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Jani Nikula]

On Tue, 16 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> Prefer the device specific debug printer.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Xe maintainers, ack for merging this via drm-misc along with the rest of
the series?

BR,
Jani.

> ---
>  drivers/gpu/drm/xe/xe_gt.c          | 2 +-
>  drivers/gpu/drm/xe/xe_gt_topology.c | 4 +++-
>  drivers/gpu/drm/xe/xe_reg_sr.c      | 2 +-
>  3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 0f2258dc4a00..16481f9b3125 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -327,7 +327,7 @@ static void dump_pat_on_error(struct xe_gt *gt)
>  	char prefix[32];
>  
>  	snprintf(prefix, sizeof(prefix), "[GT%u Error]", gt->info.id);
> -	p = drm_debug_printer(prefix);
> +	p = drm_dbg_printer(&gt_to_xe(gt)->drm, DRM_UT_DRIVER, prefix);
>  
>  	xe_pat_dump(gt, &p);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
> index a8d7f272c30a..5dc62fe1be49 100644
> --- a/drivers/gpu/drm/xe/xe_gt_topology.c
> +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
> @@ -84,7 +84,7 @@ void
>  xe_gt_topology_init(struct xe_gt *gt)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
> -	struct drm_printer p = drm_debug_printer("GT topology");
> +	struct drm_printer p;
>  	int num_geometry_regs, num_compute_regs;
>  
>  	get_num_dss_regs(xe, &num_geometry_regs, &num_compute_regs);
> @@ -107,6 +107,8 @@ xe_gt_topology_init(struct xe_gt *gt)
>  		      XE2_GT_COMPUTE_DSS_2);
>  	load_eu_mask(gt, gt->fuse_topo.eu_mask_per_dss);
>  
> +	p = drm_dbg_printer(&gt_to_xe(gt)->drm, DRM_UT_DRIVER, "GT topology");
> +
>  	xe_gt_topology_dump(gt, &p);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
> index 87adefb56024..440ac572f6e5 100644
> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
> @@ -231,7 +231,7 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
>  	if (err)
>  		goto err_force_wake;
>  
> -	p = drm_debug_printer(KBUILD_MODNAME);
> +	p = drm_dbg_printer(&xe->drm, DRM_UT_DRIVER, NULL);
>  	xa_for_each(&sr->xa, reg, entry) {
>  		if (slot == RING_MAX_NONPRIV_SLOTS) {
>  			xe_gt_err(gt,

-- 
Jani Nikula, Intel

Re: Re: [PATCH 00/10] drm: drm debug and error logging improvements

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

On Thu, Feb 01, 2024 at 02:51:13PM +0200, Jani Nikula wrote:
> On Tue, 16 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> > This is resend and more patches on top of [1]. I don't think I've
> > changed anything since then.
> 
> Hi drm-misc maintainers -
> 
> I've got R-b from Luca, but given there's no comments outside of Intel,
> are you okay with me merging this to drm-misc?

Acked-by: Maxime Ripard <mripard@kernel.org>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Re: [PATCH 09/10] drm/xe: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Lucas De Marchi]

On Thu, Feb 01, 2024 at 02:52:55PM +0200, Jani Nikula wrote:
>On Tue, 16 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
>> Prefer the device specific debug printer.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
>Xe maintainers, ack for merging this via drm-misc along with the rest of
>the series?

Do you plan to also merge the last patch where you remove
drm_debug_printer()?  Any conflict to be solved in the merge commit?

This is just a question to know what's the plan, not preferring
something else.

Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>

thanks
Lucas De Marchi

Re: Re: [PATCH 00/10] drm: drm debug and error logging improvements

[Jani Nikula]

On Thu, 01 Feb 2024, Maxime Ripard <mripard@kernel.org> wrote:
> On Thu, Feb 01, 2024 at 02:51:13PM +0200, Jani Nikula wrote:
>> On Tue, 16 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
>> > This is resend and more patches on top of [1]. I don't think I've
>> > changed anything since then.
>> 
>> Hi drm-misc maintainers -
>> 
>> I've got R-b from Luca, but given there's no comments outside of Intel,
>> are you okay with me merging this to drm-misc?
>
> Acked-by: Maxime Ripard <mripard@kernel.org>

Thanks for the reviews and acks, pushed to drm-misc-next.

BR,
Jani.

-- 
Jani Nikula, Intel

Re: Re: [PATCH 09/10] drm/xe: switch from drm_debug_printer() to device specific drm_dbg_printer()

[Jani Nikula]

On Thu, 01 Feb 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Do you plan to also merge the last patch where you remove
> drm_debug_printer()?  Any conflict to be solved in the merge commit?
>
> This is just a question to know what's the plan, not preferring
> something else.

I looked into it, didn't see any conflicts, and merged away. Hopefully
none pop up until the branches merge again.

> Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>

Thanks,
Jani.

-- 
Jani Nikula, Intel