[PATCH 04/16 v2] drm/amd/display: Add tracing to dc

Kuehling, Felix Felix.Kuehling at amd.com
Fri Dec 7 05:40:41 UTC 2018


This change seems to be breaking the build for me. I'm getting errors like this:

  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.o
In file included from ../include/trace/events/tlb.h:9:0,
                 from ../arch/x86/include/asm/mmu_context.h:10,
                 from ../include/linux/mmu_context.h:5,
                 from ../drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h:30,
                 from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:85,
                 from ../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:34:
../include/linux/tracepoint.h:285:20: error: redefinition of â__tpstrtab_amdgpu_dc_rregâ
  static const char __tpstrtab_##name[]     \
                    ^
../include/linux/tracepoint.h:293:2: note: in expansion of macro âDEFINE_TRACE_FNâ
  DEFINE_TRACE_FN(name, NULL, NULL);
  ^
../include/trace/define_trace.h:28:2: note: in expansion of macro âDEFINE_TRACEâ
  DEFINE_TRACE(name)
  ^
../drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/./amdgpu_dm_trace.h:34:1: note: in expansion of macro âTRACE_EVENTâ
 TRACE_EVENT(amdgpu_dc_rreg,
 ^

I have a local change that adds #include <amdgpu_amdkfd.h> to amdgpu.h, which pulls in include/trace/events/tlb.h. That seems to create some kind of conflict with trace definitions. Any ideas how to fix this? It seems a bit fragile if adding some random include can break the build like this.

Thanks,
  Felix

-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of David Francis
Sent: Friday, November 30, 2018 9:57 AM
To: amd-gfx at lists.freedesktop.org
Cc: Francis, David <David.Francis at amd.com>
Subject: [PATCH 04/16 v2] drm/amd/display: Add tracing to dc

[Why]
Tracing is a useful and cheap debug functionality

[How]
This creates a new trace system amdgpu_dm, currently with three trace events

amdgpu_dc_rreg and amdgpu_dc_wreg report the address and value of any dc register reads and writes

amdgpu_dc_performance requires at least one of those two to be enabled.  It counts the register reads and writes since the last entry

v2: Don't check for NULL before kfree

Signed-off-by: David Francis <David.Francis at amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland at amd.com>
Acked-by: Leo Li <sunpeng.li at amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   3 +
 .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 104 ++++++++++++++++++
 drivers/gpu/drm/amd/display/dc/core/dc.c      |  19 ++++
 drivers/gpu/drm/amd/display/dc/dc_types.h     |   8 ++
 .../amd/display/dc/dcn10/dcn10_cm_common.c    |   4 +-
 drivers/gpu/drm/amd/display/dc/dm_services.h  |  12 +-
 6 files changed, 146 insertions(+), 4 deletions(-)  create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 76b1aebdca0c..376927c8bcc6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -23,6 +23,9 @@
  *
  */
 
+/* The caprices of the preprocessor require that this be declared right 
+here */ #define CREATE_TRACE_POINTS
+
 #include "dm_services_types.h"
 #include "dc.h"
 #include "dc/inc/core_types.h"
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
new file mode 100644
index 000000000000..d898981684d5
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -0,0 +1,104 @@
+/*
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the 
+"Software"),
+ * to deal in the Software without restriction, including without 
+limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
+sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
+the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be 
+included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
+SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
+DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
+OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
+OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: AMD
+ *
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM amdgpu_dm
+
+#if !defined(_AMDGPU_DM_TRACE_H) || defined(TRACE_HEADER_MULTI_READ) 
+#define _AMDGPU_DM_TRACE_H_
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(amdgpu_dc_rreg,
+	TP_PROTO(unsigned long *read_count, uint32_t reg, uint32_t value),
+	TP_ARGS(read_count, reg, value),
+	TP_STRUCT__entry(
+			__field(uint32_t, reg)
+			__field(uint32_t, value)
+		),
+	TP_fast_assign(
+			__entry->reg = reg;
+			__entry->value = value;
+			*read_count = *read_count + 1;
+		),
+	TP_printk("reg=0x%08lx, value=0x%08lx",
+			(unsigned long)__entry->reg,
+			(unsigned long)__entry->value)
+);
+
+TRACE_EVENT(amdgpu_dc_wreg,
+	TP_PROTO(unsigned long *write_count, uint32_t reg, uint32_t value),
+	TP_ARGS(write_count, reg, value),
+	TP_STRUCT__entry(
+			__field(uint32_t, reg)
+			__field(uint32_t, value)
+		),
+	TP_fast_assign(
+			__entry->reg = reg;
+			__entry->value = value;
+			*write_count = *write_count + 1;
+		),
+	TP_printk("reg=0x%08lx, value=0x%08lx",
+			(unsigned long)__entry->reg,
+			(unsigned long)__entry->value)
+);
+
+
+TRACE_EVENT(amdgpu_dc_performance,
+	TP_PROTO(unsigned long read_count, unsigned long write_count,
+		unsigned long *last_read, unsigned long *last_write,
+		const char *func, unsigned int line),
+	TP_ARGS(read_count, write_count, last_read, last_write, func, line),
+	TP_STRUCT__entry(
+			__field(uint32_t, reads)
+			__field(uint32_t, writes)
+			__field(uint32_t, read_delta)
+			__field(uint32_t, write_delta)
+			__string(func, func)
+			__field(uint32_t, line)
+			),
+	TP_fast_assign(
+			__entry->reads = read_count;
+			__entry->writes = write_count;
+			__entry->read_delta = read_count - *last_read;
+			__entry->write_delta = write_count - *last_write;
+			__assign_str(func, func);
+			__entry->line = line;
+			*last_read = read_count;
+			*last_write = write_count;
+			),
+	TP_printk("%s:%d reads=%08ld (%08ld total), writes=%08ld (%08ld total)",
+			__get_str(func), __entry->line,
+			(unsigned long)__entry->read_delta,
+			(unsigned long)__entry->reads,
+			(unsigned long)__entry->write_delta,
+			(unsigned long)__entry->writes)
+);
+#endif /* _AMDGPU_DM_TRACE_H_ */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_FILE amdgpu_dm_trace #include 
+<trace/define_trace.h>
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index dba6b57830c7..3903b2c0a6f1 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -175,6 +175,17 @@ static bool create_links(
 	return false;
 }
 
+static struct dc_perf_trace *dc_perf_trace_create(void) {
+	return kzalloc(sizeof(struct dc_perf_trace), GFP_KERNEL); }
+
+static void dc_perf_trace_destroy(struct dc_perf_trace **perf_trace) {
+	kfree(*perf_trace);
+	*perf_trace = NULL;
+}
+
 /**
  *****************************************************************************
  *  Function: dc_stream_adjust_vmin_vmax @@ -536,6 +547,8 @@ static void destruct(struct dc *dc)
 	if (dc->ctx->created_bios)
 		dal_bios_parser_destroy(&dc->ctx->dc_bios);
 
+	dc_perf_trace_destroy(&dc->ctx->perf_trace);
+
 	kfree(dc->ctx);
 	dc->ctx = NULL;
 
@@ -659,6 +672,12 @@ static bool construct(struct dc *dc,
 		goto fail;
 	}
 
+	dc_ctx->perf_trace = dc_perf_trace_create();
+	if (!dc_ctx->perf_trace) {
+		ASSERT_CRITICAL(false);
+		goto fail;
+	}
+
 	/* Create GPIO service */
 	dc_ctx->gpio_service = dal_gpio_service_create(
 			dc_version,
diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h
index 6e12d640d020..8390baedaf71 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_types.h
@@ -73,10 +73,18 @@ struct hw_asic_id {
 	void *atombios_base_address;
 };
 
+struct dc_perf_trace {
+	unsigned long read_count;
+	unsigned long write_count;
+	unsigned long last_entry_read;
+	unsigned long last_entry_write;
+};
+
 struct dc_context {
 	struct dc *dc;
 
 	void *driver_context; /* e.g. amdgpu_device */
+	struct dc_perf_trace *perf_trace;
 	void *cgs_device;
 
 	enum dce_environment dce_environment;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
index 3eea44092a04..7469333a2c8a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
@@ -324,7 +324,7 @@ bool cm_helper_translate_curve_to_hw_format(
 	if (output_tf == NULL || lut_params == NULL || output_tf->type == TF_TYPE_BYPASS)
 		return false;
 
-	PERF_TRACE();
+	PERF_TRACE_CTX(output_tf->ctx);
 
 	corner_points = lut_params->corner_points;
 	rgb_resulted = lut_params->rgb_resulted; @@ -513,7 +513,7 @@ bool cm_helper_translate_curve_to_degamma_hw_format(
 	if (output_tf == NULL || lut_params == NULL || output_tf->type == TF_TYPE_BYPASS)
 		return false;
 
-	PERF_TRACE();
+	PERF_TRACE_CTX(output_tf->ctx);
 
 	corner_points = lut_params->corner_points;
 	rgb_resulted = lut_params->rgb_resulted; diff --git a/drivers/gpu/drm/amd/display/dc/dm_services.h b/drivers/gpu/drm/amd/display/dc/dm_services.h
index 28128c02de00..1961cc6d9143 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_services.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_services.h
@@ -31,6 +31,8 @@
 
 #define __DM_SERVICES_H__
 
+#include "amdgpu_dm_trace.h"
+
 /* TODO: remove when DC is complete. */  #include "dm_services_types.h"
 #include "logger_interface.h"
@@ -70,6 +72,7 @@ static inline uint32_t dm_read_reg_func(
 	}
 #endif
 	value = cgs_read_register(ctx->cgs_device, address);
+	trace_amdgpu_dc_rreg(&ctx->perf_trace->read_count, address, value);
 
 	return value;
 }
@@ -90,6 +93,7 @@ static inline void dm_write_reg_func(
 	}
 #endif
 	cgs_write_register(ctx->cgs_device, address, value);
+	trace_amdgpu_dc_wreg(&ctx->perf_trace->write_count, address, value);
 }
 
 static inline uint32_t dm_read_index_reg( @@ -351,8 +355,12 @@ unsigned long long dm_get_elapse_time_in_ns(struct dc_context *ctx,
 /*
  * performance tracing
  */
-void dm_perf_trace_timestamp(const char *func_name, unsigned int line);
-#define PERF_TRACE()	dm_perf_trace_timestamp(__func__, __LINE__)
+#define PERF_TRACE()	trace_amdgpu_dc_performance(CTX->perf_trace->read_count,\
+		CTX->perf_trace->write_count, &CTX->perf_trace->last_entry_read,\
+		&CTX->perf_trace->last_entry_write, __func__, __LINE__)
+#define PERF_TRACE_CTX(__CTX)	trace_amdgpu_dc_performance(__CTX->perf_trace->read_count,\
+		__CTX->perf_trace->write_count, &__CTX->perf_trace->last_entry_read,\
+		&__CTX->perf_trace->last_entry_write, __func__, __LINE__)
 
 
 /*
--
2.17.1

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


More information about the amd-gfx mailing list