diff mbox

drm/i915: debug print all of the DPCD we have

Message ID 1351151890-24891-1-git-send-email-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jani Nikula Oct. 25, 2012, 7:58 a.m. UTC
At some point the DPCD size was increased, but the debug print not. While
at it, switch to using hex dump.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

The obvious downside here is that the hex dumping to buffer is done
regardless of drm.debug value. I thought it was still better than the
alternatives:

1) DRM_DEBUG_KMS with loads of %02x as it were

2) if (drm_debug & DRM_UT_KMS)

3) print_hex_dump(KERN_DEBUG, ... )

4) adding a new drm_print_hex_dump or similar that does 2) internally
---
 drivers/gpu/drm/i915/intel_dp.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Mika Kuoppala Oct. 25, 2012, 12:40 p.m. UTC | #1
On Thu, 25 Oct 2012 10:58:10 +0300, Jani Nikula <jani.nikula@intel.com> wrote:
> At some point the DPCD size was increased, but the debug print not. While
> at it, switch to using hex dump.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> The obvious downside here is that the hex dumping to buffer is done
> regardless of drm.debug value. I thought it was still better than the
> alternatives:
> 
> 1) DRM_DEBUG_KMS with loads of %02x as it were
> 
> 2) if (drm_debug & DRM_UT_KMS)
> 
> 3) print_hex_dump(KERN_DEBUG, ... )
> 
> 4) adding a new drm_print_hex_dump or similar that does 2) internally
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 01b67d9..52eea34 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2278,6 +2278,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>  	struct drm_device *dev = intel_dp->base.base.dev;
>  	enum drm_connector_status status;
>  	struct edid *edid = NULL;
> +	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];

It seems hex_dump_to_buffer nukes trailing ' ' before inserting null.
Would make reader less nervous by adding + 1 thought, for courtesy :)

>  	intel_dp->has_audio = false;
>  
> @@ -2286,10 +2287,9 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>  	else
>  		status = g4x_dp_detect(intel_dp);
>  
> -	DRM_DEBUG_KMS("DPCD: %02hx%02hx%02hx%02hx%02hx%02hx%02hx%02hx\n",
> -		      intel_dp->dpcd[0], intel_dp->dpcd[1], intel_dp->dpcd[2],
> -		      intel_dp->dpcd[3], intel_dp->dpcd[4], intel_dp->dpcd[5],
> -		      intel_dp->dpcd[6], intel_dp->dpcd[7]);
> +	hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),
> +			   32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
> +	DRM_DEBUG_KMS("DPCD: %s\n", dpcd_hex_dump);
>  
>  	if (status != connector_status_connected)
>  		return status;
> -- 
> 1.7.9.5

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Paulo Zanoni Oct. 25, 2012, 1 p.m. UTC | #2
Hi

2012/10/25 Jani Nikula <jani.nikula@intel.com>:
> At some point the DPCD size was increased, but the debug print not. While
> at it, switch to using hex dump.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> ---
>
> The obvious downside here is that the hex dumping to buffer is done
> regardless of drm.debug value. I thought it was still better than the
> alternatives:
>
> 1) DRM_DEBUG_KMS with loads of %02x as it were
>
> 2) if (drm_debug & DRM_UT_KMS)
>
> 3) print_hex_dump(KERN_DEBUG, ... )
>
> 4) adding a new drm_print_hex_dump or similar that does 2) internally
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 01b67d9..52eea34 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2278,6 +2278,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>         struct drm_device *dev = intel_dp->base.base.dev;
>         enum drm_connector_status status;
>         struct edid *edid = NULL;
> +       char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
>
>         intel_dp->has_audio = false;
>
> @@ -2286,10 +2287,9 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>         else
>                 status = g4x_dp_detect(intel_dp);
>
> -       DRM_DEBUG_KMS("DPCD: %02hx%02hx%02hx%02hx%02hx%02hx%02hx%02hx\n",
> -                     intel_dp->dpcd[0], intel_dp->dpcd[1], intel_dp->dpcd[2],
> -                     intel_dp->dpcd[3], intel_dp->dpcd[4], intel_dp->dpcd[5],
> -                     intel_dp->dpcd[6], intel_dp->dpcd[7]);
> +       hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),
> +                          32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
> +       DRM_DEBUG_KMS("DPCD: %s\n", dpcd_hex_dump);
>
>         if (status != connector_status_connected)
>                 return status;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Oct. 26, 2012, 6:50 p.m. UTC | #3
On Thu, Oct 25, 2012 at 11:00:32AM -0200, Paulo Zanoni wrote:
> Hi
> 
> 2012/10/25 Jani Nikula <jani.nikula@intel.com>:
> > At some point the DPCD size was increased, but the debug print not. While
> > at it, switch to using hex dump.
> >
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Queued for -next, thanks for the patch.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 01b67d9..52eea34 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2278,6 +2278,7 @@  intel_dp_detect(struct drm_connector *connector, bool force)
 	struct drm_device *dev = intel_dp->base.base.dev;
 	enum drm_connector_status status;
 	struct edid *edid = NULL;
+	char dpcd_hex_dump[sizeof(intel_dp->dpcd) * 3];
 
 	intel_dp->has_audio = false;
 
@@ -2286,10 +2287,9 @@  intel_dp_detect(struct drm_connector *connector, bool force)
 	else
 		status = g4x_dp_detect(intel_dp);
 
-	DRM_DEBUG_KMS("DPCD: %02hx%02hx%02hx%02hx%02hx%02hx%02hx%02hx\n",
-		      intel_dp->dpcd[0], intel_dp->dpcd[1], intel_dp->dpcd[2],
-		      intel_dp->dpcd[3], intel_dp->dpcd[4], intel_dp->dpcd[5],
-		      intel_dp->dpcd[6], intel_dp->dpcd[7]);
+	hex_dump_to_buffer(intel_dp->dpcd, sizeof(intel_dp->dpcd),
+			   32, 1, dpcd_hex_dump, sizeof(dpcd_hex_dump), false);
+	DRM_DEBUG_KMS("DPCD: %s\n", dpcd_hex_dump);
 
 	if (status != connector_status_connected)
 		return status;