public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/sdvo: Fix up debug output to not split lines
@ 2013-11-27 11:10 Daniel Vetter
  2013-11-27 14:09 ` Mika Kuoppala
  2013-11-27 15:03 ` Daniel Vetter
  0 siblings, 2 replies; 6+ messages in thread
From: Daniel Vetter @ 2013-11-27 11:10 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

It leads to a big mess when stuff interleaves. Especially with the new
patch I've submitted for the drm core to no longer artificially split
up debug messages.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 55 ++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a583e8f718a7..e88ad95df08f 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -413,23 +413,34 @@ static const struct _sdvo_cmd_name {
 static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
 				   const void *args, int args_len)
 {
-	int i;
+	int i, pos = 0;
+#define BUF_LEN 256
+	char buffer[BUF_LEN];
+
+#define BUF_PRINT(args...) \
+	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos - 1, 0), args)
+
 
-	DRM_DEBUG_KMS("%s: W: %02X ",
-				SDVO_NAME(intel_sdvo), cmd);
-	for (i = 0; i < args_len; i++)
-		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
-	for (; i < 8; i++)
-		DRM_LOG_KMS("   ");
+	for (i = 0; i < args_len; i++) {
+		BUF_PRINT("%02X ", ((u8 *)args)[i]);
+	}
+	for (; i < 8; i++) {
+		BUF_PRINT("   ");
+	}
 	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
 		if (cmd == sdvo_cmd_names[i].cmd) {
-			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
+			BUF_PRINT("(%s)", sdvo_cmd_names[i].name);
 			break;
 		}
 	}
