[PATCH] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log

[PATCH] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

dm_log_to_buffer logs unconditionally, so calling it directly resulted
in the main message being logged even when the event type isn't enabled
in the event mask.

To fix this, remove the trailing newline from the format string and call
dm_logger_append instead.

Fixes spurious messages like

 [drm] {1920x1200, 2080x1235@154000Khz}

in dmesg when a mode is set.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/amd/display/dc/basics/log_helpers.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
index 785b943b60ed..02d43e29ded5 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
@@ -60,6 +60,8 @@ void dc_conn_log(struct dc_context *ctx,
 	va_list args;
 	struct log_entry entry = { 0 };
 	enum signal_type signal;
+	char *msg_no_newline = (char*)msg;
+	size_t msg_len = strlen(msg) - 1;
 
 	if (link->local_sink)
 		signal = link->local_sink->sink_signal;
@@ -79,17 +81,15 @@ void dc_conn_log(struct dc_context *ctx,
 			signal_type_info_tbl[i].name,
 			link->link_index);
 
-	va_start(args, msg);
-	entry.buf_offset += dm_log_to_buffer(
-		&entry.buf[entry.buf_offset],
-		LOG_MAX_LINE_SIZE - entry.buf_offset,
-		msg, args);
-
-	if (entry.buf[strlen(entry.buf) - 1] == '\n') {
-		entry.buf[strlen(entry.buf) - 1] = '\0';
-		entry.buf_offset--;
+	if (msg[msg_len] == '\n') {
+		msg_no_newline = kstrdup(msg, GFP_KERNEL);
+		msg_no_newline[msg_len] = '\0';
 	}
 
+	dm_logger_append(&entry, msg_no_newline, args);
+	if (msg_no_newline != msg)
+		kfree(msg_no_newline);
+
 	if (hex_data)
 		for (i = 0; i < hex_data_count; i++)
 			dm_logger_append(&entry, "%2.2X ", hex_data[i]);
@@ -97,6 +97,4 @@ void dc_conn_log(struct dc_context *ctx,
 	dm_logger_append(&entry, "^\n");
 	dm_helpers_dc_conn_log(ctx, &entry, event);
 	dm_logger_close(&entry);
-
-	va_end(args);
 }
-- 
2.15.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log

[Harry Wentland]

On 2017-11-23 06:51 AM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> dm_log_to_buffer logs unconditionally, so calling it directly resulted
> in the main message being logged even when the event type isn't enabled
> in the event mask.
> 
> To fix this, remove the trailing newline from the format string and call
> dm_logger_append instead.
> 
> Fixes spurious messages like
> 
>  [drm] {1920x1200, 2080x1235@154000Khz}
> 
> in dmesg when a mode is set.
> 
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Thanks for catching and fixing this. I missed this when eye-balling the code yesterday.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/basics/log_helpers.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> index 785b943b60ed..02d43e29ded5 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> @@ -60,6 +60,8 @@ void dc_conn_log(struct dc_context *ctx,
>  	va_list args;
>  	struct log_entry entry = { 0 };
>  	enum signal_type signal;
> +	char *msg_no_newline = (char*)msg;
> +	size_t msg_len = strlen(msg) - 1;
>  
>  	if (link->local_sink)
>  		signal = link->local_sink->sink_signal;
> @@ -79,17 +81,15 @@ void dc_conn_log(struct dc_context *ctx,
>  			signal_type_info_tbl[i].name,
>  			link->link_index);
>  
> -	va_start(args, msg);
> -	entry.buf_offset += dm_log_to_buffer(
> -		&entry.buf[entry.buf_offset],
> -		LOG_MAX_LINE_SIZE - entry.buf_offset,
> -		msg, args);
> -
> -	if (entry.buf[strlen(entry.buf) - 1] == '\n') {
> -		entry.buf[strlen(entry.buf) - 1] = '\0';
> -		entry.buf_offset--;
> +	if (msg[msg_len] == '\n') {
> +		msg_no_newline = kstrdup(msg, GFP_KERNEL);
> +		msg_no_newline[msg_len] = '\0';
>  	}
>  
> +	dm_logger_append(&entry, msg_no_newline, args);
> +	if (msg_no_newline != msg)
> +		kfree(msg_no_newline);
> +
>  	if (hex_data)
>  		for (i = 0; i < hex_data_count; i++)
>  			dm_logger_append(&entry, "%2.2X ", hex_data[i]);
> @@ -97,6 +97,4 @@ void dc_conn_log(struct dc_context *ctx,
>  	dm_logger_append(&entry, "^\n");
>  	dm_helpers_dc_conn_log(ctx, &entry, event);
>  	dm_logger_close(&entry);
> -
> -	va_end(args);
>  }
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/2] drm/amd/display: Add dm_logger_append_va API

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

Same as dm_logger_append, except it takes a va_list instead of a
variable number of arguments. dm_logger_append is now a minimal wrapper
around dm_logger_append_va.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/amd/display/dc/basics/logger.c         | 17 ++++++++++++-----
 drivers/gpu/drm/amd/display/include/logger_interface.h |  5 +++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/basics/logger.c b/drivers/gpu/drm/amd/display/dc/basics/logger.c
index 2ff5b467603d..180a9d69d351 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/logger.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/logger.c
@@ -311,6 +311,18 @@ void dm_logger_append(
 	struct log_entry *entry,
 	const char *msg,
 	...)
+{
+	va_list args;
+
+	va_start(args, msg);
+	dm_logger_append_va(entry, msg, args);
+	va_end(args);
+}
+
+void dm_logger_append_va(
+	struct log_entry *entry,
+	const char *msg,
+	va_list args)
 {
 	struct dal_logger *logger;
 
@@ -325,11 +337,8 @@ void dm_logger_append(
 		dal_logger_should_log(logger, entry->type)) {
 
 		uint32_t size;
-		va_list args;
 		char buffer[LOG_MAX_LINE_SIZE];
 
-		va_start(args, msg);
-
 		size = dm_log_to_buffer(
 			buffer, LOG_MAX_LINE_SIZE, msg, args);
 
@@ -338,8 +347,6 @@ void dm_logger_append(
 		} else {
 			append_entry(entry, "LOG_ERROR, line too long\n", 27);
 		}
-
-		va_end(args);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/display/include/logger_interface.h b/drivers/gpu/drm/amd/display/include/logger_interface.h
index 8e1fe70097be..28dee960d509 100644
--- a/drivers/gpu/drm/amd/display/include/logger_interface.h
+++ b/drivers/gpu/drm/amd/display/include/logger_interface.h
@@ -57,6 +57,11 @@ void dm_logger_append(
 		const char *msg,
 		...);
 
+void dm_logger_append_va(
+		struct log_entry *entry,
+		const char *msg,
+		va_list args);
+
 void dm_logger_open(
 		struct dal_logger *logger,
 		struct log_entry *entry,
-- 
2.15.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 2/2] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

dm_log_to_buffer logs unconditionally, so calling it directly resulted
in the main message being logged even when the event type isn't enabled
in the event mask.

To fix this, use the new dm_logger_append_va API.

Fixes spurious messages like

 [drm] {1920x1200, 2080x1235@154000Khz}

in dmesg when a mode is set.

v2:
* Use new dm_logger_append_va API, fixes incorrect va_list usage in v1
* Just use and decrease entry.buf_offset to get rid of the trailing
  newline

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/amd/display/dc/basics/log_helpers.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
index 785b943b60ed..fe1648f81d71 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
@@ -80,15 +80,11 @@ void dc_conn_log(struct dc_context *ctx,
 			link->link_index);
 
 	va_start(args, msg);
-	entry.buf_offset += dm_log_to_buffer(
-		&entry.buf[entry.buf_offset],
-		LOG_MAX_LINE_SIZE - entry.buf_offset,
-		msg, args);
+	dm_logger_append_va(&entry, msg, args);
 
-	if (entry.buf[strlen(entry.buf) - 1] == '\n') {
-		entry.buf[strlen(entry.buf) - 1] = '\0';
+	if (entry.buf_offset > 0 &&
+	    entry.buf[entry.buf_offset - 1] == '\n')
 		entry.buf_offset--;
-	}
 
 	if (hex_data)
 		for (i = 0; i < hex_data_count; i++)
-- 
2.15.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/2] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log

[Michel Dänzer]

Ping on this series.

This patch is v2 of a previously single patch, which was reviewed by Harry.


On 2017-11-23 06:48 PM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> dm_log_to_buffer logs unconditionally, so calling it directly resulted
> in the main message being logged even when the event type isn't enabled
> in the event mask.
> 
> To fix this, use the new dm_logger_append_va API.
> 
> Fixes spurious messages like
> 
>  [drm] {1920x1200, 2080x1235@154000Khz}
> 
> in dmesg when a mode is set.
> 
> v2:
> * Use new dm_logger_append_va API, fixes incorrect va_list usage in v1
> * Just use and decrease entry.buf_offset to get rid of the trailing
>   newline
> 
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>  drivers/gpu/drm/amd/display/dc/basics/log_helpers.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> index 785b943b60ed..fe1648f81d71 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
> @@ -80,15 +80,11 @@ void dc_conn_log(struct dc_context *ctx,
>  			link->link_index);
>  
>  	va_start(args, msg);
> -	entry.buf_offset += dm_log_to_buffer(
> -		&entry.buf[entry.buf_offset],
> -		LOG_MAX_LINE_SIZE - entry.buf_offset,
> -		msg, args);
> +	dm_logger_append_va(&entry, msg, args);
>  
> -	if (entry.buf[strlen(entry.buf) - 1] == '\n') {
> -		entry.buf[strlen(entry.buf) - 1] = '\0';
> +	if (entry.buf_offset > 0 &&
> +	    entry.buf[entry.buf_offset - 1] == '\n')
>  		entry.buf_offset--;
> -	}
>  
>  	if (hex_data)
>  		for (i = 0; i < hex_data_count; i++)
> 


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/2] drm/amd/display: Don't call dm_log_to_buffer directly in dc_conn_log

[Alex Deucher]

On Tue, Nov 28, 2017 at 6:09 AM, Michel Dänzer <michel@daenzer.net> wrote:
>
> Ping on this series.
>
> This patch is v2 of a previously single patch, which was reviewed by Harry.
>
>

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> On 2017-11-23 06:48 PM, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> dm_log_to_buffer logs unconditionally, so calling it directly resulted
>> in the main message being logged even when the event type isn't enabled
>> in the event mask.
>>
>> To fix this, use the new dm_logger_append_va API.
>>
>> Fixes spurious messages like
>>
>>  [drm] {1920x1200, 2080x1235@154000Khz}
>>
>> in dmesg when a mode is set.
>>
>> v2:
>> * Use new dm_logger_append_va API, fixes incorrect va_list usage in v1
>> * Just use and decrease entry.buf_offset to get rid of the trailing
>>   newline
>>
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> ---
>>  drivers/gpu/drm/amd/display/dc/basics/log_helpers.c | 10 +++-------
>>  1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
>> index 785b943b60ed..fe1648f81d71 100644
>> --- a/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
>> +++ b/drivers/gpu/drm/amd/display/dc/basics/log_helpers.c
>> @@ -80,15 +80,11 @@ void dc_conn_log(struct dc_context *ctx,
>>                       link->link_index);
>>
>>       va_start(args, msg);
>> -     entry.buf_offset += dm_log_to_buffer(
>> -             &entry.buf[entry.buf_offset],
>> -             LOG_MAX_LINE_SIZE - entry.buf_offset,
>> -             msg, args);
>> +     dm_logger_append_va(&entry, msg, args);
>>
>> -     if (entry.buf[strlen(entry.buf) - 1] == '\n') {
>> -             entry.buf[strlen(entry.buf) - 1] = '\0';
>> +     if (entry.buf_offset > 0 &&
>> +         entry.buf[entry.buf_offset - 1] == '\n')
>>               entry.buf_offset--;
>> -     }
>>
>>       if (hex_data)
>>               for (i = 0; i < hex_data_count; i++)
>>
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx