[1/7] drm: Add a helper function for printing a debugfs_regset32.
diff mbox series

Message ID 20190220210343.28157-1-eric@anholt.net
State New
Headers show
Series
  • [1/7] drm: Add a helper function for printing a debugfs_regset32.
Related show

Commit Message

Eric Anholt Feb. 20, 2019, 9:03 p.m. UTC
The debugfs_regset32 is nice to use for reducing boilerplate in
dumping a bunch of regs in debugfs, but we also want to be able to
print to dmesg them at runtime for driver debugging.  drm_printer lets
us format debugfs and the printk the same way.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
 include/drm/drm_print.h     |  2 ++
 2 files changed, 18 insertions(+)

Comments

Daniel Vetter Feb. 21, 2019, 9:37 a.m. UTC | #1
On Wed, Feb 20, 2019 at 01:03:37PM -0800, Eric Anholt wrote:
> The debugfs_regset32 is nice to use for reducing boilerplate in
> dumping a bunch of regs in debugfs, but we also want to be able to
> print to dmesg them at runtime for driver debugging.  drm_printer lets
> us format debugfs and the printk the same way.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
>  include/drm/drm_print.h     |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
>  	va_end(args);
>  }
>  EXPORT_SYMBOL(drm_err);
> +

A bit of kerneldoc would be nice. With that:

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

> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
> +{
> +	int namelen = 0;
> +	int i;
> +
> +	for (i = 0; i < regset->nregs; i++)
> +		namelen = max(namelen, (int)strlen(regset->regs[i].name));
> +
> +	for (i = 0; i < regset->nregs; i++) {
> +		drm_printf(p, "%*s = 0x%08x\n",
> +			   namelen, regset->regs[i].name,
> +			   readl(regset->base + regset->regs[i].offset));
> +	}
> +}
> +EXPORT_SYMBOL(drm_print_regset32);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index afbc3beef089..3a4247319e63 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -30,6 +30,7 @@
>  #include <linux/printk.h>
>  #include <linux/seq_file.h>
>  #include <linux/device.h>
> +#include <linux/debugfs.h>
>  
>  /**
>   * DOC: print
> @@ -84,6 +85,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
>  __printf(2, 3)
>  void drm_printf(struct drm_printer *p, const char *f, ...);
>  void drm_puts(struct drm_printer *p, const char *str);
> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
>  
>  __printf(2, 0)
>  /**
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Paul Kocialkowski March 22, 2019, 10:30 a.m. UTC | #2
Hi,

Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
> The debugfs_regset32 is nice to use for reducing boilerplate in
> dumping a bunch of regs in debugfs, but we also want to be able to
> print to dmesg them at runtime for driver debugging.  drm_printer lets
> us format debugfs and the printk the same way.

Looks good to me!

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
>  include/drm/drm_print.h     |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
>  	va_end(args);
>  }
>  EXPORT_SYMBOL(drm_err);
> +
> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
> +{
> +	int namelen = 0;
> +	int i;
> +
> +	for (i = 0; i < regset->nregs; i++)
> +		namelen = max(namelen, (int)strlen(regset->regs[i].name));
> +
> +	for (i = 0; i < regset->nregs; i++) {
> +		drm_printf(p, "%*s = 0x%08x\n",
> +			   namelen, regset->regs[i].name,
> +			   readl(regset->base + regset->regs[i].offset));
> +	}
> +}
> +EXPORT_SYMBOL(drm_print_regset32);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index afbc3beef089..3a4247319e63 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -30,6 +30,7 @@
>  #include <linux/printk.h>
>  #include <linux/seq_file.h>
>  #include <linux/device.h>
> +#include <linux/debugfs.h>
>  
>  /**
>   * DOC: print
> @@ -84,6 +85,7 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
>  __printf(2, 3)
>  void drm_printf(struct drm_printer *p, const char *f, ...);
>  void drm_puts(struct drm_printer *p, const char *str);
> +void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
>  
>  __printf(2, 0)
>  /**
Eric Anholt March 25, 2019, 4:43 p.m. UTC | #3
Daniel Vetter <daniel@ffwll.ch> writes:

> On Wed, Feb 20, 2019 at 01:03:37PM -0800, Eric Anholt wrote:
>> The debugfs_regset32 is nice to use for reducing boilerplate in
>> dumping a bunch of regs in debugfs, but we also want to be able to
>> print to dmesg them at runtime for driver debugging.  drm_printer lets
>> us format debugfs and the printk the same way.
>> 
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>  drivers/gpu/drm/drm_print.c | 16 ++++++++++++++++
>>  include/drm/drm_print.h     |  2 ++
>>  2 files changed, 18 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
>> index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
>> --- a/drivers/gpu/drm/drm_print.c
>> +++ b/drivers/gpu/drm/drm_print.c
>> @@ -253,3 +253,19 @@ void drm_err(const char *format, ...)
>>  	va_end(args);
>>  }
>>  EXPORT_SYMBOL(drm_err);
>> +
>
> A bit of kerneldoc would be nice. With that:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

+/**
+ * drm_print_regset32 - print the contents of registers to a
+ * &drm_printer stream.
+ *
+ * @p: the &drm printer
+ * @regset: the list of registers to print.
+ *
+ * Often in driver debug, it's useful to be able to either capture the
+ * contents of registers in the steady state using debugfs or at
+ * specific points during operation.  This lets the driver have a
+ * single list of registers for both.
+ */
Eric Anholt April 1, 2019, 5:52 p.m. UTC | #4
Paul Kocialkowski <paul.kocialkowski@bootlin.com> writes:

> Hi,
>
> Le mercredi 20 février 2019 à 13:03 -0800, Eric Anholt a écrit :
>> The debugfs_regset32 is nice to use for reducing boilerplate in
>> dumping a bunch of regs in debugfs, but we also want to be able to
>> print to dmesg them at runtime for driver debugging.  drm_printer lets
>> us format debugfs and the printk the same way.
>
> Looks good to me!
>
> Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Thanks!  Merged all but patch 3 (unreviewed) and patch 5 (needs patch 3
to avoid oopsing).  Sending out a rebased version shortly.

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 0e7fc3e7dfb4..5ecc0f04cd0c 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -253,3 +253,19 @@  void drm_err(const char *format, ...)
 	va_end(args);
 }
 EXPORT_SYMBOL(drm_err);
+
+void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
+{
+	int namelen = 0;
+	int i;
+
+	for (i = 0; i < regset->nregs; i++)
+		namelen = max(namelen, (int)strlen(regset->regs[i].name));
+
+	for (i = 0; i < regset->nregs; i++) {
+		drm_printf(p, "%*s = 0x%08x\n",
+			   namelen, regset->regs[i].name,
+			   readl(regset->base + regset->regs[i].offset));
+	}
+}
+EXPORT_SYMBOL(drm_print_regset32);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index afbc3beef089..3a4247319e63 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -30,6 +30,7 @@ 
 #include <linux/printk.h>
 #include <linux/seq_file.h>
 #include <linux/device.h>
+#include <linux/debugfs.h>
 
 /**
  * DOC: print
@@ -84,6 +85,7 @@  void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
 __printf(2, 3)
 void drm_printf(struct drm_printer *p, const char *f, ...);
 void drm_puts(struct drm_printer *p, const char *str);
+void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
 
 __printf(2, 0)
 /**