-	if (i == ARRAY_SIZE(sdvo_cmd_names))
-		DRM_LOG_KMS("(%02X)", cmd);
-	DRM_LOG_KMS("\n");
+	if (i == ARRAY_SIZE(sdvo_cmd_names)) {
+		BUF_PRINT("(%02X)", cmd);
+	}
+	BUG_ON(pos >= BUF_LEN - 1);
+#undef BUF_PRINT
+#undef BUF_LEN
+
+	DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
 }
 
 static const char *cmd_status_names[] = {
@@ -512,9 +523,10 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 {
 	u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
 	u8 status;
-	int i;
+	int i, pos = 0;
+#define BUF_LEN 256
+	char buffer[BUF_LEN];
 
-	DRM_DEBUG_KMS("%s: R: ", SDVO_NAME(intel_sdvo));
 
 	/*
 	 * The documentation states that all commands will be
@@ -551,10 +563,13 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 			goto log_fail;
 	}
 
+#define BUF_PRINT(args...) \
+	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos - 1, 0), args)
+
 	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
-		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
+		BUF_PRINT("(%s)", cmd_status_names[status]);
 	else
-		DRM_LOG_KMS("(??? %d)", status);
+		BUF_PRINT("(??? %d)", status);
 
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		goto log_fail;
@@ -565,13 +580,17 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 					  SDVO_I2C_RETURN_0 + i,
 					  &((u8 *)response)[i]))
 			goto log_fail;
-		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
+		BUF_PRINT(" %02X", ((u8 *)response)[i]);
 	}
-	DRM_LOG_KMS("\n");
+	BUG_ON(pos >= BUF_LEN - 1);
+#undef BUF_PRINT
+#undef BUF_LEN
+
+	DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
 	return true;
 
 log_fail:
-	DRM_LOG_KMS("... failed\n");
+	DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
 	return false;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/sdvo: Fix up debug output to not split lines
  2013-11-27 11:10 [PATCH] drm/i915/sdvo: Fix up debug output to not split lines Daniel Vetter
@ 2013-11-27 14:09 ` Mika Kuoppala
  2013-11-27 14:26   ` Daniel Vetter
  2013-11-27 15:03 ` Daniel Vetter
  1 sibling, 1 reply; 6+ messages in thread
From: Mika Kuoppala @ 2013-11-27 14:09 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter


Hi Daniel,

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> It leads to a big mess when stuff interleaves. Especially with the new
> patch I've submitted for the drm core to no longer artificially split
> up debug messages.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 55 ++++++++++++++++++++++++++-------------
>  1 file changed, 37 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index a583e8f718a7..e88ad95df08f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -413,23 +413,34 @@ static const struct _sdvo_cmd_name {
>  static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
>  				   const void *args, int args_len)
>  {
> -	int i;
> +	int i, pos = 0;
> +#define BUF_LEN 256
> +	char buffer[BUF_LEN];
> +
> +#define BUF_PRINT(args...) \
> +	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos - 1, 0), args)
> +

You want scnprintf here as it returns the true number of bytes written
instead of what would have been written. Also the size argument includes
the the trailing null space. Consider:

BUG_ON((pos += scnprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)) >= BUF_LEN)

> -	DRM_DEBUG_KMS("%s: W: %02X ",
> -				SDVO_NAME(intel_sdvo), cmd);
> -	for (i = 0; i < args_len; i++)
> -		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
> -	for (; i < 8; i++)
> -		DRM_LOG_KMS("   ");
> +	for (i = 0; i < args_len; i++) {
> +		BUF_PRINT("%02X ", ((u8 *)args)[i]);
> +	}
> +	for (; i < 8; i++) {
> +		BUF_PRINT("   ");
> +	}
>  	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
>  		if (cmd == sdvo_cmd_names[i].cmd) {
> -			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
> +			BUF_PRINT("(%s)", sdvo_cmd_names[i].name);
>  			break;
>  		}
>  	}
> -	if (i == ARRAY_SIZE(sdvo_cmd_names))
> -		DRM_LOG_KMS("(%02X)", cmd);
> -	DRM_LOG_KMS("\n");
> +	if (i == ARRAY_SIZE(sdvo_cmd_names)) {
> +		BUF_PRINT("(%02X)", cmd);
> +	}
> +	BUG_ON(pos >= BUF_LEN - 1);

pos >= BUF_LEN is enough if you choose not to BUG_ON on each scnprintf

> +#undef BUF_PRINT
> +#undef BUF_LEN
> +
> +	DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
>  }
>  
>  static const char *cmd_status_names[] = {
> @@ -512,9 +523,10 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>  {
>  	u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
>  	u8 status;
> -	int i;
> +	int i, pos = 0;
> +#define BUF_LEN 256
> +	char buffer[BUF_LEN];
>  
> -	DRM_DEBUG_KMS("%s: R: ", SDVO_NAME(intel_sdvo));
>  
>  	/*
>  	 * The documentation states that all commands will be
> @@ -551,10 +563,13 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>  			goto log_fail;
>  	}
>  
> +#define BUF_PRINT(args...) \
> +	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos - 1, 0), args)
> +
^^ Ditto

>  	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
> -		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
> +		BUF_PRINT("(%s)", cmd_status_names[status]);
>  	else
> -		DRM_LOG_KMS("(??? %d)", status);
> +		BUF_PRINT("(??? %d)", status);
>  
>  	if (status != SDVO_CMD_STATUS_SUCCESS)
>  		goto log_fail;
> @@ -565,13 +580,17 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>  					  SDVO_I2C_RETURN_0 + i,
>  					  &((u8 *)response)[i]))
>  			goto log_fail;
> -		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
> +		BUF_PRINT(" %02X", ((u8 *)response)[i]);
>  	}
> -	DRM_LOG_KMS("\n");
> +	BUG_ON(pos >= BUF_LEN - 1);
> +#undef BUF_PRINT
> +#undef BUF_LEN
> +
> +	DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
>  	return true;
>  
>  log_fail:
> -	DRM_LOG_KMS("... failed\n");
> +	DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
>  	return false;
>  }
>  
> -- 
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/sdvo: Fix up debug output to not split lines
  2013-11-27 14:09 ` Mika Kuoppala
@ 2013-11-27 14:26   ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2013-11-27 14:26 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: Intel Graphics Development

On Wed, Nov 27, 2013 at 3:09 PM, Mika Kuoppala
<mika.kuoppala@linux.intel.com> wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
>> It leads to a big mess when stuff interleaves. Especially with the new
>> patch I've submitted for the drm core to no longer artificially split
>> up debug messages.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  drivers/gpu/drm/i915/intel_sdvo.c | 55 ++++++++++++++++++++++++++-------------
>>  1 file changed, 37 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>> index a583e8f718a7..e88ad95df08f 100644
>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>> @@ -413,23 +413,34 @@ static const struct _sdvo_cmd_name {
>>  static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
>>                                  const void *args, int args_len)
>>  {
>> -     int i;
>> +     int i, pos = 0;
>> +#define BUF_LEN 256
>> +     char buffer[BUF_LEN];
>> +
>> +#define BUF_PRINT(args...) \
>> +     pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos - 1, 0), args)
>> +
>
> You want scnprintf here as it returns the true number of bytes written
> instead of what would have been written. Also the size argument includes
> the the trailing null space. Consider:

Actually the semantics of snprintf which only returns the actual
characters written without the terminating 0 at the end is what I want
- the next BUF_PRINT should overwrite things.

In case of a buffer overrunt the max should make sure that we don't
actually overwrite anything, and the BUG_ON below makes sure that the
string we feed to printk is still 0-terminated (hence the - 1 there).

> BUG_ON((pos += scnprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)) >= BUF_LEN)
>
>> -     DRM_DEBUG_KMS("%s: W: %02X ",
>> -                             SDVO_NAME(intel_sdvo), cmd);
>> -     for (i = 0; i < args_len; i++)
>> -             DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
>> -     for (; i < 8; i++)
>> -             DRM_LOG_KMS("   ");
>> +     for (i = 0; i < args_len; i++) {
>> +             BUF_PRINT("%02X ", ((u8 *)args)[i]);
>> +     }
>> +     for (; i < 8; i++) {
>> +             BUF_PRINT("   ");
>> +     }
>>       for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
>>               if (cmd == sdvo_cmd_names[i].cmd) {
>> -                     DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
>> +                     BUF_PRINT("(%s)", sdvo_cmd_names[i].name);
>>                       break;
>>               }
>>       }
>> -     if (i == ARRAY_SIZE(sdvo_cmd_names))
>> -             DRM_LOG_KMS("(%02X)", cmd);
>> -     DRM_LOG_KMS("\n");
>> +     if (i == ARRAY_SIZE(sdvo_cmd_names)) {
>> +             BUF_PRINT("(%02X)", cmd);
>> +     }
>> +     BUG_ON(pos >= BUF_LEN - 1);
>
> pos >= BUF_LEN is enough if you choose not to BUG_ON on each scnprintf

See above, with snprintf we need to make sure we have a terminating 0.
Without that the printk will go rampant.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] drm/i915/sdvo: Fix up debug output to not split lines
  2013-11-27 11:10 [PATCH] drm/i915/sdvo: Fix up debug output to not split lines Daniel Vetter
  2013-11-27 14:09 ` Mika Kuoppala
@ 2013-11-27 15:03 ` Daniel Vetter
  2013-11-27 15:15   ` Mika Kuoppala
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2013-11-27 15:03 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

It leads to a big mess when stuff interleaves. Especially with the new
patch I've submitted for the drm core to no longer artificially split
up debug messages.

v2: The size parameter to snprintf includes the terminating 0, but the
return value does not. Adjust the logic accordingly. Spotted by Mika.

Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 55 ++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a583e8f718a7..e4f9918ab859 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -413,23 +413,34 @@ static const struct _sdvo_cmd_name {
 static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
 				   const void *args, int args_len)
 {
-	int i;
+	int i, pos = 0;
+#define BUF_LEN 256
+	char buffer[BUF_LEN];
+
+#define BUF_PRINT(args...) \
+	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)
+
 
-	DRM_DEBUG_KMS("%s: W: %02X ",
-				SDVO_NAME(intel_sdvo), cmd);
-	for (i = 0; i < args_len; i++)
-		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
-	for (; i < 8; i++)
-		DRM_LOG_KMS("   ");
+	for (i = 0; i < args_len; i++) {
+		BUF_PRINT("%02X ", ((u8 *)args)[i]);
+	}
+	for (; i < 8; i++) {
+		BUF_PRINT("   ");
+	}
 	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
 		if (cmd == sdvo_cmd_names[i].cmd) {
-			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
+			BUF_PRINT("(%s)", sdvo_cmd_names[i].name);
 			break;
 		}
 	}
-	if (i == ARRAY_SIZE(sdvo_cmd_names))
-		DRM_LOG_KMS("(%02X)", cmd);
-	DRM_LOG_KMS("\n");
+	if (i == ARRAY_SIZE(sdvo_cmd_names)) {
+		BUF_PRINT("(%02X)", cmd);
+	}
+	BUG_ON(pos >= BUF_LEN - 1);
+#undef BUF_PRINT
+#undef BUF_LEN
+
+	DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
 }
 
 static const char *cmd_status_names[] = {
@@ -512,9 +523,10 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 {
 	u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
 	u8 status;
-	int i;
+	int i, pos = 0;
+#define BUF_LEN 256
+	char buffer[BUF_LEN];
 
-	DRM_DEBUG_KMS("%s: R: ", SDVO_NAME(intel_sdvo));
 
 	/*
 	 * The documentation states that all commands will be
@@ -551,10 +563,13 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 			goto log_fail;
 	}
 
+#define BUF_PRINT(args...) \
+	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)
+
 	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
-		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
+		BUF_PRINT("(%s)", cmd_status_names[status]);
 	else
-		DRM_LOG_KMS("(??? %d)", status);
+		BUF_PRINT("(??? %d)", status);
 
 	if (status != SDVO_CMD_STATUS_SUCCESS)
 		goto log_fail;
@@ -565,13 +580,17 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 					  SDVO_I2C_RETURN_0 + i,
 					  &((u8 *)response)[i]))
 			goto log_fail;
-		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
+		BUF_PRINT(" %02X", ((u8 *)response)[i]);
 	}
-	DRM_LOG_KMS("\n");
+	BUG_ON(pos >= BUF_LEN - 1);
+#undef BUF_PRINT
+#undef BUF_LEN
+
+	DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
 	return true;
 
 log_fail:
-	DRM_LOG_KMS("... failed\n");
+	DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
 	return false;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/sdvo: Fix up debug output to not split lines
  2013-11-27 15:03 ` Daniel Vetter
@ 2013-11-27 15:15   ` Mika Kuoppala
  2013-11-27 21:53     ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Mika Kuoppala @ 2013-11-27 15:15 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

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

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 55 ++++++++++++++++++++++++++-------------
>  1 file changed, 37 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index a583e8f718a7..e4f9918ab859 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -413,23 +413,34 @@ static const struct _sdvo_cmd_name {
>  static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
>  				   const void *args, int args_len)
>  {
> -	int i;
> +	int i, pos = 0;
> +#define BUF_LEN 256
> +	char buffer[BUF_LEN];
> +
> +#define BUF_PRINT(args...) \
> +	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)
> +
>  
> -	DRM_DEBUG_KMS("%s: W: %02X ",
> -				SDVO_NAME(intel_sdvo), cmd);
> -	for (i = 0; i < args_len; i++)
> -		DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
> -	for (; i < 8; i++)
> -		DRM_LOG_KMS("   ");
> +	for (i = 0; i < args_len; i++) {
> +		BUF_PRINT("%02X ", ((u8 *)args)[i]);
> +	}
> +	for (; i < 8; i++) {
> +		BUF_PRINT("   ");
> +	}
>  	for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) {
>  		if (cmd == sdvo_cmd_names[i].cmd) {
> -			DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
> +			BUF_PRINT("(%s)", sdvo_cmd_names[i].name);
>  			break;
>  		}
>  	}
> -	if (i == ARRAY_SIZE(sdvo_cmd_names))
> -		DRM_LOG_KMS("(%02X)", cmd);
> -	DRM_LOG_KMS("\n");
> +	if (i == ARRAY_SIZE(sdvo_cmd_names)) {
> +		BUF_PRINT("(%02X)", cmd);
> +	}
> +	BUG_ON(pos >= BUF_LEN - 1);
> +#undef BUF_PRINT
> +#undef BUF_LEN
> +
> +	DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
>  }
>  
>  static const char *cmd_status_names[] = {
> @@ -512,9 +523,10 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>  {
>  	u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
>  	u8 status;
> -	int i;
> +	int i, pos = 0;
> +#define BUF_LEN 256
> +	char buffer[BUF_LEN];
>  
> -	DRM_DEBUG_KMS("%s: R: ", SDVO_NAME(intel_sdvo));
>  
>  	/*
>  	 * The documentation states that all commands will be
> @@ -551,10 +563,13 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>  			goto log_fail;
>  	}
>  
> +#define BUF_PRINT(args...) \
> +	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)
> +
>  	if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
> -		DRM_LOG_KMS("(%s)", cmd_status_names[status]);
> +		BUF_PRINT("(%s)", cmd_status_names[status]);
>  	else
> -		DRM_LOG_KMS("(??? %d)", status);
> +		BUF_PRINT("(??? %d)", status);
>  
>  	if (status != SDVO_CMD_STATUS_SUCCESS)
>  		goto log_fail;
> @@ -565,13 +580,17 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>  					  SDVO_I2C_RETURN_0 + i,
>  					  &((u8 *)response)[i]))
>  			goto log_fail;
> -		DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
> +		BUF_PRINT(" %02X", ((u8 *)response)[i]);
>  	}
> -	DRM_LOG_KMS("\n");
> +	BUG_ON(pos >= BUF_LEN - 1);
> +#undef BUF_PRINT
> +#undef BUF_LEN
> +
> +	DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
>  	return true;
>  
>  log_fail:
> -	DRM_LOG_KMS("... failed\n");
> +	DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
>  	return false;
>  }
>  
> -- 
> 1.8.3.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915/sdvo: Fix up debug output to not split lines
  2013-11-27 15:15   ` Mika Kuoppala
@ 2013-11-27 21:53     ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2013-11-27 21:53 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: Daniel Vetter, Intel Graphics Development

On Wed, Nov 27, 2013 at 05:15:56PM +0200, Mika Kuoppala wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> 
> > It leads to a big mess when stuff interleaves. Especially with the new
> > patch I've submitted for the drm core to no longer artificially split
> > up debug messages.
> >
> > v2: The size parameter to snprintf includes the terminating 0, but the
> > return value does not. Adjust the logic accordingly. Spotted by Mika.
> >
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-11-27 21:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 11:10 [PATCH] drm/i915/sdvo: Fix up debug output to not split lines Daniel Vetter
2013-11-27 14:09 ` Mika Kuoppala
2013-11-27 14:26   ` Daniel Vetter
2013-11-27 15:03 ` Daniel Vetter
2013-11-27 15:15   ` Mika Kuoppala
2013-11-27 21:53     ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox