[PATCH 0/3] drm: add drm_printer based hex dumper and use it

[PATCH 0/3] drm: add drm_printer based hex dumper and use it

[Jani Nikula]

Add a hex dumper for drm_printer, and use it to simplify hex dumping in
i915.

Jani Nikula (3):
  drm/print: add drm_print_hex_dump()
  drm/i915/display: use drm_print_hex_dump() for crtc state dump
  drm/i915/display: use drm_print_hex_dump() for buffer mismatch dumps

 drivers/gpu/drm/drm_print.c                   | 23 +++++++++++++++++++
 .../drm/i915/display/intel_crtc_state_dump.c  | 14 ++---------
 drivers/gpu/drm/i915/display/intel_display.c  | 17 ++------------
 include/drm/drm_print.h                       |  2 ++
 4 files changed, 29 insertions(+), 27 deletions(-)

-- 
2.39.5

[PATCH 1/3] drm/print: add drm_print_hex_dump()

[Jani Nikula]

Add a helper to print a hex dump to a struct drm_printer. There's no
fancy formatting stuff, just 16 space-separated bytes per line, with an
optional prefix.

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

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 08cfea04e22b..79517bd4418f 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -390,3 +390,26 @@ void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
 	}
 }
 EXPORT_SYMBOL(drm_print_regset32);
+
+/**
+ * drm_print_hex_dump - print a hex dump to a &drm_printer stream
+ * @p: The &drm_printer
+ * @prefix: Prefix for each line, may be NULL for no prefix
+ * @buf: Buffer to dump
+ * @len: Length of buffer
+ *
+ * Print hex dump to &drm_printer, with 16 space-separated hex bytes per line,
+ * optionally with a prefix on each line. No separator is added after prefix.
+ */
+void drm_print_hex_dump(struct drm_printer *p, const char *prefix,
+			const u8 *buf, size_t len)
+{
+	int i;
+
+	for (i = 0; i < len; i += 16) {
+		int bytes_per_line = min(16, len - i);
+
+		drm_printf(p, "%s%*ph\n", prefix ?: "", bytes_per_line, buf + i);
+	}
+}
+EXPORT_SYMBOL(drm_print_hex_dump);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index b3906dc04388..f77fe1531cf8 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -199,6 +199,8 @@ void drm_puts(struct drm_printer *p, const char *str);
 void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
 void drm_print_bits(struct drm_printer *p, unsigned long value,
 		    const char * const bits[], unsigned int nbits);
+void drm_print_hex_dump(struct drm_printer *p, const char *prefix,
+			const u8 *buf, size_t len);
 
 __printf(2, 0)
 /**
-- 
2.39.5

[PATCH 2/3] drm/i915/display: use drm_print_hex_dump() for crtc state dump

[Jani Nikula]

Use the drm_printer based printer to get the device specific printing of
the hex dump.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../gpu/drm/i915/display/intel_crtc_state_dump.c   | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index 705ec5ad385c..1faef60be472 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -50,16 +50,6 @@ intel_dump_infoframe(struct drm_i915_private *i915,
 	hdmi_infoframe_log(KERN_DEBUG, i915->drm.dev, frame);
 }
 
-static void
-intel_dump_buffer(const char *prefix, const u8 *buf, size_t len)
-{
-	if (!drm_debug_enabled(DRM_UT_KMS))
-		return;
-
-	print_hex_dump(KERN_DEBUG, prefix, DUMP_PREFIX_NONE,
-		       16, 0, buf, len, false);
-}
-
 #define OUTPUT_TYPE(x) [INTEL_OUTPUT_ ## x] = #x
 
 static const char * const output_type_str[] = {
@@ -293,8 +283,8 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 		drm_dp_as_sdp_log(&p, &pipe_config->infoframes.as_sdp);
 
 	if (pipe_config->has_audio)
-		intel_dump_buffer("ELD: ", pipe_config->eld,
-				  drm_eld_size(pipe_config->eld));
+		drm_print_hex_dump(&p, "ELD: ", pipe_config->eld,
+				   drm_eld_size(pipe_config->eld));
 
 	drm_printf(&p, "vrr: %s, vmin: %d, vmax: %d, pipeline full: %d, guardband: %d flipline: %d, vmin vblank: %d, vmax vblank: %d\n",
 		   str_yes_no(pipe_config->vrr.enable),
-- 
2.39.5

Re: [PATCH 1/3] drm/print: add drm_print_hex_dump()

[Andi Shyti]

Hi Jani,

On Thu, Dec 05, 2024 at 11:49:33AM +0200, Jani Nikula wrote:
> Add a helper to print a hex dump to a struct drm_printer. There's no
> fancy formatting stuff, just 16 space-separated bytes per line, with an
> optional prefix.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

Re: [PATCH 2/3] drm/i915/display: use drm_print_hex_dump() for crtc state dump

[Andi Shyti]

Hi Jani,

On Thu, Dec 05, 2024 at 11:49:34AM +0200, Jani Nikula wrote:
> Use the drm_printer based printer to get the device specific printing of
> the hex dump.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

...

> -static void
> -intel_dump_buffer(const char *prefix, const u8 *buf, size_t len)
> -{
> -	if (!drm_debug_enabled(DRM_UT_KMS))
> -		return;

We lose this check now, anyway,

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

Re: [PATCH 3/3] drm/i915/display: use drm_print_hex_dump() for buffer mismatch dumps

[Andi Shyti]

Hi Jani,

On Thu, Dec 05, 2024 at 11:49:35AM +0200, Jani Nikula wrote:
> Use the drm_printer based printer to get the device specific printing of
> the hex dump, and avoid the manual loglevel hacking.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

Re: [PATCH 2/3] drm/i915/display: use drm_print_hex_dump() for crtc state dump

[Jani Nikula]

On Thu, 05 Dec 2024, Andi Shyti <andi.shyti@linux.intel.com> wrote:
> Hi Jani,
>
> On Thu, Dec 05, 2024 at 11:49:34AM +0200, Jani Nikula wrote:
>> Use the drm_printer based printer to get the device specific printing of
>> the hex dump.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ...
>
>> -static void
>> -intel_dump_buffer(const char *prefix, const u8 *buf, size_t len)
>> -{
>> -	if (!drm_debug_enabled(DRM_UT_KMS))
>> -		return;
>
> We lose this check now, anyway,

That now depends on the drm_printer, as it should.

Moreover, intel_crtc_state_dump() already has that check, so this is
completely redundant.

> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Jani.

>
> Thanks,
> Andi

-- 
Jani Nikula, Intel

Re: [PATCH 1/3] drm/print: add drm_print_hex_dump()

[Jani Nikula]

On Thu, 05 Dec 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> Add a helper to print a hex dump to a struct drm_printer. There's no
> fancy formatting stuff, just 16 space-separated bytes per line, with an
> optional prefix.

drm-misc maintainers, ack for merging this via drm-intel-next?

BR,
Jani.


>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_print.c | 23 +++++++++++++++++++++++
>  include/drm/drm_print.h     |  2 ++
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 08cfea04e22b..79517bd4418f 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -390,3 +390,26 @@ void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset)
>  	}
>  }
>  EXPORT_SYMBOL(drm_print_regset32);
> +
> +/**
> + * drm_print_hex_dump - print a hex dump to a &drm_printer stream
> + * @p: The &drm_printer
> + * @prefix: Prefix for each line, may be NULL for no prefix
> + * @buf: Buffer to dump
> + * @len: Length of buffer
> + *
> + * Print hex dump to &drm_printer, with 16 space-separated hex bytes per line,
> + * optionally with a prefix on each line. No separator is added after prefix.
> + */
> +void drm_print_hex_dump(struct drm_printer *p, const char *prefix,
> +			const u8 *buf, size_t len)
> +{
> +	int i;
> +
> +	for (i = 0; i < len; i += 16) {
> +		int bytes_per_line = min(16, len - i);
> +
> +		drm_printf(p, "%s%*ph\n", prefix ?: "", bytes_per_line, buf + i);
> +	}
> +}
> +EXPORT_SYMBOL(drm_print_hex_dump);
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index b3906dc04388..f77fe1531cf8 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -199,6 +199,8 @@ void drm_puts(struct drm_printer *p, const char *str);
>  void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
>  void drm_print_bits(struct drm_printer *p, unsigned long value,
>  		    const char * const bits[], unsigned int nbits);
> +void drm_print_hex_dump(struct drm_printer *p, const char *prefix,
> +			const u8 *buf, size_t len);
>  
>  __printf(2, 0)
>  /**

-- 
Jani Nikula, Intel

Re: [PATCH 1/3] drm/print: add drm_print_hex_dump()

[Jani Nikula]

On Thu, 05 Dec 2024, Andi Shyti <andi.shyti@linux.intel.com> wrote:
> Hi Jani,
>
> On Thu, Dec 05, 2024 at 11:49:33AM +0200, Jani Nikula wrote:
>> Add a helper to print a hex dump to a struct drm_printer. There's no
>> fancy formatting stuff, just 16 space-separated bytes per line, with an
>> optional prefix.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks for the review, pushed the series to drm-intel-next with Thomas'
IRC ack on patch 1.

BR,
Jani.

>
> Thanks,
> Andi

-- 
Jani Nikula, Intel