[RESEND PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs

[RESEND PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Hi all,
I just had the pleasure of rebasing this set on our CrOS downstream
kernel and wanted to resend it for consideration once again. There
hasn't been any resistence to the set AFAIK, just perhaps not enough
motivation for anyone to hit the go bit. There was some interest from
the msm folks about a month ago, and it has been an invaluable tool
on CrOS for the past ~year. Hopefully someone can dig into this and
provide some feedback so we can move this forward.

Thanks!

Sean

Changes since last v6:
-Rebased on drm-tip

Original v6 of the set available here:
https://patchwork.freedesktop.org/series/78133/
https://lore.kernel.org/dri-devel/20200818210510.49730-1-sean@poorly.run/

Sean Paul (14):
  drm/mipi_dbi: Convert pr_debug calls to DRM_DEBUG_DRIVER
  drm/sil164: Convert dev_printk to drm_dev_dbg
  drm/i915/utils: Replace dev_printk with drm helpers
  drm/msm/dpu: Replace definitions for dpu debug macros
  drm/print: rename drm_debug* to be more syslog-centric
  drm/amd: Gate i2c transaction logs on drm_debug_syslog
  drm/etnaviv: Change buffer dump checks to target syslog
  drm/nouveau: Change debug checks to specifically target syslog
  drm/i915: Change infoframe debug checks to specify syslog
  drm/print: Add drm_debug_category_printer
  drm/mst: Convert debug printers to debug category printers
  drm/i915: Use debug category printer for welcome message
  drm/atomic: Use debug category printer for atomic state printer
  drm/print: Add tracefs support to the drm logging helpers

 Documentation/gpu/drm-uapi.rst               |   6 +
 drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c   |   4 +-
 drivers/gpu/drm/drm_atomic_uapi.c            |   2 +-
 drivers/gpu/drm/drm_dp_mst_topology.c        |   9 +-
 drivers/gpu/drm/drm_drv.c                    |   3 +
 drivers/gpu/drm/drm_mipi_dbi.c               |   8 +-
 drivers/gpu/drm/drm_print.c                  | 242 ++++++++++++++++---
 drivers/gpu/drm/etnaviv/etnaviv_buffer.c     |   8 +-
 drivers/gpu/drm/i2c/sil164_drv.c             |  12 +-
 drivers/gpu/drm/i915/display/intel_display.c |   4 +-
 drivers/gpu/drm/i915/i915_drv.c              |   3 +-
 drivers/gpu/drm/i915/i915_utils.c            |   5 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h      |  20 +-
 drivers/gpu/drm/nouveau/nouveau_drv.h        |   4 +-
 include/drm/drm_print.h                      |  96 +++++++-
 15 files changed, 331 insertions(+), 95 deletions(-)

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[RESEND PATCH v6 01/14] drm/mipi_dbi: Convert pr_debug calls to DRM_DEBUG_DRIVER

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Use the drm logging helpers to output these messages to ensure they'll
be included by the drm tracefs instance.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-2-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/drm_mipi_dbi.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 10b4e59384ae..2f661d7ff774 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -763,9 +763,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc,
 	int i, ret;
 	u8 *dst;
 
-	if (drm_debug_enabled(DRM_UT_DRIVER))
-		pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n",
-			 __func__, dc, max_chunk);
+	DRM_DEBUG_DRIVER("dc=%d, max_chunk=%zu, transfers:\n", dc, max_chunk);
 
 	tr.speed_hz = mipi_dbi_spi_cmd_max_speed(spi, len);
 	spi_message_init_with_transfers(&m, &tr, 1);
@@ -887,9 +885,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc,
 	max_chunk = dbi->tx_buf9_len;
 	dst16 = dbi->tx_buf9;
 
-	if (drm_debug_enabled(DRM_UT_DRIVER))
-		pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n",
-			 __func__, dc, max_chunk);
+	DRM_DEBUG_DRIVER("dc=%d, max_chunk=%zu, transfers:\n", dc, max_chunk);
 
 	max_chunk = min(max_chunk / 2, len);
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 01/14] drm/mipi_dbi: Convert pr_debug calls to DRM_DEBUG_DRIVER

[jim.cromie]

On Wed, Jul 21, 2021 at 1:55 PM Sean Paul <sean@poorly.run> wrote:
>
> From: Sean Paul <seanpaul@chromium.org>
>
> Use the drm logging helpers to output these messages to ensure they'll
> be included by the drm tracefs instance.
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-2-sean@poorly.run #v5
>
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/drm_mipi_dbi.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index 10b4e59384ae..2f661d7ff774 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -763,9 +763,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *dbi, int dc,
>         int i, ret;
>         u8 *dst;
>
> -       if (drm_debug_enabled(DRM_UT_DRIVER))
> -               pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n",
> -                        __func__, dc, max_chunk);
> +       DRM_DEBUG_DRIVER("dc=%d, max_chunk=%zu, transfers:\n", dc, max_chunk);
>
>         tr.speed_hz = mipi_dbi_spi_cmd_max_speed(spi, len);
>         spi_message_init_with_transfers(&m, &tr, 1);
> @@ -887,9 +885,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *dbi, int dc,
>         max_chunk = dbi->tx_buf9_len;
>         dst16 = dbi->tx_buf9;
>
> -       if (drm_debug_enabled(DRM_UT_DRIVER))
> -               pr_debug("[drm:%s] dc=%d, max_chunk=%zu, transfers:\n",
> -                        __func__, dc, max_chunk);
> +       DRM_DEBUG_DRIVER("dc=%d, max_chunk=%zu, transfers:\n", dc, max_chunk);
>

This makes sense generically.

I note you have dropped the "[drm:__func__]" prefix.

dyndbg can provide that if user wants (+pf), so dropping from msg-body
is brevity.

but this means that dyndbg cannot select on it using
   echo "format ^[drm:funcname] +p" > control

thats ok, since "function funcname" would work,
but theres an element of moving target.

obviously, w/o dyndbg, all this is different, putting more balls in the air.

[RESEND PATCH v6 02/14] drm/sil164: Convert dev_printk to drm_dev_dbg

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Use the drm debug helper instead of dev_printk in order to leverage the
upcoming tracefs support

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-3-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/i2c/sil164_drv.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i2c/sil164_drv.c b/drivers/gpu/drm/i2c/sil164_drv.c
index 741886b54419..b315a789fca2 100644
--- a/drivers/gpu/drm/i2c/sil164_drv.c
+++ b/drivers/gpu/drm/i2c/sil164_drv.c
@@ -43,11 +43,6 @@ struct sil164_priv {
 #define to_sil164_priv(x) \
 	((struct sil164_priv *)to_encoder_slave(x)->slave_priv)
 
-#define sil164_dbg(client, format, ...) do {				\
-		if (drm_debug_enabled(DRM_UT_KMS))			\
-			dev_printk(KERN_DEBUG, &client->dev,		\
-				   "%s: " format, __func__, ## __VA_ARGS__); \
-	} while (0)
 #define sil164_info(client, format, ...)		\
 	dev_info(&client->dev, format, __VA_ARGS__)
 #define sil164_err(client, format, ...)			\
@@ -359,8 +354,8 @@ sil164_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	int rev = sil164_read(client, SIL164_REVISION);
 
 	if (vendor != 0x1 || device != 0x6) {
-		sil164_dbg(client, "Unknown device %x:%x.%x\n",
-			   vendor, device, rev);
+		drm_dev_dbg(&client->dev, DRM_UT_KMS,
+			    "Unknown device %x:%x.%x\n", vendor, device, rev);
 		return -ENODEV;
 	}
 
@@ -389,7 +384,8 @@ sil164_detect_slave(struct i2c_client *client)
 	};
 
 	if (i2c_transfer(adap, &msg, 1) != 1) {
-		sil164_dbg(adap, "No dual-link slave found.");
+		drm_dev_dbg(&adap->dev, DRM_UT_KMS,
+			    "No dual-link slave found.");
 		return NULL;
 	}
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 02/14] drm/sil164: Convert dev_printk to drm_dev_dbg

[jim.cromie]

hi Sean,

one little niggle here.

On Wed, Jul 21, 2021 at 1:55 PM Sean Paul <sean@poorly.run> wrote:
>
> From: Sean Paul <seanpaul@chromium.org>
>
> Use the drm debug helper instead of dev_printk in order to leverage the
> upcoming tracefs support
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-3-sean@poorly.run #v5
>
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/i2c/sil164_drv.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i2c/sil164_drv.c b/drivers/gpu/drm/i2c/sil164_drv.c
> index 741886b54419..b315a789fca2 100644
> --- a/drivers/gpu/drm/i2c/sil164_drv.c
> +++ b/drivers/gpu/drm/i2c/sil164_drv.c
> @@ -43,11 +43,6 @@ struct sil164_priv {
>  #define to_sil164_priv(x) \
>         ((struct sil164_priv *)to_encoder_slave(x)->slave_priv)
>
> -#define sil164_dbg(client, format, ...) do {                           \
> -               if (drm_debug_enabled(DRM_UT_KMS))                      \
> -                       dev_printk(KERN_DEBUG, &client->dev,            \
> -                                  "%s: " format, __func__, ## __VA_ARGS__); \
> -       } while (0)
>  #define sil164_info(client, format, ...)               \
>         dev_info(&client->dev, format, __VA_ARGS__)
>  #define sil164_err(client, format, ...)                        \
> @@ -359,8 +354,8 @@ sil164_probe(struct i2c_client *client, const struct i2c_device_id *id)
>         int rev = sil164_read(client, SIL164_REVISION);
>
>         if (vendor != 0x1 || device != 0x6) {
> -               sil164_dbg(client, "Unknown device %x:%x.%x\n",
> -                          vendor, device, rev);
> +               drm_dev_dbg(&client->dev, DRM_UT_KMS,
> +                           "Unknown device %x:%x.%x\n", vendor, device, rev);

we have a macro for that :-)

 #define drm_dbg_kms(drm, fmt, ...)                                     \
-       drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_KMS, fmt, ##__VA_ARGS__)
+       drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CLASS_KMS, fmt,
##__VA_ARGS__)

and again below.

[RESEND PATCH v6 03/14] drm/i915/utils: Replace dev_printk with drm helpers

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Use drm logging helpers to add support for the upcoming tracefs
implementation.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-4-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/i915/i915_utils.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_utils.c b/drivers/gpu/drm/i915/i915_utils.c
index f9e780dee9de..d858c92c6997 100644
--- a/drivers/gpu/drm/i915/i915_utils.c
+++ b/drivers/gpu/drm/i915/i915_utils.c
@@ -30,10 +30,9 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
 	vaf.va = &args;
 
 	if (is_error)
-		dev_printk(level, kdev, "%pV", &vaf);
+		drm_dev_printk(kdev, level, "%pV", &vaf);
 	else
-		dev_printk(level, kdev, "[" DRM_NAME ":%ps] %pV",
-			   __builtin_return_address(0), &vaf);
+		drm_err(&dev_priv->drm, "%pV", &vaf);
 
 	va_end(args);
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 03/14] drm/i915/utils: Replace dev_printk with drm helpers

[jim.cromie]

On Wed, Jul 21, 2021 at 1:55 PM Sean Paul <sean@poorly.run> wrote:
>
> From: Sean Paul <seanpaul@chromium.org>
>
> Use drm logging helpers to add support for the upcoming tracefs
> implementation.
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-4-sean@poorly.run #v5
>
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/i915/i915_utils.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_utils.c b/drivers/gpu/drm/i915/i915_utils.c
> index f9e780dee9de..d858c92c6997 100644
> --- a/drivers/gpu/drm/i915/i915_utils.c
> +++ b/drivers/gpu/drm/i915/i915_utils.c
> @@ -30,10 +30,9 @@ __i915_printk(struct drm_i915_private *dev_priv, const char *level,
>         vaf.va = &args;
>
>         if (is_error)
> -               dev_printk(level, kdev, "%pV", &vaf);
> +               drm_dev_printk(kdev, level, "%pV", &vaf);
>         else
> -               dev_printk(level, kdev, "[" DRM_NAME ":%ps] %pV",
> -                          __builtin_return_address(0), &vaf);
> +               drm_err(&dev_priv->drm, "%pV", &vaf);

its slightly jarring to see drm_err() in the !is_err branch.
warn or notice seems better.


>
>         va_end(args);
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
>

[RESEND PATCH v6 04/14] drm/msm/dpu: Replace definitions for dpu debug macros

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

The debug messages shouldn't be logged as errors when debug categories
are enabled. Use the drm logging helpers to do the right thing

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-5-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 323a6bce9e64..c33164d3944a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -31,27 +31,15 @@
  * DPU_DEBUG - macro for kms/plane/crtc/encoder/connector logs
  * @fmt: Pointer to format string
  */
-#define DPU_DEBUG(fmt, ...)                                                \
-	do {                                                               \
-		if (drm_debug_enabled(DRM_UT_KMS))                         \
-			DRM_DEBUG(fmt, ##__VA_ARGS__); \
-		else                                                       \
-			pr_debug(fmt, ##__VA_ARGS__);                      \
-	} while (0)
+#define DPU_DEBUG(fmt, ...) DRM_DEBUG_KMS(fmt, ##__VA_ARGS__)
 
 /**
  * DPU_DEBUG_DRIVER - macro for hardware driver logging
  * @fmt: Pointer to format string
  */
-#define DPU_DEBUG_DRIVER(fmt, ...)                                         \
-	do {                                                               \
-		if (drm_debug_enabled(DRM_UT_DRIVER))                      \
-			DRM_ERROR(fmt, ##__VA_ARGS__); \
-		else                                                       \
-			pr_debug(fmt, ##__VA_ARGS__);                      \
-	} while (0)
-
-#define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
+#define DPU_DEBUG_DRIVER(fmt, ...) DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
+
+#define DPU_ERROR(fmt, ...) DRM_ERROR(fmt, ##__VA_ARGS__)
 
 /**
  * ktime_compare_safe - compare two ktime structures
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 04/14] drm/msm/dpu: Replace definitions for dpu debug macros

[Dmitry Baryshkov]

On 21/07/2021 20:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> The debug messages shouldn't be logged as errors when debug categories
> are enabled. Use the drm logging helpers to do the right thing
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-5-sean@poorly.run #v5

The patch seems correct, but I'd propose another approach.
There is just 1 user of DPU_DEBUG_DRIVER and 29 users of DPU_DEBUG. So 
we might instead replace them with DRM_DEBUG_* calls and drop the 
DPU_DEBUG/DPU_DEBUG_DRIVER altogether. DPU_ERROR is used in ~190 places, 
so it will be more intrusive, but still doable.

> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 20 ++++----------------
>   1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 323a6bce9e64..c33164d3944a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -31,27 +31,15 @@
>    * DPU_DEBUG - macro for kms/plane/crtc/encoder/connector logs
>    * @fmt: Pointer to format string
>    */
> -#define DPU_DEBUG(fmt, ...)                                                \
> -	do {                                                               \
> -		if (drm_debug_enabled(DRM_UT_KMS))                         \
> -			DRM_DEBUG(fmt, ##__VA_ARGS__); \
> -		else                                                       \
> -			pr_debug(fmt, ##__VA_ARGS__);                      \
> -	} while (0)
> +#define DPU_DEBUG(fmt, ...) DRM_DEBUG_KMS(fmt, ##__VA_ARGS__)
>   
>   /**
>    * DPU_DEBUG_DRIVER - macro for hardware driver logging
>    * @fmt: Pointer to format string
>    */
> -#define DPU_DEBUG_DRIVER(fmt, ...)                                         \
> -	do {                                                               \
> -		if (drm_debug_enabled(DRM_UT_DRIVER))                      \
> -			DRM_ERROR(fmt, ##__VA_ARGS__); \
> -		else                                                       \
> -			pr_debug(fmt, ##__VA_ARGS__);                      \
> -	} while (0)
> -
> -#define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
> +#define DPU_DEBUG_DRIVER(fmt, ...) DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
> +
> +#define DPU_ERROR(fmt, ...) DRM_ERROR(fmt, ##__VA_ARGS__)
>   
>   /**
>    * ktime_compare_safe - compare two ktime structures
> 


-- 
With best wishes
Dmitry

Re: [Freedreno] [RESEND PATCH v6 04/14] drm/msm/dpu: Replace definitions for dpu debug macros

[abhinavk]

On 2021-07-21 10:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> The debug messages shouldn't be logged as errors when debug categories
> are enabled. Use the drm logging helpers to do the right thing
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-5-sean@poorly.run
> #v5
> 
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 323a6bce9e64..c33164d3944a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -31,27 +31,15 @@
>   * DPU_DEBUG - macro for kms/plane/crtc/encoder/connector logs
>   * @fmt: Pointer to format string
>   */
> -#define DPU_DEBUG(fmt, ...)                                            
>     \
> -	do {                                                               \
> -		if (drm_debug_enabled(DRM_UT_KMS))                         \
> -			DRM_DEBUG(fmt, ##__VA_ARGS__); \
> -		else                                                       \
> -			pr_debug(fmt, ##__VA_ARGS__);                      \
> -	} while (0)
> +#define DPU_DEBUG(fmt, ...) DRM_DEBUG_KMS(fmt, ##__VA_ARGS__)
> 
>  /**
>   * DPU_DEBUG_DRIVER - macro for hardware driver logging
>   * @fmt: Pointer to format string
>   */
> -#define DPU_DEBUG_DRIVER(fmt, ...)                                     
>     \
> -	do {                                                               \
> -		if (drm_debug_enabled(DRM_UT_DRIVER))                      \
> -			DRM_ERROR(fmt, ##__VA_ARGS__); \
> -		else                                                       \
> -			pr_debug(fmt, ##__VA_ARGS__);                      \
> -	} while (0)
> -
> -#define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
> +#define DPU_DEBUG_DRIVER(fmt, ...) DRM_DEBUG_DRIVER(fmt, 
> ##__VA_ARGS__)
> +
> +#define DPU_ERROR(fmt, ...) DRM_ERROR(fmt, ##__VA_ARGS__)
> 
>  /**
>   * ktime_compare_safe - compare two ktime structures

[RESEND PATCH v6 05/14] drm/print: rename drm_debug* to be more syslog-centric

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

In preparation for tracefs support, rename drm_debug related functions
to reflect that it targets the syslog. This will allow us to selectively
target syslog and/or tracefs.

No functional changes here.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-6-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/drm_print.c | 12 ++++++------
 include/drm/drm_print.h     | 13 +++++++++----
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 111b932cf2a9..2ff7a6ecc632 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -37,11 +37,11 @@
 #include <drm/drm_print.h>
 
 /*
- * __drm_debug: Enable debug output.
+ * __drm_debug_syslog: Enable debug output to system logs
  * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
  */
-unsigned int __drm_debug;
-EXPORT_SYMBOL(__drm_debug);
+unsigned int __drm_debug_syslog;
+EXPORT_SYMBOL(__drm_debug_syslog);
 
 MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
 "\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n"
@@ -52,7 +52,7 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
 "\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
 "\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
 "\t\tBit 8 (0x100) will enable DP messages (displayport code)");
-module_param_named(debug, __drm_debug, int, 0600);
+module_param_named(debug, __drm_debug_syslog, int, 0600);
 
 void __drm_puts_coredump(struct drm_printer *p, const char *str)
 {
@@ -160,11 +160,11 @@ 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)
+void __drm_printfn_debug_syslog(struct drm_printer *p, struct va_format *vaf)
 {
 	pr_debug("%s %pV", p->prefix, vaf);
 }
-EXPORT_SYMBOL(__drm_printfn_debug);
+EXPORT_SYMBOL(__drm_printfn_debug_syslog);
 
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
 {
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 9b66be54dd16..2ea0ffd9c1ce 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -35,7 +35,7 @@
 #include <drm/drm.h>
 
 /* Do *not* use outside of drm_print.[ch]! */
-extern unsigned int __drm_debug;
+extern unsigned int __drm_debug_syslog;
 
 /**
  * DOC: print
@@ -85,7 +85,7 @@ 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_debug_syslog(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
 
 __printf(2, 3)
@@ -227,7 +227,7 @@ static inline struct drm_printer drm_info_printer(struct device *dev)
 static inline struct drm_printer drm_debug_printer(const char *prefix)
 {
 	struct drm_printer p = {
-		.printfn = __drm_printfn_debug,
+		.printfn = __drm_printfn_debug_syslog,
 		.prefix = prefix
 	};
 	return p;
@@ -319,9 +319,14 @@ enum drm_debug_category {
 	DRM_UT_DRMRES		= 0x200,
 };
 
+static inline bool drm_debug_syslog_enabled(enum drm_debug_category category)
+{
+	return unlikely(__drm_debug_syslog & category);
+}
+
 static inline bool drm_debug_enabled(enum drm_debug_category category)
 {
-	return unlikely(__drm_debug & category);
+	return drm_debug_syslog_enabled(category);
 }
 
 /*
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 05/14] drm/print: rename drm_debug* to be more syslog-centric

[jim.cromie]

On Wed, Jul 21, 2021 at 1:55 PM Sean Paul <sean@poorly.run> wrote:
>
> From: Sean Paul <seanpaul@chromium.org>
>
> In preparation for tracefs support, rename drm_debug related functions
> to reflect that it targets the syslog. This will allow us to selectively
> target syslog and/or tracefs.
>
> No functional changes here.

I feel like a fish asking "whats water?"

from a parochial dynamic-debug view, there is only syslog.
(maybe theres also console, I try not to think of the rogue tty gonna eat me)

is there something in the tracefs work that makes this necessary ?

Re: [RESEND PATCH v6 05/14] drm/print: rename drm_debug* to be more syslog-centric

[jim.cromie]

On Wed, Jul 21, 2021 at 1:55 PM Sean Paul <sean@poorly.run> wrote:
>
> From: Sean Paul <seanpaul@chromium.org>
>
> In preparation for tracefs support, rename drm_debug related functions
> to reflect that it targets the syslog. This will allow us to selectively
> target syslog and/or tracefs.
>
> No functional changes here.
>
...
> +static inline bool drm_debug_syslog_enabled(enum drm_debug_category category)
> +{
> +       return unlikely(__drm_debug_syslog & category);
> +}
> +
>  static inline bool drm_debug_enabled(enum drm_debug_category category)
>  {
> -       return unlikely(__drm_debug & category);
> +       return drm_debug_syslog_enabled(category);
>  }
>


ok so the distinction is  this has 2  drm_debug_enabled() fns,
allowing separate decisions later.

IIUC,  in the long run, @danvet wants 0 of these.

Re: [RESEND PATCH v6 05/14] drm/print: rename drm_debug* to be more syslog-centric

[abhinavk]

On 2021-07-21 10:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> In preparation for tracefs support, rename drm_debug related functions
> to reflect that it targets the syslog. This will allow us to 
> selectively
> target syslog and/or tracefs.
> 
> No functional changes here.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-6-sean@poorly.run
> #v5
> 
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/drm_print.c | 12 ++++++------
>  include/drm/drm_print.h     | 13 +++++++++----
>  2 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 111b932cf2a9..2ff7a6ecc632 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -37,11 +37,11 @@
>  #include <drm/drm_print.h>
> 
>  /*
> - * __drm_debug: Enable debug output.
> + * __drm_debug_syslog: Enable debug output to system logs
>   * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
>   */
> -unsigned int __drm_debug;
> -EXPORT_SYMBOL(__drm_debug);
> +unsigned int __drm_debug_syslog;
> +EXPORT_SYMBOL(__drm_debug_syslog);
> 
>  MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables
> a debug category.\n"
>  "\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n"
> @@ -52,7 +52,7 @@ MODULE_PARM_DESC(debug, "Enable debug output, where
> each bit enables a debug cat
>  "\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
>  "\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
>  "\t\tBit 8 (0x100) will enable DP messages (displayport code)");
> -module_param_named(debug, __drm_debug, int, 0600);
> +module_param_named(debug, __drm_debug_syslog, int, 0600);
> 
>  void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  {
> @@ -160,11 +160,11 @@ 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)
> +void __drm_printfn_debug_syslog(struct drm_printer *p, struct 
> va_format *vaf)
>  {
>  	pr_debug("%s %pV", p->prefix, vaf);
>  }
> -EXPORT_SYMBOL(__drm_printfn_debug);
> +EXPORT_SYMBOL(__drm_printfn_debug_syslog);
> 
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
>  {
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 9b66be54dd16..2ea0ffd9c1ce 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -35,7 +35,7 @@
>  #include <drm/drm.h>
> 
>  /* Do *not* use outside of drm_print.[ch]! */
> -extern unsigned int __drm_debug;
> +extern unsigned int __drm_debug_syslog;
> 
>  /**
>   * DOC: print
> @@ -85,7 +85,7 @@ 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_debug_syslog(struct drm_printer *p, struct 
> va_format *vaf);
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
> 
>  __printf(2, 3)
> @@ -227,7 +227,7 @@ static inline struct drm_printer
> drm_info_printer(struct device *dev)
>  static inline struct drm_printer drm_debug_printer(const char *prefix)
>  {
>  	struct drm_printer p = {
> -		.printfn = __drm_printfn_debug,
> +		.printfn = __drm_printfn_debug_syslog,
>  		.prefix = prefix
>  	};
>  	return p;
> @@ -319,9 +319,14 @@ enum drm_debug_category {
>  	DRM_UT_DRMRES		= 0x200,
>  };
> 
> +static inline bool drm_debug_syslog_enabled(enum drm_debug_category 
> category)
> +{
> +	return unlikely(__drm_debug_syslog & category);
> +}
> +
>  static inline bool drm_debug_enabled(enum drm_debug_category category)
>  {
> -	return unlikely(__drm_debug & category);
> +	return drm_debug_syslog_enabled(category);
>  }
> 
>  /*

Re: [RESEND PATCH v6 05/14] drm/print: rename drm_debug* to be more syslog-centric

[abhinavk]

On 2021-07-21 10:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> In preparation for tracefs support, rename drm_debug related functions
> to reflect that it targets the syslog. This will allow us to 
> selectively
> target syslog and/or tracefs.
> 
> No functional changes here.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-6-sean@poorly.run
> #v5
> 
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/drm_print.c | 12 ++++++------
>  include/drm/drm_print.h     | 13 +++++++++----
>  2 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 111b932cf2a9..2ff7a6ecc632 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -37,11 +37,11 @@
>  #include <drm/drm_print.h>
> 
>  /*
> - * __drm_debug: Enable debug output.
> + * __drm_debug_syslog: Enable debug output to system logs
>   * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
>   */
> -unsigned int __drm_debug;
> -EXPORT_SYMBOL(__drm_debug);
> +unsigned int __drm_debug_syslog;
> +EXPORT_SYMBOL(__drm_debug_syslog);
> 
>  MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables
> a debug category.\n"
>  "\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n"
> @@ -52,7 +52,7 @@ MODULE_PARM_DESC(debug, "Enable debug output, where
> each bit enables a debug cat
>  "\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
>  "\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
>  "\t\tBit 8 (0x100) will enable DP messages (displayport code)");
> -module_param_named(debug, __drm_debug, int, 0600);
> +module_param_named(debug, __drm_debug_syslog, int, 0600);
> 
>  void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  {
> @@ -160,11 +160,11 @@ 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)
> +void __drm_printfn_debug_syslog(struct drm_printer *p, struct 
> va_format *vaf)
>  {
>  	pr_debug("%s %pV", p->prefix, vaf);
>  }
> -EXPORT_SYMBOL(__drm_printfn_debug);
> +EXPORT_SYMBOL(__drm_printfn_debug_syslog);
> 
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
>  {
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 9b66be54dd16..2ea0ffd9c1ce 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -35,7 +35,7 @@
>  #include <drm/drm.h>
> 
>  /* Do *not* use outside of drm_print.[ch]! */
> -extern unsigned int __drm_debug;
> +extern unsigned int __drm_debug_syslog;
> 
>  /**
>   * DOC: print
> @@ -85,7 +85,7 @@ 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_debug_syslog(struct drm_printer *p, struct 
> va_format *vaf);
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
> 
>  __printf(2, 3)
> @@ -227,7 +227,7 @@ static inline struct drm_printer
> drm_info_printer(struct device *dev)
>  static inline struct drm_printer drm_debug_printer(const char *prefix)
>  {
>  	struct drm_printer p = {
> -		.printfn = __drm_printfn_debug,
> +		.printfn = __drm_printfn_debug_syslog,
>  		.prefix = prefix
>  	};
>  	return p;
> @@ -319,9 +319,14 @@ enum drm_debug_category {
>  	DRM_UT_DRMRES		= 0x200,
>  };
> 
> +static inline bool drm_debug_syslog_enabled(enum drm_debug_category 
> category)
> +{
> +	return unlikely(__drm_debug_syslog & category);
> +}
> +
>  static inline bool drm_debug_enabled(enum drm_debug_category category)
>  {
> -	return unlikely(__drm_debug & category);
> +	return drm_debug_syslog_enabled(category);
>  }
> 
>  /*

[RESEND PATCH v6 06/14] drm/amd: Gate i2c transaction logs on drm_debug_syslog

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Since the logs protected by these checks specifically target syslog,
use the new drm_debug_syslog_enabled() call to avoid triggering
these prints when only trace is enabled.

Acked-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-7-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
index 5c7d769aee3f..d9ceab332060 100644
--- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
+++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
@@ -233,7 +233,7 @@ static uint32_t smu_v11_0_i2c_transmit(struct i2c_adapter *control,
 	DRM_DEBUG_DRIVER("I2C_Transmit(), address = %x, bytes = %d , data: ",
 		 (uint16_t)address, numbytes);
 
-	if (drm_debug_enabled(DRM_UT_DRIVER)) {
+	if (drm_debug_syslog_enabled(DRM_UT_DRIVER)) {
 		print_hex_dump(KERN_INFO, "data: ", DUMP_PREFIX_NONE,
 			       16, 1, data, numbytes, false);
 	}
@@ -389,7 +389,7 @@ static uint32_t smu_v11_0_i2c_receive(struct i2c_adapter *control,
 	DRM_DEBUG_DRIVER("I2C_Receive(), address = %x, bytes = %d, data :",
 		  (uint16_t)address, bytes_received);
 
-	if (drm_debug_enabled(DRM_UT_DRIVER)) {
+	if (drm_debug_syslog_enabled(DRM_UT_DRIVER)) {
 		print_hex_dump(KERN_INFO, "data: ", DUMP_PREFIX_NONE,
 			       16, 1, data, bytes_received, false);
 	}
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[RESEND PATCH v6 07/14] drm/etnaviv: Change buffer dump checks to target syslog

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Since the logs protected by these checks specifically target syslog,
use the new drm_debug_syslog_enabled() call to avoid triggering
these prints when only trace is enabled.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-8-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
index 76d38561c910..7713474800e8 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
@@ -353,7 +353,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
 
 	lockdep_assert_held(&gpu->lock);
 
-	if (drm_debug_enabled(DRM_UT_DRIVER))
+	if (drm_debug_syslog_enabled(DRM_UT_DRIVER))
 		etnaviv_buffer_dump(gpu, buffer, 0, 0x50);
 
 	link_target = etnaviv_cmdbuf_get_va(cmdbuf,
@@ -509,13 +509,13 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
 		 etnaviv_cmdbuf_get_va(buffer, &gpu->mmu_context->cmdbuf_mapping)
 		 + buffer->user_size - 4);
 
-	if (drm_debug_enabled(DRM_UT_DRIVER))
+	if (drm_debug_syslog_enabled(DRM_UT_DRIVER))
 		pr_info("stream link to 0x%08x @ 0x%08x %p\n",
 			return_target,
 			etnaviv_cmdbuf_get_va(cmdbuf, &gpu->mmu_context->cmdbuf_mapping),
 			cmdbuf->vaddr);
 
-	if (drm_debug_enabled(DRM_UT_DRIVER)) {
+	if (drm_debug_syslog_enabled(DRM_UT_DRIVER)) {
 		print_hex_dump(KERN_INFO, "cmd ", DUMP_PREFIX_OFFSET, 16, 4,
 			       cmdbuf->vaddr, cmdbuf->size, 0);
 
@@ -534,6 +534,6 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
 				    VIV_FE_LINK_HEADER_PREFETCH(link_dwords),
 				    link_target);
 
-	if (drm_debug_enabled(DRM_UT_DRIVER))
+	if (drm_debug_syslog_enabled(DRM_UT_DRIVER))
 		etnaviv_buffer_dump(gpu, buffer, 0, 0x50);
 }
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 07/14] drm/etnaviv: Change buffer dump checks to target syslog

[Lucas Stach]

Am Mittwoch, dem 21.07.2021 um 13:55 -0400 schrieb Sean Paul:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Since the logs protected by these checks specifically target syslog,
> use the new drm_debug_syslog_enabled() call to avoid triggering
> these prints when only trace is enabled.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Lucas Stach <l.stach@pengutronix.de>

> Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-8-sean@poorly.run #v5
> 
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> index 76d38561c910..7713474800e8 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> @@ -353,7 +353,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
>  
>  	lockdep_assert_held(&gpu->lock);
>  
> -	if (drm_debug_enabled(DRM_UT_DRIVER))
> +	if (drm_debug_syslog_enabled(DRM_UT_DRIVER))
>  		etnaviv_buffer_dump(gpu, buffer, 0, 0x50);
>  
>  	link_target = etnaviv_cmdbuf_get_va(cmdbuf,
> @@ -509,13 +509,13 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
>  		 etnaviv_cmdbuf_get_va(buffer, &gpu->mmu_context->cmdbuf_mapping)
>  		 + buffer->user_size - 4);
>  
> -	if (drm_debug_enabled(DRM_UT_DRIVER))
> +	if (drm_debug_syslog_enabled(DRM_UT_DRIVER))
>  		pr_info("stream link to 0x%08x @ 0x%08x %p\n",
>  			return_target,
>  			etnaviv_cmdbuf_get_va(cmdbuf, &gpu->mmu_context->cmdbuf_mapping),
>  			cmdbuf->vaddr);
>  
> -	if (drm_debug_enabled(DRM_UT_DRIVER)) {
> +	if (drm_debug_syslog_enabled(DRM_UT_DRIVER)) {
>  		print_hex_dump(KERN_INFO, "cmd ", DUMP_PREFIX_OFFSET, 16, 4,
>  			       cmdbuf->vaddr, cmdbuf->size, 0);
>  
> @@ -534,6 +534,6 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
>  				    VIV_FE_LINK_HEADER_PREFETCH(link_dwords),
>  				    link_target);
>  
> -	if (drm_debug_enabled(DRM_UT_DRIVER))
> +	if (drm_debug_syslog_enabled(DRM_UT_DRIVER))
>  		etnaviv_buffer_dump(gpu, buffer, 0, 0x50);
>  }

[RESEND PATCH v6 08/14] drm/nouveau: Change debug checks to specifically target syslog

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Since the logs protected by these checks specifically target syslog,
use the new drm_debug_syslog_enabled() call to avoid triggering
these prints when only trace is enabled.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-9-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-Rebased on drm-tip, changes in dispnv50/disp.h were rebased out
---
 drivers/gpu/drm/nouveau/nouveau_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index ba65f136cf48..2caa5720f451 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -260,11 +260,11 @@ void nouveau_drm_device_remove(struct drm_device *dev);
 #define NV_INFO(drm,f,a...) NV_PRINTK(info, &(drm)->client, f, ##a)
 
 #define NV_DEBUG(drm,f,a...) do {                                              \
-	if (drm_debug_enabled(DRM_UT_DRIVER))                                  \
+	if (drm_debug_syslog_enabled(DRM_UT_DRIVER))                                  \
 		NV_PRINTK(info, &(drm)->client, f, ##a);                       \
 } while(0)
 #define NV_ATOMIC(drm,f,a...) do {                                             \
-	if (drm_debug_enabled(DRM_UT_ATOMIC))                                  \
+	if (drm_debug_syslog_enabled(DRM_UT_ATOMIC))                                  \
 		NV_PRINTK(info, &(drm)->client, f, ##a);                       \
 } while(0)
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[RESEND PATCH v6 09/14] drm/i915: Change infoframe debug checks to specify syslog

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

Since the logs protected by these checks specifically target syslog,
use the new drm_debug_syslog_enabled() call to avoid triggering
these prints when only trace is enabled.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-10-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 65ddb6ca16e6..048d7335196b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7494,7 +7494,7 @@ static void
 intel_dump_infoframe(struct drm_i915_private *dev_priv,
 		     const union hdmi_infoframe *frame)
 {
-	if (!drm_debug_enabled(DRM_UT_KMS))
+	if (!drm_debug_syslog_enabled(DRM_UT_KMS))
 		return;
 
 	hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, frame);
@@ -8215,7 +8215,7 @@ pipe_config_infoframe_mismatch(struct drm_i915_private *dev_priv,
 			       const union hdmi_infoframe *b)
 {
 	if (fastset) {
-		if (!drm_debug_enabled(DRM_UT_KMS))
+		if (!drm_debug_syslog_enabled(DRM_UT_KMS))
 			return;
 
 		drm_dbg_kms(&dev_priv->drm,
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[RESEND PATCH v6 10/14] drm/print: Add drm_debug_category_printer

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This patch adds a new printer which will select the appropriate output
for a given debug category. Currently there is only one output target,
which is syslog. However in the future we'll have tracefs and it will be
useful to print to syslog, tracefs, or both. Drivers just need to create
the printer for the appropriate category and the printer will decide
where to send the output.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-11-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/drm_print.c |  5 +++++
 include/drm/drm_print.h     | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 2ff7a6ecc632..4d984a01b3a3 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -172,6 +172,11 @@ void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
 }
 EXPORT_SYMBOL(__drm_printfn_err);
 
+void __drm_printfn_noop(struct drm_printer *p, struct va_format *vaf)
+{
+}
+EXPORT_SYMBOL(__drm_printfn_noop);
+
 /**
  * drm_puts - print a const string to a &drm_printer stream
  * @p: the &drm printer
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2ea0ffd9c1ce..af31beeb82a1 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -87,6 +87,7 @@ 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_syslog(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
+void __drm_printfn_noop(struct drm_printer *p, struct va_format *vaf);
 
 __printf(2, 3)
 void drm_printf(struct drm_printer *p, const char *f, ...);
@@ -329,6 +330,33 @@ static inline bool drm_debug_enabled(enum drm_debug_category category)
 	return drm_debug_syslog_enabled(category);
 }
 
+/**
+ * drm_debug_category_printer - construct a &drm_printer that outputs to
+ * pr_debug() if enabled for the given category.
+ * @category: the DRM_UT_* message category this message belongs to
+ * @prefix: trace output prefix
+ *
+ * RETURNS:
+ * The &drm_printer object
+ */
+static inline struct drm_printer
+drm_debug_category_printer(enum drm_debug_category category,
+			   const char *prefix)
+{
+	struct drm_printer p = {
+		.prefix = prefix
+	};
+
+	if (drm_debug_syslog_enabled(category)) {
+		p.printfn = __drm_printfn_debug_syslog;
+	} else {
+		WARN(1, "Debug category %d is inactive.", category);
+		p.printfn = __drm_printfn_noop;
+	}
+
+	return p;
+}
+
 /*
  * struct device based logging
  *
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 10/14] drm/print: Add drm_debug_category_printer

[abhinavk]

On 2021-07-21 10:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds a new printer which will select the appropriate output
> for a given debug category. Currently there is only one output target,
> which is syslog. However in the future we'll have tracefs and it will 
> be
> useful to print to syslog, tracefs, or both. Drivers just need to 
> create
> the printer for the appropriate category and the printer will decide
> where to send the output.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-11-sean@poorly.run
> #v5
> 
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> ---
>  drivers/gpu/drm/drm_print.c |  5 +++++
>  include/drm/drm_print.h     | 28 ++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 2ff7a6ecc632..4d984a01b3a3 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -172,6 +172,11 @@ void __drm_printfn_err(struct drm_printer *p,
> struct va_format *vaf)
>  }
>  EXPORT_SYMBOL(__drm_printfn_err);
> 
> +void __drm_printfn_noop(struct drm_printer *p, struct va_format *vaf)
> +{
> +}
> +EXPORT_SYMBOL(__drm_printfn_noop);
> +
>  /**
>   * drm_puts - print a const string to a &drm_printer stream
>   * @p: the &drm printer
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 2ea0ffd9c1ce..af31beeb82a1 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -87,6 +87,7 @@ 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_syslog(struct drm_printer *p, struct 
> va_format *vaf);
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
> +void __drm_printfn_noop(struct drm_printer *p, struct va_format *vaf);
> 
>  __printf(2, 3)
>  void drm_printf(struct drm_printer *p, const char *f, ...);
> @@ -329,6 +330,33 @@ static inline bool drm_debug_enabled(enum
> drm_debug_category category)
>  	return drm_debug_syslog_enabled(category);
>  }
> 
> +/**
> + * drm_debug_category_printer - construct a &drm_printer that outputs 
> to
> + * pr_debug() if enabled for the given category.
> + * @category: the DRM_UT_* message category this message belongs to
> + * @prefix: trace output prefix
> + *
> + * RETURNS:
> + * The &drm_printer object
> + */
> +static inline struct drm_printer
> +drm_debug_category_printer(enum drm_debug_category category,
> +			   const char *prefix)
> +{
> +	struct drm_printer p = {
> +		.prefix = prefix
> +	};
> +
> +	if (drm_debug_syslog_enabled(category)) {
> +		p.printfn = __drm_printfn_debug_syslog;
> +	} else {
> +		WARN(1, "Debug category %d is inactive.", category);
> +		p.printfn = __drm_printfn_noop;
> +	}
> +
> +	return p;
> +}
> +
>  /*
>   * struct device based logging
>   *

[RESEND PATCH v6 11/14] drm/mst: Convert debug printers to debug category printers

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

The printers in dp_mst are meant to be gated on DRM_UT_DP, so use the
debug category printer to avoid dumping mst transactions to the wrong
place.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-12-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
Reviewed-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index ad0795afc21c..b1dddecad4c6 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1356,7 +1356,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_debug_category_printer(DRM_UT_DP,
+								  DBG_PREFIX);
 
 		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
 	}
@@ -2873,7 +2874,8 @@ static int process_single_tx_qlock(struct drm_dp_mst_topology_mgr *mgr,
 
 	ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
 	if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
-		struct drm_printer p = drm_debug_printer(DBG_PREFIX);
+		struct drm_printer p = drm_debug_category_printer(DRM_UT_DP,
+								  DBG_PREFIX);
 
 		drm_printf(&p, "sideband msg failed to send\n");
 		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
@@ -2917,7 +2919,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_debug_category_printer(DRM_UT_DP,
+								  DBG_PREFIX);
 
 		drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
 	}
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 11/14] drm/mst: Convert debug printers to debug category printers

[Lyude Paul]

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Wed, 2021-07-21 at 13:55 -0400, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> The printers in dp_mst are meant to be gated on DRM_UT_DP, so use the
> debug category printer to avoid dumping mst transactions to the wrong
> place.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-12-sean@poorly.run
>  #v5
> 
> Changes in v5:
> -Added to the set
> Changes in v6:
> -None
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index ad0795afc21c..b1dddecad4c6 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1356,7 +1356,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_debug_category_printer(DRM_UT_DP,
> +                                                                
> DBG_PREFIX);
>  
>                 drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
>         }
> @@ -2873,7 +2874,8 @@ static int process_single_tx_qlock(struct
> drm_dp_mst_topology_mgr *mgr,
>  
>         ret = drm_dp_send_sideband_msg(mgr, up, chunk, idx);
>         if (unlikely(ret) && drm_debug_enabled(DRM_UT_DP)) {
> -               struct drm_printer p = drm_debug_printer(DBG_PREFIX);
> +               struct drm_printer p = drm_debug_category_printer(DRM_UT_DP,
> +                                                                
> DBG_PREFIX);
>  
>                 drm_printf(&p, "sideband msg failed to send\n");
>                 drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> @@ -2917,7 +2919,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_debug_category_printer(DRM_UT_DP,
> +                                                                
> DBG_PREFIX);
>  
>                 drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
>         }

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

[RESEND PATCH v6 12/14] drm/i915: Use debug category printer for welcome message

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

The welcome printer is meant to be gated on DRM_UT_DRIVER, so use the
debug category printer to avoid dumping the message in the wrong
place.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-13-sean@poorly.run #v5

Changes in v5:
-Added to the set
Changes in v6:
-None
---
 drivers/gpu/drm/i915/i915_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c43b698bf0b9..93299af1e853 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -731,7 +731,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 static void i915_welcome_messages(struct drm_i915_private *dev_priv)
 {
 	if (drm_debug_enabled(DRM_UT_DRIVER)) {
-		struct drm_printer p = drm_debug_printer("i915 device info:");
+		struct drm_printer p = drm_debug_category_printer(DRM_UT_DRIVER,
+						"i915 device info:");
 
 		drm_printf(&p, "pciid=0x%04x rev=0x%02x platform=%s (subplatform=0x%x) gen=%i\n",
 			   INTEL_DEVID(dev_priv),
-- 
Sean Paul, Software Engineer, Google / Chromium OS

[RESEND PATCH v6 13/14] drm/atomic: Use debug category printer for atomic state printer

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

The atomic state is printed if the DRM_UT_STATE is active, but it's
printed at INFO level. This patch converts it to use the debug
category printer so:

a- it's consistent with other DRM_UT_STATE logging
b- it's properly routed through drm_trace when introduced

Signed-off-by: Sean Paul <seanpaul@chromium.org>

Changes in v6:
-Added to the set
---
 drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 7e48d40600ff..7615ded60195 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1322,7 +1322,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 	struct drm_out_fence_state *fence_state;
 	int ret = 0;
 	unsigned int i, j, num_fences;
-	struct drm_printer p = drm_info_printer(dev->dev);
+	struct drm_printer p = drm_debug_category_printer(DRM_UT_STATE, "commit_state");
 
 	/* disallow for drivers not supporting atomic: */
 	if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 13/14] drm/atomic: Use debug category printer for atomic state printer

[abhinavk]

On 2021-07-21 10:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> The atomic state is printed if the DRM_UT_STATE is active, but it's
> printed at INFO level. This patch converts it to use the debug
> category printer so:
> 
> a- it's consistent with other DRM_UT_STATE logging
> b- it's properly routed through drm_trace when introduced
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Abhinav Kumar <abhinavk@codeaurora.org>
> 
> Changes in v6:
> -Added to the set
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index 7e48d40600ff..7615ded60195 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1322,7 +1322,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  	struct drm_out_fence_state *fence_state;
>  	int ret = 0;
>  	unsigned int i, j, num_fences;
> -	struct drm_printer p = drm_info_printer(dev->dev);
> +	struct drm_printer p = drm_debug_category_printer(DRM_UT_STATE,
> "commit_state");
> 
>  	/* disallow for drivers not supporting atomic: */
>  	if (!drm_core_check_feature(dev, DRIVER_ATOMIC))

[RESEND PATCH v6 14/14] drm/print: Add tracefs support to the drm logging helpers

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This patch adds a new module parameter called drm.trace which accepts
the same mask as drm.debug. When a debug category is enabled, log
messages will be put in a new tracefs instance called drm for
consumption.

Using the new tracefs instance will allow distros to enable drm logging
in production without impacting performance or spamming the system
logs.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Steven Rostedt <rostedt@goodmis.org>
Reported-by: kernel test robot <lkp@intel.com> # warning reported in v6
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-sean@poorly.run #v1
Link: https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html #v2
Link: https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-sean@poorly.run #v3
Link: https://patchwork.freedesktop.org/patch/msgid/20200114172155.215463-1-sean@poorly.run #v4
Link: https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-14-sean@poorly.run #v5
Link: https://patchwork.freedesktop.org/patch/msgid/20200818210510.49730-15-sean@poorly.run #v6

Changes in v5:
-Re-write to use trace_array and the tracefs instance support
Changes in v6:
-Use the new trace_array_init_printk() to initialize global trace
 buffers
Changes in v6.5:
-Fix kernel test robot warning
-Add a trace printf in __drm_err
---
 Documentation/gpu/drm-uapi.rst |   6 +
 drivers/gpu/drm/drm_drv.c      |   3 +
 drivers/gpu/drm/drm_print.c    | 223 ++++++++++++++++++++++++++++-----
 include/drm/drm_print.h        |  63 ++++++++--
 4 files changed, 255 insertions(+), 40 deletions(-)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 7e51dd40bf6e..ce1ea39fb4b9 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -424,6 +424,12 @@ Debugfs Support
 .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
    :export:
 
+DRM Tracing
+---------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_print.c
+   :doc: DRM Tracing
+
 Sysfs Support
 =============
 
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8804ec7d3215..71dc0b161b51 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1034,12 +1034,15 @@ static void drm_core_exit(void)
 	drm_sysfs_destroy();
 	idr_destroy(&drm_minors_idr);
 	drm_connector_ida_destroy();
+	drm_trace_cleanup();
 }
 
 static int __init drm_core_init(void)
 {
 	int ret;
 
+	drm_trace_init();
+
 	drm_connector_ida_init();
 	idr_init(&drm_minors_idr);
 	drm_memcpy_init_early();
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 4d984a01b3a3..64d9a724c2df 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -31,6 +31,7 @@
 #include <linux/moduleparam.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
+#include <linux/trace.h>
 
 #include <drm/drm.h>
 #include <drm/drm_drv.h>
@@ -43,17 +44,34 @@
 unsigned int __drm_debug_syslog;
 EXPORT_SYMBOL(__drm_debug_syslog);
 
-MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
-"\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n"
-"\t\tBit 1 (0x02)  will enable DRIVER messages (drm controller code)\n"
-"\t\tBit 2 (0x04)  will enable KMS messages (modesetting code)\n"
-"\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n"
-"\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n"
-"\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
-"\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
-"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
+/*
+ * __drm_debug_trace: Enable debug output in drm tracing instance.
+ * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
+ */
+unsigned int __drm_debug_trace;
+EXPORT_SYMBOL(__drm_debug_trace);
+
+#define DEBUG_PARM_DESC(dst) \
+"Enable debug output to " dst ", where each bit enables a debug category.\n" \
+"\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n" \
+"\t\tBit 1 (0x02)  will enable DRIVER messages (drm controller code)\n" \
+"\t\tBit 2 (0x04)  will enable KMS messages (modesetting code)\n" \
+"\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n" \
+"\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n" \
+"\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n" \
+"\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n" \
+"\t\tBit 8 (0x100) will enable DP messages (displayport code)"
+
+MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog"));
 module_param_named(debug, __drm_debug_syslog, int, 0600);
 
+MODULE_PARM_DESC(trace, DEBUG_PARM_DESC("tracefs"));
+module_param_named(trace, __drm_debug_trace, int, 0600);
+
+#ifdef CONFIG_TRACING
+struct trace_array *trace_arr;
+#endif
+
 void __drm_puts_coredump(struct drm_printer *p, const char *str)
 {
 	struct drm_print_iterator *iterator = p->arg;
@@ -166,6 +184,20 @@ void __drm_printfn_debug_syslog(struct drm_printer *p, struct va_format *vaf)
 }
 EXPORT_SYMBOL(__drm_printfn_debug_syslog);
 
+void __drm_printfn_trace(struct drm_printer *p, struct va_format *vaf)
+{
+	drm_trace_printf("%s %pV", p->prefix, vaf);
+}
+EXPORT_SYMBOL(__drm_printfn_trace);
+
+void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
+					   struct va_format *vaf)
+{
+	pr_debug("%s %pV", p->prefix, vaf);
+	drm_trace_printf("%s %pV", p->prefix, vaf);
+}
+EXPORT_SYMBOL(__drm_printfn_debug_syslog_and_trace);
+
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
 {
 	pr_err("*ERROR* %s %pV", p->prefix, vaf);
@@ -246,6 +278,14 @@ void drm_dev_printk(const struct device *dev, const char *level,
 	struct va_format vaf;
 	va_list args;
 
+	va_start(args, format);
+	vaf.fmt = format;
+	vaf.va = &args;
+	drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
+			 dev ? dev_name(dev) : "",dev ? " " : "",
+			 __builtin_return_address(0), &vaf);
+	va_end(args);
+
 	va_start(args, format);
 	vaf.fmt = format;
 	vaf.va = &args;
@@ -267,21 +307,30 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
 	struct va_format vaf;
 	va_list args;
 
-	if (!drm_debug_enabled(category))
-		return;
+	if (drm_debug_syslog_enabled(category)) {
+		va_start(args, format);
+		vaf.fmt = format;
+		vaf.va = &args;
 
-	va_start(args, format);
-	vaf.fmt = format;
-	vaf.va = &args;
+		if (dev)
+			dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
+				__builtin_return_address(0), &vaf);
+		else
+			printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
+			__builtin_return_address(0), &vaf);
 
-	if (dev)
-		dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
-			   __builtin_return_address(0), &vaf);
-	else
-		printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
-		       __builtin_return_address(0), &vaf);
+		va_end(args);
+	}
 
-	va_end(args);
+	if (drm_debug_trace_enabled(category)) {
+		va_start(args, format);
+		vaf.fmt = format;
+		vaf.va = &args;
+		drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
+				 dev ? dev_name(dev) : "", dev ? " " : "",
+				 __builtin_return_address(0), &vaf);
+		va_end(args);
+	}
 }
 EXPORT_SYMBOL(drm_dev_dbg);
 
@@ -290,17 +339,25 @@ void __drm_dbg(enum drm_debug_category category, const char *format, ...)
 	struct va_format vaf;
 	va_list args;
 
-	if (!drm_debug_enabled(category))
-		return;
+	if (drm_debug_syslog_enabled(category)) {
+		va_start(args, format);
+		vaf.fmt = format;
+		vaf.va = &args;
 
-	va_start(args, format);
-	vaf.fmt = format;
-	vaf.va = &args;
+		printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
+		__builtin_return_address(0), &vaf);
 
-	printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
-	       __builtin_return_address(0), &vaf);
+		va_end(args);
+	}
 
-	va_end(args);
+	if (drm_debug_trace_enabled(category)) {
+		va_start(args, format);
+		vaf.fmt = format;
+		vaf.va = &args;
+		drm_trace_printf("[" DRM_NAME ":%ps] %pV",
+				 __builtin_return_address(0), &vaf);
+		va_end(args);
+	}
 }
 EXPORT_SYMBOL(__drm_dbg);
 
@@ -317,6 +374,13 @@ void __drm_err(const char *format, ...)
 	       __builtin_return_address(0), &vaf);
 
 	va_end(args);
+
+	va_start(args, format);
+	vaf.fmt = format;
+	vaf.va = &args;
+	drm_trace_printf("[" DRM_NAME ":%ps] *ERROR* %pV",
+				__builtin_return_address(0), &vaf);
+	va_end(args);
 }
 EXPORT_SYMBOL(__drm_err);
 
@@ -347,3 +411,104 @@ void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
 	}
 }
 EXPORT_SYMBOL(drm_print_regset32);
+
+
+/**
+ * DOC: DRM Tracing
+ *
+ * *tl;dr* DRM tracing is a lightweight alternative to traditional DRM debug
+ * logging.
+ *
+ * While DRM logging is quite convenient when reproducing a specific issue, it
+ * doesn't help when something goes wrong unexpectedly. There are a couple
+ * reasons why one does not want to enable DRM logging at all times:
+ *
+ * 1. We don't want to overwhelm syslog with drm spam, others have to use it too
+ * 2. Console logging is slow
+ *
+ * DRM tracing aims to solve both these problems.
+ *
+ * To use DRM tracing, set the drm.trace module parameter (via cmdline or sysfs)
+ * to a DRM debug category mask (this is a bitmask of &drm_debug_category
+ * values):
+ * ::
+ *
+ *    eg: echo 0x106 > /sys/module/drm/parameters/trace
+ *
+ * Once active, all log messages in the specified categories will be written to
+ * the DRM trace. Once at capacity, the trace will overwrite old messages with
+ * new ones. At any point, one can read the trace file to extract the previous N
+ * DRM messages:
+ * ::
+ *
+ *    eg: cat /sys/kernel/tracing/instances/drm/trace
+ *
+ * Considerations
+ * **************
+ * The trace is subsystem wide, so if you have multiple devices active, they
+ * will be adding logs to the same trace.
+ *
+ * The contents of the DRM Trace are **not** considered UABI. **DO NOT depend on
+ * the values of these traces in your userspace.** These traces are intended for
+ * entertainment purposes only. The contents of these logs carry no warranty,
+ * expressed or implied.
+ */
+
+
+#ifdef CONFIG_TRACING
+
+/**
+ * drm_trace_init - initializes the drm trace array
+ *
+ * This function fetches (or creates) the drm trace array. This should be called
+ * once on drm subsystem creation and matched with drm_trace_cleanup().
+ */
+void drm_trace_init(void)
+{
+	int ret;
+
+	trace_arr = trace_array_get_by_name("drm");
+	if (!trace_arr)
+		return;
+
+	ret = trace_array_init_printk(trace_arr);
+	if (ret)
+		drm_trace_cleanup();
+}
+EXPORT_SYMBOL(drm_trace_init);
+
+/**
+ * drm_trace_printf - adds an entry to the drm tracefs instance
+ * @format: printf format of the message to add to the trace
+ *
+ * This function adds a new entry in the drm tracefs instance
+ */
+void drm_trace_printf(const char *format, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, format);
+	vaf.fmt = format;
+	vaf.va = &args;
+	trace_array_printk(trace_arr, _THIS_IP_, "%pV", &vaf);
+	va_end(args);
+}
+
+/**
+ * drm_trace_cleanup - destroys the drm trace array
+ *
+ * This function destroys the drm trace array created with drm_trace_init. This
+ * should be called once on drm subsystem close and matched with
+ * drm_trace_init().
+ */
+void drm_trace_cleanup(void)
+{
+	if (trace_arr) {
+		trace_array_put(trace_arr);
+		trace_array_destroy(trace_arr);
+		trace_arr = NULL;
+	}
+}
+EXPORT_SYMBOL(drm_trace_cleanup);
+#endif
\ No newline at end of file
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index af31beeb82a1..4609a2f4a425 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -36,12 +36,13 @@
 
 /* Do *not* use outside of drm_print.[ch]! */
 extern unsigned int __drm_debug_syslog;
+extern unsigned int __drm_debug_trace;
 
 /**
  * DOC: print
  *
  * A simple wrapper for dev_printk(), seq_printf(), etc.  Allows same
- * debug code to be used for both debugfs and printk logging.
+ * debug code to be used for debugfs, printk and tracefs logging.
  *
  * For example::
  *
@@ -86,6 +87,9 @@ 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_syslog(struct drm_printer *p, struct va_format *vaf);
+void __drm_printfn_trace(struct drm_printer *p, struct va_format *vaf);
+void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
+					   struct va_format *vaf);
 void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
 void __drm_printfn_noop(struct drm_printer *p, struct va_format *vaf);
 
@@ -219,7 +223,8 @@ static inline struct drm_printer drm_info_printer(struct device *dev)
 }
 
 /**
- * drm_debug_printer - construct a &drm_printer that outputs to pr_debug()
+ * drm_debug_printer - construct a &drm_printer that outputs to pr_debug() and
+ * drm tracefs
  * @prefix: debug output prefix
  *
  * RETURNS:
@@ -228,7 +233,7 @@ static inline struct drm_printer drm_info_printer(struct device *dev)
 static inline struct drm_printer drm_debug_printer(const char *prefix)
 {
 	struct drm_printer p = {
-		.printfn = __drm_printfn_debug_syslog,
+		.printfn = __drm_printfn_debug_syslog_and_trace,
 		.prefix = prefix
 	};
 	return p;
@@ -254,14 +259,14 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
  * 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.
+ * is filtered by the drm.debug and drm.trace module parameters. 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:
+ * Enabling verbose debug messages is done through the drm.debug and drm.trace
+ * parameters, each category being enabled by a bit:
  *
  *  - drm.debug=0x1 will enable CORE messages
  *  - drm.debug=0x2 will enable DRIVER messages
@@ -270,10 +275,14 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
  *  - drm.debug=0x1ff will enable all messages
  *
  * An interesting feature is that it's possible to enable verbose logging at
- * run-time by echoing the debug value in its sysfs node::
+ * run-time by echoing the debug category value in its sysfs node::
  *
+ *   # For syslog logging:
  *   # echo 0xf > /sys/module/drm/parameters/debug
  *
+ *   # For tracefs logging:
+ *   # echo 0xf > /sys/module/drm/parameters/trace
+ *
  */
 enum drm_debug_category {
 	/**
@@ -325,14 +334,20 @@ static inline bool drm_debug_syslog_enabled(enum drm_debug_category category)
 	return unlikely(__drm_debug_syslog & category);
 }
 
+static inline bool drm_debug_trace_enabled(enum drm_debug_category category)
+{
+	return unlikely(__drm_debug_trace & category);
+}
+
 static inline bool drm_debug_enabled(enum drm_debug_category category)
 {
-	return drm_debug_syslog_enabled(category);
+	return drm_debug_syslog_enabled(category) ||
+	       drm_debug_trace_enabled(category);
 }
 
 /**
  * drm_debug_category_printer - construct a &drm_printer that outputs to
- * pr_debug() if enabled for the given category.
+ * pr_debug() and/or the drm tracefs instance if enabled for the given category.
  * @category: the DRM_UT_* message category this message belongs to
  * @prefix: trace output prefix
  *
@@ -347,8 +362,13 @@ drm_debug_category_printer(enum drm_debug_category category,
 		.prefix = prefix
 	};
 
-	if (drm_debug_syslog_enabled(category)) {
+	if (drm_debug_syslog_enabled(category) &&
+	    drm_debug_trace_enabled(category)) {
+		p.printfn = __drm_printfn_debug_syslog_and_trace;
+	} else if (drm_debug_syslog_enabled(category)) {
 		p.printfn = __drm_printfn_debug_syslog;
+	} else if (drm_debug_trace_enabled(category)) {
+		p.printfn = __drm_printfn_trace;
 	} else {
 		WARN(1, "Debug category %d is inactive.", category);
 		p.printfn = __drm_printfn_noop;
@@ -357,6 +377,27 @@ drm_debug_category_printer(enum drm_debug_category category,
 	return p;
 }
 
+
+#ifdef CONFIG_TRACING
+void drm_trace_init(void);
+__printf(1, 2)
+void drm_trace_printf(const char *format, ...);
+void drm_trace_cleanup(void);
+#else
+static inline void drm_trace_init(void)
+{
+}
+
+__printf(1, 2)
+static inline void drm_trace_printf(const char *format, ...)
+{
+}
+
+static inline void drm_trace_cleanup(void)
+{
+}
+#endif
+
 /*
  * struct device based logging
  *
-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [RESEND PATCH v6 14/14] drm/print: Add tracefs support to the drm logging helpers

[Rodrigo Siqueira]

On 07/21, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds a new module parameter called drm.trace which accepts
> the same mask as drm.debug. When a debug category is enabled, log
> messages will be put in a new tracefs instance called drm for
> consumption.
> 
> Using the new tracefs instance will allow distros to enable drm logging
> in production without impacting performance or spamming the system
> logs.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Reported-by: kernel test robot <lkp@intel.com> # warning reported in v6
> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2Fmsgid%2F20191010204823.195540-1-sean%40poorly.run&amp;data=04%7C01%7CRodrigo.Siqueira%40amd.com%7Cbb3d63341a974f70c29208d94c70e119%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637624869979178761%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NEMZqbDJcEq%2B5Ext3F3%2BLkLNe7JHFa7HNWozh5boJ9M%3D&amp;reserved=0 #v1
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2019-November%2F243230.html&amp;data=04%7C01%7CRodrigo.Siqueira%40amd.com%7Cbb3d63341a974f70c29208d94c70e119%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637624869979178761%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=nA9bS%2BIsuMxcqdTzf9aClkl3ovccPVnInyFyBpDT8Do%3D&amp;reserved=0 #v2
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2Fmsgid%2F20191212203301.142437-1-sean%40poorly.run&amp;data=04%7C01%7CRodrigo.Siqueira%40amd.com%7Cbb3d63341a974f70c29208d94c70e119%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637624869979178761%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Vz6I8SF%2BjoajAR48%2BzJmx6llQAHtww9VROLT4krkZK4%3D&amp;reserved=0 #v3
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2Fmsgid%2F20200114172155.215463-1-sean%40poorly.run&amp;data=04%7C01%7CRodrigo.Siqueira%40amd.com%7Cbb3d63341a974f70c29208d94c70e119%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637624869979178761%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Vcjao5eEZEE6o1Q%2B8CyIalz0UTAiqydOxR8qISbh61I%3D&amp;reserved=0 #v4
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2Fmsgid%2F20200608210505.48519-14-sean%40poorly.run&amp;data=04%7C01%7CRodrigo.Siqueira%40amd.com%7Cbb3d63341a974f70c29208d94c70e119%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637624869979178761%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=rc1O7r2SQqR3I3Rn0bSRbS9mZBBH%2Fg7%2BEmGcLexheO0%3D&amp;reserved=0 #v5
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2Fmsgid%2F20200818210510.49730-15-sean%40poorly.run&amp;data=04%7C01%7CRodrigo.Siqueira%40amd.com%7Cbb3d63341a974f70c29208d94c70e119%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637624869979178761%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=PblinqQ5Y8NgHJKGZ%2BwUU7hIlZHwiy0SOsXhB7NPegw%3D&amp;reserved=0 #v6
> 
> Changes in v5:
> -Re-write to use trace_array and the tracefs instance support
> Changes in v6:
> -Use the new trace_array_init_printk() to initialize global trace
>  buffers
> Changes in v6.5:
> -Fix kernel test robot warning
> -Add a trace printf in __drm_err
> ---
>  Documentation/gpu/drm-uapi.rst |   6 +
>  drivers/gpu/drm/drm_drv.c      |   3 +
>  drivers/gpu/drm/drm_print.c    | 223 ++++++++++++++++++++++++++++-----
>  include/drm/drm_print.h        |  63 ++++++++--
>  4 files changed, 255 insertions(+), 40 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 7e51dd40bf6e..ce1ea39fb4b9 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -424,6 +424,12 @@ Debugfs Support
>  .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
>     :export:
>  
> +DRM Tracing
> +---------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_print.c
> +   :doc: DRM Tracing
> +
>  Sysfs Support
>  =============
>  
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8804ec7d3215..71dc0b161b51 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1034,12 +1034,15 @@ static void drm_core_exit(void)
>  	drm_sysfs_destroy();
>  	idr_destroy(&drm_minors_idr);
>  	drm_connector_ida_destroy();
> +	drm_trace_cleanup();
>  }
>  
>  static int __init drm_core_init(void)
>  {
>  	int ret;
>  
> +	drm_trace_init();
> +
>  	drm_connector_ida_init();
>  	idr_init(&drm_minors_idr);
>  	drm_memcpy_init_early();
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 4d984a01b3a3..64d9a724c2df 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -31,6 +31,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> +#include <linux/trace.h>
>  
>  #include <drm/drm.h>
>  #include <drm/drm_drv.h>
> @@ -43,17 +44,34 @@
>  unsigned int __drm_debug_syslog;
>  EXPORT_SYMBOL(__drm_debug_syslog);
>  
> -MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
> -"\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n"
> -"\t\tBit 1 (0x02)  will enable DRIVER messages (drm controller code)\n"
> -"\t\tBit 2 (0x04)  will enable KMS messages (modesetting code)\n"
> -"\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n"
> -"\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n"
> -"\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
> -"\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
> -"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
> +/*
> + * __drm_debug_trace: Enable debug output in drm tracing instance.
> + * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
> + */
> +unsigned int __drm_debug_trace;
> +EXPORT_SYMBOL(__drm_debug_trace);
> +
> +#define DEBUG_PARM_DESC(dst) \
> +"Enable debug output to " dst ", where each bit enables a debug category.\n" \
> +"\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n" \
> +"\t\tBit 1 (0x02)  will enable DRIVER messages (drm controller code)\n" \
> +"\t\tBit 2 (0x04)  will enable KMS messages (modesetting code)\n" \
> +"\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n" \
> +"\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n" \
> +"\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n" \
> +"\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n" \
> +"\t\tBit 8 (0x100) will enable DP messages (displayport code)"
> +
> +MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog"));
>  module_param_named(debug, __drm_debug_syslog, int, 0600);
>  
> +MODULE_PARM_DESC(trace, DEBUG_PARM_DESC("tracefs"));
> +module_param_named(trace, __drm_debug_trace, int, 0600);
> +
> +#ifdef CONFIG_TRACING
> +struct trace_array *trace_arr;
> +#endif
> +
>  void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  {
>  	struct drm_print_iterator *iterator = p->arg;
> @@ -166,6 +184,20 @@ void __drm_printfn_debug_syslog(struct drm_printer *p, struct va_format *vaf)
>  }
>  EXPORT_SYMBOL(__drm_printfn_debug_syslog);
>  
> +void __drm_printfn_trace(struct drm_printer *p, struct va_format *vaf)
> +{
> +	drm_trace_printf("%s %pV", p->prefix, vaf);
> +}
> +EXPORT_SYMBOL(__drm_printfn_trace);
> +
> +void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
> +					   struct va_format *vaf)
> +{
> +	pr_debug("%s %pV", p->prefix, vaf);
> +	drm_trace_printf("%s %pV", p->prefix, vaf);
> +}
> +EXPORT_SYMBOL(__drm_printfn_debug_syslog_and_trace);
> +
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
>  {
>  	pr_err("*ERROR* %s %pV", p->prefix, vaf);
> @@ -246,6 +278,14 @@ void drm_dev_printk(const struct device *dev, const char *level,
>  	struct va_format vaf;
>  	va_list args;
>  
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +	drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
> +			 dev ? dev_name(dev) : "",dev ? " " : "",
> +			 __builtin_return_address(0), &vaf);
> +	va_end(args);
> +
>  	va_start(args, format);
>  	vaf.fmt = format;
>  	vaf.va = &args;
> @@ -267,21 +307,30 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
>  	struct va_format vaf;
>  	va_list args;
>  
> -	if (!drm_debug_enabled(category))
> -		return;
> +	if (drm_debug_syslog_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
>  
> -	va_start(args, format);
> -	vaf.fmt = format;
> -	vaf.va = &args;
> +		if (dev)
> +			dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
> +				__builtin_return_address(0), &vaf);
> +		else
> +			printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> +			__builtin_return_address(0), &vaf);
>  
> -	if (dev)
> -		dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
> -			   __builtin_return_address(0), &vaf);
> -	else
> -		printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> -		       __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
>  
> -	va_end(args);
> +	if (drm_debug_trace_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
> +		drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
> +				 dev ? dev_name(dev) : "", dev ? " " : "",
> +				 __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
>  }
>  EXPORT_SYMBOL(drm_dev_dbg);
>  
> @@ -290,17 +339,25 @@ void __drm_dbg(enum drm_debug_category category, const char *format, ...)
>  	struct va_format vaf;
>  	va_list args;
>  
> -	if (!drm_debug_enabled(category))
> -		return;
> +	if (drm_debug_syslog_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
>  
> -	va_start(args, format);
> -	vaf.fmt = format;
> -	vaf.va = &args;
> +		printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> +		__builtin_return_address(0), &vaf);
>  
> -	printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> -	       __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
>  
> -	va_end(args);
> +	if (drm_debug_trace_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
> +		drm_trace_printf("[" DRM_NAME ":%ps] %pV",
> +				 __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
>  }
>  EXPORT_SYMBOL(__drm_dbg);
>  
> @@ -317,6 +374,13 @@ void __drm_err(const char *format, ...)
>  	       __builtin_return_address(0), &vaf);
>  
>  	va_end(args);
> +
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +	drm_trace_printf("[" DRM_NAME ":%ps] *ERROR* %pV",
> +				__builtin_return_address(0), &vaf);
> +	va_end(args);
>  }
>  EXPORT_SYMBOL(__drm_err);
>  
> @@ -347,3 +411,104 @@ void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
>  	}
>  }
>  EXPORT_SYMBOL(drm_print_regset32);
> +
> +
> +/**
> + * DOC: DRM Tracing
> + *
> + * *tl;dr* DRM tracing is a lightweight alternative to traditional DRM debug
> + * logging.
> + *
> + * While DRM logging is quite convenient when reproducing a specific issue, it
> + * doesn't help when something goes wrong unexpectedly. There are a couple
> + * reasons why one does not want to enable DRM logging at all times:
> + *
> + * 1. We don't want to overwhelm syslog with drm spam, others have to use it too
> + * 2. Console logging is slow
> + *
> + * DRM tracing aims to solve both these problems.
> + *
> + * To use DRM tracing, set the drm.trace module parameter (via cmdline or sysfs)
> + * to a DRM debug category mask (this is a bitmask of &drm_debug_category
> + * values):
> + * ::
> + *
> + *    eg: echo 0x106 > /sys/module/drm/parameters/trace
> + *
> + * Once active, all log messages in the specified categories will be written to
> + * the DRM trace. Once at capacity, the trace will overwrite old messages with
> + * new ones. At any point, one can read the trace file to extract the previous N
> + * DRM messages:
> + * ::
> + *
> + *    eg: cat /sys/kernel/tracing/instances/drm/trace
> + *
> + * Considerations
> + * **************
> + * The trace is subsystem wide, so if you have multiple devices active, they
> + * will be adding logs to the same trace.
> + *
> + * The contents of the DRM Trace are **not** considered UABI. **DO NOT depend on
> + * the values of these traces in your userspace.** These traces are intended for
> + * entertainment purposes only. The contents of these logs carry no warranty,
> + * expressed or implied.
> + */
> +
> +
> +#ifdef CONFIG_TRACING
> +
> +/**
> + * drm_trace_init - initializes the drm trace array
> + *
> + * This function fetches (or creates) the drm trace array. This should be called
> + * once on drm subsystem creation and matched with drm_trace_cleanup().
> + */
> +void drm_trace_init(void)
> +{
> +	int ret;
> +
> +	trace_arr = trace_array_get_by_name("drm");
> +	if (!trace_arr)
> +		return;
> +
> +	ret = trace_array_init_printk(trace_arr);
> +	if (ret)
> +		drm_trace_cleanup();
> +}
> +EXPORT_SYMBOL(drm_trace_init);
> +
> +/**
> + * drm_trace_printf - adds an entry to the drm tracefs instance
> + * @format: printf format of the message to add to the trace
> + *
> + * This function adds a new entry in the drm tracefs instance
> + */
> +void drm_trace_printf(const char *format, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +	trace_array_printk(trace_arr, _THIS_IP_, "%pV", &vaf);
> +	va_end(args);
> +}
> +
> +/**
> + * drm_trace_cleanup - destroys the drm trace array
> + *
> + * This function destroys the drm trace array created with drm_trace_init. This
> + * should be called once on drm subsystem close and matched with
> + * drm_trace_init().
> + */
> +void drm_trace_cleanup(void)
> +{
> +	if (trace_arr) {
> +		trace_array_put(trace_arr);
> +		trace_array_destroy(trace_arr);
> +		trace_arr = NULL;
> +	}
> +}
> +EXPORT_SYMBOL(drm_trace_cleanup);
> +#endif
> \ No newline at end of file
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index af31beeb82a1..4609a2f4a425 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -36,12 +36,13 @@
>  
>  /* Do *not* use outside of drm_print.[ch]! */
>  extern unsigned int __drm_debug_syslog;
> +extern unsigned int __drm_debug_trace;
>  
>  /**
>   * DOC: print
>   *
>   * A simple wrapper for dev_printk(), seq_printf(), etc.  Allows same
> - * debug code to be used for both debugfs and printk logging.
> + * debug code to be used for debugfs, printk and tracefs logging.
>   *
>   * For example::
>   *
> @@ -86,6 +87,9 @@ 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_syslog(struct drm_printer *p, struct va_format *vaf);
> +void __drm_printfn_trace(struct drm_printer *p, struct va_format *vaf);
> +void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
> +					   struct va_format *vaf);
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_noop(struct drm_printer *p, struct va_format *vaf);
>  
> @@ -219,7 +223,8 @@ static inline struct drm_printer drm_info_printer(struct device *dev)
>  }
>  
>  /**
> - * drm_debug_printer - construct a &drm_printer that outputs to pr_debug()
> + * drm_debug_printer - construct a &drm_printer that outputs to pr_debug() and
> + * drm tracefs
>   * @prefix: debug output prefix
>   *
>   * RETURNS:
> @@ -228,7 +233,7 @@ static inline struct drm_printer drm_info_printer(struct device *dev)
>  static inline struct drm_printer drm_debug_printer(const char *prefix)
>  {
>  	struct drm_printer p = {
> -		.printfn = __drm_printfn_debug_syslog,
> +		.printfn = __drm_printfn_debug_syslog_and_trace,
>  		.prefix = prefix
>  	};
>  	return p;
> @@ -254,14 +259,14 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
>   * 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.
> + * is filtered by the drm.debug and drm.trace module parameters. 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:
> + * Enabling verbose debug messages is done through the drm.debug and drm.trace
> + * parameters, each category being enabled by a bit:
>   *
>   *  - drm.debug=0x1 will enable CORE messages
>   *  - drm.debug=0x2 will enable DRIVER messages
> @@ -270,10 +275,14 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
>   *  - drm.debug=0x1ff will enable all messages
>   *
>   * An interesting feature is that it's possible to enable verbose logging at
> - * run-time by echoing the debug value in its sysfs node::
> + * run-time by echoing the debug category value in its sysfs node::
>   *
> + *   # For syslog logging:
>   *   # echo 0xf > /sys/module/drm/parameters/debug
>   *
> + *   # For tracefs logging:
> + *   # echo 0xf > /sys/module/drm/parameters/trace
> + *
>   */
>  enum drm_debug_category {
>  	/**
> @@ -325,14 +334,20 @@ static inline bool drm_debug_syslog_enabled(enum drm_debug_category category)
>  	return unlikely(__drm_debug_syslog & category);
>  }
>  
> +static inline bool drm_debug_trace_enabled(enum drm_debug_category category)
> +{
> +	return unlikely(__drm_debug_trace & category);
> +}
> +
>  static inline bool drm_debug_enabled(enum drm_debug_category category)
>  {
> -	return drm_debug_syslog_enabled(category);
> +	return drm_debug_syslog_enabled(category) ||
> +	       drm_debug_trace_enabled(category);
>  }
>  
>  /**
>   * drm_debug_category_printer - construct a &drm_printer that outputs to
> - * pr_debug() if enabled for the given category.
> + * pr_debug() and/or the drm tracefs instance if enabled for the given category.
>   * @category: the DRM_UT_* message category this message belongs to
>   * @prefix: trace output prefix
>   *
> @@ -347,8 +362,13 @@ drm_debug_category_printer(enum drm_debug_category category,
>  		.prefix = prefix
>  	};
>  
> -	if (drm_debug_syslog_enabled(category)) {
> +	if (drm_debug_syslog_enabled(category) &&
> +	    drm_debug_trace_enabled(category)) {
> +		p.printfn = __drm_printfn_debug_syslog_and_trace;
> +	} else if (drm_debug_syslog_enabled(category)) {
>  		p.printfn = __drm_printfn_debug_syslog;
> +	} else if (drm_debug_trace_enabled(category)) {
> +		p.printfn = __drm_printfn_trace;
>  	} else {
>  		WARN(1, "Debug category %d is inactive.", category);
>  		p.printfn = __drm_printfn_noop;
> @@ -357,6 +377,27 @@ drm_debug_category_printer(enum drm_debug_category category,
>  	return p;
>  }
>  
> +
> +#ifdef CONFIG_TRACING
> +void drm_trace_init(void);
> +__printf(1, 2)
> +void drm_trace_printf(const char *format, ...);
> +void drm_trace_cleanup(void);
> +#else
> +static inline void drm_trace_init(void)
> +{
> +}
> +
> +__printf(1, 2)
> +static inline void drm_trace_printf(const char *format, ...)
> +{
> +}
> +
> +static inline void drm_trace_cleanup(void)
> +{
> +}
> +#endif
> +
>  /*
>   * struct device based logging
>   *
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
>

Hi Sean,

Nice patch! I tested and reviewed it and everthing lgtm.

Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>

-- 
Rodrigo Siqueira
https://siqueira.tech

Re: [RESEND PATCH v6 14/14] drm/print: Add tracefs support to the drm logging helpers

[jim.cromie]

On Wed, Jul 21, 2021 at 11:56 AM Sean Paul <sean@poorly.run> wrote:
>
> From: Sean Paul <seanpaul@chromium.org>
>
> This patch adds a new module parameter called drm.trace which accepts
> the same mask as drm.debug. When a debug category is enabled, log
> messages will be put in a new tracefs instance called drm for
> consumption.
>
> Using the new tracefs instance will allow distros to enable drm logging
> in production without impacting performance or spamming the system
> logs.

hi Paul,

I should have started reading here, not at patch-1
While I still think some of the _syslog name changes are extra,
drm_debug_enabled() needs it - rename goes with narrower purpose.

a couple comments below, trimming heavily.


> +#define DEBUG_PARM_DESC(dst) \
> +"Enable debug output to " dst ", where each bit enables a debug category.\n" \

I will be borrowing that idea.

> +
> +MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog"));
>  module_param_named(debug, __drm_debug_syslog, int, 0600);
>


> +void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
> +                                          struct va_format *vaf)
> +{
> +       pr_debug("%s %pV", p->prefix, vaf);

Im not sure about prefixing this way.
dyndbg query needs to see the prefix in the format string at compile-time.

My RFC patch does the prefixing in the macro layer, when enabled.
when disabled, prefixing at runtime is fine.
This looks easy to shake out..
















> +       drm_trace_printf("%s %pV", p->prefix, vaf);
> +}
> +EXPORT_SYMBOL(__drm_printfn_debug_syslog_and_trace);
> +
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
>  {
>         pr_err("*ERROR* %s %pV", p->prefix, vaf);
> @@ -246,6 +278,14 @@ void drm_dev_printk(const struct device *dev, const char *level,
>         struct va_format vaf;
>         va_list args;
>
> +       va_start(args, format);
> +       vaf.fmt = format;
> +       vaf.va = &args;
> +       drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
> +                        dev ? dev_name(dev) : "",dev ? " " : "",
> +                        __builtin_return_address(0), &vaf);
> +       va_end(args);
> +
>         va_start(args, format);
>         vaf.fmt = format;
>         vaf.va = &args;

here and below, you re-prepare vaf.
can you just prepare once, and print 2x to different printers ?
or is that not allowed for some reason?

maybe macros to reduce linecount ?

 thanks
Jim

Re: [RESEND PATCH v6 14/14] drm/print: Add tracefs support to the drm logging helpers

[jim.cromie]

On Thu, Jul 29, 2021 at 2:24 PM <jim.cromie@gmail.com> wrote:
>
> On Wed, Jul 21, 2021 at 11:56 AM Sean Paul <sean@poorly.run> wrote:
> >
> > From: Sean Paul <seanpaul@chromium.org>
> >
> > This patch adds a new module parameter called drm.trace which accepts
> > the same mask as drm.debug. When a debug category is enabled, log
> > messages will be put in a new tracefs instance called drm for
> > consumption.
> >
> > Using the new tracefs instance will allow distros to enable drm logging
> > in production without impacting performance or spamming the system
> > logs.
>
> hi Paul,
>
> I should have started reading here, not at patch-1
> While I still think some of the _syslog name changes are extra,
> drm_debug_enabled() needs it - rename goes with narrower purpose.
>
> a couple comments below, trimming heavily.
>
>
> > +#define DEBUG_PARM_DESC(dst) \
> > +"Enable debug output to " dst ", where each bit enables a debug category.\n" \
>
> I will be borrowing that idea.
>
> > +
> > +MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog"));
> >  module_param_named(debug, __drm_debug_syslog, int, 0600);
> >
>
>
> > +void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
> > +                                          struct va_format *vaf)
> > +{
> > +       pr_debug("%s %pV", p->prefix, vaf);
>
> Im not sure about prefixing this way.
> dyndbg query needs to see the prefix in the format string at compile-time.
>
> My RFC patch does the prefixing in the macro layer, when enabled.
> when disabled, prefixing at runtime is fine.
> This looks easy to shake out..
>

Actually, this is more problematic than that.
this pr_debug has just a single point of >control,
whereas all those in macro expansions get individual lines in control file.

I had this problem in an earlier version of RFC patch
where I called pr_debug from within drm_dev_dbg etc.

Re: [RESEND PATCH v6 14/14] drm/print: Add tracefs support to the drm logging helpers

[abhinavk]

On 2021-07-21 10:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch adds a new module parameter called drm.trace which accepts
> the same mask as drm.debug. When a debug category is enabled, log
> messages will be put in a new tracefs instance called drm for
> consumption.
> 
> Using the new tracefs instance will allow distros to enable drm logging
> in production without impacting performance or spamming the system
> logs.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Reported-by: kernel test robot <lkp@intel.com> # warning reported in v6
> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191010204823.195540-1-sean@poorly.run
> #v1
> Link:
> https://lists.freedesktop.org/archives/dri-devel/2019-November/243230.html
> #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20191212203301.142437-1-sean@poorly.run
> #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200114172155.215463-1-sean@poorly.run
> #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200608210505.48519-14-sean@poorly.run
> #v5
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20200818210510.49730-15-sean@poorly.run
> #v6
> 
> Changes in v5:
> -Re-write to use trace_array and the tracefs instance support
> Changes in v6:
> -Use the new trace_array_init_printk() to initialize global trace
>  buffers
> Changes in v6.5:
> -Fix kernel test robot warning
> -Add a trace printf in __drm_err
> ---
>  Documentation/gpu/drm-uapi.rst |   6 +
>  drivers/gpu/drm/drm_drv.c      |   3 +
>  drivers/gpu/drm/drm_print.c    | 223 ++++++++++++++++++++++++++++-----
>  include/drm/drm_print.h        |  63 ++++++++--
>  4 files changed, 255 insertions(+), 40 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst 
> b/Documentation/gpu/drm-uapi.rst
> index 7e51dd40bf6e..ce1ea39fb4b9 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -424,6 +424,12 @@ Debugfs Support
>  .. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
>     :export:
> 
> +DRM Tracing
> +---------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_print.c
> +   :doc: DRM Tracing
> +
>  Sysfs Support
>  =============
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8804ec7d3215..71dc0b161b51 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1034,12 +1034,15 @@ static void drm_core_exit(void)
>  	drm_sysfs_destroy();
>  	idr_destroy(&drm_minors_idr);
>  	drm_connector_ida_destroy();
> +	drm_trace_cleanup();
>  }
> 
>  static int __init drm_core_init(void)
>  {
>  	int ret;
> 
> +	drm_trace_init();

Can we move the drm_trace_init() to drm_dev_register()?
The reason is while creating the tracefs node, I think its useful to 
have the drm device id to support
multiple DRM devices. So while creating the tracefs node, we can do 
something like
trace_array_get_by_name("drm_<device_id");

Then usermode will just read all the traces from /instances/drm_***

> +
>  	drm_connector_ida_init();
>  	idr_init(&drm_minors_idr);
>  	drm_memcpy_init_early();
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 4d984a01b3a3..64d9a724c2df 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -31,6 +31,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
> +#include <linux/trace.h>
> 
>  #include <drm/drm.h>
>  #include <drm/drm_drv.h>
> @@ -43,17 +44,34 @@
>  unsigned int __drm_debug_syslog;
>  EXPORT_SYMBOL(__drm_debug_syslog);
> 
> -MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables
> a debug category.\n"
> -"\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n"
> -"\t\tBit 1 (0x02)  will enable DRIVER messages (drm controller 
> code)\n"
> -"\t\tBit 2 (0x04)  will enable KMS messages (modesetting code)\n"
> -"\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n"
> -"\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n"
> -"\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n"
> -"\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n"
> -"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
> +/*
> + * __drm_debug_trace: Enable debug output in drm tracing instance.
> + * Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
> + */
> +unsigned int __drm_debug_trace;
> +EXPORT_SYMBOL(__drm_debug_trace);
> +
> +#define DEBUG_PARM_DESC(dst) \
> +"Enable debug output to " dst ", where each bit enables a debug 
> category.\n" \
> +"\t\tBit 0 (0x01)  will enable CORE messages (drm core code)\n" \
> +"\t\tBit 1 (0x02)  will enable DRIVER messages (drm controller 
> code)\n" \
> +"\t\tBit 2 (0x04)  will enable KMS messages (modesetting code)\n" \
> +"\t\tBit 3 (0x08)  will enable PRIME messages (prime code)\n" \
> +"\t\tBit 4 (0x10)  will enable ATOMIC messages (atomic code)\n" \
> +"\t\tBit 5 (0x20)  will enable VBL messages (vblank code)\n" \
> +"\t\tBit 7 (0x80)  will enable LEASE messages (leasing code)\n" \
> +"\t\tBit 8 (0x100) will enable DP messages (displayport code)"

This is certainly a great start. Moving forward a bit, is there a 
possibility to subcatergorize
even further? Like plane, crtc, connector etc?

> +
> +MODULE_PARM_DESC(debug, DEBUG_PARM_DESC("syslog"));
>  module_param_named(debug, __drm_debug_syslog, int, 0600);
> 
> +MODULE_PARM_DESC(trace, DEBUG_PARM_DESC("tracefs"));
> +module_param_named(trace, __drm_debug_trace, int, 0600);
> +
> +#ifdef CONFIG_TRACING
> +struct trace_array *trace_arr;
> +#endif
> +
>  void __drm_puts_coredump(struct drm_printer *p, const char *str)
>  {
>  	struct drm_print_iterator *iterator = p->arg;
> @@ -166,6 +184,20 @@ void __drm_printfn_debug_syslog(struct
> drm_printer *p, struct va_format *vaf)
>  }
>  EXPORT_SYMBOL(__drm_printfn_debug_syslog);
> 
> +void __drm_printfn_trace(struct drm_printer *p, struct va_format *vaf)
> +{
> +	drm_trace_printf("%s %pV", p->prefix, vaf);
> +}
> +EXPORT_SYMBOL(__drm_printfn_trace);
> +
> +void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
> +					   struct va_format *vaf)
> +{
> +	pr_debug("%s %pV", p->prefix, vaf);
> +	drm_trace_printf("%s %pV", p->prefix, vaf);
> +}
> +EXPORT_SYMBOL(__drm_printfn_debug_syslog_and_trace);
> +
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf)
>  {
>  	pr_err("*ERROR* %s %pV", p->prefix, vaf);
> @@ -246,6 +278,14 @@ void drm_dev_printk(const struct device *dev,
> const char *level,
>  	struct va_format vaf;
>  	va_list args;
> 
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +	drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
> +			 dev ? dev_name(dev) : "",dev ? " " : "",
> +			 __builtin_return_address(0), &vaf);
> +	va_end(args);
> +
>  	va_start(args, format);
>  	vaf.fmt = format;
>  	vaf.va = &args;
> @@ -267,21 +307,30 @@ void drm_dev_dbg(const struct device *dev, enum
> drm_debug_category category,
>  	struct va_format vaf;
>  	va_list args;
> 
> -	if (!drm_debug_enabled(category))
> -		return;
> +	if (drm_debug_syslog_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
> 
> -	va_start(args, format);
> -	vaf.fmt = format;
> -	vaf.va = &args;
> +		if (dev)
> +			dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
> +				__builtin_return_address(0), &vaf);
> +		else
> +			printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> +			__builtin_return_address(0), &vaf);
> 
> -	if (dev)
> -		dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
> -			   __builtin_return_address(0), &vaf);
> -	else
> -		printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> -		       __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
> 
> -	va_end(args);
> +	if (drm_debug_trace_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
> +		drm_trace_printf("%s%s[" DRM_NAME ":%ps] %pV",
> +				 dev ? dev_name(dev) : "", dev ? " " : "",
> +				 __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
>  }
>  EXPORT_SYMBOL(drm_dev_dbg);
> 
> @@ -290,17 +339,25 @@ void __drm_dbg(enum drm_debug_category category,
> const char *format, ...)
>  	struct va_format vaf;
>  	va_list args;
> 
> -	if (!drm_debug_enabled(category))
> -		return;
> +	if (drm_debug_syslog_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
> 
> -	va_start(args, format);
> -	vaf.fmt = format;
> -	vaf.va = &args;
> +		printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> +		__builtin_return_address(0), &vaf);
> 
> -	printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> -	       __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
> 
> -	va_end(args);
> +	if (drm_debug_trace_enabled(category)) {
> +		va_start(args, format);
> +		vaf.fmt = format;
> +		vaf.va = &args;
> +		drm_trace_printf("[" DRM_NAME ":%ps] %pV",
> +				 __builtin_return_address(0), &vaf);
> +		va_end(args);
> +	}
>  }
>  EXPORT_SYMBOL(__drm_dbg);
> 
> @@ -317,6 +374,13 @@ void __drm_err(const char *format, ...)
>  	       __builtin_return_address(0), &vaf);
> 
>  	va_end(args);
> +
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +	drm_trace_printf("[" DRM_NAME ":%ps] *ERROR* %pV",
> +				__builtin_return_address(0), &vaf);
> +	va_end(args);
>  }
>  EXPORT_SYMBOL(__drm_err);
> 
> @@ -347,3 +411,104 @@ void drm_print_regset32(struct drm_printer *p,
> struct debugfs_regset32 *regset)
>  	}
>  }
>  EXPORT_SYMBOL(drm_print_regset32);
> +
> +
> +/**
> + * DOC: DRM Tracing
> + *
> + * *tl;dr* DRM tracing is a lightweight alternative to traditional DRM 
> debug
> + * logging.
> + *
> + * While DRM logging is quite convenient when reproducing a specific 
> issue, it
> + * doesn't help when something goes wrong unexpectedly. There are a 
> couple
> + * reasons why one does not want to enable DRM logging at all times:
> + *
> + * 1. We don't want to overwhelm syslog with drm spam, others have to
> use it too
> + * 2. Console logging is slow
> + *
> + * DRM tracing aims to solve both these problems.
> + *
> + * To use DRM tracing, set the drm.trace module parameter (via
> cmdline or sysfs)
> + * to a DRM debug category mask (this is a bitmask of 
> &drm_debug_category
> + * values):
> + * ::
> + *
> + *    eg: echo 0x106 > /sys/module/drm/parameters/trace
> + *
> + * Once active, all log messages in the specified categories will be 
> written to
> + * the DRM trace. Once at capacity, the trace will overwrite old 
> messages with
> + * new ones. At any point, one can read the trace file to extract the
> previous N
> + * DRM messages:
> + * ::
> + *
> + *    eg: cat /sys/kernel/tracing/instances/drm/trace
> + *
> + * Considerations
> + * **************
> + * The trace is subsystem wide, so if you have multiple devices 
> active, they
> + * will be adding logs to the same trace.
> + *
> + * The contents of the DRM Trace are **not** considered UABI. **DO
> NOT depend on
> + * the values of these traces in your userspace.** These traces are
> intended for
> + * entertainment purposes only. The contents of these logs carry no 
> warranty,
> + * expressed or implied.
> + */
> +
> +
> +#ifdef CONFIG_TRACING
> +
> +/**
> + * drm_trace_init - initializes the drm trace array
> + *
> + * This function fetches (or creates) the drm trace array. This
> should be called
> + * once on drm subsystem creation and matched with 
> drm_trace_cleanup().
> + */
> +void drm_trace_init(void)
> +{
> +	int ret;
> +
> +	trace_arr = trace_array_get_by_name("drm");
> +	if (!trace_arr)
> +		return;
> +
> +	ret = trace_array_init_printk(trace_arr);
> +	if (ret)
> +		drm_trace_cleanup();
> +}
> +EXPORT_SYMBOL(drm_trace_init);
> +
> +/**
> + * drm_trace_printf - adds an entry to the drm tracefs instance
> + * @format: printf format of the message to add to the trace
> + *
> + * This function adds a new entry in the drm tracefs instance
> + */
> +void drm_trace_printf(const char *format, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, format);
> +	vaf.fmt = format;
> +	vaf.va = &args;
> +	trace_array_printk(trace_arr, _THIS_IP_, "%pV", &vaf);
> +	va_end(args);
> +}
> +
> +/**
> + * drm_trace_cleanup - destroys the drm trace array
> + *
> + * This function destroys the drm trace array created with 
> drm_trace_init. This
> + * should be called once on drm subsystem close and matched with
> + * drm_trace_init().
> + */
> +void drm_trace_cleanup(void)
> +{
> +	if (trace_arr) {
> +		trace_array_put(trace_arr);
> +		trace_array_destroy(trace_arr);
> +		trace_arr = NULL;
> +	}
> +}
> +EXPORT_SYMBOL(drm_trace_cleanup);
> +#endif
> \ No newline at end of file
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index af31beeb82a1..4609a2f4a425 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -36,12 +36,13 @@
> 
>  /* Do *not* use outside of drm_print.[ch]! */
>  extern unsigned int __drm_debug_syslog;
> +extern unsigned int __drm_debug_trace;
> 
>  /**
>   * DOC: print
>   *
>   * A simple wrapper for dev_printk(), seq_printf(), etc.  Allows same
> - * debug code to be used for both debugfs and printk logging.
> + * debug code to be used for debugfs, printk and tracefs logging.
>   *
>   * For example::
>   *
> @@ -86,6 +87,9 @@ 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_syslog(struct drm_printer *p, struct 
> va_format *vaf);
> +void __drm_printfn_trace(struct drm_printer *p, struct va_format 
> *vaf);
> +void __drm_printfn_debug_syslog_and_trace(struct drm_printer *p,
> +					   struct va_format *vaf);
>  void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf);
>  void __drm_printfn_noop(struct drm_printer *p, struct va_format *vaf);
> 
> @@ -219,7 +223,8 @@ static inline struct drm_printer
> drm_info_printer(struct device *dev)
>  }
> 
>  /**
> - * drm_debug_printer - construct a &drm_printer that outputs to 
> pr_debug()
> + * drm_debug_printer - construct a &drm_printer that outputs to 
> pr_debug() and
> + * drm tracefs
>   * @prefix: debug output prefix
>   *
>   * RETURNS:
> @@ -228,7 +233,7 @@ static inline struct drm_printer
> drm_info_printer(struct device *dev)
>  static inline struct drm_printer drm_debug_printer(const char *prefix)
>  {
>  	struct drm_printer p = {
> -		.printfn = __drm_printfn_debug_syslog,
> +		.printfn = __drm_printfn_debug_syslog_and_trace,
>  		.prefix = prefix
>  	};
>  	return p;
> @@ -254,14 +259,14 @@ static inline struct drm_printer
> drm_err_printer(const char *prefix)
>   * 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.
> + * is filtered by the drm.debug and drm.trace module parameters. 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:
> + * Enabling verbose debug messages is done through the drm.debug and 
> drm.trace
> + * parameters, each category being enabled by a bit:
>   *
>   *  - drm.debug=0x1 will enable CORE messages
>   *  - drm.debug=0x2 will enable DRIVER messages
> @@ -270,10 +275,14 @@ static inline struct drm_printer
> drm_err_printer(const char *prefix)
>   *  - drm.debug=0x1ff will enable all messages
>   *
>   * An interesting feature is that it's possible to enable verbose 
> logging at
> - * run-time by echoing the debug value in its sysfs node::
> + * run-time by echoing the debug category value in its sysfs node::
>   *
> + *   # For syslog logging:
>   *   # echo 0xf > /sys/module/drm/parameters/debug
>   *
> + *   # For tracefs logging:
> + *   # echo 0xf > /sys/module/drm/parameters/trace
> + *
>   */
>  enum drm_debug_category {
>  	/**
> @@ -325,14 +334,20 @@ static inline bool drm_debug_syslog_enabled(enum
> drm_debug_category category)
>  	return unlikely(__drm_debug_syslog & category);
>  }
> 
> +static inline bool drm_debug_trace_enabled(enum drm_debug_category 
> category)
> +{
> +	return unlikely(__drm_debug_trace & category);
> +}
> +
>  static inline bool drm_debug_enabled(enum drm_debug_category category)
>  {
> -	return drm_debug_syslog_enabled(category);
> +	return drm_debug_syslog_enabled(category) ||
> +	       drm_debug_trace_enabled(category);
>  }
> 
>  /**
>   * drm_debug_category_printer - construct a &drm_printer that outputs 
> to
> - * pr_debug() if enabled for the given category.
> + * pr_debug() and/or the drm tracefs instance if enabled for the
> given category.
>   * @category: the DRM_UT_* message category this message belongs to
>   * @prefix: trace output prefix
>   *
> @@ -347,8 +362,13 @@ drm_debug_category_printer(enum
> drm_debug_category category,
>  		.prefix = prefix
>  	};
> 
> -	if (drm_debug_syslog_enabled(category)) {
> +	if (drm_debug_syslog_enabled(category) &&
> +	    drm_debug_trace_enabled(category)) {
> +		p.printfn = __drm_printfn_debug_syslog_and_trace;
> +	} else if (drm_debug_syslog_enabled(category)) {
>  		p.printfn = __drm_printfn_debug_syslog;
> +	} else if (drm_debug_trace_enabled(category)) {
> +		p.printfn = __drm_printfn_trace;
>  	} else {
>  		WARN(1, "Debug category %d is inactive.", category);
>  		p.printfn = __drm_printfn_noop;
> @@ -357,6 +377,27 @@ drm_debug_category_printer(enum
> drm_debug_category category,
>  	return p;
>  }
> 
> +
> +#ifdef CONFIG_TRACING
> +void drm_trace_init(void);
> +__printf(1, 2)
> +void drm_trace_printf(const char *format, ...);
> +void drm_trace_cleanup(void);
> +#else
> +static inline void drm_trace_init(void)
> +{
> +}
> +
> +__printf(1, 2)
> +static inline void drm_trace_printf(const char *format, ...)
> +{
> +}
> +
> +static inline void drm_trace_cleanup(void)
> +{
> +}
> +#endif
> +
>  /*
>   * struct device based logging
>   *

Re: [RESEND PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs

[Pekka Paalanen]

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

On Wed, 21 Jul 2021 13:55:07 -0400
Sean Paul <sean@poorly.run> wrote:

> From: Sean Paul <seanpaul@chromium.org>
> 
> Hi all,
> I just had the pleasure of rebasing this set on our CrOS downstream
> kernel and wanted to resend it for consideration once again. There
> hasn't been any resistence to the set AFAIK, just perhaps not enough
> motivation for anyone to hit the go bit. There was some interest from
> the msm folks about a month ago, and it has been an invaluable tool
> on CrOS for the past ~year. Hopefully someone can dig into this and
> provide some feedback so we can move this forward.
> 
> Thanks!
> 
> Sean
> 
> Changes since last v6:
> -Rebased on drm-tip
> 
> Original v6 of the set available here:
> https://patchwork.freedesktop.org/series/78133/
> https://lore.kernel.org/dri-devel/20200818210510.49730-1-sean@poorly.run/
> 

Woo! Yes!

Do you have a link to your userspace?

You wouldn't happen to have already written a privileged userspace
service that would deliver on request the logs to non-privileged actors
like a compositor to be dumped in an error report?


Thanks,
pq

> Sean Paul (14):
>   drm/mipi_dbi: Convert pr_debug calls to DRM_DEBUG_DRIVER
>   drm/sil164: Convert dev_printk to drm_dev_dbg
>   drm/i915/utils: Replace dev_printk with drm helpers
>   drm/msm/dpu: Replace definitions for dpu debug macros
>   drm/print: rename drm_debug* to be more syslog-centric
>   drm/amd: Gate i2c transaction logs on drm_debug_syslog
>   drm/etnaviv: Change buffer dump checks to target syslog
>   drm/nouveau: Change debug checks to specifically target syslog
>   drm/i915: Change infoframe debug checks to specify syslog
>   drm/print: Add drm_debug_category_printer
>   drm/mst: Convert debug printers to debug category printers
>   drm/i915: Use debug category printer for welcome message
>   drm/atomic: Use debug category printer for atomic state printer
>   drm/print: Add tracefs support to the drm logging helpers
> 
>  Documentation/gpu/drm-uapi.rst               |   6 +
>  drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c   |   4 +-
>  drivers/gpu/drm/drm_atomic_uapi.c            |   2 +-
>  drivers/gpu/drm/drm_dp_mst_topology.c        |   9 +-
>  drivers/gpu/drm/drm_drv.c                    |   3 +
>  drivers/gpu/drm/drm_mipi_dbi.c               |   8 +-
>  drivers/gpu/drm/drm_print.c                  | 242 ++++++++++++++++---
>  drivers/gpu/drm/etnaviv/etnaviv_buffer.c     |   8 +-
>  drivers/gpu/drm/i2c/sil164_drv.c             |  12 +-
>  drivers/gpu/drm/i915/display/intel_display.c |   4 +-
>  drivers/gpu/drm/i915/i915_drv.c              |   3 +-
>  drivers/gpu/drm/i915/i915_utils.c            |   5 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h      |  20 +-
>  drivers/gpu/drm/nouveau/nouveau_drv.h        |   4 +-
>  include/drm/drm_print.h                      |  96 +++++++-
>  15 files changed, 331 insertions(+), 95 deletions(-)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs

[Sean Paul]

On Thu, Jul 22, 2021 at 3:49 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Wed, 21 Jul 2021 13:55:07 -0400
> Sean Paul <sean@poorly.run> wrote:
>
> > From: Sean Paul <seanpaul@chromium.org>
> >
> > Hi all,
> > I just had the pleasure of rebasing this set on our CrOS downstream
> > kernel and wanted to resend it for consideration once again. There
> > hasn't been any resistence to the set AFAIK, just perhaps not enough
> > motivation for anyone to hit the go bit. There was some interest from
> > the msm folks about a month ago, and it has been an invaluable tool
> > on CrOS for the past ~year. Hopefully someone can dig into this and
> > provide some feedback so we can move this forward.
> >
> > Thanks!
> >
> > Sean
> >
> > Changes since last v6:
> > -Rebased on drm-tip
> >
> > Original v6 of the set available here:
> > https://patchwork.freedesktop.org/series/78133/
> > https://lore.kernel.org/dri-devel/20200818210510.49730-1-sean@poorly.run/
> >
>
> Woo! Yes!
>
> Do you have a link to your userspace?
>

Hi Pekka,
Probably less interesting that you're hoping for, but here are the
CrOS patches to enable and collect tracing:

https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2354674
https://chromium-review.googlesource.com/c/chromiumos/platform/crosutils/+/2354392


> You wouldn't happen to have already written a privileged userspace
> service that would deliver on request the logs to non-privileged actors
> like a compositor to be dumped in an error report?

Our feedback report generation (log_tool.cc above) collects the logs
(depending on user log preferences) from across the system and
packages them up for submission when requested by the user. For
drm_trace, we grab them from debugfs since we don't have tracefs
mounted.

You could adapt this code to change the delivery method, but I'm not
sure how much value it would add beyond writing your own purpose-built
service.

Sean

>
>
> Thanks,
> pq
>
> > Sean Paul (14):
> >   drm/mipi_dbi: Convert pr_debug calls to DRM_DEBUG_DRIVER
> >   drm/sil164: Convert dev_printk to drm_dev_dbg
> >   drm/i915/utils: Replace dev_printk with drm helpers
> >   drm/msm/dpu: Replace definitions for dpu debug macros
> >   drm/print: rename drm_debug* to be more syslog-centric
> >   drm/amd: Gate i2c transaction logs on drm_debug_syslog
> >   drm/etnaviv: Change buffer dump checks to target syslog
> >   drm/nouveau: Change debug checks to specifically target syslog
> >   drm/i915: Change infoframe debug checks to specify syslog
> >   drm/print: Add drm_debug_category_printer
> >   drm/mst: Convert debug printers to debug category printers
> >   drm/i915: Use debug category printer for welcome message
> >   drm/atomic: Use debug category printer for atomic state printer
> >   drm/print: Add tracefs support to the drm logging helpers
> >
> >  Documentation/gpu/drm-uapi.rst               |   6 +
> >  drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c   |   4 +-
> >  drivers/gpu/drm/drm_atomic_uapi.c            |   2 +-
> >  drivers/gpu/drm/drm_dp_mst_topology.c        |   9 +-
> >  drivers/gpu/drm/drm_drv.c                    |   3 +
> >  drivers/gpu/drm/drm_mipi_dbi.c               |   8 +-
> >  drivers/gpu/drm/drm_print.c                  | 242 ++++++++++++++++---
> >  drivers/gpu/drm/etnaviv/etnaviv_buffer.c     |   8 +-
> >  drivers/gpu/drm/i2c/sil164_drv.c             |  12 +-
> >  drivers/gpu/drm/i915/display/intel_display.c |   4 +-
> >  drivers/gpu/drm/i915/i915_drv.c              |   3 +-
> >  drivers/gpu/drm/i915/i915_utils.c            |   5 +-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h      |  20 +-
> >  drivers/gpu/drm/nouveau/nouveau_drv.h        |   4 +-
> >  include/drm/drm_print.h                      |  96 +++++++-
> >  15 files changed, 331 insertions(+), 95 deletions(-)
> >
>

Re: [RESEND PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs

[Sean Paul]

On Thu, Jul 22, 2021 at 9:48 AM Sean Paul <sean@poorly.run> wrote:
>
> On Thu, Jul 22, 2021 at 3:49 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > On Wed, 21 Jul 2021 13:55:07 -0400
> > Sean Paul <sean@poorly.run> wrote:
> >
> > > From: Sean Paul <seanpaul@chromium.org>
> > >
> > > Hi all,
> > > I just had the pleasure of rebasing this set on our CrOS downstream
> > > kernel and wanted to resend it for consideration once again. There
> > > hasn't been any resistence to the set AFAIK, just perhaps not enough
> > > motivation for anyone to hit the go bit. There was some interest from
> > > the msm folks about a month ago, and it has been an invaluable tool
> > > on CrOS for the past ~year. Hopefully someone can dig into this and
> > > provide some feedback so we can move this forward.
> > >
> > > Thanks!
> > >
> > > Sean
> > >
> > > Changes since last v6:
> > > -Rebased on drm-tip
> > >
> > > Original v6 of the set available here:
> > > https://patchwork.freedesktop.org/series/78133/
> > > https://lore.kernel.org/dri-devel/20200818210510.49730-1-sean@poorly.run/
> > >
> >
> > Woo! Yes!
> >
> > Do you have a link to your userspace?
> >
>
> Hi Pekka,
> Probably less interesting that you're hoping for, but here are the
> CrOS patches to enable and collect tracing:
>
> https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2354674
> https://chromium-review.googlesource.com/c/chromiumos/platform/crosutils/+/2354392
>
>
> > You wouldn't happen to have already written a privileged userspace
> > service that would deliver on request the logs to non-privileged actors
> > like a compositor to be dumped in an error report?
>
> Our feedback report generation (log_tool.cc above) collects the logs
> (depending on user log preferences) from across the system and
> packages them up for submission when requested by the user. For
> drm_trace, we grab them from debugfs since we don't have tracefs
> mounted.
>

One more note:

If you have a chromebook with a 5.4+ kernel, you can check out the
output yourself by navigating to chrome://system and expanding the
"drm_trace" field.

Sean

> You could adapt this code to change the delivery method, but I'm not
> sure how much value it would add beyond writing your own purpose-built
> service.
>
> Sean
>
> >
> >
> > Thanks,
> > pq
> >
> > > Sean Paul (14):
> > >   drm/mipi_dbi: Convert pr_debug calls to DRM_DEBUG_DRIVER
> > >   drm/sil164: Convert dev_printk to drm_dev_dbg
> > >   drm/i915/utils: Replace dev_printk with drm helpers
> > >   drm/msm/dpu: Replace definitions for dpu debug macros
> > >   drm/print: rename drm_debug* to be more syslog-centric
> > >   drm/amd: Gate i2c transaction logs on drm_debug_syslog
> > >   drm/etnaviv: Change buffer dump checks to target syslog
> > >   drm/nouveau: Change debug checks to specifically target syslog
> > >   drm/i915: Change infoframe debug checks to specify syslog
> > >   drm/print: Add drm_debug_category_printer
> > >   drm/mst: Convert debug printers to debug category printers
> > >   drm/i915: Use debug category printer for welcome message
> > >   drm/atomic: Use debug category printer for atomic state printer
> > >   drm/print: Add tracefs support to the drm logging helpers
> > >
> > >  Documentation/gpu/drm-uapi.rst               |   6 +
> > >  drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c   |   4 +-
> > >  drivers/gpu/drm/drm_atomic_uapi.c            |   2 +-
> > >  drivers/gpu/drm/drm_dp_mst_topology.c        |   9 +-
> > >  drivers/gpu/drm/drm_drv.c                    |   3 +
> > >  drivers/gpu/drm/drm_mipi_dbi.c               |   8 +-
> > >  drivers/gpu/drm/drm_print.c                  | 242 ++++++++++++++++---
> > >  drivers/gpu/drm/etnaviv/etnaviv_buffer.c     |   8 +-
> > >  drivers/gpu/drm/i2c/sil164_drv.c             |  12 +-
> > >  drivers/gpu/drm/i915/display/intel_display.c |   4 +-
> > >  drivers/gpu/drm/i915/i915_drv.c              |   3 +-
> > >  drivers/gpu/drm/i915/i915_utils.c            |   5 +-
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h      |  20 +-
> > >  drivers/gpu/drm/nouveau/nouveau_drv.h        |   4 +-
> > >  include/drm/drm_print.h                      |  96 +++++++-
> > >  15 files changed, 331 insertions(+), 95 deletions(-)
> > >
> >

Re: [RESEND PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs

[Pekka Paalanen]

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

On Thu, 22 Jul 2021 09:48:00 -0400
Sean Paul <sean@poorly.run> wrote:

> On Thu, Jul 22, 2021 at 3:49 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > On Wed, 21 Jul 2021 13:55:07 -0400
> > Sean Paul <sean@poorly.run> wrote:
> >  
> > > From: Sean Paul <seanpaul@chromium.org>
> > >
> > > Hi all,
> > > I just had the pleasure of rebasing this set on our CrOS downstream
> > > kernel and wanted to resend it for consideration once again. There
> > > hasn't been any resistence to the set AFAIK, just perhaps not enough
> > > motivation for anyone to hit the go bit. There was some interest from
> > > the msm folks about a month ago, and it has been an invaluable tool
> > > on CrOS for the past ~year. Hopefully someone can dig into this and
> > > provide some feedback so we can move this forward.
> > >
> > > Thanks!
> > >
> > > Sean
> > >
> > > Changes since last v6:
> > > -Rebased on drm-tip
> > >
> > > Original v6 of the set available here:
> > > https://patchwork.freedesktop.org/series/78133/
> > > https://lore.kernel.org/dri-devel/20200818210510.49730-1-sean@poorly.run/
> > >  
> >
> > Woo! Yes!
> >
> > Do you have a link to your userspace?
> >  
> 
> Hi Pekka,
> Probably less interesting that you're hoping for, but here are the
> CrOS patches to enable and collect tracing:
> 
> https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2354674
> https://chromium-review.googlesource.com/c/chromiumos/platform/crosutils/+/2354392
> 
> 
> > You wouldn't happen to have already written a privileged userspace
> > service that would deliver on request the logs to non-privileged actors
> > like a compositor to be dumped in an error report?  
> 
> Our feedback report generation (log_tool.cc above) collects the logs
> (depending on user log preferences) from across the system and
> packages them up for submission when requested by the user. For
> drm_trace, we grab them from debugfs since we don't have tracefs
> mounted.
> 
> You could adapt this code to change the delivery method, but I'm not
> sure how much value it would add beyond writing your own purpose-built
> service.

Indeed. Thanks anyway! :-)
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RESEND PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs

[abhinavk]

On 2021-07-21 10:55, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Hi all,
> I just had the pleasure of rebasing this set on our CrOS downstream
> kernel and wanted to resend it for consideration once again. There
> hasn't been any resistence to the set AFAIK, just perhaps not enough
> motivation for anyone to hit the go bit. There was some interest from
> the msm folks about a month ago, and it has been an invaluable tool
> on CrOS for the past ~year. Hopefully someone can dig into this and
> provide some feedback so we can move this forward.
> 
> Thanks!
> 
> Sean

Totally agree, This tool has been valuable for us in debugging many 
issues.
FWIW, I will go through the core bits and msm pieces to give my RB.

> 
> Changes since last v6:
> -Rebased on drm-tip
> 
> Original v6 of the set available here:
> https://patchwork.freedesktop.org/series/78133/
> https://lore.kernel.org/dri-devel/20200818210510.49730-1-sean@poorly.run/
> 
> Sean Paul (14):
>   drm/mipi_dbi: Convert pr_debug calls to DRM_DEBUG_DRIVER
>   drm/sil164: Convert dev_printk to drm_dev_dbg
>   drm/i915/utils: Replace dev_printk with drm helpers
>   drm/msm/dpu: Replace definitions for dpu debug macros
>   drm/print: rename drm_debug* to be more syslog-centric
>   drm/amd: Gate i2c transaction logs on drm_debug_syslog
>   drm/etnaviv: Change buffer dump checks to target syslog
>   drm/nouveau: Change debug checks to specifically target syslog
>   drm/i915: Change infoframe debug checks to specify syslog
>   drm/print: Add drm_debug_category_printer
>   drm/mst: Convert debug printers to debug category printers
>   drm/i915: Use debug category printer for welcome message
>   drm/atomic: Use debug category printer for atomic state printer
>   drm/print: Add tracefs support to the drm logging helpers
> 
>  Documentation/gpu/drm-uapi.rst               |   6 +
>  drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c   |   4 +-
>  drivers/gpu/drm/drm_atomic_uapi.c            |   2 +-
>  drivers/gpu/drm/drm_dp_mst_topology.c        |   9 +-
>  drivers/gpu/drm/drm_drv.c                    |   3 +
>  drivers/gpu/drm/drm_mipi_dbi.c               |   8 +-
>  drivers/gpu/drm/drm_print.c                  | 242 ++++++++++++++++---
>  drivers/gpu/drm/etnaviv/etnaviv_buffer.c     |   8 +-
>  drivers/gpu/drm/i2c/sil164_drv.c             |  12 +-
>  drivers/gpu/drm/i915/display/intel_display.c |   4 +-
>  drivers/gpu/drm/i915/i915_drv.c              |   3 +-
>  drivers/gpu/drm/i915/i915_utils.c            |   5 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h      |  20 +-
>  drivers/gpu/drm/nouveau/nouveau_drv.h        |   4 +-
>  include/drm/drm_print.h                      |  96 +++++++-
>  15 files changed, 331 insertions(+), 95 deletions(-)