[PATCH 00/11] A few patches around DRM logging

[PATCH 00/11] A few patches around DRM logging

[Damien Lespiau]

As patch 8/11 explains, I noticed that we where evaluating the arguments to
drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good
reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(),
we skip those instructions that only need to be executed when logging is on.

The remaining patches are bonus clean-ups, with the main goal of removing
DRM_LOG* macros that are not really used throughout the code base. After that,
it's possible to simplify a bit drm_ut_debug_printk(). All pretty
straightforward cleanups, but still worthwhile IMHO.

For driver-specific patches (why some of you are Cced in that series), I'd love
if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you
have any objection if the driver specific patch go through the DRM tree?
should people judge that series worthwhile, of course.

-- 
Damien

Damien Lespiau (11):
  drm: Refresh the explanation of debug categories
  drm: Remove the unused (and unusable) DRM_LOG_MODE()
  drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
  drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
  drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
  staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
  drm: Remove the now unused DRM_LOG* macros
  drm: Pull the test on drm_debug in the logging macros
  drm: drm_ut_debug_printk() isn't called with NULL anywmore
  drm: Remove the prefix argument of drm_ut_debug_printk()
  drm: Remove the ',' after the function name in debug logs

 drivers/gpu/drm/drm_stub.c                |  24 +++----
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   2 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.c |   2 +-
 drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  20 +++---
 drivers/gpu/drm/i915/dvo_ch7xxx.c         |   4 +-
 drivers/gpu/drm/i915/dvo_ivch.c           |  30 ++++-----
 drivers/gpu/drm/i915/dvo_ns2501.c         |  10 +--
 drivers/gpu/drm/i915/dvo_sil164.c         |  10 +--
 drivers/gpu/drm/i915/dvo_tfp410.c         |  24 +++----
 drivers/staging/imx-drm/ipuv3-plane.c     |   2 +-
 include/drm/drmP.h                        | 106 +++++++++++-------------------
 11 files changed, 98 insertions(+), 136 deletions(-)

-- 
1.8.3.1

[PATCH 01/11] drm: Refresh the explanation of debug categories

[Damien Lespiau]

That comment wasn't super-readable, so I tried to improve it:

- Put the comment before the values it's documenting
- Add a mention to PRIME
- Reword things a bit to be a lighter read
- Add a note about the option to set the debug value at run-time

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 include/drm/drmP.h | 57 ++++++++++++++++++++++++++----------------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3857450..97900b7 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -88,41 +88,38 @@ struct videomode;
 #include <drm/drm_hashtab.h>
 #include <drm/drm_mm.h>
 
-#define DRM_UT_CORE 		0x01
-#define DRM_UT_DRIVER		0x02
-#define DRM_UT_KMS		0x04
-#define DRM_UT_PRIME		0x08
 /*
- * Three debug levels are defined.
- * drm_core, drm_driver, drm_kms
- * drm_core level can be used in the generic drm code. For example:
- * 	drm_ioctl, drm_mm, drm_memory
- * The macro definition of DRM_DEBUG is used.
- * 	DRM_DEBUG(fmt, args...)
- * 	The debug info by using the DRM_DEBUG can be obtained by adding
- * 	the boot option of "drm.debug=1".
+ * 4 debug 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.
  *
- * drm_driver level can be used in the specific drm driver. It is used
- * to add the debug info related with the drm driver. For example:
- * i915_drv, i915_dma, i915_gem, radeon_drv,
- * 	The macro definition of DRM_DEBUG_DRIVER can be used.
- * 	DRM_DEBUG_DRIVER(fmt, args...)
- * 	The debug info by using the DRM_DEBUG_DRIVER can be obtained by
- * 	adding the boot option of "drm.debug=0x02"
+ * KMS: used in the modesetting code.
+ *	This is the category used by the DRM_DEBUG_KMS() macro.
  *
- * drm_kms level can be used in the KMS code related with specific drm driver.
- * It is used to add the debug info related with KMS mode. For example:
- * the connector/crtc ,
- * 	The macro definition of DRM_DEBUG_KMS can be used.
- * 	DRM_DEBUG_KMS(fmt, args...)
- * 	The debug info by using the DRM_DEBUG_KMS can be obtained by
- * 	adding the boot option of "drm.debug=0x04"
+ * PRIME: used in the prime code.
+ *	  This is the category used by the DRM_DEBUG_PRIME() macro.
  *
- * If we add the boot option of "drm.debug=0x06", we can get the debug info by
- * using the DRM_DEBUG_KMS and DRM_DEBUG_DRIVER.
- * If we add the boot option of "drm.debug=0x05", we can get the debug info by
- * using the DRM_DEBUG_KMS and DRM_DEBUG.
+ * 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=0xf 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
  */
+#define DRM_UT_CORE 		0x01
+#define DRM_UT_DRIVER		0x02
+#define DRM_UT_KMS		0x04
+#define DRM_UT_PRIME		0x08
 
 extern __printf(4, 5)
 void drm_ut_debug_printk(unsigned int request_level,
-- 
1.8.3.1

[PATCH 02/11] drm: Remove the unused (and unusable) DRM_LOG_MODE()

[Damien Lespiau]

This macro was trying to use the non existing DRM_UT_MODE debug category
and looks like it should be covered by DRM_LOG_KMS().

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 include/drm/drmP.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 97900b7..1455e58 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -238,11 +238,6 @@ int drm_err(const char *func, const char *format, ...);
 		drm_ut_debug_printk(DRM_UT_KMS, NULL,			\
 					NULL, fmt, ##args);		\
 	} while (0)
-#define DRM_LOG_MODE(fmt, args...)					\
-	do {								\
-		drm_ut_debug_printk(DRM_UT_MODE, NULL,			\
-					NULL, fmt, ##args);		\
-	} while (0)
 #define DRM_LOG_DRIVER(fmt, args...)					\
 	do {								\
 		drm_ut_debug_printk(DRM_UT_DRIVER, NULL,		\
@@ -255,7 +250,6 @@ int drm_err(const char *func, const char *format, ...);
 #define DRM_DEBUG(fmt, arg...)		 do { } while (0)
 #define DRM_LOG(fmt, arg...)		do { } while (0)
 #define DRM_LOG_KMS(fmt, args...) do { } while (0)
-#define DRM_LOG_MODE(fmt, arg...) do { } while (0)
 #define DRM_LOG_DRIVER(fmt, arg...) do { } while (0)
 
 #endif
-- 
1.8.3.1

[PATCH 03/11] drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

[Damien Lespiau]

There are only a few users of the DRM_LOG_KMS() macro. We can simplify
the DRM code a bit by replacing them by DRM_DEBUG_KMS().

Cc: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index e7c2f2d..5fa342e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -90,7 +90,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
 	/* RGB formats use only one buffer */
 	buffer = exynos_drm_fb_buffer(fb, 0);
 	if (!buffer) {
-		DRM_LOG_KMS("buffer is null.\n");
+		DRM_DEBUG_KMS("buffer is null.\n");
 		return -EFAULT;
 	}
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index fcb0652..2c765d5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -87,7 +87,7 @@ int exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
 		struct exynos_drm_gem_buf *buffer = exynos_drm_fb_buffer(fb, i);
 
 		if (!buffer) {
-			DRM_LOG_KMS("buffer is null\n");
+			DRM_DEBUG_KMS("buffer is null\n");
 			return -EFAULT;
 		}
 
-- 
1.8.3.1

[PATCH 04/11] drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

[Damien Lespiau]

There are only a few users of the DRM_LOG_KMS() macro. We can simplify
the DRM code a bit by replacing them by DRM_DEBUG_KMS().

Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 07d3a9e..681efec 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct psb_intel_sdvo *psb_intel_sdvo, u8
 	DRM_DEBUG_KMS("%s: W: %02X ",
 				SDVO_NAME(psb_intel_sdvo), cmd);
 	for (i = 0; i < args_len; i++)
-		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
+		DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]);
 	for (; i < 8; i++)
-		DRM_LOG_KMS("   ");
+		DRM_DEBUG_KMS("   ");
 	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
 		if (cmd == sdvo_cmd_names[i].cmd) {
-			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
+			DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name);
 			break;
 		}
 	}
 	if (i == ARRAY_SIZE(sdvo_cmd_names))
-		DRM_LOG_KMS("(%02X)", cmd);
-	DRM_LOG_KMS("\n");
+		DRM_DEBUG_KMS("(%02X)", cmd);
+	DRM_DEBUG_KMS("\n");
 }
 
 static const char *cmd_status_names[] = {
@@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
 	}
 
 	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
-		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
+		DRM_DEBUG_KMS("(%s)", cmd_status_names[status]);
 	else
-		DRM_LOG_KMS("(??? %d)", status);
+		DRM_DEBUG_KMS("(??? %d)", status);
 
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		goto log_fail;
@@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
 					  SDVO_I2C_RETURN_0 + i,
 					  &((u8 *)response)[i]))
 			goto log_fail;
-		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
+		DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]);
 	}
-	DRM_LOG_KMS("\n");
+	DRM_DEBUG_KMS("\n");
 	return true;
 
 log_fail:
-	DRM_LOG_KMS("... failed\n");
+	DRM_DEBUG_KMS("... failed\n");
 	return false;
 }
 
-- 
1.8.3.1

[PATCH 05/11] drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

[Damien Lespiau]

There are only a few users of the DRM_LOG_KMS() macro. We can simplify
the DRM code a bit by replacing them by DRM_DEBUG_KMS().

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/dvo_ch7xxx.c |  4 ++--
 drivers/gpu/drm/i915/dvo_ivch.c   | 30 +++++++++++++++---------------
 drivers/gpu/drm/i915/dvo_ns2501.c | 10 +++++-----
 drivers/gpu/drm/i915/dvo_sil164.c | 10 +++++-----
 drivers/gpu/drm/i915/dvo_tfp410.c | 24 ++++++++++++------------
 5 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c b/drivers/gpu/drm/i915/dvo_ch7xxx.c
index af42e94..a0f5bdd 100644
--- a/drivers/gpu/drm/i915/dvo_ch7xxx.c
+++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c
@@ -340,9 +340,9 @@ static void ch7xxx_dump_regs(struct intel_dvo_device *dvo)
 	for (i = 0; i < CH7xxx_NUM_REGS; i++) {
 		uint8_t val;
 		if ((i % 8) == 0)
-			DRM_LOG_KMS("\n %02X: ", i);
+			DRM_DEBUG_KMS("\n %02X: ", i);
 		ch7xxx_readb(dvo, i, &val);
-		DRM_LOG_KMS("%02X ", val);
+		DRM_DEBUG_KMS("%02X ", val);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c
index baaf65b..0f1865d 100644
--- a/drivers/gpu/drm/i915/dvo_ivch.c
+++ b/drivers/gpu/drm/i915/dvo_ivch.c
@@ -377,41 +377,41 @@ static void ivch_dump_regs(struct intel_dvo_device *dvo)
 	uint16_t val;
 
 	ivch_read(dvo, VR00, &val);
-	DRM_LOG_KMS("VR00: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR00: 0x%04x\n", val);
 	ivch_read(dvo, VR01, &val);
-	DRM_LOG_KMS("VR01: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR01: 0x%04x\n", val);
 	ivch_read(dvo, VR30, &val);
-	DRM_LOG_KMS("VR30: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR30: 0x%04x\n", val);
 	ivch_read(dvo, VR40, &val);
-	DRM_LOG_KMS("VR40: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR40: 0x%04x\n", val);
 
 	/* GPIO registers */
 	ivch_read(dvo, VR80, &val);
-	DRM_LOG_KMS("VR80: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR80: 0x%04x\n", val);
 	ivch_read(dvo, VR81, &val);
-	DRM_LOG_KMS("VR81: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR81: 0x%04x\n", val);
 	ivch_read(dvo, VR82, &val);
-	DRM_LOG_KMS("VR82: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR82: 0x%04x\n", val);
 	ivch_read(dvo, VR83, &val);
-	DRM_LOG_KMS("VR83: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR83: 0x%04x\n", val);
 	ivch_read(dvo, VR84, &val);
-	DRM_LOG_KMS("VR84: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR84: 0x%04x\n", val);
 	ivch_read(dvo, VR85, &val);
-	DRM_LOG_KMS("VR85: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR85: 0x%04x\n", val);
 	ivch_read(dvo, VR86, &val);
-	DRM_LOG_KMS("VR86: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR86: 0x%04x\n", val);
 	ivch_read(dvo, VR87, &val);
-	DRM_LOG_KMS("VR87: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR87: 0x%04x\n", val);
 	ivch_read(dvo, VR88, &val);
-	DRM_LOG_KMS("VR88: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR88: 0x%04x\n", val);
 
 	/* Scratch register 0 - AIM Panel type */
 	ivch_read(dvo, VR8E, &val);
-	DRM_LOG_KMS("VR8E: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR8E: 0x%04x\n", val);
 
 	/* Scratch register 1 - Status register */
 	ivch_read(dvo, VR8F, &val);
-	DRM_LOG_KMS("VR8F: 0x%04x\n", val);
+	DRM_DEBUG_KMS("VR8F: 0x%04x\n", val);
 }
 
 static void ivch_destroy(struct intel_dvo_device *dvo)
diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c
index 954acb2..8155ded 100644
--- a/drivers/gpu/drm/i915/dvo_ns2501.c
+++ b/drivers/gpu/drm/i915/dvo_ns2501.c
@@ -490,15 +490,15 @@ static void ns2501_dump_regs(struct intel_dvo_device *dvo)
 	uint8_t val;
 
 	ns2501_readb(dvo, NS2501_FREQ_LO, &val);
-	DRM_LOG_KMS("NS2501_FREQ_LO: 0x%02x\n", val);
+	DRM_DEBUG_KMS("NS2501_FREQ_LO: 0x%02x\n", val);
 	ns2501_readb(dvo, NS2501_FREQ_HI, &val);
-	DRM_LOG_KMS("NS2501_FREQ_HI: 0x%02x\n", val);
+	DRM_DEBUG_KMS("NS2501_FREQ_HI: 0x%02x\n", val);
 	ns2501_readb(dvo, NS2501_REG8, &val);
-	DRM_LOG_KMS("NS2501_REG8: 0x%02x\n", val);
+	DRM_DEBUG_KMS("NS2501_REG8: 0x%02x\n", val);
 	ns2501_readb(dvo, NS2501_REG9, &val);
-	DRM_LOG_KMS("NS2501_REG9: 0x%02x\n", val);
+	DRM_DEBUG_KMS("NS2501_REG9: 0x%02x\n", val);
 	ns2501_readb(dvo, NS2501_REGC, &val);
-	DRM_LOG_KMS("NS2501_REGC: 0x%02x\n", val);
+	DRM_DEBUG_KMS("NS2501_REGC: 0x%02x\n", val);
 }
 
 static void ns2501_destroy(struct intel_dvo_device *dvo)
diff --git a/drivers/gpu/drm/i915/dvo_sil164.c b/drivers/gpu/drm/i915/dvo_sil164.c
index 4debd32..7b3e9e9 100644
--- a/drivers/gpu/drm/i915/dvo_sil164.c
+++ b/drivers/gpu/drm/i915/dvo_sil164.c
@@ -246,15 +246,15 @@ static void sil164_dump_regs(struct intel_dvo_device *dvo)
 	uint8_t val;
 
 	sil164_readb(dvo, SIL164_FREQ_LO, &val);
-	DRM_LOG_KMS("SIL164_FREQ_LO: 0x%02x\n", val);
+	DRM_DEBUG_KMS("SIL164_FREQ_LO: 0x%02x\n", val);
 	sil164_readb(dvo, SIL164_FREQ_HI, &val);
-	DRM_LOG_KMS("SIL164_FREQ_HI: 0x%02x\n", val);
+	DRM_DEBUG_KMS("SIL164_FREQ_HI: 0x%02x\n", val);
 	sil164_readb(dvo, SIL164_REG8, &val);
-	DRM_LOG_KMS("SIL164_REG8: 0x%02x\n", val);
+	DRM_DEBUG_KMS("SIL164_REG8: 0x%02x\n", val);
 	sil164_readb(dvo, SIL164_REG9, &val);
-	DRM_LOG_KMS("SIL164_REG9: 0x%02x\n", val);
+	DRM_DEBUG_KMS("SIL164_REG9: 0x%02x\n", val);
 	sil164_readb(dvo, SIL164_REGC, &val);
-	DRM_LOG_KMS("SIL164_REGC: 0x%02x\n", val);
+	DRM_DEBUG_KMS("SIL164_REGC: 0x%02x\n", val);
 }
 
 static void sil164_destroy(struct intel_dvo_device *dvo)
diff --git a/drivers/gpu/drm/i915/dvo_tfp410.c b/drivers/gpu/drm/i915/dvo_tfp410.c
index e17f1b0..12ea4b1 100644
--- a/drivers/gpu/drm/i915/dvo_tfp410.c
+++ b/drivers/gpu/drm/i915/dvo_tfp410.c
@@ -267,33 +267,33 @@ static void tfp410_dump_regs(struct intel_dvo_device *dvo)
 	uint8_t val, val2;
 
 	tfp410_readb(dvo, TFP410_REV, &val);
-	DRM_LOG_KMS("TFP410_REV: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_REV: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_CTL_1, &val);
-	DRM_LOG_KMS("TFP410_CTL1: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_CTL1: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_CTL_2, &val);
-	DRM_LOG_KMS("TFP410_CTL2: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_CTL2: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_CTL_3, &val);
-	DRM_LOG_KMS("TFP410_CTL3: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_CTL3: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_USERCFG, &val);
-	DRM_LOG_KMS("TFP410_USERCFG: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_USERCFG: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_DE_DLY, &val);
-	DRM_LOG_KMS("TFP410_DE_DLY: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_DE_DLY: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_DE_CTL, &val);
-	DRM_LOG_KMS("TFP410_DE_CTL: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_DE_CTL: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_DE_TOP, &val);
-	DRM_LOG_KMS("TFP410_DE_TOP: 0x%02X\n", val);
+	DRM_DEBUG_KMS("TFP410_DE_TOP: 0x%02X\n", val);
 	tfp410_readb(dvo, TFP410_DE_CNT_LO, &val);
 	tfp410_readb(dvo, TFP410_DE_CNT_HI, &val2);
-	DRM_LOG_KMS("TFP410_DE_CNT: 0x%02X%02X\n", val2, val);
+	DRM_DEBUG_KMS("TFP410_DE_CNT: 0x%02X%02X\n", val2, val);
 	tfp410_readb(dvo, TFP410_DE_LIN_LO, &val);
 	tfp410_readb(dvo, TFP410_DE_LIN_HI, &val2);
-	DRM_LOG_KMS("TFP410_DE_LIN: 0x%02X%02X\n", val2, val);
+	DRM_DEBUG_KMS("TFP410_DE_LIN: 0x%02X%02X\n", val2, val);
 	tfp410_readb(dvo, TFP410_H_RES_LO, &val);
 	tfp410_readb(dvo, TFP410_H_RES_HI, &val2);
-	DRM_LOG_KMS("TFP410_H_RES: 0x%02X%02X\n", val2, val);
+	DRM_DEBUG_KMS("TFP410_H_RES: 0x%02X%02X\n", val2, val);
 	tfp410_readb(dvo, TFP410_V_RES_LO, &val);
 	tfp410_readb(dvo, TFP410_V_RES_HI, &val2);
-	DRM_LOG_KMS("TFP410_V_RES: 0x%02X%02X\n", val2, val);
+	DRM_DEBUG_KMS("TFP410_V_RES: 0x%02X%02X\n", val2, val);
 }
 
 static void tfp410_destroy(struct intel_dvo_device *dvo)
-- 
1.8.3.1

[PATCH 06/11] staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

[Damien Lespiau]

There are only a few users of the DRM_LOG_KMS() macro. We can simplify
the DRM code a bit by replacing them by DRM_DEBUG_KMS().

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/staging/imx-drm/ipuv3-plane.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c
index 34b642a..5128dc39 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -68,7 +68,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 
 	cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
 	if (!cma_obj) {
-		DRM_LOG_KMS("entry is null.\n");
+		DRM_DEBUG_KMS("entry is null.\n");
 		return -EFAULT;
 	}
 
-- 
1.8.3.1

[PATCH 07/11] drm: Remove the now unused DRM_LOG* macros

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 include/drm/drmP.h | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 1455e58..3055b36 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -228,30 +228,11 @@ int drm_err(const char *func, const char *format, ...);
 		drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME,		\
 					__func__, fmt, ##args);		\
 	} while (0)
-#define DRM_LOG(fmt, args...)						\
-	do {								\
-		drm_ut_debug_printk(DRM_UT_CORE, NULL,			\
-					NULL, fmt, ##args);		\
-	} while (0)
-#define DRM_LOG_KMS(fmt, args...)					\
-	do {								\
-		drm_ut_debug_printk(DRM_UT_KMS, NULL,			\
-					NULL, fmt, ##args);		\
-	} while (0)
-#define DRM_LOG_DRIVER(fmt, args...)					\
-	do {								\
-		drm_ut_debug_printk(DRM_UT_DRIVER, NULL,		\
-					NULL, fmt, ##args);		\
-	} while (0)
 #else
 #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
 #define DRM_DEBUG_KMS(fmt, args...)	do { } while (0)
 #define DRM_DEBUG_PRIME(fmt, args...)	do { } while (0)
 #define DRM_DEBUG(fmt, arg...)		 do { } while (0)
-#define DRM_LOG(fmt, arg...)		do { } while (0)
-#define DRM_LOG_KMS(fmt, args...) do { } while (0)
-#define DRM_LOG_DRIVER(fmt, arg...) do { } while (0)
-
 #endif
 
 /*@}*/
-- 
1.8.3.1

[PATCH 08/11] drm: Pull the test on drm_debug in the logging macros

[Damien Lespiau]

In the logging code, we are currently checking is we need to output in
drm_ut_debug_printk(). This is too late. The problem is that when we write
something like:

    DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
                     connector->base.id,
                     drm_get_connector_name(connector),
                     connector->encoder->base.id,
                     drm_get_encoder_name(connector->encoder));

We start by evaluating the arguments (so call drm_get_connector_name() and
drm_get_connector_name()) before ending up in drm_ut_debug_printk() which will
then does nothing.

This means we execute a lot of instructions (drm_get_connector_name(), in turn,
calls snprintf() for example) to happily discard them in the normal case,
drm.debug=0.

So, let's put the test on drm_debug earlier, in the macros themselves.
Sprinkle an unlikely() as well for good measure.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_stub.c | 26 ++++++++++++--------------
 include/drm/drmP.h         | 27 +++++++++++++++------------
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index dc2c609..81ed832 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -97,26 +97,24 @@ int drm_err(const char *func, const char *format, ...)
 }
 EXPORT_SYMBOL(drm_err);
 
-void drm_ut_debug_printk(unsigned int request_level,
-			 const char *prefix,
+void drm_ut_debug_printk(const char *prefix,
 			 const char *function_name,
 			 const char *format, ...)
 {
 	struct va_format vaf;
 	va_list args;
 
-	if (drm_debug & request_level) {
-		va_start(args, format);
-		vaf.fmt = format;
-		vaf.va = &args;
-
-		if (function_name)
-			printk(KERN_DEBUG "[%s:%s], %pV", prefix,
-			       function_name, &vaf);
-		else
-			printk(KERN_DEBUG "%pV", &vaf);
-		va_end(args);
-	}
+	va_start(args, format);
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	if (function_name)
+		printk(KERN_DEBUG "[%s:%s], %pV", prefix,
+		       function_name, &vaf);
+	else
+		printk(KERN_DEBUG "%pV", &vaf);
+
+	va_end(args);
 }
 EXPORT_SYMBOL(drm_ut_debug_printk);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3055b36..87b558a 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -121,9 +121,8 @@ struct videomode;
 #define DRM_UT_KMS		0x04
 #define DRM_UT_PRIME		0x08
 
-extern __printf(4, 5)
-void drm_ut_debug_printk(unsigned int request_level,
-			 const char *prefix,
+extern __printf(3, 4)
+void drm_ut_debug_printk(const char *prefix,
 			 const char *function_name,
 			 const char *format, ...);
 extern __printf(2, 3)
@@ -209,24 +208,28 @@ int drm_err(const char *func, const char *format, ...);
 #if DRM_DEBUG_CODE
 #define DRM_DEBUG(fmt, args...)						\
 	do {								\
-		drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, 		\
-					__func__, fmt, ##args);		\
+		if (unlikely(drm_debug & DRM_UT_CORE))			\
+			drm_ut_debug_printk(DRM_NAME, __func__,		\
+					    fmt, ##args);		\
 	} while (0)
 
 #define DRM_DEBUG_DRIVER(fmt, args...)					\
 	do {								\
-		drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME,		\
-					__func__, fmt, ##args);		\
+		if (unlikely(drm_debug & DRM_UT_DRIVER))		\
+			drm_ut_debug_printk(DRM_NAME, __func__,		\
+					    fmt, ##args);		\
 	} while (0)
-#define DRM_DEBUG_KMS(fmt, args...)				\
+#define DRM_DEBUG_KMS(fmt, args...)					\
 	do {								\
-		drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, 		\
-					 __func__, fmt, ##args);	\
+		if (unlikely(drm_debug & DRM_UT_KMS))			\
+			drm_ut_debug_printk(DRM_NAME, __func__,		\
+					    fmt, ##args);		\
 	} while (0)
 #define DRM_DEBUG_PRIME(fmt, args...)					\
 	do {								\
-		drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME,		\
-					__func__, fmt, ##args);		\
+		if (unlikely(drm_debug & DRM_UT_PRIME))			\
+			drm_ut_debug_printk(DRM_NAME, __func__,		\
+					    fmt, ##args);		\
 	} while (0)
 #else
 #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
-- 
1.8.3.1

[PATCH 09/11] drm: drm_ut_debug_printk() isn't called with NULL anywmore

[Damien Lespiau]

The DRM_LOG* macros where the only sites where drm_ut_debug_printk was
called with NULL arguments for prefix and function_name. Now that they
are gone, we can remove that case.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_stub.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 81ed832..0b1a912 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -108,11 +108,7 @@ void drm_ut_debug_printk(const char *prefix,
 	vaf.fmt = format;
 	vaf.va = &args;
 
-	if (function_name)
-		printk(KERN_DEBUG "[%s:%s], %pV", prefix,
-		       function_name, &vaf);
-	else
-		printk(KERN_DEBUG "%pV", &vaf);
+	printk(KERN_DEBUG "[%s:%s], %pV", prefix, function_name, &vaf);
 
 	va_end(args);
 }
-- 
1.8.3.1

[PATCH 10/11] drm: Remove the prefix argument of drm_ut_debug_printk()

[Damien Lespiau]

This is always DRM_NAME, so we can just make it part of the format
string instead of asking prink to do it for us.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_stub.c |  6 ++----
 include/drm/drmP.h         | 17 ++++++-----------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 0b1a912..9b14873 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -97,9 +97,7 @@ int drm_err(const char *func, const char *format, ...)
 }
 EXPORT_SYMBOL(drm_err);
 
-void drm_ut_debug_printk(const char *prefix,
-			 const char *function_name,
-			 const char *format, ...)
+void drm_ut_debug_printk(const char *function_name, const char *format, ...)
 {
 	struct va_format vaf;
 	va_list args;
@@ -108,7 +106,7 @@ void drm_ut_debug_printk(const char *prefix,
 	vaf.fmt = format;
 	vaf.va = &args;
 
-	printk(KERN_DEBUG "[%s:%s], %pV", prefix, function_name, &vaf);
+	printk(KERN_DEBUG "[" DRM_NAME ":%s], %pV", function_name, &vaf);
 
 	va_end(args);
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 87b558a..2f49510 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -121,9 +121,8 @@ struct videomode;
 #define DRM_UT_KMS		0x04
 #define DRM_UT_PRIME		0x08
 
-extern __printf(3, 4)
-void drm_ut_debug_printk(const char *prefix,
-			 const char *function_name,
+extern __printf(2, 3)
+void drm_ut_debug_printk(const char *function_name,
 			 const char *format, ...);
 extern __printf(2, 3)
 int drm_err(const char *func, const char *format, ...);
@@ -209,27 +208,23 @@ int drm_err(const char *func, const char *format, ...);
 #define DRM_DEBUG(fmt, args...)						\
 	do {								\
 		if (unlikely(drm_debug & DRM_UT_CORE))			\
-			drm_ut_debug_printk(DRM_NAME, __func__,		\
-					    fmt, ##args);		\
+			drm_ut_debug_printk(__func__, fmt, ##args);	\
 	} while (0)
 
 #define DRM_DEBUG_DRIVER(fmt, args...)					\
 	do {								\
 		if (unlikely(drm_debug & DRM_UT_DRIVER))		\
-			drm_ut_debug_printk(DRM_NAME, __func__,		\
-					    fmt, ##args);		\
+			drm_ut_debug_printk(__func__, fmt, ##args);	\
 	} while (0)
 #define DRM_DEBUG_KMS(fmt, args...)					\
 	do {								\
 		if (unlikely(drm_debug & DRM_UT_KMS))			\
-			drm_ut_debug_printk(DRM_NAME, __func__,		\
-					    fmt, ##args);		\
+			drm_ut_debug_printk(__func__, fmt, ##args);	\
 	} while (0)
 #define DRM_DEBUG_PRIME(fmt, args...)					\
 	do {								\
 		if (unlikely(drm_debug & DRM_UT_PRIME))			\
-			drm_ut_debug_printk(DRM_NAME, __func__,		\
-					    fmt, ##args);		\
+			drm_ut_debug_printk(__func__, fmt, ##args);	\
 	} while (0)
 #else
 #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
-- 
1.8.3.1

[PATCH 11/11] drm: Remove the ', ' after the function name in debug logs

[Damien Lespiau]

Right now a debug message looks like:

  [drm:drm_ioctl], pid=860, dev=0xe200, auth=1, DRM_IOCTL_MODE_GETCRTC

That first comma looks weird as we already have ']' as a separator.
Remove it.

If anyone sees this commit message and also thinks that auth=1 isn't the
most useful info to have here, let's just say I'd happily review a patch
removing it. If I don't get annoyed enough to submit a patch, that is.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/drm_stub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 9b14873..ed8a995 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -106,7 +106,7 @@ void drm_ut_debug_printk(const char *function_name, const char *format, ...)
 	vaf.fmt = format;
 	vaf.va = &args;
 
-	printk(KERN_DEBUG "[" DRM_NAME ":%s], %pV", function_name, &vaf);
+	printk(KERN_DEBUG "[" DRM_NAME ":%s] %pV", function_name, &vaf);
 
 	va_end(args);
 }
-- 
1.8.3.1

Re: [PATCH 04/11] drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

[Daniel Vetter]

On Mon, Mar 24, 2014 at 03:53:11PM +0000, Damien Lespiau wrote:
> There are only a few users of the DRM_LOG_KMS() macro. We can simplify
> the DRM code a bit by replacing them by DRM_DEBUG_KMS().
> 
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Note that the point here of using LOG_KMS is to esssentially have
continuations. Which means your patch here will make the output extremely
noisy and hard to read.

But the current code is already racy which annoyed me sufficently a while
ago in i915's copy to fix it all up properly in

commit 84fcb46977e57bafba40bde32067bacc1e510f9c                                                                                      
Author: Daniel Vetter <daniel.vetter@ffwll.ch>                                                                                       
Date:   Wed Nov 27 16:03:01 2013 +0100                                                                                               

    drm/i915/sdvo: Fix up debug output to not split lines
    
    It leads to a big mess when stuff interleaves. Especially with the new
    patch I've submitted for the drm core to no longer artificially split
    up debug messages.
    
    v2: The size parameter to snprintf includes the terminating 0, but the
    return value does not. Adjust the logic accordingly. Spotted by Mika.
    
    Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
    Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Just my 2c.

Cheers, Daniel
> ---
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> index 07d3a9e..681efec 100644
> --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> @@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct psb_intel_sdvo *psb_intel_sdvo, u8
>  	DRM_DEBUG_KMS("%s: W: %02X ",
>  				SDVO_NAME(psb_intel_sdvo), cmd);
>  	for (i = 0; i < args_len; i++)
> -		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
> +		DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]);
>  	for (; i < 8; i++)
> -		DRM_LOG_KMS("   ");
> +		DRM_DEBUG_KMS("   ");
>  	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
>  		if (cmd == sdvo_cmd_names[i].cmd) {
> -			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
> +			DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name);
>  			break;
>  		}
>  	}
>  	if (i == ARRAY_SIZE(sdvo_cmd_names))
> -		DRM_LOG_KMS("(%02X)", cmd);
> -	DRM_LOG_KMS("\n");
> +		DRM_DEBUG_KMS("(%02X)", cmd);
> +	DRM_DEBUG_KMS("\n");
>  }
>  
>  static const char *cmd_status_names[] = {
> @@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
>  	}
>  
>  	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
> -		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
> +		DRM_DEBUG_KMS("(%s)", cmd_status_names[status]);
>  	else
> -		DRM_LOG_KMS("(??? %d)", status);
> +		DRM_DEBUG_KMS("(??? %d)", status);
>  
>  	if (status != SDVO_CMD_STATUS_SUCCESS)
>  		goto log_fail;
> @@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo,
>  					  SDVO_I2C_RETURN_0 + i,
>  					  &((u8 *)response)[i]))
>  			goto log_fail;
> -		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
> +		DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]);
>  	}
> -	DRM_LOG_KMS("\n");
> +	DRM_DEBUG_KMS("\n");
>  	return true;
>  
>  log_fail:
> -	DRM_LOG_KMS("... failed\n");
> +	DRM_DEBUG_KMS("... failed\n");
>  	return false;
>  }
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/11] A few patches around DRM logging

[Daniel Vetter]

On Mon, Mar 24, 2014 at 03:53:07PM +0000, Damien Lespiau wrote:
> As patch 8/11 explains, I noticed that we where evaluating the arguments to
> drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good
> reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(),
> we skip those instructions that only need to be executed when logging is on.
> 
> The remaining patches are bonus clean-ups, with the main goal of removing
> DRM_LOG* macros that are not really used throughout the code base. After that,
> it's possible to simplify a bit drm_ut_debug_printk(). All pretty
> straightforward cleanups, but still worthwhile IMHO.
> 
> For driver-specific patches (why some of you are Cced in that series), I'd love
> if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you
> have any objection if the driver specific patch go through the DRM tree?
> should people judge that series worthwhile, of course.

Except for the gma500 patch this series is

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

For the gma500 I've replied with my opinion but will defer to Patrik.
-Daniel

> 
> -- 
> Damien
> 
> Damien Lespiau (11):
>   drm: Refresh the explanation of debug categories
>   drm: Remove the unused (and unusable) DRM_LOG_MODE()
>   drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm: Remove the now unused DRM_LOG* macros
>   drm: Pull the test on drm_debug in the logging macros
>   drm: drm_ut_debug_printk() isn't called with NULL anywmore
>   drm: Remove the prefix argument of drm_ut_debug_printk()
>   drm: Remove the ',' after the function name in debug logs
> 
>  drivers/gpu/drm/drm_stub.c                |  24 +++----
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |   2 +-
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  20 +++---
>  drivers/gpu/drm/i915/dvo_ch7xxx.c         |   4 +-
>  drivers/gpu/drm/i915/dvo_ivch.c           |  30 ++++-----
>  drivers/gpu/drm/i915/dvo_ns2501.c         |  10 +--
>  drivers/gpu/drm/i915/dvo_sil164.c         |  10 +--
>  drivers/gpu/drm/i915/dvo_tfp410.c         |  24 +++----
>  drivers/staging/imx-drm/ipuv3-plane.c     |   2 +-
>  include/drm/drmP.h                        | 106 +++++++++++-------------------
>  11 files changed, 98 insertions(+), 136 deletions(-)
> 
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 04/11] drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

[Patrik Jakobsson]

[-- Attachment #1.1: Type: text/plain, Size: 4729 bytes --]

On Mon, Mar 24, 2014 at 9:39 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Mar 24, 2014 at 03:53:11PM +0000, Damien Lespiau wrote:
> > There are only a few users of the DRM_LOG_KMS() macro. We can simplify
> > the DRM code a bit by replacing them by DRM_DEBUG_KMS().
> >
> > Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
>
> Note that the point here of using LOG_KMS is to esssentially have
> continuations. Which means your patch here will make the output extremely
> noisy and hard to read.
>

The noise is already there (tons of empty lines) so this patch will not make
it any worse. It needs to be fixed like in i915. I'll put it on my
todo-list.

Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>



> But the current code is already racy which annoyed me sufficently a while
> ago in i915's copy to fix it all up properly in
>
> commit 84fcb46977e57bafba40bde32067bacc1e510f9c
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Wed Nov 27 16:03:01 2013 +0100
>
>     drm/i915/sdvo: Fix up debug output to not split lines
>
>     It leads to a big mess when stuff interleaves. Especially with the new
>     patch I've submitted for the drm core to no longer artificially split
>     up debug messages.
>
>     v2: The size parameter to snprintf includes the terminating 0, but the
>     return value does not. Adjust the logic accordingly. Spotted by Mika.
>
>     Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>     Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
>     Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Just my 2c.
>
> Cheers, Daniel
> > ---
> >  drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> > index 07d3a9e..681efec 100644
> > --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> > +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
> > @@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct
> psb_intel_sdvo *psb_intel_sdvo, u8
> >       DRM_DEBUG_KMS("%s: W: %02X ",
> >                               SDVO_NAME(psb_intel_sdvo), cmd);
> >       for (i = 0; i < args_len; i++)
> > -             DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
> > +             DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]);
> >       for (; i < 8; i++)
> > -             DRM_LOG_KMS("   ");
> > +             DRM_DEBUG_KMS("   ");
> >       for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
> >               if (cmd == sdvo_cmd_names[i].cmd) {
> > -                     DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
> > +                     DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name);
> >                       break;
> >               }
> >       }
> >       if (i == ARRAY_SIZE(sdvo_cmd_names))
> > -             DRM_LOG_KMS("(%02X)", cmd);
> > -     DRM_LOG_KMS("\n");
> > +             DRM_DEBUG_KMS("(%02X)", cmd);
> > +     DRM_DEBUG_KMS("\n");
> >  }
> >
> >  static const char *cmd_status_names[] = {
> > @@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct
> psb_intel_sdvo *psb_intel_sdvo,
> >       }
> >
> >       if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
> > -             DRM_LOG_KMS("(%s)", cmd_status_names[status]);
> > +             DRM_DEBUG_KMS("(%s)", cmd_status_names[status]);
> >       else
> > -             DRM_LOG_KMS("(??? %d)", status);
> > +             DRM_DEBUG_KMS("(??? %d)", status);
> >
> >       if (status != SDVO_CMD_STATUS_SUCCESS)
> >               goto log_fail;
> > @@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct
> psb_intel_sdvo *psb_intel_sdvo,
> >                                         SDVO_I2C_RETURN_0 + i,
> >                                         &((u8 *)response)[i]))
> >                       goto log_fail;
> > -             DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
> > +             DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]);
> >       }
> > -     DRM_LOG_KMS("\n");
> > +     DRM_DEBUG_KMS("\n");
> >       return true;
> >
> >  log_fail:
> > -     DRM_LOG_KMS("... failed\n");
> > +     DRM_DEBUG_KMS("... failed\n");
> >       return false;
> >  }
> >
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 6906 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/11] A few patches around DRM logging

[Patrik Jakobsson]

[-- Attachment #1.1: Type: text/plain, Size: 2499 bytes --]

On Mon, Mar 24, 2014 at 4:53 PM, Damien Lespiau <damien.lespiau@intel.com>wrote:

> As patch 8/11 explains, I noticed that we where evaluating the arguments to
> drm_ut_debug_printk() even when drm.debug was 0, doing some work for no
> good
> reason. By pulling the test on drm_debug before calling
> drm_ut_debug_printk(),
> we skip those instructions that only need to be executed when logging is
> on.
>
> The remaining patches are bonus clean-ups, with the main goal of removing
> DRM_LOG* macros that are not really used throughout the code base. After
> that,
> it's possible to simplify a bit drm_ut_debug_printk(). All pretty
> straightforward cleanups, but still worthwhile IMHO.
>
> For driver-specific patches (why some of you are Cced in that series), I'd
> love
> if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do
> you
> have any objection if the driver specific patch go through the DRM tree?
> should people judge that series worthwhile, of course.
>
>
I'm ok with this going through the DRM tree.

Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>


> --
> Damien
>
> Damien Lespiau (11):
>   drm: Refresh the explanation of debug categories
>   drm: Remove the unused (and unusable) DRM_LOG_MODE()
>   drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm: Remove the now unused DRM_LOG* macros
>   drm: Pull the test on drm_debug in the logging macros
>   drm: drm_ut_debug_printk() isn't called with NULL anywmore
>   drm: Remove the prefix argument of drm_ut_debug_printk()
>   drm: Remove the ',' after the function name in debug logs
>
>  drivers/gpu/drm/drm_stub.c                |  24 +++----
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |   2 +-
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  20 +++---
>  drivers/gpu/drm/i915/dvo_ch7xxx.c         |   4 +-
>  drivers/gpu/drm/i915/dvo_ivch.c           |  30 ++++-----
>  drivers/gpu/drm/i915/dvo_ns2501.c         |  10 +--
>  drivers/gpu/drm/i915/dvo_sil164.c         |  10 +--
>  drivers/gpu/drm/i915/dvo_tfp410.c         |  24 +++----
>  drivers/staging/imx-drm/ipuv3-plane.c     |   2 +-
>  include/drm/drmP.h                        | 106
> +++++++++++-------------------
>  11 files changed, 98 insertions(+), 136 deletions(-)
>
> --
> 1.8.3.1
>
>

[-- Attachment #1.2: Type: text/html, Size: 3221 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/11] A few patches around DRM logging

[Inki Dae]

2014-03-25 0:53 GMT+09:00 Damien Lespiau <damien.lespiau@intel.com>:
> As patch 8/11 explains, I noticed that we where evaluating the arguments to
> drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good
> reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(),
> we skip those instructions that only need to be executed when logging is on.
>
> The remaining patches are bonus clean-ups, with the main goal of removing
> DRM_LOG* macros that are not really used throughout the code base. After that,
> it's possible to simplify a bit drm_ut_debug_printk(). All pretty
> straightforward cleanups, but still worthwhile IMHO.
>
> For driver-specific patches (why some of you are Cced in that series), I'd love
> if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you
> have any objection if the driver specific patch go through the DRM tree?
> should people judge that series worthwhile, of course.

For exynos parts,

Reviewed-by: Inki Dae <inki.dae@samsung.com>

Thanks,
Inki Dae

>
> --
> Damien
>
> Damien Lespiau (11):
>   drm: Refresh the explanation of debug categories
>   drm: Remove the unused (and unusable) DRM_LOG_MODE()
>   drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()
>   drm: Remove the now unused DRM_LOG* macros
>   drm: Pull the test on drm_debug in the logging macros
>   drm: drm_ut_debug_printk() isn't called with NULL anywmore
>   drm: Remove the prefix argument of drm_ut_debug_printk()
>   drm: Remove the ',' after the function name in debug logs
>
>  drivers/gpu/drm/drm_stub.c                |  24 +++----
>  drivers/gpu/drm/exynos/exynos_drm_fbdev.c |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_plane.c |   2 +-
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c   |  20 +++---
>  drivers/gpu/drm/i915/dvo_ch7xxx.c         |   4 +-
>  drivers/gpu/drm/i915/dvo_ivch.c           |  30 ++++-----
>  drivers/gpu/drm/i915/dvo_ns2501.c         |  10 +--
>  drivers/gpu/drm/i915/dvo_sil164.c         |  10 +--
>  drivers/gpu/drm/i915/dvo_tfp410.c         |  24 +++----
>  drivers/staging/imx-drm/ipuv3-plane.c     |   2 +-
>  include/drm/drmP.h                        | 106 +++++++++++-------------------
>  11 files changed, 98 insertions(+), 136 deletions(-)
>
> --
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/11] staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS()

[Philipp Zabel]

Am Montag, den 24.03.2014, 15:53 +0000 schrieb Damien Lespiau:
> There are only a few users of the DRM_LOG_KMS() macro. We can simplify
> the DRM code a bit by replacing them by DRM_DEBUG_KMS().
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

>  drivers/staging/imx-drm/ipuv3-plane.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c
> index 34b642a..5128dc39 100644
> --- a/drivers/staging/imx-drm/ipuv3-plane.c
> +++ b/drivers/staging/imx-drm/ipuv3-plane.c
> @@ -68,7 +68,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
>  
>  	cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
>  	if (!cma_obj) {
> -		DRM_LOG_KMS("entry is null.\n");
> +		DRM_DEBUG_KMS("entry is null.\n");
>  		return -EFAULT;
>  	}
>  

Re: [PATCH 08/11] drm: Pull the test on drm_debug in the logging macros

[Jani Nikula]

On Mon, 24 Mar 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
> In the logging code, we are currently checking is we need to output in

s/is/if/

> drm_ut_debug_printk(). This is too late. The problem is that when we write
> something like:
>
>     DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
>                      connector->base.id,
>                      drm_get_connector_name(connector),
>                      connector->encoder->base.id,
>                      drm_get_encoder_name(connector->encoder));
>
> We start by evaluating the arguments (so call drm_get_connector_name() and
> drm_get_connector_name()) before ending up in drm_ut_debug_printk() which will
> then does nothing.

s/does/do/

> This means we execute a lot of instructions (drm_get_connector_name(), in turn,
> calls snprintf() for example) to happily discard them in the normal case,
> drm.debug=0.
>
> So, let's put the test on drm_debug earlier, in the macros themselves.
> Sprinkle an unlikely() as well for good measure.

Bikeshed, is it likely that the unlikely matters all that much?
https://lwn.net/Articles/70473/

I won't insist on removing them, it's more the casual nature of the
"sprinkling unlikely for good measure" that bugs me.


BR,
Jani.


>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/drm_stub.c | 26 ++++++++++++--------------
>  include/drm/drmP.h         | 27 +++++++++++++++------------
>  2 files changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index dc2c609..81ed832 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -97,26 +97,24 @@ int drm_err(const char *func, const char *format, ...)
>  }
>  EXPORT_SYMBOL(drm_err);
>  
> -void drm_ut_debug_printk(unsigned int request_level,
> -			 const char *prefix,
> +void drm_ut_debug_printk(const char *prefix,
>  			 const char *function_name,
>  			 const char *format, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
>  
> -	if (drm_debug & request_level) {
> -		va_start(args, format);
> -		vaf.fmt = format;
> -		vaf.va = &args;
> -
> -		if (function_name)
> -			printk(KERN_DEBUG "[%s:%s], %pV", prefix,
> -			       function_name, &vaf);
> -		else
> -			printk(KERN_DEBUG "%pV", &vaf);
> -		va_end(args);
> -	}
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +
> +	if (function_name)
> +		printk(KERN_DEBUG "[%s:%s], %pV", prefix,
> +		       function_name, &vaf);
> +	else
> +		printk(KERN_DEBUG "%pV", &vaf);
> +
> +	va_end(args);
>  }
>  EXPORT_SYMBOL(drm_ut_debug_printk);
>  
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 3055b36..87b558a 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -121,9 +121,8 @@ struct videomode;
>  #define DRM_UT_KMS		0x04
>  #define DRM_UT_PRIME		0x08
>  
> -extern __printf(4, 5)
> -void drm_ut_debug_printk(unsigned int request_level,
> -			 const char *prefix,
> +extern __printf(3, 4)
> +void drm_ut_debug_printk(const char *prefix,
>  			 const char *function_name,
>  			 const char *format, ...);
>  extern __printf(2, 3)
> @@ -209,24 +208,28 @@ int drm_err(const char *func, const char *format, ...);
>  #if DRM_DEBUG_CODE
>  #define DRM_DEBUG(fmt, args...)						\
>  	do {								\
> -		drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, 		\
> -					__func__, fmt, ##args);		\
> +		if (unlikely(drm_debug & DRM_UT_CORE))			\
> +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> +					    fmt, ##args);		\
>  	} while (0)
>  
>  #define DRM_DEBUG_DRIVER(fmt, args...)					\
>  	do {								\
> -		drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME,		\
> -					__func__, fmt, ##args);		\
> +		if (unlikely(drm_debug & DRM_UT_DRIVER))		\
> +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> +					    fmt, ##args);		\
>  	} while (0)
> -#define DRM_DEBUG_KMS(fmt, args...)				\
> +#define DRM_DEBUG_KMS(fmt, args...)					\
>  	do {								\
> -		drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, 		\
> -					 __func__, fmt, ##args);	\
> +		if (unlikely(drm_debug & DRM_UT_KMS))			\
> +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> +					    fmt, ##args);		\
>  	} while (0)
>  #define DRM_DEBUG_PRIME(fmt, args...)					\
>  	do {								\
> -		drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME,		\
> -					__func__, fmt, ##args);		\
> +		if (unlikely(drm_debug & DRM_UT_PRIME))			\
> +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> +					    fmt, ##args);		\
>  	} while (0)
>  #else
>  #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
> -- 
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 08/11] drm: Pull the test on drm_debug in the logging macros

[Daniel Vetter]

On Tue, Mar 25, 2014 at 11:56:24AM +0200, Jani Nikula wrote:
> On Mon, 24 Mar 2014, Damien Lespiau <damien.lespiau@intel.com> wrote:
> > In the logging code, we are currently checking is we need to output in
> 
> s/is/if/
> 
> > drm_ut_debug_printk(). This is too late. The problem is that when we write
> > something like:
> >
> >     DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
> >                      connector->base.id,
> >                      drm_get_connector_name(connector),
> >                      connector->encoder->base.id,
> >                      drm_get_encoder_name(connector->encoder));
> >
> > We start by evaluating the arguments (so call drm_get_connector_name() and
> > drm_get_connector_name()) before ending up in drm_ut_debug_printk() which will
> > then does nothing.
> 
> s/does/do/
> 
> > This means we execute a lot of instructions (drm_get_connector_name(), in turn,
> > calls snprintf() for example) to happily discard them in the normal case,
> > drm.debug=0.
> >
> > So, let's put the test on drm_debug earlier, in the macros themselves.
> > Sprinkle an unlikely() as well for good measure.
> 
> Bikeshed, is it likely that the unlikely matters all that much?
> https://lwn.net/Articles/70473/
> 
> I won't insist on removing them, it's more the casual nature of the
> "sprinkling unlikely for good measure" that bugs me.

I've considered this and since most users don't set debug every we should
be easily able to hit the 1:1M ratio seemingly required to make debug
work.

And once you enable debugging the actual printk overhead will wash out
anything you pay in branch mispredicts anyway.
-Daniel

> 
> 
> BR,
> Jani.
> 
> 
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/drm_stub.c | 26 ++++++++++++--------------
> >  include/drm/drmP.h         | 27 +++++++++++++++------------
> >  2 files changed, 27 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> > index dc2c609..81ed832 100644
> > --- a/drivers/gpu/drm/drm_stub.c
> > +++ b/drivers/gpu/drm/drm_stub.c
> > @@ -97,26 +97,24 @@ int drm_err(const char *func, const char *format, ...)
> >  }
> >  EXPORT_SYMBOL(drm_err);
> >  
> > -void drm_ut_debug_printk(unsigned int request_level,
> > -			 const char *prefix,
> > +void drm_ut_debug_printk(const char *prefix,
> >  			 const char *function_name,
> >  			 const char *format, ...)
> >  {
> >  	struct va_format vaf;
> >  	va_list args;
> >  
> > -	if (drm_debug & request_level) {
> > -		va_start(args, format);
> > -		vaf.fmt = format;
> > -		vaf.va = &args;
> > -
> > -		if (function_name)
> > -			printk(KERN_DEBUG "[%s:%s], %pV", prefix,
> > -			       function_name, &vaf);
> > -		else
> > -			printk(KERN_DEBUG "%pV", &vaf);
> > -		va_end(args);
> > -	}
> > +	va_start(args, format);
> > +	vaf.fmt = format;
> > +	vaf.va = &args;
> > +
> > +	if (function_name)
> > +		printk(KERN_DEBUG "[%s:%s], %pV", prefix,
> > +		       function_name, &vaf);
> > +	else
> > +		printk(KERN_DEBUG "%pV", &vaf);
> > +
> > +	va_end(args);
> >  }
> >  EXPORT_SYMBOL(drm_ut_debug_printk);
> >  
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 3055b36..87b558a 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -121,9 +121,8 @@ struct videomode;
> >  #define DRM_UT_KMS		0x04
> >  #define DRM_UT_PRIME		0x08
> >  
> > -extern __printf(4, 5)
> > -void drm_ut_debug_printk(unsigned int request_level,
> > -			 const char *prefix,
> > +extern __printf(3, 4)
> > +void drm_ut_debug_printk(const char *prefix,
> >  			 const char *function_name,
> >  			 const char *format, ...);
> >  extern __printf(2, 3)
> > @@ -209,24 +208,28 @@ int drm_err(const char *func, const char *format, ...);
> >  #if DRM_DEBUG_CODE
> >  #define DRM_DEBUG(fmt, args...)						\
> >  	do {								\
> > -		drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, 		\
> > -					__func__, fmt, ##args);		\
> > +		if (unlikely(drm_debug & DRM_UT_CORE))			\
> > +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> > +					    fmt, ##args);		\
> >  	} while (0)
> >  
> >  #define DRM_DEBUG_DRIVER(fmt, args...)					\
> >  	do {								\
> > -		drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME,		\
> > -					__func__, fmt, ##args);		\
> > +		if (unlikely(drm_debug & DRM_UT_DRIVER))		\
> > +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> > +					    fmt, ##args);		\
> >  	} while (0)
> > -#define DRM_DEBUG_KMS(fmt, args...)				\
> > +#define DRM_DEBUG_KMS(fmt, args...)					\
> >  	do {								\
> > -		drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, 		\
> > -					 __func__, fmt, ##args);	\
> > +		if (unlikely(drm_debug & DRM_UT_KMS))			\
> > +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> > +					    fmt, ##args);		\
> >  	} while (0)
> >  #define DRM_DEBUG_PRIME(fmt, args...)					\
> >  	do {								\
> > -		drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME,		\
> > -					__func__, fmt, ##args);		\
> > +		if (unlikely(drm_debug & DRM_UT_PRIME))			\
> > +			drm_ut_debug_printk(DRM_NAME, __func__,		\
> > +					    fmt, ##args);		\
> >  	} while (0)
> >  #else
> >  #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
> > -- 
> > 1.8.3.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/11] A few patches around DRM logging

[Dave Airlie]

On Tue, Mar 25, 2014 at 4:35 PM, Inki Dae <inki.dae@samsung.com> wrote:
> 2014-03-25 0:53 GMT+09:00 Damien Lespiau <damien.lespiau@intel.com>:
>> As patch 8/11 explains, I noticed that we where evaluating the arguments to
>> drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good
>> reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(),
>> we skip those instructions that only need to be executed when logging is on.
>>
>> The remaining patches are bonus clean-ups, with the main goal of removing
>> DRM_LOG* macros that are not really used throughout the code base. After that,
>> it's possible to simplify a bit drm_ut_debug_printk(). All pretty
>> straightforward cleanups, but still worthwhile IMHO.
>>
>> For driver-specific patches (why some of you are Cced in that series), I'd love
>> if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you
>> have any objection if the driver specific patch go through the DRM tree?
>> should people judge that series worthwhile, of course.
>
All pulled into -next, thanks,

Dave.