[PATCH 00/16] DC Patches Nov 29, 2018

[PATCH 00/16] DC Patches Nov 29, 2018

[sunpeng.li-5C7GfCeVMHo]

From: Leo Li <sunpeng.li@amd.com>

Summary of change:
* Initial documentation of DC
* Implement tracing in DC
* Fix AUX transaction race

Chiawen Huang (1):
  drm/amd/display: Add customizable tracing event

David Francis (3):
  drm/amd/display: Start documentation of DC
  drm/amd/display: Add tracing to dc
  drm/amd/display: Allow clock lower on dce100

Fatemeh Darbehani (1):
  drm/amd/display: Clean up for DCN1 clock debug logging

Harmanprit Tatla (1):
  drm/amd/display: Info frame cleanup

Joshua Aberback (1):
  drm/amd/display: Remove unused panel patch "disconnect_delay"

Krunoslav Kovac (1):
  drm/amd/display: Fix spelling of axis in modules/color/color_gamma.c

Nevenko Stupar (1):
  drm/amd/display: Re-arrange GFX9 fields

Nicholas Kazlauskas (1):
  drm/amd/display: Copy crc_enabled when duplicating dm_crtc_state

SivapiriyanKumarasamy (1):
  drm/amd/display: Program dithering if requested

Steven Chiu (2):
  drm/amd/display: 3.2.07
  drm/amd/display: 3.2.08

Yogesh Mohan Marimuthu (1):
  drm/amd/display: fix sporadic multiple aux transaction failure

abdoulaye berthe (1):
  drm/amd/display: CTS 4.2.2.7

hersen wu (1):
  drm/amd/display: fbc state could not reach while enable fbc

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  10 +-
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h    | 104 +++++++++++++++
 drivers/gpu/drm/amd/display/dc/core/dc.c           | 100 +++++++++++++-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c      |  21 ++-
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   |   2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c  | 145 ++++++---------------
 drivers/gpu/drm/amd/display/dc/core/dc_stream.c    |   2 +-
 drivers/gpu/drm/amd/display/dc/dc.h                |   2 +-
 drivers/gpu/drm/amd/display/dc/dc_hw_types.h       |   7 +-
 drivers/gpu/drm/amd/display/dc/dc_link.h           |   6 +-
 drivers/gpu/drm/amd/display/dc/dc_stream.h         |   3 +
 drivers/gpu/drm/amd/display/dc/dc_types.h          |   9 +-
 .../amd/display/dc/dce100/dce100_hw_sequencer.c    |  14 +-
 .../drm/amd/display/dc/dce110/dce110_compressor.c  |  91 +++++--------
 .../amd/display/dc/dce110/dce110_hw_sequencer.c    |  57 ++++----
 .../gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.c   |   4 +-
 .../gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.h   |   6 +
 .../gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c |   4 +-
 .../display/dc/dcn10/dcn10_hw_sequencer_debug.c    |  39 +++---
 .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c  |   4 -
 drivers/gpu/drm/amd/display/dc/dm_event_log.h      |   1 +
 drivers/gpu/drm/amd/display/dc/dm_pp_smu.h         |   2 +-
 drivers/gpu/drm/amd/display/dc/dm_services.h       |  12 +-
 drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c |  65 +++------
 drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h |   7 +-
 drivers/gpu/drm/amd/display/dc/inc/compressor.h    |   1 +
 drivers/gpu/drm/amd/display/dc/inc/core_types.h    |  12 +-
 .../drm/amd/display/modules/color/color_gamma.c    |  16 +--
 .../drm/amd/display/modules/freesync/freesync.c    |  10 +-
 .../drm/amd/display/modules/inc/mod_info_packet.h  |  14 +-
 .../gpu/drm/amd/display/modules/inc/mod_shared.h   |  27 ++--
 .../amd/display/modules/info_packet/info_packet.c  |  15 +--
 32 files changed, 481 insertions(+), 331 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h

-- 
2.7.4

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

[PATCH 01/16] drm/amd/display: fix sporadic multiple aux transaction failure

[sunpeng.li-5C7GfCeVMHo]

From: Yogesh Mohan Marimuthu <yogesh.mohanmarimuthu@amd.com>

[why]
When there are multiple aux transaction in parallel, it is sometime
sporadically the aux transaction starts to continuously fail. The
aux transaction was failing because the busy bit for the given gpio
pin was always set. The busy bit was alway set because the
programming sequence to read, modify and write busy bit was not
atomic. Due to which when multiple threads are trying to modify the
busy bits for their gpio pins in the same integer variable sometimes
the busy bits integer variable is written with old data causing
failure.

[how]
Instead of using individual bits to track gpio pins and grouping
them to integers, one byte will be allcoated for each gpio pin.
Now whenever a gpio pin needs to be set to mark being used, only
writing a value of one to that byte is sufficient, other bytes
are not impacted. Also no need to have atomicity with bytes unlike
with bits.

Signed-off-by: Yogesh Mohan Marimuthu <yogesh.mohanmarimuthu@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 65 ++++++----------------
 drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h |  7 +--
 2 files changed, 21 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
index f20161c..dada042 100644
--- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
+++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
@@ -56,7 +56,6 @@ struct gpio_service *dal_gpio_service_create(
 	struct dc_context *ctx)
 {
 	struct gpio_service *service;
-
 	uint32_t index_of_id;
 
 	service = kzalloc(sizeof(struct gpio_service), GFP_KERNEL);
@@ -78,44 +77,33 @@ struct gpio_service *dal_gpio_service_create(
 		goto failure_1;
 	}
 
-	/* allocate and initialize business storage */
+	/* allocate and initialize busyness storage */
 	{
-		const uint32_t bits_per_uint = sizeof(uint32_t) << 3;
-
 		index_of_id = 0;
 		service->ctx = ctx;
 
 		do {
 			uint32_t number_of_bits =
 				service->factory.number_of_pins[index_of_id];
+			uint32_t i = 0;
 
-			uint32_t number_of_uints =
-				(number_of_bits + bits_per_uint - 1) /
-				bits_per_uint;
-
-			uint32_t *slot;
-
-			if (number_of_bits) {
-				uint32_t index_of_uint = 0;
+			if (number_of_bits)  {
+				service->busyness[index_of_id] =
+					kcalloc(number_of_bits, sizeof(char),
+						GFP_KERNEL);
 
-				slot = kcalloc(number_of_uints,
-					       sizeof(uint32_t),
-					       GFP_KERNEL);
-
-				if (!slot) {
+				if (!service->busyness[index_of_id]) {
 					BREAK_TO_DEBUGGER();
 					goto failure_2;
 				}
 
 				do {
-					slot[index_of_uint] = 0;
-
-					++index_of_uint;
-				} while (index_of_uint < number_of_uints);
-			} else
-				slot = NULL;
-
-			service->busyness[index_of_id] = slot;
+					service->busyness[index_of_id][i] = 0;
+					++i;
+				} while (i < number_of_bits);
+			} else {
+				service->busyness[index_of_id] = NULL;
+			}
 
 			++index_of_id;
 		} while (index_of_id < GPIO_ID_COUNT);
@@ -125,13 +113,8 @@ struct gpio_service *dal_gpio_service_create(
 
 failure_2:
 	while (index_of_id) {
-		uint32_t *slot;
-
 		--index_of_id;
-
-		slot = service->busyness[index_of_id];
-
-		kfree(slot);
+		kfree(service->busyness[index_of_id]);
 	}
 
 failure_1:
@@ -169,9 +152,7 @@ void dal_gpio_service_destroy(
 		uint32_t index_of_id = 0;
 
 		do {
-			uint32_t *slot = (*ptr)->busyness[index_of_id];
-
-			kfree(slot);
+			kfree((*ptr)->busyness[index_of_id]);
 
 			++index_of_id;
 		} while (index_of_id < GPIO_ID_COUNT);
@@ -192,11 +173,7 @@ static bool is_pin_busy(
 	enum gpio_id id,
 	uint32_t en)
 {
-	const uint32_t bits_per_uint = sizeof(uint32_t) << 3;
-
-	const uint32_t *slot = service->busyness[id] + (en / bits_per_uint);
-
-	return 0 != (*slot & (1 << (en % bits_per_uint)));
+	return service->busyness[id][en];
 }
 
 static void set_pin_busy(
@@ -204,10 +181,7 @@ static void set_pin_busy(
 	enum gpio_id id,
 	uint32_t en)
 {
-	const uint32_t bits_per_uint = sizeof(uint32_t) << 3;
-
-	service->busyness[id][en / bits_per_uint] |=
-		(1 << (en % bits_per_uint));
+	service->busyness[id][en] = true;
 }
 
 static void set_pin_free(
@@ -215,10 +189,7 @@ static void set_pin_free(
 	enum gpio_id id,
 	uint32_t en)
 {
-	const uint32_t bits_per_uint = sizeof(uint32_t) << 3;
-
-	service->busyness[id][en / bits_per_uint] &=
-		~(1 << (en % bits_per_uint));
+	service->busyness[id][en] = false;
 }
 
 enum gpio_result dal_gpio_service_open(
diff --git a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h
index c7f3081..1d501a4 100644
--- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h
+++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.h
@@ -36,10 +36,9 @@ struct gpio_service {
 	/*
 	 * @brief
 	 * Business storage.
-	 * For each member of 'enum gpio_id',
-	 * store array of bits (packed into uint32_t slots),
-	 * index individual bit by 'en' value */
-	uint32_t *busyness[GPIO_ID_COUNT];
+	 * one byte For each member of 'enum gpio_id'
+	 */
+	char *busyness[GPIO_ID_COUNT];
 };
 
 enum gpio_result dal_gpio_service_open(
-- 
2.7.4

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

[PATCH 02/16] drm/amd/display: 3.2.07

[sunpeng.li-5C7GfCeVMHo]

From: Steven Chiu <steven.chiu@amd.com>

Signed-off-by: Steven Chiu <steven.chiu@amd.com>
Reviewed-by: Shahin Khayyer <Shahin.Khayyer@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/dc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index dea8bc3..70873d2 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -39,7 +39,7 @@
 #include "inc/hw/dmcu.h"
 #include "dml/display_mode_lib.h"
 
-#define DC_VER "3.2.06"
+#define DC_VER "3.2.07"
 
 #define MAX_SURFACES 3
 #define MAX_STREAMS 6
-- 
2.7.4

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

[PATCH 03/16] drm/amd/display: Start documentation of DC

[sunpeng.li-5C7GfCeVMHo]

From: David Francis <David.Francis@amd.com>

[Why]
There are a lot of unintuitive parts of the dm-dc interface.
It would help us if these were documented to provide
a common understanding of what they are supposed to do

[How]
Most of this documentation is stubs, to be filled out more
thoroughly by the experts

Not every dm-accessible function and struct is mentioned.
Simple functions like getters, setters, retain, release,
create, destroy can be left unadorned.

Signed-off-by: David Francis <David.Francis@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc.c          | 72 ++++++++++++++++++++++-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c     | 21 ++++++-
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 35 +++++++++--
 drivers/gpu/drm/amd/display/dc/core/dc_stream.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/dc_link.h          |  6 +-
 drivers/gpu/drm/amd/display/dc/inc/core_types.h   | 12 +++-
 6 files changed, 135 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index dba6b57..8edd030 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -62,6 +62,55 @@
 
 const static char DC_BUILD_ID[] = "production-build";
 
+/**
+ * DOC: Overview
+ *
+ * DC is the OS-agnostic component of the amdgpu DC driver.
+ *
+ * DC maintains and validates a set of structs representing the state of the
+ * driver and writes that state to AMD hardware
+ *
+ * Main DC HW structs:
+ *
+ * struct dc - The central struct.  One per driver.  Created on driver load,
+ * destroyed on driver unload.
+ *
+ * struct dc_context - One per driver.
+ * Used as a backpointer by most other structs in dc.
+ *
+ * struct dc_link - One per connector (the physical DP, HDMI, miniDP, or eDP
+ * plugpoints).  Created on driver load, destroyed on driver unload.
+ *
+ * struct dc_sink - One per display.  Created on boot or hotplug.
+ * Destroyed on shutdown or hotunplug.  A dc_link can have a local sink
+ * (the display directly attached).  It may also have one or more remote
+ * sinks (in the Multi-Stream Transport case)
+ *
+ * struct resource_pool - One per driver.  Represents the hw blocks not in the
+ * main pipeline.  Not directly accessible by dm.
+ *
+ * Main dc state structs:
+ *
+ * These structs can be created and destroyed as needed.  There is a full set of
+ * these structs in dc->current_state representing the currently programmed state.
+ *
+ * struct dc_state - The global DC state to track global state information,
+ * such as bandwidth values.
+ *
+ * struct dc_stream_state - Represents the hw configuration for the pipeline from
+ * a framebuffer to a display.  Maps one-to-one with dc_sink.
+ *
+ * struct dc_plane_state - Represents a framebuffer.  Each stream has at least one,
+ * and may have more in the Multi-Plane Overlay case.
+ *
+ * struct resource_context - Represents the programmable state of everything in
+ * the resource_pool.  Not directly accessible by dm.
+ *
+ * struct pipe_ctx - A member of struct resource_context.  Represents the
+ * internal hardware pipeline components.  Each dc_plane_state has either
+ * one or two (in the pipe-split case).
+ */
+
 /*******************************************************************************
  * Private functions
  ******************************************************************************/
@@ -240,7 +289,7 @@ bool dc_stream_get_crtc_position(struct dc *dc,
 }
 
 /**
- * dc_stream_configure_crc: Configure CRC capture for the given stream.
+ * dc_stream_configure_crc() - Configure CRC capture for the given stream.
  * @dc: DC Object
  * @stream: The stream to configure CRC on.
  * @enable: Enable CRC if true, disable otherwise.
@@ -292,7 +341,7 @@ bool dc_stream_configure_crc(struct dc *dc, struct dc_stream_state *stream,
 }
 
 /**
- * dc_stream_get_crc: Get CRC values for the given stream.
+ * dc_stream_get_crc() - Get CRC values for the given stream.
  * @dc: DC object
  * @stream: The DC stream state of the stream to get CRCs from.
  * @r_cr, g_y, b_cb: CRC values for the three channels are stored here.
@@ -1329,6 +1378,11 @@ static enum surface_update_type check_update_surfaces_for_stream(
 	return overall_type;
 }
 
+/**
+ * dc_check_update_surfaces_for_stream() - Determine update type (fast, med, or full)
+ *
+ * See :c:type:`enum surface_update_type <surface_update_type>` for explanation of update types
+ */
 enum surface_update_type dc_check_update_surfaces_for_stream(
 		struct dc *dc,
 		struct dc_surface_update *updates,
@@ -1631,6 +1685,9 @@ enum dc_irq_source dc_interrupt_to_irq_source(
 	return dal_irq_service_to_irq_source(dc->res_pool->irqs, src_id, ext_id);
 }
 
+/**
+ * dc_interrupt_set() - Enable/disable an AMD hw interrupt source
+ */
 bool dc_interrupt_set(struct dc *dc, enum dc_irq_source src, bool enable)
 {
 
@@ -1724,6 +1781,11 @@ static bool link_add_remote_sink_helper(struct dc_link *dc_link, struct dc_sink
 	return true;
 }
 
+/**
+ * dc_link_add_remote_sink() - Create a sink and attach it to an existing link
+ *
+ * EDID length is in bytes
+ */
 struct dc_sink *dc_link_add_remote_sink(
 		struct dc_link *link,
 		const uint8_t *edid,
@@ -1782,6 +1844,12 @@ struct dc_sink *dc_link_add_remote_sink(
 	return NULL;
 }
 
+/**
+ * dc_link_remove_remote_sink() - Remove a remote sink from a dc_link
+ *
+ * Note that this just removes the struct dc_sink - it doesn't
+ * program hardware or alter other members of dc_link
+ */
 void dc_link_remove_remote_sink(struct dc_link *link, struct dc_sink *sink)
 {
 	int i;
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 948596a..4dc5846 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -198,6 +198,13 @@ static bool program_hpd_filter(
 	return result;
 }
 
+/**
+ * dc_link_detect_sink() - Determine if there is a sink connected
+ *
+ * @type: Returned connection type
+ * Does not detect downstream devices, such as MST sinks
+ * or display connected through active dongles
+ */
 bool dc_link_detect_sink(struct dc_link *link, enum dc_connection_type *type)
 {
 	uint32_t is_hpd_high = 0;
@@ -324,9 +331,9 @@ static enum signal_type get_basic_signal_type(
 	return SIGNAL_TYPE_NONE;
 }
 
-/*
- * @brief
- * Check whether there is a dongle on DP connector
+/**
+ * dc_link_is_dp_sink_present() - Check if there is a native DP
+ * or passive DP-HDMI dongle connected
  */
 bool dc_link_is_dp_sink_present(struct dc_link *link)
 {
@@ -593,6 +600,14 @@ static bool is_same_edid(struct dc_edid *old_edid, struct dc_edid *new_edid)
 	return (memcmp(old_edid->raw_edid, new_edid->raw_edid, new_edid->length) == 0);
 }
 
+/**
+ * dc_link_detect() - Detect if a sink is attached to a given link
+ *
+ * link->local_sink is created or destroyed as needed.
+ *
+ * This does not create remote sinks but will trigger DM
+ * to start MST detection if a branch is detected.
+ */
 bool dc_link_detect(struct dc_link *link, enum dc_detect_reason reason)
 {
 	struct dc_sink_init_data sink_init_data = { 0 };
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index 0bb844a..d4fd1d1 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -1447,6 +1447,14 @@ static bool are_stream_backends_same(
 	return true;
 }
 
+/**
+ * dc_is_stream_unchanged() - Compare two stream states for equivalence.
+ *
+ * Checks if there a difference between the two states
+ * that would require a mode change.
+ *
+ * Does not compare cursor position or attributes.
+ */
 bool dc_is_stream_unchanged(
 	struct dc_stream_state *old_stream, struct dc_stream_state *stream)
 {
@@ -1457,6 +1465,9 @@ bool dc_is_stream_unchanged(
 	return true;
 }
 
+/**
+ * dc_is_stream_scaling_unchanged() - Compare scaling rectangles of two streams.
+ */
 bool dc_is_stream_scaling_unchanged(
 	struct dc_stream_state *old_stream, struct dc_stream_state *stream)
 {
@@ -1616,6 +1627,9 @@ bool resource_is_stream_unchanged(
 	return false;
 }
 
+/**
+ * dc_add_stream_to_ctx() - Add a new dc_stream_state to a dc_state.
+ */
 enum dc_status dc_add_stream_to_ctx(
 		struct dc *dc,
 		struct dc_state *new_ctx,
@@ -1640,6 +1654,9 @@ enum dc_status dc_add_stream_to_ctx(
 	return res;
 }
 
+/**
+ * dc_remove_stream_from_ctx() - Remove a stream from a dc_state.
+ */
 enum dc_status dc_remove_stream_from_ctx(
 			struct dc *dc,
 			struct dc_state *new_ctx,
@@ -1860,6 +1877,12 @@ enum dc_status resource_map_pool_resources(
 	return DC_ERROR_UNEXPECTED;
 }
 
+/**
+ * dc_resource_state_copy_construct_current() - Creates a new dc_state from existing state
+ * Is a shallow copy.  Increments refcounts on existing streams and planes.
+ * @dc: copy out of dc->current_state
+ * @dst_ctx: copy into this
+ */
 void dc_resource_state_copy_construct_current(
 		const struct dc *dc,
 		struct dc_state *dst_ctx)
@@ -1875,6 +1898,14 @@ void dc_resource_state_construct(
 	dst_ctx->dccg = dc->res_pool->clk_mgr;
 }
 
+/**
+ * dc_validate_global_state() - Determine if HW can support a given state
+ * Checks HW resource availability and bandwidth requirement.
+ * @dc: dc struct for this driver
+ * @new_ctx: state to be validated
+ *
+ * Return: DC_OK if the result can be programmed.  Otherwise, an error code.
+ */
 enum dc_status dc_validate_global_state(
 		struct dc *dc,
 		struct dc_state *new_ctx)
@@ -2364,10 +2395,6 @@ void dc_resource_state_destruct(struct dc_state *context)
 	}
 }
 
-/*
- * Copy src_ctx into dst_ctx and retain all surfaces and streams referenced
- * by the src_ctx
- */
 void dc_resource_state_copy_construct(
 		const struct dc_state *src_ctx,
 		struct dc_state *dst_ctx)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index 780838a..66e5c46 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -170,7 +170,7 @@ struct dc_stream_status *dc_stream_get_status(
 }
 
 /**
- * Update the cursor attributes and set cursor surface address
+ * dc_stream_set_cursor_attributes() - Update cursor attributes and set cursor surface address
  */
 bool dc_stream_set_cursor_attributes(
 	struct dc_stream_state *stream,
diff --git a/drivers/gpu/drm/amd/display/dc/dc_link.h b/drivers/gpu/drm/amd/display/dc/dc_link.h
index 8738f27..29f19d5 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_link.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_link.h
@@ -128,8 +128,10 @@ struct dc_link {
 
 const struct dc_link_status *dc_link_get_status(const struct dc_link *dc_link);
 
-/*
- * Return an enumerated dc_link.  dc_link order is constant and determined at
+/**
+ * dc_get_link_at_index() - Return an enumerated dc_link.
+ *
+ * dc_link order is constant and determined at
  * boot time.  They cannot be created or destroyed.
  * Use dc_get_caps() to get number of links.
  */
diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
index e3ee96a..b168a5e 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h
@@ -272,6 +272,17 @@ union bw_context {
 	struct dce_bw_output dce;
 };
 
+/**
+ * struct dc_state - The full description of a state requested by a user
+ *
+ * @streams: Stream properties
+ * @stream_status: The planes on a given stream
+ * @res_ctx: Persistent state of resources
+ * @bw: The output from bandwidth and watermark calculations
+ * @pp_display_cfg: PowerPlay clocks and settings
+ * @dcn_bw_vars: non-stack memory to support bandwidth calculations
+ *
+ */
 struct dc_state {
 	struct dc_stream_state *streams[MAX_PIPES];
 	struct dc_stream_status stream_status[MAX_PIPES];
@@ -279,7 +290,6 @@ struct dc_state {
 
 	struct resource_context res_ctx;
 
-	/* The output from BW and WM calculations. */
 	union bw_context bw;
 
 	/* Note: these are big structures, do *not* put on stack! */
-- 
2.7.4

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

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

[sunpeng.li-5C7GfCeVMHo]

From: David Francis <David.Francis@amd.com>

[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

Signed-off-by: David Francis <David.Francis@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   3 +
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h    | 104 +++++++++++++++++++++
 drivers/gpu/drm/amd/display/dc/core/dc.c           |  20 ++++
 drivers/gpu/drm/amd/display/dc/dc_types.h          |   8 ++
 .../gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c |   4 +-
 drivers/gpu/drm/amd/display/dc/dm_services.h       |  12 ++-
 6 files changed, 147 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 ce00e56..2490f66 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 0000000..d898981
--- /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 8edd030..dffd083 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -224,6 +224,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
@@ -585,6 +596,9 @@ static void destruct(struct dc *dc)
 	if (dc->ctx->created_bios)
 		dal_bios_parser_destroy(&dc->ctx->dc_bios);
 
+	if (dc->ctx->perf_trace)
+		dc_perf_trace_destroy(&dc->ctx->perf_trace);
+
 	kfree(dc->ctx);
 	dc->ctx = NULL;
 
@@ -708,6 +722,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 6e12d64..8390bae 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 3eea440..7469333 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 28128c0..1961cc6 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.7.4

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

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

[Christian König]

Am 29.11.18 um 16:52 schrieb sunpeng.li@amd.com:
> From: David Francis <David.Francis@amd.com>
>
> [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
>
> Signed-off-by: David Francis <David.Francis@amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
> Acked-by: Leo Li <sunpeng.li@amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   3 +
>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h    | 104 +++++++++++++++++++++
>   drivers/gpu/drm/amd/display/dc/core/dc.c           |  20 ++++
>   drivers/gpu/drm/amd/display/dc/dc_types.h          |   8 ++
>   .../gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c |   4 +-
>   drivers/gpu/drm/amd/display/dc/dm_services.h       |  12 ++-
>   6 files changed, 147 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 ce00e56..2490f66 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 0000000..d898981
> --- /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 8edd030..dffd083 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -224,6 +224,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
> @@ -585,6 +596,9 @@ static void destruct(struct dc *dc)
>   	if (dc->ctx->created_bios)
>   		dal_bios_parser_destroy(&dc->ctx->dc_bios);
>   
> +	if (dc->ctx->perf_trace)
> +		dc_perf_trace_destroy(&dc->ctx->perf_trace);
> +

kfree is NULL pointer save, please drop all additional "if 
(dc->ctx->perf_trace)" checks around it.

Christian.

>   	kfree(dc->ctx);
>   	dc->ctx = NULL;
>   
> @@ -708,6 +722,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 6e12d64..8390bae 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 3eea440..7469333 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 28128c0..1961cc6 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__)
>   
>   
>   /*

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

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

[David Francis]

[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@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Acked-by: Leo Li <sunpeng.li@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Kuehling, Felix]

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@lists.freedesktop.org> On Behalf Of David Francis
Sent: Friday, November 30, 2018 9:57 AM
To: amd-gfx@lists.freedesktop.org
Cc: Francis, David <David.Francis@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@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Acked-by: Leo Li <sunpeng.li@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@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

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

[Wentland, Harry]

On 2018-12-07 12:40 a.m., Kuehling, Felix wrote:
> 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.
> 

That's the trace subsystem for you. David and I are trying to understand what's going on. I think the problem is that both tlb.h and amdgpu_dm_trace.h define trace events and we now include both here.

I think we'd want to include neither trace events from amdgpu.h to avoid this problem in the future, so we'll probably have to (a) clean up the dc.h include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main includes for our respective drivers but are also wholesale included in amdgpu.h.

Harry

> Thanks,
>    Felix
> 
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of David Francis
> Sent: Friday, November 30, 2018 9:57 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Francis, David <David.Francis@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@amd.com>
> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
> Acked-by: Leo Li <sunpeng.li@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@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
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Wentland, Harry]

On 2018-12-07 9:41 a.m., Wentland, Harry wrote:
> On 2018-12-07 12:40 a.m., Kuehling, Felix wrote:
>> 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.
>>
> 
> That's the trace subsystem for you. David and I are trying to understand what's going on. I think the problem is that both tlb.h and amdgpu_dm_trace.h define trace events and we now include both here.
> 
> I think we'd want to include neither trace events from amdgpu.h to avoid this problem in the future, so we'll probably have to (a) clean up the dc.h include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main includes for our respective drivers but are also wholesale included in amdgpu.h.
> 

Apologies for the spam.

Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace events (from tlb.h) which we don't expect and care about. I think we should make sure amdgpu.h won't include anything that defines TRACE_EVENTs.

Harry

> Harry
> 
>> Thanks,
>>     Felix
>>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of David Francis
>> Sent: Friday, November 30, 2018 9:57 AM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Francis, David <David.Francis@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@amd.com>
>> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
>> Acked-by: Leo Li <sunpeng.li@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@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
>>
> _______________________________________________
> 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

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

[Kuehling, Felix]

On 2018-12-07 9:46 a.m., Wentland, Harry wrote:
> On 2018-12-07 9:41 a.m., Wentland, Harry wrote:
>> On 2018-12-07 12:40 a.m., Kuehling, Felix wrote:
>>> 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.
>>>
>> That's the trace subsystem for you. David and I are trying to understand what's going on. I think the problem is that both tlb.h and amdgpu_dm_trace.h define trace events and we now include both here.
>>
>> I think we'd want to include neither trace events from amdgpu.h to avoid this problem in the future, so we'll probably have to (a) clean up the dc.h include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main includes for our respective drivers but are also wholesale included in amdgpu.h.
>>
> Apologies for the spam.
>
> Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace events (from tlb.h) which we don't expect and care about. I think we should make sure amdgpu.h won't include anything that defines TRACE_EVENTs.

amdgpu_amdkfd.h defines a macro that uses use_mm and unuse_mm. Therefore
it needs to include mmu_context.h, which pulls in the conflicting trace
events. Maybe we can move that into a different header file that doesn't
get included in amdgpu.h. Or find another way to avoid including
amdgpu_amdkfd.h in amdgpu.h.

Thanks,
  Felix



>
> Harry
>
>> Harry
>>
>>> Thanks,
>>>     Felix
>>>
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of David Francis
>>> Sent: Friday, November 30, 2018 9:57 AM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Francis, David <David.Francis@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@amd.com>
>>> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
>>> Acked-by: Leo Li <sunpeng.li@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@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
>>>
>> _______________________________________________
>> 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

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

[Wentland, Harry]

On 2018-12-07 3:32 p.m., Kuehling, Felix wrote:
> On 2018-12-07 9:46 a.m., Wentland, Harry wrote:
>> On 2018-12-07 9:41 a.m., Wentland, Harry wrote:
>>> On 2018-12-07 12:40 a.m., Kuehling, Felix wrote:
>>>> 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.
>>>>
>>> That's the trace subsystem for you. David and I are trying to understand what's going on. I think the problem is that both tlb.h and amdgpu_dm_trace.h define trace events and we now include both here.
>>>
>>> I think we'd want to include neither trace events from amdgpu.h to avoid this problem in the future, so we'll probably have to (a) clean up the dc.h include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main includes for our respective drivers but are also wholesale included in amdgpu.h.
>>>
>> Apologies for the spam.
>>
>> Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace events (from tlb.h) which we don't expect and care about. I think we should make sure amdgpu.h won't include anything that defines TRACE_EVENTs.
> 
> amdgpu_amdkfd.h defines a macro that uses use_mm and unuse_mm. Therefore
> it needs to include mmu_context.h, which pulls in the conflicting trace
> events. Maybe we can move that into a different header file that doesn't
> get included in amdgpu.h. Or find another way to avoid including
> amdgpu_amdkfd.h in amdgpu.h.
> 

It's defined but not used in amdgpu_amdkfd.h (at least in the amd-stg tree). Couldn't you include mmu_context.h in the files that use it (amdgpu_amdkfd_gfx_v7/8/9.c) instead and avoid the problem that way?

Harry

> Thanks,
>   Felix
> 
> 
> 
>>
>> Harry
>>
>>> Harry
>>>
>>>> Thanks,
>>>>     Felix
>>>>
>>>> -----Original Message-----
>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of David Francis
>>>> Sent: Friday, November 30, 2018 9:57 AM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Francis, David <David.Francis@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@amd.com>
>>>> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
>>>> Acked-by: Leo Li <sunpeng.li@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@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
>>>>
>>> _______________________________________________
>>> 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

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

[Kuehling, Felix]

On 2018-12-14 2:31 p.m., Wentland, Harry wrote:
> On 2018-12-07 3:32 p.m., Kuehling, Felix wrote:
>> On 2018-12-07 9:46 a.m., Wentland, Harry wrote:
>>> On 2018-12-07 9:41 a.m., Wentland, Harry wrote:
>>>> On 2018-12-07 12:40 a.m., Kuehling, Felix wrote:
>>>>> 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.
>>>>>
>>>> That's the trace subsystem for you. David and I are trying to understand what's going on. I think the problem is that both tlb.h and amdgpu_dm_trace.h define trace events and we now include both here.
>>>>
>>>> I think we'd want to include neither trace events from amdgpu.h to avoid this problem in the future, so we'll probably have to (a) clean up the dc.h include to only contain what amdgpu.h needs and (b) clean up amdgpu_amdkfd.h to only contain what amdgpu.h needs. At the end amdgpu.h doesn't care about the tracer. The problem seems that dc.h and amdgpu_amdkfd.h are the main includes for our respective drivers but are also wholesale included in amdgpu.h.
>>>>
>>> Apologies for the spam.
>>>
>>> Just noticed that amdgpu.h includes only amdgpu_dm.h which is clean. The problem is that including amdgpu.h from amdgpu_dm.c now pulls in your trace events (from tlb.h) which we don't expect and care about. I think we should make sure amdgpu.h won't include anything that defines TRACE_EVENTs.
>> amdgpu_amdkfd.h defines a macro that uses use_mm and unuse_mm. Therefore
>> it needs to include mmu_context.h, which pulls in the conflicting trace
>> events. Maybe we can move that into a different header file that doesn't
>> get included in amdgpu.h. Or find another way to avoid including
>> amdgpu_amdkfd.h in amdgpu.h.
>>
> It's defined but not used in amdgpu_amdkfd.h (at least in the amd-stg tree). Couldn't you include mmu_context.h in the files that use it (amdgpu_amdkfd_gfx_v7/8/9.c) instead and avoid the problem that way?
Yes, that's what I ended up doing. I already fixed it in this commit:

commit 2a90860cfb895ff55d961a1b727d58bb68e213ba
Author: Felix Kuehling <Felix.Kuehling@amd.com>
Date:   Fri Dec 7 17:01:27 2018 -0500

    drm/amdgpu: Workaround build failure due to trace conflict
   
    Avoid including mmu_context.h in amdgpu_amdkfd.h since that may be
    included in other header files that define traces. This leads to
    conflicts due to traces defined in other headers included via
    mmu_context.h.
   
    Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
    Acked-by: Alex Deucher <alexander.deucher@amd.com>

Regards,
  Felix


>
> Harry
>
>> Thanks,
>>   Felix
>>
>>
>>
>>> Harry
>>>
>>>> Harry
>>>>
>>>>> Thanks,
>>>>>     Felix
>>>>>
>>>>> -----Original Message-----
>>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of David Francis
>>>>> Sent: Friday, November 30, 2018 9:57 AM
>>>>> To: amd-gfx@lists.freedesktop.org
>>>>> Cc: Francis, David <David.Francis@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@amd.com>
>>>>> Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
>>>>> Acked-by: Leo Li <sunpeng.li@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@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
>>>>>
>>>> _______________________________________________
>>>> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/16] drm/amd/display: Remove unused panel patch "disconnect_delay"

[sunpeng.li-5C7GfCeVMHo]

From: Joshua Aberback <joshua.aberback@amd.com>

[Why]
This patch is for use by dm, no need for it in dc.

Signed-off-by: Joshua Aberback <joshua.aberback@amd.com>
Reviewed-by: Jun Lei <Jun.Lei@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/dc_types.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h
index 8390bae..0b20ae2 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_types.h
@@ -199,7 +199,6 @@ union display_content_support {
 };
 
 struct dc_panel_patch {
-	unsigned int disconnect_delay;
 	unsigned int dppowerup_delay;
 	unsigned int extra_t12_ms;
 };
-- 
2.7.4

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

[PATCH 06/16] drm/amd/display: Fix spelling of axis in modules/color/color_gamma.c

[sunpeng.li-5C7GfCeVMHo]

From: Krunoslav Kovac <Krunoslav.Kovac@amd.com>

Use axis instead of axix

Signed-off-by: Krunoslav Kovac <Krunoslav.Kovac@amd.com>
Reviewed-by: Aric Cyr <Aric.Cyr@amd.com>
Acked-by: Anthony Koo <Anthony.Koo@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
index bbecbae..479b77c 100644
--- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
+++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c
@@ -1761,7 +1761,7 @@ bool mod_color_calculate_degamma_params(struct dc_transfer_func *input_tf,
 
 	struct pwl_float_data *rgb_user = NULL;
 	struct pwl_float_data_ex *curve = NULL;
-	struct gamma_pixel *axix_x = NULL;
+	struct gamma_pixel *axis_x = NULL;
 	struct pixel_gamma_point *coeff = NULL;
 	enum dc_transfer_func_predefined tf = TRANSFER_FUNCTION_SRGB;
 	bool ret = false;
@@ -1787,10 +1787,10 @@ bool mod_color_calculate_degamma_params(struct dc_transfer_func *input_tf,
 			 GFP_KERNEL);
 	if (!curve)
 		goto curve_alloc_fail;
-	axix_x = kvcalloc(ramp->num_entries + _EXTRA_POINTS, sizeof(*axix_x),
+	axis_x = kvcalloc(ramp->num_entries + _EXTRA_POINTS, sizeof(*axis_x),
 			  GFP_KERNEL);
-	if (!axix_x)
-		goto axix_x_alloc_fail;
+	if (!axis_x)
+		goto axis_x_alloc_fail;
 	coeff = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS, sizeof(*coeff),
 			 GFP_KERNEL);
 	if (!coeff)
@@ -1803,7 +1803,7 @@ bool mod_color_calculate_degamma_params(struct dc_transfer_func *input_tf,
 	tf = input_tf->tf;
 
 	build_evenly_distributed_points(
-			axix_x,
+			axis_x,
 			ramp->num_entries,
 			dividers);
 
@@ -1828,7 +1828,7 @@ bool mod_color_calculate_degamma_params(struct dc_transfer_func *input_tf,
 	tf_pts->x_point_at_y1_blue = 1;
 
 	map_regamma_hw_to_x_user(ramp, coeff, rgb_user,
-			coordinates_x, axix_x, curve,
+			coordinates_x, axis_x, curve,
 			MAX_HW_POINTS, tf_pts,
 			mapUserRamp && ramp->type != GAMMA_CUSTOM);
 	if (ramp->type == GAMMA_CUSTOM)
@@ -1838,8 +1838,8 @@ bool mod_color_calculate_degamma_params(struct dc_transfer_func *input_tf,
 
 	kvfree(coeff);
 coeff_alloc_fail:
-	kvfree(axix_x);
-axix_x_alloc_fail:
+	kvfree(axis_x);
+axis_x_alloc_fail:
 	kvfree(curve);
 curve_alloc_fail:
 	kvfree(rgb_user);
-- 
2.7.4

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

[PATCH 07/16] drm/amd/display: CTS 4.2.2.7

[sunpeng.li-5C7GfCeVMHo]

From: abdoulaye berthe <abdoulaye.berthe@amd.com>

[Why]
Failure to read Detailed Capabilities Info.

[How]
Read Detailed Capbilities Info 80h-08Fh.

Signed-off-by: abdoulaye berthe <abdoulaye.berthe@amd.com>
Reviewed-by: Wenjing Liu <Wenjing.Liu@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 4d1f8ac..849a3a3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -2196,7 +2196,7 @@ static void get_active_converter_info(
 	}
 
 	if (link->dpcd_caps.dpcd_rev.raw >= DPCD_REV_11) {
-		uint8_t det_caps[4];
+		uint8_t det_caps[16]; /* CTS 4.2.2.7 expects source to read Detailed Capabilities Info : 00080h-0008F.*/
 		union dwnstream_port_caps_byte0 *port_caps =
 			(union dwnstream_port_caps_byte0 *)det_caps;
 		core_link_read_dpcd(link, DP_DOWNSTREAM_PORT_0,
-- 
2.7.4

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

[PATCH 08/16] drm/amd/display: Info frame cleanup

[sunpeng.li-5C7GfCeVMHo]

From: Harmanprit Tatla <Harmanprit.Tatla@amd.com>

* Use provided infopacket in stream (if valid) instead of reconstructing
  in set_vendor_info_packet()
* Use proper format for enums
* Use dc info packet struct instead

Signed-off-by: Harmanprit Tatla <Harmanprit.Tatla@amd.com>
Reviewed-by: Anthony Koo <Anthony.Koo@amd.com>
Acked-by: Krunoslav Kovac <Krunoslav.Kovac@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   6 +-
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c  | 110 ++-------------------
 drivers/gpu/drm/amd/display/dc/dc_stream.h         |   2 +
 .../drm/amd/display/modules/freesync/freesync.c    |  10 +-
 .../drm/amd/display/modules/inc/mod_info_packet.h  |  14 ++-
 .../gpu/drm/amd/display/modules/inc/mod_shared.h   |  27 +++--
 .../amd/display/modules/info_packet/info_packet.c  |  15 ++-
 7 files changed, 42 insertions(+), 142 deletions(-)

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 2490f66..6c865ce 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -75,6 +75,7 @@
 
 #include "modules/inc/mod_freesync.h"
 #include "modules/power/power_helpers.h"
+#include "modules/inc/mod_info_packet.h"
 
 #define FIRMWARE_RAVEN_DMCU		"amdgpu/raven_dmcu.bin"
 MODULE_FIRMWARE(FIRMWARE_RAVEN_DMCU);
@@ -2934,6 +2935,7 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
 
 	if (dm_state && dm_state->freesync_capable)
 		stream->ignore_msa_timing_param = true;
+
 finish:
 	if (sink && sink->sink_signal == SIGNAL_TYPE_VIRTUAL && aconnector->base.force != DRM_FORCE_ON)
 		dc_sink_release(sink);
@@ -4427,8 +4429,8 @@ static void update_freesync_state_on_stream(
 		dm->freesync_module,
 		new_stream,
 		&vrr,
-		packet_type_vrr,
-		transfer_func_unknown,
+		PACKET_TYPE_VRR,
+		TRANSFER_FUNC_UNKNOWN,
 		&vrr_infopacket);
 
 	new_crtc_state->freesync_timing_changed =
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index d4fd1d1..c347afd 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -2233,113 +2233,15 @@ static void set_vendor_info_packet(
 		struct dc_info_packet *info_packet,
 		struct dc_stream_state *stream)
 {
-	uint32_t length = 0;
-	bool hdmi_vic_mode = false;
-	uint8_t checksum = 0;
-	uint32_t i = 0;
-	enum dc_timing_3d_format format;
-	// Can be different depending on packet content /*todo*/
-	// unsigned int length = pPathMode->dolbyVision ? 24 : 5;
-
-	info_packet->valid = false;
-
-	format = stream->timing.timing_3d_format;
-	if (stream->view_format == VIEW_3D_FORMAT_NONE)
-		format = TIMING_3D_FORMAT_NONE;
-
-	/* Can be different depending on packet content */
-	length = 5;
-
-	if (stream->timing.hdmi_vic != 0
-			&& stream->timing.h_total >= 3840
-			&& stream->timing.v_total >= 2160)
-		hdmi_vic_mode = true;
-
-	/* According to HDMI 1.4a CTS, VSIF should be sent
-	 * for both 3D stereo and HDMI VIC modes.
-	 * For all other modes, there is no VSIF sent.  */
+	/* SPD info packet for FreeSync */
 
-	if (format == TIMING_3D_FORMAT_NONE && !hdmi_vic_mode)
+	/* Check if Freesync is supported. Return if false. If true,
+	 * set the corresponding bit in the info packet
+	 */
+	if (!stream->vsp_infopacket.valid)
 		return;
 
-	/* 24bit IEEE Registration identifier (0x000c03). LSB first. */
-	info_packet->sb[1] = 0x03;
-	info_packet->sb[2] = 0x0C;
-	info_packet->sb[3] = 0x00;
-
-	/*PB4: 5 lower bytes = 0 (reserved). 3 higher bits = HDMI_Video_Format.
-	 * The value for HDMI_Video_Format are:
-	 * 0x0 (0b000) - No additional HDMI video format is presented in this
-	 * packet
-	 * 0x1 (0b001) - Extended resolution format present. 1 byte of HDMI_VIC
-	 * parameter follows
-	 * 0x2 (0b010) - 3D format indication present. 3D_Structure and
-	 * potentially 3D_Ext_Data follows
-	 * 0x3..0x7 (0b011..0b111) - reserved for future use */
-	if (format != TIMING_3D_FORMAT_NONE)
-		info_packet->sb[4] = (2 << 5);
-	else if (hdmi_vic_mode)
-		info_packet->sb[4] = (1 << 5);
-
-	/* PB5: If PB4 claims 3D timing (HDMI_Video_Format = 0x2):
-	 * 4 lower bites = 0 (reserved). 4 higher bits = 3D_Structure.
-	 * The value for 3D_Structure are:
-	 * 0x0 - Frame Packing
-	 * 0x1 - Field Alternative
-	 * 0x2 - Line Alternative
-	 * 0x3 - Side-by-Side (full)
-	 * 0x4 - L + depth
-	 * 0x5 - L + depth + graphics + graphics-depth
-	 * 0x6 - Top-and-Bottom
-	 * 0x7 - Reserved for future use
-	 * 0x8 - Side-by-Side (Half)
-	 * 0x9..0xE - Reserved for future use
-	 * 0xF - Not used */
-	switch (format) {
-	case TIMING_3D_FORMAT_HW_FRAME_PACKING:
-	case TIMING_3D_FORMAT_SW_FRAME_PACKING:
-		info_packet->sb[5] = (0x0 << 4);
-		break;
-
-	case TIMING_3D_FORMAT_SIDE_BY_SIDE:
-	case TIMING_3D_FORMAT_SBS_SW_PACKED:
-		info_packet->sb[5] = (0x8 << 4);
-		length = 6;
-		break;
-
-	case TIMING_3D_FORMAT_TOP_AND_BOTTOM:
-	case TIMING_3D_FORMAT_TB_SW_PACKED:
-		info_packet->sb[5] = (0x6 << 4);
-		break;
-
-	default:
-		break;
-	}
-
-	/*PB5: If PB4 is set to 0x1 (extended resolution format)
-	 * fill PB5 with the correct HDMI VIC code */
-	if (hdmi_vic_mode)
-		info_packet->sb[5] = stream->timing.hdmi_vic;
-
-	/* Header */
-	info_packet->hb0 = HDMI_INFOFRAME_TYPE_VENDOR; /* VSIF packet type. */
-	info_packet->hb1 = 0x01; /* Version */
-
-	/* 4 lower bits = Length, 4 higher bits = 0 (reserved) */
-	info_packet->hb2 = (uint8_t) (length);
-
-	/* Calculate checksum */
-	checksum = 0;
-	checksum += info_packet->hb0;
-	checksum += info_packet->hb1;
-	checksum += info_packet->hb2;
-
-	for (i = 1; i <= length; i++)
-		checksum += info_packet->sb[i];
-
-	info_packet->sb[0] = (uint8_t) (0x100 - checksum);
-
-	info_packet->valid = true;
+	*info_packet = stream->vsp_infopacket;
 }
 
 static void set_spd_info_packet(
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index 771d9f1..0c42418 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -56,6 +56,7 @@ struct dc_stream_state {
 	struct dc_crtc_timing_adjust adjust;
 	struct dc_info_packet vrr_infopacket;
 	struct dc_info_packet vsc_infopacket;
+	struct dc_info_packet vsp_infopacket;
 
 	struct rect src; /* composition area */
 	struct rect dst; /* stream addressable area */
@@ -129,6 +130,7 @@ struct dc_stream_update {
 	struct dc_crtc_timing_adjust *adjust;
 	struct dc_info_packet *vrr_infopacket;
 	struct dc_info_packet *vsc_infopacket;
+	struct dc_info_packet *vsp_infopacket;
 
 	bool *dpms_off;
 
diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index 620a171..1544ed3 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -608,12 +608,12 @@ static void build_vrr_infopacket_data(const struct mod_vrr_params *vrr,
 static void build_vrr_infopacket_fs2_data(enum color_transfer_func app_tf,
 		struct dc_info_packet *infopacket)
 {
-	if (app_tf != transfer_func_unknown) {
+	if (app_tf != TRANSFER_FUNC_UNKNOWN) {
 		infopacket->valid = true;
 
 		infopacket->sb[6] |= 0x08;  // PB6 = [Bit 3 = Native Color Active]
 
-		if (app_tf == transfer_func_gamma_22) {
+		if (app_tf == TRANSFER_FUNC_GAMMA_22) {
 			infopacket->sb[9] |= 0x04;  // PB6 = [Bit 2 = Gamma 2.2 EOTF Active]
 		}
 	}
@@ -688,11 +688,11 @@ void mod_freesync_build_vrr_infopacket(struct mod_freesync *mod_freesync,
 		return;
 
 	switch (packet_type) {
-	case packet_type_fs2:
+	case PACKET_TYPE_FS2:
 		build_vrr_infopacket_v2(stream->signal, vrr, app_tf, infopacket);
 		break;
-	case packet_type_vrr:
-	case packet_type_fs1:
+	case PACKET_TYPE_VRR:
+	case PACKET_TYPE_FS1:
 	default:
 		build_vrr_infopacket_v1(stream->signal, vrr, infopacket);
 	}
diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h b/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h
index 786b343..5b1c9a4 100644
--- a/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h
+++ b/drivers/gpu/drm/amd/display/modules/inc/mod_info_packet.h
@@ -26,15 +26,13 @@
 #ifndef MOD_INFO_PACKET_H_
 #define MOD_INFO_PACKET_H_
 
-struct info_packet_inputs {
-	const struct dc_stream_state *pStream;
-};
+#include "mod_shared.h"
 
-struct info_packets {
-	struct dc_info_packet *pVscInfoPacket;
-};
+//Forward Declarations
+struct dc_stream_state;
+struct dc_info_packet;
 
-void mod_build_infopackets(struct info_packet_inputs *inputs,
-		struct info_packets *info_packets);
+void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
+		struct dc_info_packet *info_packet);
 
 #endif
diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_shared.h b/drivers/gpu/drm/amd/display/modules/inc/mod_shared.h
index 238c431..1bd02c0 100644
--- a/drivers/gpu/drm/amd/display/modules/inc/mod_shared.h
+++ b/drivers/gpu/drm/amd/display/modules/inc/mod_shared.h
@@ -23,27 +23,26 @@
  *
  */
 
-
 #ifndef MOD_SHARED_H_
 #define MOD_SHARED_H_
 
 enum color_transfer_func {
-	transfer_func_unknown,
-	transfer_func_srgb,
-	transfer_func_bt709,
-	transfer_func_pq2084,
-	transfer_func_pq2084_interim,
-	transfer_func_linear_0_1,
-	transfer_func_linear_0_125,
-	transfer_func_dolbyvision,
-	transfer_func_gamma_22,
-	transfer_func_gamma_26
+	TRANSFER_FUNC_UNKNOWN,
+	TRANSFER_FUNC_SRGB,
+	TRANSFER_FUNC_BT709,
+	TRANSFER_FUNC_PQ2084,
+	TRANSFER_FUNC_PQ2084_INTERIM,
+	TRANSFER_FUNC_LINEAR_0_1,
+	TRANSFER_FUNC_LINEAR_0_125,
+	TRANSFER_FUNC_GAMMA_22,
+	TRANSFER_FUNC_GAMMA_26
 };
 
 enum vrr_packet_type {
-	packet_type_vrr,
-	packet_type_fs1,
-	packet_type_fs2
+	PACKET_TYPE_VRR,
+	PACKET_TYPE_FS1,
+	PACKET_TYPE_FS2
 };
 
+
 #endif /* MOD_SHARED_H_ */
diff --git a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
index ff8bfb9..db06fab 100644
--- a/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
+++ b/drivers/gpu/drm/amd/display/modules/info_packet/info_packet.c
@@ -25,6 +25,10 @@
 
 #include "mod_info_packet.h"
 #include "core_types.h"
+#include "dc_types.h"
+#include "mod_shared.h"
+
+#define HDMI_INFOFRAME_TYPE_VENDOR 0x81
 
 enum ColorimetryRGBDP {
 	ColorimetryRGB_DP_sRGB               = 0,
@@ -41,7 +45,7 @@ enum ColorimetryYCCDP {
 	ColorimetryYCC_DP_ITU2020YCbCr  = 7,
 };
 
-static void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
+void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
 		struct dc_info_packet *info_packet)
 {
 	unsigned int vscPacketRevision = 0;
@@ -159,7 +163,7 @@ static void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
 	 *   DPCD register is exposed in the new Extended Receiver Capability field for DPCD Rev. 1.4
 	 *   (and higher). When MISC1. bit 6. is Set to 1, a Source device uses a VSC SDP to indicate
 	 *   the Pixel Encoding/Colorimetry Format and that a Sink device must ignore MISC1, bit 7, and
-	 *   MISC0, bits 7:1 (MISC1, bit 7. and MISC0, bits 7:1 become “don’t care”).)
+	 *   MISC0, bits 7:1 (MISC1, bit 7. and MISC0, bits 7:1 become "don't care").)
 	 */
 	if (vscPacketRevision == 0x5) {
 		/* Secondary-data Packet ID = 0 */
@@ -320,10 +324,3 @@ static void mod_build_vsc_infopacket(const struct dc_stream_state *stream,
 
 }
 
-void mod_build_infopackets(struct info_packet_inputs *inputs,
-		struct info_packets *info_packets)
-{
-	if (info_packets->pVscInfoPacket != NULL)
-		mod_build_vsc_infopacket(inputs->pStream, info_packets->pVscInfoPacket);
-}
-
-- 
2.7.4

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

[PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

[sunpeng.li-5C7GfCeVMHo]

From: hersen wu <hersenxs.wu@amd.com>

   [WHY] fbc is within the data path from memory to dce. while
   re-configure mc dmif, fbc should be enabled. otherwise, fbc
   may not be enabled properly.

   [HOW] before re-configure mc dmif, disable fbc, only after
   dmif re-configuration fully done, enable fbc again.

Signed-off-by: hersen wu <hersenxs.wu@amd.com>
Reviewed-by: Roman Li <Roman.Li@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 .../drm/amd/display/dc/dce110/dce110_compressor.c  | 91 ++++++++--------------
 .../amd/display/dc/dce110/dce110_hw_sequencer.c    | 57 ++++++++------
 drivers/gpu/drm/amd/display/dc/inc/compressor.h    |  1 +
 3 files changed, 66 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
index 1f7f250..52d50e2 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
@@ -64,65 +64,37 @@ static const struct dce110_compressor_reg_offsets reg_offsets[] = {
 
 static const uint32_t dce11_one_lpt_channel_max_resolution = 2560 * 1600;
 
-enum fbc_idle_force {
-	/* Bit 0 - Display registers updated */
-	FBC_IDLE_FORCE_DISPLAY_REGISTER_UPDATE = 0x00000001,
-
-	/* Bit 2 - FBC_GRPH_COMP_EN register updated */
-	FBC_IDLE_FORCE_GRPH_COMP_EN = 0x00000002,
-	/* Bit 3 - FBC_SRC_SEL register updated */
-	FBC_IDLE_FORCE_SRC_SEL_CHANGE = 0x00000004,
-	/* Bit 4 - FBC_MIN_COMPRESSION register updated */
-	FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE = 0x00000008,
-	/* Bit 5 - FBC_ALPHA_COMP_EN register updated */
-	FBC_IDLE_FORCE_ALPHA_COMP_EN = 0x00000010,
-	/* Bit 6 - FBC_ZERO_ALPHA_CHUNK_SKIP_EN register updated */
-	FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN = 0x00000020,
-	/* Bit 7 - FBC_FORCE_COPY_TO_COMP_BUF register updated */
-	FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF = 0x00000040,
-
-	/* Bit 24 - Memory write to region 0 defined by MC registers. */
-	FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION0 = 0x01000000,
-	/* Bit 25 - Memory write to region 1 defined by MC registers */
-	FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION1 = 0x02000000,
-	/* Bit 26 - Memory write to region 2 defined by MC registers */
-	FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION2 = 0x04000000,
-	/* Bit 27 - Memory write to region 3 defined by MC registers. */
-	FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION3 = 0x08000000,
-
-	/* Bit 28 - Memory write from any client other than MCIF */
-	FBC_IDLE_FORCE_MEMORY_WRITE_OTHER_THAN_MCIF = 0x10000000,
-	/* Bit 29 - CG statics screen signal is inactive */
-	FBC_IDLE_FORCE_CG_STATIC_SCREEN_IS_INACTIVE = 0x20000000,
-};
-
-
 static uint32_t align_to_chunks_number_per_line(uint32_t pixels)
 {
 	return 256 * ((pixels + 255) / 256);
 }
 
-static void reset_lb_on_vblank(struct dc_context *ctx)
+static void reset_lb_on_vblank(struct compressor *compressor, uint32_t crtc_inst)
 {
-	uint32_t value, frame_count;
+	uint32_t value;
+	uint32_t frame_count;
+	uint32_t status_pos;
 	uint32_t retry = 0;
-	uint32_t status_pos =
-			dm_read_reg(ctx, mmCRTC_STATUS_POSITION);
+	struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+
+	cp110->offsets = reg_offsets[crtc_inst];
+
+	status_pos = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION));
 
 
 	/* Only if CRTC is enabled and counter is moving we wait for one frame. */
-	if (status_pos != dm_read_reg(ctx, mmCRTC_STATUS_POSITION)) {
+	if (status_pos != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION))) {
 		/* Resetting LB on VBlank */
-		value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+		value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
 		set_reg_field_value(value, 3, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
 		set_reg_field_value(value, 1, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-		dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
+		dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);
 
-		frame_count = dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT);
+		frame_count = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT));
 
 
 		for (retry = 10000; retry > 0; retry--) {
-			if (frame_count != dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT))
+			if (frame_count != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT)))
 				break;
 			udelay(10);
 		}
@@ -130,13 +102,11 @@ static void reset_lb_on_vblank(struct dc_context *ctx)
 			dm_error("Frame count did not increase for 100ms.\n");
 
 		/* Resetting LB on VBlank */
-		value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+		value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
 		set_reg_field_value(value, 2, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
 		set_reg_field_value(value, 0, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-		dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
-
+		dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);
 	}
-
 }
 
 static void wait_for_fbc_state_changed(
@@ -226,10 +196,10 @@ void dce110_compressor_enable_fbc(
 		uint32_t addr;
 		uint32_t value, misc_value;
 
-
 		addr = mmFBC_CNTL;
 		value = dm_read_reg(compressor->ctx, addr);
 		set_reg_field_value(value, 1, FBC_CNTL, FBC_GRPH_COMP_EN);
+		/* params->inst is valid HW CRTC instance start from 0 */
 		set_reg_field_value(
 			value,
 			params->inst,
@@ -238,8 +208,10 @@ void dce110_compressor_enable_fbc(
 
 		/* Keep track of enum controller_id FBC is attached to */
 		compressor->is_enabled = true;
-		compressor->attached_inst = params->inst;
-		cp110->offsets = reg_offsets[params->inst];
+		/* attached_inst is SW CRTC instance start from 1
+		 * 0 = CONTROLLER_ID_UNDEFINED means not attached crtc
+		 */
+		compressor->attached_inst = params->inst + CONTROLLER_ID_D0;
 
 		/* Toggle it as there is bug in HW */
 		set_reg_field_value(value, 0, FBC_CNTL, FBC_GRPH_COMP_EN);
@@ -268,9 +240,10 @@ void dce110_compressor_enable_fbc(
 void dce110_compressor_disable_fbc(struct compressor *compressor)
 {
 	struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+	uint32_t crtc_inst = 0;
 
 	if (compressor->options.bits.FBC_SUPPORT) {
-		if (dce110_compressor_is_fbc_enabled_in_hw(compressor, NULL)) {
+		if (dce110_compressor_is_fbc_enabled_in_hw(compressor, &crtc_inst)) {
 			uint32_t reg_data;
 			/* Turn off compression */
 			reg_data = dm_read_reg(compressor->ctx, mmFBC_CNTL);
@@ -284,8 +257,10 @@ void dce110_compressor_disable_fbc(struct compressor *compressor)
 			wait_for_fbc_state_changed(cp110, false);
 		}
 
-		/* Sync line buffer  - dce100/110 only*/
-		reset_lb_on_vblank(compressor->ctx);
+		/* Sync line buffer which fbc was attached to dce100/110 only */
+		if (crtc_inst > CONTROLLER_ID_UNDEFINED && crtc_inst < CONTROLLER_ID_D3)
+			reset_lb_on_vblank(compressor,
+					crtc_inst - CONTROLLER_ID_D0);
 	}
 }
 
@@ -328,6 +303,8 @@ void dce110_compressor_program_compressed_surface_address_and_pitch(
 	uint32_t compressed_surf_address_low_part =
 		compressor->compr_surface_address.addr.low_part;
 
+	cp110->offsets = reg_offsets[params->inst];
+
 	/* Clear content first. */
 	dm_write_reg(
 		compressor->ctx,
@@ -410,13 +387,7 @@ void dce110_compressor_set_fbc_invalidation_triggers(
 	value = dm_read_reg(compressor->ctx, addr);
 	set_reg_field_value(
 		value,
-		fbc_trigger |
-		FBC_IDLE_FORCE_GRPH_COMP_EN |
-		FBC_IDLE_FORCE_SRC_SEL_CHANGE |
-		FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE |
-		FBC_IDLE_FORCE_ALPHA_COMP_EN |
-		FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN |
-		FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF,
+		fbc_trigger,
 		FBC_IDLE_FORCE_CLEAR_MASK,
 		FBC_IDLE_FORCE_CLEAR_MASK);
 	dm_write_reg(compressor->ctx, addr, value);
@@ -549,7 +520,7 @@ void dce110_compressor_construct(struct dce110_compressor *compressor,
 	compressor->base.channel_interleave_size = 0;
 	compressor->base.dram_channels_num = 0;
 	compressor->base.lpt_channels_num = 0;
-	compressor->base.attached_inst = 0;
+	compressor->base.attached_inst = CONTROLLER_ID_UNDEFINED;
 	compressor->base.is_enabled = false;
 	compressor->base.funcs = &dce110_compressor_funcs;
 
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index 2f062ba..6349ba7 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1766,12 +1766,13 @@ static void set_static_screen_control(struct pipe_ctx **pipe_ctx,
  *  Check if FBC can be enabled
  */
 static bool should_enable_fbc(struct dc *dc,
-			      struct dc_state *context,
-			      uint32_t *pipe_idx)
+		struct dc_state *context,
+		uint32_t *pipe_idx)
 {
 	uint32_t i;
 	struct pipe_ctx *pipe_ctx = NULL;
 	struct resource_context *res_ctx = &context->res_ctx;
+	unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;
 
 
 	ASSERT(dc->fbc_compressor);
@@ -1786,14 +1787,28 @@ static bool should_enable_fbc(struct dc *dc,
 
 	for (i = 0; i < dc->res_pool->pipe_count; i++) {
 		if (res_ctx->pipe_ctx[i].stream) {
+
 			pipe_ctx = &res_ctx->pipe_ctx[i];
-			*pipe_idx = i;
-			break;
+
+			if (!pipe_ctx)
+				continue;
+
+			/* fbc not applicable on underlay pipe */
+			if (pipe_ctx->pipe_idx != underlay_idx) {
+				*pipe_idx = i;
+				break;
+			}
 		}
 	}
 
-	/* Pipe context should be found */
-	ASSERT(pipe_ctx);
+	if (i == dc->res_pool->pipe_count)
+		return false;
+
+	if (!pipe_ctx->stream->sink)
+		return false;
+
+	if (!pipe_ctx->stream->sink->link)
+		return false;
 
 	/* Only supports eDP */
 	if (pipe_ctx->stream->sink->link->connector_signal != SIGNAL_TYPE_EDP)
@@ -1817,8 +1832,9 @@ static bool should_enable_fbc(struct dc *dc,
 /*
  *  Enable FBC
  */
-static void enable_fbc(struct dc *dc,
-		       struct dc_state *context)
+static void enable_fbc(
+		struct dc *dc,
+		struct dc_state *context)
 {
 	uint32_t pipe_idx = 0;
 
@@ -1828,10 +1844,9 @@ static void enable_fbc(struct dc *dc,
 		struct compressor *compr = dc->fbc_compressor;
 		struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[pipe_idx];
 
-
 		params.source_view_width = pipe_ctx->stream->timing.h_addressable;
 		params.source_view_height = pipe_ctx->stream->timing.v_addressable;
-
+		params.inst = pipe_ctx->stream_res.tg->inst;
 		compr->compr_surface_address.quad_part = dc->ctx->fbc_gpu_addr;
 
 		compr->funcs->surface_address_and_pitch(compr, &params);
@@ -2046,10 +2061,10 @@ enum dc_status dce110_apply_ctx_to_hw(
 			return status;
 	}
 
-	dcb->funcs->set_scratch_critical_state(dcb, false);
-
 	if (dc->fbc_compressor)
-		enable_fbc(dc, context);
+		enable_fbc(dc, dc->current_state);
+
+	dcb->funcs->set_scratch_critical_state(dcb, false);
 
 	return DC_OK;
 }
@@ -2408,7 +2423,6 @@ static void dce110_program_front_end_for_pipe(
 	struct dc_plane_state *plane_state = pipe_ctx->plane_state;
 	struct xfm_grph_csc_adjustment adjust;
 	struct out_csc_color_matrix tbl_entry;
-	unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;
 	unsigned int i;
 	DC_LOGGER_INIT();
 	memset(&tbl_entry, 0, sizeof(tbl_entry));
@@ -2449,15 +2463,6 @@ static void dce110_program_front_end_for_pipe(
 
 	program_scaler(dc, pipe_ctx);
 
-	/* fbc not applicable on Underlay pipe */
-	if (dc->fbc_compressor && old_pipe->stream &&
-	    pipe_ctx->pipe_idx != underlay_idx) {
-		if (plane_state->tiling_info.gfx8.array_mode == DC_ARRAY_LINEAR_GENERAL)
-			dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
-		else
-			enable_fbc(dc, dc->current_state);
-	}
-
 	mi->funcs->mem_input_program_surface_config(
 			mi,
 			plane_state->format,
@@ -2534,6 +2539,9 @@ static void dce110_apply_ctx_for_surface(
 	if (num_planes == 0)
 		return;
 
+	if (dc->fbc_compressor)
+		dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
+
 	for (i = 0; i < dc->res_pool->pipe_count; i++) {
 		struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[i];
 		struct pipe_ctx *old_pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
@@ -2576,6 +2584,9 @@ static void dce110_apply_ctx_for_surface(
 			(pipe_ctx->plane_state || old_pipe_ctx->plane_state))
 			dc->hwss.pipe_control_lock(dc, pipe_ctx, false);
 	}
+
+	if (dc->fbc_compressor)
+		enable_fbc(dc, dc->current_state);
 }
 
 static void dce110_power_down_fe(struct dc *dc, struct pipe_ctx *pipe_ctx)
diff --git a/drivers/gpu/drm/amd/display/dc/inc/compressor.h b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
index bcb18f5..7a147a9 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/compressor.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
@@ -77,6 +77,7 @@ struct compressor_funcs {
 };
 struct compressor {
 	struct dc_context *ctx;
+	/* CONTROLLER_ID_D0 + instance, CONTROLLER_ID_UNDEFINED = 0 */
 	uint32_t attached_inst;
 	bool is_enabled;
 	const struct compressor_funcs *funcs;
-- 
2.7.4

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

Re: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 16326 bytes --]

Do you think this will fix this bug?

https://bugs.freedesktop.org/show_bug.cgi?id=108577

If so, we can re-enable fbc.


Alex

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of sunpeng.li-5C7GfCeVMHo@public.gmane.org <sunpeng.li-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, November 29, 2018 10:52:16 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Wu, Hersen
Subject: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

From: hersen wu <hersenxs.wu-5C7GfCeVMHo@public.gmane.org>

   [WHY] fbc is within the data path from memory to dce. while
   re-configure mc dmif, fbc should be enabled. otherwise, fbc
   may not be enabled properly.

   [HOW] before re-configure mc dmif, disable fbc, only after
   dmif re-configuration fully done, enable fbc again.

Signed-off-by: hersen wu <hersenxs.wu-5C7GfCeVMHo@public.gmane.org>
Reviewed-by: Roman Li <Roman.Li-5C7GfCeVMHo@public.gmane.org>
Acked-by: Leo Li <sunpeng.li-5C7GfCeVMHo@public.gmane.org>
---
 .../drm/amd/display/dc/dce110/dce110_compressor.c  | 91 ++++++++--------------
 .../amd/display/dc/dce110/dce110_hw_sequencer.c    | 57 ++++++++------
 drivers/gpu/drm/amd/display/dc/inc/compressor.h    |  1 +
 3 files changed, 66 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
index 1f7f250..52d50e2 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
@@ -64,65 +64,37 @@ static const struct dce110_compressor_reg_offsets reg_offsets[] = {

 static const uint32_t dce11_one_lpt_channel_max_resolution = 2560 * 1600;

-enum fbc_idle_force {
-       /* Bit 0 - Display registers updated */
-       FBC_IDLE_FORCE_DISPLAY_REGISTER_UPDATE = 0x00000001,
-
-       /* Bit 2 - FBC_GRPH_COMP_EN register updated */
-       FBC_IDLE_FORCE_GRPH_COMP_EN = 0x00000002,
-       /* Bit 3 - FBC_SRC_SEL register updated */
-       FBC_IDLE_FORCE_SRC_SEL_CHANGE = 0x00000004,
-       /* Bit 4 - FBC_MIN_COMPRESSION register updated */
-       FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE = 0x00000008,
-       /* Bit 5 - FBC_ALPHA_COMP_EN register updated */
-       FBC_IDLE_FORCE_ALPHA_COMP_EN = 0x00000010,
-       /* Bit 6 - FBC_ZERO_ALPHA_CHUNK_SKIP_EN register updated */
-       FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN = 0x00000020,
-       /* Bit 7 - FBC_FORCE_COPY_TO_COMP_BUF register updated */
-       FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF = 0x00000040,
-
-       /* Bit 24 - Memory write to region 0 defined by MC registers. */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION0 = 0x01000000,
-       /* Bit 25 - Memory write to region 1 defined by MC registers */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION1 = 0x02000000,
-       /* Bit 26 - Memory write to region 2 defined by MC registers */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION2 = 0x04000000,
-       /* Bit 27 - Memory write to region 3 defined by MC registers. */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION3 = 0x08000000,
-
-       /* Bit 28 - Memory write from any client other than MCIF */
-       FBC_IDLE_FORCE_MEMORY_WRITE_OTHER_THAN_MCIF = 0x10000000,
-       /* Bit 29 - CG statics screen signal is inactive */
-       FBC_IDLE_FORCE_CG_STATIC_SCREEN_IS_INACTIVE = 0x20000000,
-};
-
-
 static uint32_t align_to_chunks_number_per_line(uint32_t pixels)
 {
         return 256 * ((pixels + 255) / 256);
 }

-static void reset_lb_on_vblank(struct dc_context *ctx)
+static void reset_lb_on_vblank(struct compressor *compressor, uint32_t crtc_inst)
 {
-       uint32_t value, frame_count;
+       uint32_t value;
+       uint32_t frame_count;
+       uint32_t status_pos;
         uint32_t retry = 0;
-       uint32_t status_pos =
-                       dm_read_reg(ctx, mmCRTC_STATUS_POSITION);
+       struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+
+       cp110->offsets = reg_offsets[crtc_inst];
+
+       status_pos = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION));


         /* Only if CRTC is enabled and counter is moving we wait for one frame. */
-       if (status_pos != dm_read_reg(ctx, mmCRTC_STATUS_POSITION)) {
+       if (status_pos != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION))) {
                 /* Resetting LB on VBlank */
-               value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+               value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
                 set_reg_field_value(value, 3, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
                 set_reg_field_value(value, 1, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-               dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
+               dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);

-               frame_count = dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT);
+               frame_count = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT));


                 for (retry = 10000; retry > 0; retry--) {
-                       if (frame_count != dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT))
+                       if (frame_count != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT)))
                                 break;
                         udelay(10);
                 }
@@ -130,13 +102,11 @@ static void reset_lb_on_vblank(struct dc_context *ctx)
                         dm_error("Frame count did not increase for 100ms.\n");

                 /* Resetting LB on VBlank */
-               value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+               value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
                 set_reg_field_value(value, 2, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
                 set_reg_field_value(value, 0, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-               dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
-
+               dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);
         }
-
 }

 static void wait_for_fbc_state_changed(
@@ -226,10 +196,10 @@ void dce110_compressor_enable_fbc(
                 uint32_t addr;
                 uint32_t value, misc_value;

-
                 addr = mmFBC_CNTL;
                 value = dm_read_reg(compressor->ctx, addr);
                 set_reg_field_value(value, 1, FBC_CNTL, FBC_GRPH_COMP_EN);
+               /* params->inst is valid HW CRTC instance start from 0 */
                 set_reg_field_value(
                         value,
                         params->inst,
@@ -238,8 +208,10 @@ void dce110_compressor_enable_fbc(

                 /* Keep track of enum controller_id FBC is attached to */
                 compressor->is_enabled = true;
-               compressor->attached_inst = params->inst;
-               cp110->offsets = reg_offsets[params->inst];
+               /* attached_inst is SW CRTC instance start from 1
+                * 0 = CONTROLLER_ID_UNDEFINED means not attached crtc
+                */
+               compressor->attached_inst = params->inst + CONTROLLER_ID_D0;

                 /* Toggle it as there is bug in HW */
                 set_reg_field_value(value, 0, FBC_CNTL, FBC_GRPH_COMP_EN);
@@ -268,9 +240,10 @@ void dce110_compressor_enable_fbc(
 void dce110_compressor_disable_fbc(struct compressor *compressor)
 {
         struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+       uint32_t crtc_inst = 0;

         if (compressor->options.bits.FBC_SUPPORT) {
-               if (dce110_compressor_is_fbc_enabled_in_hw(compressor, NULL)) {
+               if (dce110_compressor_is_fbc_enabled_in_hw(compressor, &crtc_inst)) {
                         uint32_t reg_data;
                         /* Turn off compression */
                         reg_data = dm_read_reg(compressor->ctx, mmFBC_CNTL);
@@ -284,8 +257,10 @@ void dce110_compressor_disable_fbc(struct compressor *compressor)
                         wait_for_fbc_state_changed(cp110, false);
                 }

-               /* Sync line buffer  - dce100/110 only*/
-               reset_lb_on_vblank(compressor->ctx);
+               /* Sync line buffer which fbc was attached to dce100/110 only */
+               if (crtc_inst > CONTROLLER_ID_UNDEFINED && crtc_inst < CONTROLLER_ID_D3)
+                       reset_lb_on_vblank(compressor,
+                                       crtc_inst - CONTROLLER_ID_D0);
         }
 }

@@ -328,6 +303,8 @@ void dce110_compressor_program_compressed_surface_address_and_pitch(
         uint32_t compressed_surf_address_low_part =
                 compressor->compr_surface_address.addr.low_part;

+       cp110->offsets = reg_offsets[params->inst];
+
         /* Clear content first. */
         dm_write_reg(
                 compressor->ctx,
@@ -410,13 +387,7 @@ void dce110_compressor_set_fbc_invalidation_triggers(
         value = dm_read_reg(compressor->ctx, addr);
         set_reg_field_value(
                 value,
-               fbc_trigger |
-               FBC_IDLE_FORCE_GRPH_COMP_EN |
-               FBC_IDLE_FORCE_SRC_SEL_CHANGE |
-               FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE |
-               FBC_IDLE_FORCE_ALPHA_COMP_EN |
-               FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN |
-               FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF,
+               fbc_trigger,
                 FBC_IDLE_FORCE_CLEAR_MASK,
                 FBC_IDLE_FORCE_CLEAR_MASK);
         dm_write_reg(compressor->ctx, addr, value);
@@ -549,7 +520,7 @@ void dce110_compressor_construct(struct dce110_compressor *compressor,
         compressor->base.channel_interleave_size = 0;
         compressor->base.dram_channels_num = 0;
         compressor->base.lpt_channels_num = 0;
-       compressor->base.attached_inst = 0;
+       compressor->base.attached_inst = CONTROLLER_ID_UNDEFINED;
         compressor->base.is_enabled = false;
         compressor->base.funcs = &dce110_compressor_funcs;

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index 2f062ba..6349ba7 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1766,12 +1766,13 @@ static void set_static_screen_control(struct pipe_ctx **pipe_ctx,
  *  Check if FBC can be enabled
  */
 static bool should_enable_fbc(struct dc *dc,
-                             struct dc_state *context,
-                             uint32_t *pipe_idx)
+               struct dc_state *context,
+               uint32_t *pipe_idx)
 {
         uint32_t i;
         struct pipe_ctx *pipe_ctx = NULL;
         struct resource_context *res_ctx = &context->res_ctx;
+       unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;


         ASSERT(dc->fbc_compressor);
@@ -1786,14 +1787,28 @@ static bool should_enable_fbc(struct dc *dc,

         for (i = 0; i < dc->res_pool->pipe_count; i++) {
                 if (res_ctx->pipe_ctx[i].stream) {
+
                         pipe_ctx = &res_ctx->pipe_ctx[i];
-                       *pipe_idx = i;
-                       break;
+
+                       if (!pipe_ctx)
+                               continue;
+
+                       /* fbc not applicable on underlay pipe */
+                       if (pipe_ctx->pipe_idx != underlay_idx) {
+                               *pipe_idx = i;
+                               break;
+                       }
                 }
         }

-       /* Pipe context should be found */
-       ASSERT(pipe_ctx);
+       if (i == dc->res_pool->pipe_count)
+               return false;
+
+       if (!pipe_ctx->stream->sink)
+               return false;
+
+       if (!pipe_ctx->stream->sink->link)
+               return false;

         /* Only supports eDP */
         if (pipe_ctx->stream->sink->link->connector_signal != SIGNAL_TYPE_EDP)
@@ -1817,8 +1832,9 @@ static bool should_enable_fbc(struct dc *dc,
 /*
  *  Enable FBC
  */
-static void enable_fbc(struct dc *dc,
-                      struct dc_state *context)
+static void enable_fbc(
+               struct dc *dc,
+               struct dc_state *context)
 {
         uint32_t pipe_idx = 0;

@@ -1828,10 +1844,9 @@ static void enable_fbc(struct dc *dc,
                 struct compressor *compr = dc->fbc_compressor;
                 struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[pipe_idx];

-
                 params.source_view_width = pipe_ctx->stream->timing.h_addressable;
                 params.source_view_height = pipe_ctx->stream->timing.v_addressable;
-
+               params.inst = pipe_ctx->stream_res.tg->inst;
                 compr->compr_surface_address.quad_part = dc->ctx->fbc_gpu_addr;

                 compr->funcs->surface_address_and_pitch(compr, &params);
@@ -2046,10 +2061,10 @@ enum dc_status dce110_apply_ctx_to_hw(
                         return status;
         }

-       dcb->funcs->set_scratch_critical_state(dcb, false);
-
         if (dc->fbc_compressor)
-               enable_fbc(dc, context);
+               enable_fbc(dc, dc->current_state);
+
+       dcb->funcs->set_scratch_critical_state(dcb, false);

         return DC_OK;
 }
@@ -2408,7 +2423,6 @@ static void dce110_program_front_end_for_pipe(
         struct dc_plane_state *plane_state = pipe_ctx->plane_state;
         struct xfm_grph_csc_adjustment adjust;
         struct out_csc_color_matrix tbl_entry;
-       unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;
         unsigned int i;
         DC_LOGGER_INIT();
         memset(&tbl_entry, 0, sizeof(tbl_entry));
@@ -2449,15 +2463,6 @@ static void dce110_program_front_end_for_pipe(

         program_scaler(dc, pipe_ctx);

-       /* fbc not applicable on Underlay pipe */
-       if (dc->fbc_compressor && old_pipe->stream &&
-           pipe_ctx->pipe_idx != underlay_idx) {
-               if (plane_state->tiling_info.gfx8.array_mode == DC_ARRAY_LINEAR_GENERAL)
-                       dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
-               else
-                       enable_fbc(dc, dc->current_state);
-       }
-
         mi->funcs->mem_input_program_surface_config(
                         mi,
                         plane_state->format,
@@ -2534,6 +2539,9 @@ static void dce110_apply_ctx_for_surface(
         if (num_planes == 0)
                 return;

+       if (dc->fbc_compressor)
+               dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
+
         for (i = 0; i < dc->res_pool->pipe_count; i++) {
                 struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[i];
                 struct pipe_ctx *old_pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
@@ -2576,6 +2584,9 @@ static void dce110_apply_ctx_for_surface(
                         (pipe_ctx->plane_state || old_pipe_ctx->plane_state))
                         dc->hwss.pipe_control_lock(dc, pipe_ctx, false);
         }
+
+       if (dc->fbc_compressor)
+               enable_fbc(dc, dc->current_state);
 }

 static void dce110_power_down_fe(struct dc *dc, struct pipe_ctx *pipe_ctx)
diff --git a/drivers/gpu/drm/amd/display/dc/inc/compressor.h b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
index bcb18f5..7a147a9 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/compressor.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
@@ -77,6 +77,7 @@ struct compressor_funcs {
 };
 struct compressor {
         struct dc_context *ctx;
+       /* CONTROLLER_ID_D0 + instance, CONTROLLER_ID_UNDEFINED = 0 */
         uint32_t attached_inst;
         bool is_enabled;
         const struct compressor_funcs *funcs;
--
2.7.4

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

[-- Attachment #1.2: Type: text/html, Size: 33851 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

RE: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

[Li, Roman]

[-- Attachment #1.1: Type: text/plain, Size: 17304 bytes --]

Unfortunately, not. I sent this patch to the reporter to try and it  didn't work.

- Roman

From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> On Behalf Of Deucher, Alexander
Sent: Thursday, November 29, 2018 11:27 AM
To: Li, Sun peng (Leo) <Sunpeng.Li-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Wu, Hersen <hersenxs.wu-5C7GfCeVMHo@public.gmane.org>
Subject: Re: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc


Do you think this will fix this bug?

https://bugs.freedesktop.org/show_bug.cgi?id=108577

If so, we can re-enable fbc.



Alex

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of sunpeng.li-5C7GfCeVMHo@public.gmane.org<mailto:sunpeng.li@amd.com> <sunpeng.li-5C7GfCeVMHo@public.gmane.org<mailto:sunpeng.li-5C7GfCeVMHo@public.gmane.org>>
Sent: Thursday, November 29, 2018 10:52:16 AM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Cc: Wu, Hersen
Subject: [PATCH 09/16] drm/amd/display: fbc state could not reach while enable fbc

From: hersen wu <hersenxs.wu-5C7GfCeVMHo@public.gmane.org<mailto:hersenxs.wu-5C7GfCeVMHo@public.gmane.org>>

   [WHY] fbc is within the data path from memory to dce. while
   re-configure mc dmif, fbc should be enabled. otherwise, fbc
   may not be enabled properly.

   [HOW] before re-configure mc dmif, disable fbc, only after
   dmif re-configuration fully done, enable fbc again.

Signed-off-by: hersen wu <hersenxs.wu-5C7GfCeVMHo@public.gmane.org<mailto:hersenxs.wu-5C7GfCeVMHo@public.gmane.org>>
Reviewed-by: Roman Li <Roman.Li-5C7GfCeVMHo@public.gmane.org<mailto:Roman.Li-5C7GfCeVMHo@public.gmane.org>>
Acked-by: Leo Li <sunpeng.li-5C7GfCeVMHo@public.gmane.org<mailto:sunpeng.li-5C7GfCeVMHo@public.gmane.org>>
---
 .../drm/amd/display/dc/dce110/dce110_compressor.c  | 91 ++++++++--------------
 .../amd/display/dc/dce110/dce110_hw_sequencer.c    | 57 ++++++++------
 drivers/gpu/drm/amd/display/dc/inc/compressor.h    |  1 +
 3 files changed, 66 insertions(+), 83 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
index 1f7f250..52d50e2 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_compressor.c
@@ -64,65 +64,37 @@ static const struct dce110_compressor_reg_offsets reg_offsets[] = {

 static const uint32_t dce11_one_lpt_channel_max_resolution = 2560 * 1600;

-enum fbc_idle_force {
-       /* Bit 0 - Display registers updated */
-       FBC_IDLE_FORCE_DISPLAY_REGISTER_UPDATE = 0x00000001,
-
-       /* Bit 2 - FBC_GRPH_COMP_EN register updated */
-       FBC_IDLE_FORCE_GRPH_COMP_EN = 0x00000002,
-       /* Bit 3 - FBC_SRC_SEL register updated */
-       FBC_IDLE_FORCE_SRC_SEL_CHANGE = 0x00000004,
-       /* Bit 4 - FBC_MIN_COMPRESSION register updated */
-       FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE = 0x00000008,
-       /* Bit 5 - FBC_ALPHA_COMP_EN register updated */
-       FBC_IDLE_FORCE_ALPHA_COMP_EN = 0x00000010,
-       /* Bit 6 - FBC_ZERO_ALPHA_CHUNK_SKIP_EN register updated */
-       FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN = 0x00000020,
-       /* Bit 7 - FBC_FORCE_COPY_TO_COMP_BUF register updated */
-       FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF = 0x00000040,
-
-       /* Bit 24 - Memory write to region 0 defined by MC registers. */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION0 = 0x01000000,
-       /* Bit 25 - Memory write to region 1 defined by MC registers */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION1 = 0x02000000,
-       /* Bit 26 - Memory write to region 2 defined by MC registers */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION2 = 0x04000000,
-       /* Bit 27 - Memory write to region 3 defined by MC registers. */
-       FBC_IDLE_FORCE_MEMORY_WRITE_TO_REGION3 = 0x08000000,
-
-       /* Bit 28 - Memory write from any client other than MCIF */
-       FBC_IDLE_FORCE_MEMORY_WRITE_OTHER_THAN_MCIF = 0x10000000,
-       /* Bit 29 - CG statics screen signal is inactive */
-       FBC_IDLE_FORCE_CG_STATIC_SCREEN_IS_INACTIVE = 0x20000000,
-};
-
-
 static uint32_t align_to_chunks_number_per_line(uint32_t pixels)
 {
         return 256 * ((pixels + 255) / 256);
 }

-static void reset_lb_on_vblank(struct dc_context *ctx)
+static void reset_lb_on_vblank(struct compressor *compressor, uint32_t crtc_inst)
 {
-       uint32_t value, frame_count;
+       uint32_t value;
+       uint32_t frame_count;
+       uint32_t status_pos;
         uint32_t retry = 0;
-       uint32_t status_pos =
-                       dm_read_reg(ctx, mmCRTC_STATUS_POSITION);
+       struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+
+       cp110->offsets = reg_offsets[crtc_inst];
+
+       status_pos = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION));


         /* Only if CRTC is enabled and counter is moving we wait for one frame. */
-       if (status_pos != dm_read_reg(ctx, mmCRTC_STATUS_POSITION)) {
+       if (status_pos != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_POSITION))) {
                 /* Resetting LB on VBlank */
-               value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+               value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
                 set_reg_field_value(value, 3, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
                 set_reg_field_value(value, 1, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-               dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
+               dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);

-               frame_count = dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT);
+               frame_count = dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT));


                 for (retry = 10000; retry > 0; retry--) {
-                       if (frame_count != dm_read_reg(ctx, mmCRTC_STATUS_FRAME_COUNT))
+                       if (frame_count != dm_read_reg(compressor->ctx, DCP_REG(mmCRTC_STATUS_FRAME_COUNT)))
                                 break;
                         udelay(10);
                 }
@@ -130,13 +102,11 @@ static void reset_lb_on_vblank(struct dc_context *ctx)
                         dm_error("Frame count did not increase for 100ms.\n");

                 /* Resetting LB on VBlank */
-               value = dm_read_reg(ctx, mmLB_SYNC_RESET_SEL);
+               value = dm_read_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL));
                 set_reg_field_value(value, 2, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL);
                 set_reg_field_value(value, 0, LB_SYNC_RESET_SEL, LB_SYNC_RESET_SEL2);
-               dm_write_reg(ctx, mmLB_SYNC_RESET_SEL, value);
-
+               dm_write_reg(compressor->ctx, DCP_REG(mmLB_SYNC_RESET_SEL), value);
         }
-
 }

 static void wait_for_fbc_state_changed(
@@ -226,10 +196,10 @@ void dce110_compressor_enable_fbc(
                 uint32_t addr;
                 uint32_t value, misc_value;

-
                 addr = mmFBC_CNTL;
                 value = dm_read_reg(compressor->ctx, addr);
                 set_reg_field_value(value, 1, FBC_CNTL, FBC_GRPH_COMP_EN);
+               /* params->inst is valid HW CRTC instance start from 0 */
                 set_reg_field_value(
                         value,
                         params->inst,
@@ -238,8 +208,10 @@ void dce110_compressor_enable_fbc(

                 /* Keep track of enum controller_id FBC is attached to */
                 compressor->is_enabled = true;
-               compressor->attached_inst = params->inst;
-               cp110->offsets = reg_offsets[params->inst];
+               /* attached_inst is SW CRTC instance start from 1
+                * 0 = CONTROLLER_ID_UNDEFINED means not attached crtc
+                */
+               compressor->attached_inst = params->inst + CONTROLLER_ID_D0;

                 /* Toggle it as there is bug in HW */
                 set_reg_field_value(value, 0, FBC_CNTL, FBC_GRPH_COMP_EN);
@@ -268,9 +240,10 @@ void dce110_compressor_enable_fbc(
 void dce110_compressor_disable_fbc(struct compressor *compressor)
 {
         struct dce110_compressor *cp110 = TO_DCE110_COMPRESSOR(compressor);
+       uint32_t crtc_inst = 0;

         if (compressor->options.bits.FBC_SUPPORT) {
-               if (dce110_compressor_is_fbc_enabled_in_hw(compressor, NULL)) {
+               if (dce110_compressor_is_fbc_enabled_in_hw(compressor, &crtc_inst)) {
                         uint32_t reg_data;
                         /* Turn off compression */
                         reg_data = dm_read_reg(compressor->ctx, mmFBC_CNTL);
@@ -284,8 +257,10 @@ void dce110_compressor_disable_fbc(struct compressor *compressor)
                         wait_for_fbc_state_changed(cp110, false);
                 }

-               /* Sync line buffer  - dce100/110 only*/
-               reset_lb_on_vblank(compressor->ctx);
+               /* Sync line buffer which fbc was attached to dce100/110 only */
+               if (crtc_inst > CONTROLLER_ID_UNDEFINED && crtc_inst < CONTROLLER_ID_D3)
+                       reset_lb_on_vblank(compressor,
+                                       crtc_inst - CONTROLLER_ID_D0);
         }
 }

@@ -328,6 +303,8 @@ void dce110_compressor_program_compressed_surface_address_and_pitch(
         uint32_t compressed_surf_address_low_part =
                 compressor->compr_surface_address.addr.low_part;

+       cp110->offsets = reg_offsets[params->inst];
+
         /* Clear content first. */
         dm_write_reg(
                 compressor->ctx,
@@ -410,13 +387,7 @@ void dce110_compressor_set_fbc_invalidation_triggers(
         value = dm_read_reg(compressor->ctx, addr);
         set_reg_field_value(
                 value,
-               fbc_trigger |
-               FBC_IDLE_FORCE_GRPH_COMP_EN |
-               FBC_IDLE_FORCE_SRC_SEL_CHANGE |
-               FBC_IDLE_FORCE_MIN_COMPRESSION_CHANGE |
-               FBC_IDLE_FORCE_ALPHA_COMP_EN |
-               FBC_IDLE_FORCE_ZERO_ALPHA_CHUNK_SKIP_EN |
-               FBC_IDLE_FORCE_FORCE_COPY_TO_COMP_BUF,
+               fbc_trigger,
                 FBC_IDLE_FORCE_CLEAR_MASK,
                 FBC_IDLE_FORCE_CLEAR_MASK);
         dm_write_reg(compressor->ctx, addr, value);
@@ -549,7 +520,7 @@ void dce110_compressor_construct(struct dce110_compressor *compressor,
         compressor->base.channel_interleave_size = 0;
         compressor->base.dram_channels_num = 0;
         compressor->base.lpt_channels_num = 0;
-       compressor->base.attached_inst = 0;
+       compressor->base.attached_inst = CONTROLLER_ID_UNDEFINED;
         compressor->base.is_enabled = false;
         compressor->base.funcs = &dce110_compressor_funcs;

diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
index 2f062ba..6349ba7 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
@@ -1766,12 +1766,13 @@ static void set_static_screen_control(struct pipe_ctx **pipe_ctx,
  *  Check if FBC can be enabled
  */
 static bool should_enable_fbc(struct dc *dc,
-                             struct dc_state *context,
-                             uint32_t *pipe_idx)
+               struct dc_state *context,
+               uint32_t *pipe_idx)
 {
         uint32_t i;
         struct pipe_ctx *pipe_ctx = NULL;
         struct resource_context *res_ctx = &context->res_ctx;
+       unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;


         ASSERT(dc->fbc_compressor);
@@ -1786,14 +1787,28 @@ static bool should_enable_fbc(struct dc *dc,

         for (i = 0; i < dc->res_pool->pipe_count; i++) {
                 if (res_ctx->pipe_ctx[i].stream) {
+
                         pipe_ctx = &res_ctx->pipe_ctx[i];
-                       *pipe_idx = i;
-                       break;
+
+                       if (!pipe_ctx)
+                               continue;
+
+                       /* fbc not applicable on underlay pipe */
+                       if (pipe_ctx->pipe_idx != underlay_idx) {
+                               *pipe_idx = i;
+                               break;
+                       }
                 }
         }

-       /* Pipe context should be found */
-       ASSERT(pipe_ctx);
+       if (i == dc->res_pool->pipe_count)
+               return false;
+
+       if (!pipe_ctx->stream->sink)
+               return false;
+
+       if (!pipe_ctx->stream->sink->link)
+               return false;

         /* Only supports eDP */
         if (pipe_ctx->stream->sink->link->connector_signal != SIGNAL_TYPE_EDP)
@@ -1817,8 +1832,9 @@ static bool should_enable_fbc(struct dc *dc,
 /*
  *  Enable FBC
  */
-static void enable_fbc(struct dc *dc,
-                      struct dc_state *context)
+static void enable_fbc(
+               struct dc *dc,
+               struct dc_state *context)
 {
         uint32_t pipe_idx = 0;

@@ -1828,10 +1844,9 @@ static void enable_fbc(struct dc *dc,
                 struct compressor *compr = dc->fbc_compressor;
                 struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[pipe_idx];

-
                 params.source_view_width = pipe_ctx->stream->timing.h_addressable;
                 params.source_view_height = pipe_ctx->stream->timing.v_addressable;
-
+               params.inst = pipe_ctx->stream_res.tg->inst;
                 compr->compr_surface_address.quad_part = dc->ctx->fbc_gpu_addr;

                 compr->funcs->surface_address_and_pitch(compr, &params);
@@ -2046,10 +2061,10 @@ enum dc_status dce110_apply_ctx_to_hw(
                         return status;
         }

-       dcb->funcs->set_scratch_critical_state(dcb, false);
-
         if (dc->fbc_compressor)
-               enable_fbc(dc, context);
+               enable_fbc(dc, dc->current_state);
+
+       dcb->funcs->set_scratch_critical_state(dcb, false);

         return DC_OK;
 }
@@ -2408,7 +2423,6 @@ static void dce110_program_front_end_for_pipe(
         struct dc_plane_state *plane_state = pipe_ctx->plane_state;
         struct xfm_grph_csc_adjustment adjust;
         struct out_csc_color_matrix tbl_entry;
-       unsigned int underlay_idx = dc->res_pool->underlay_pipe_index;
         unsigned int i;
         DC_LOGGER_INIT();
         memset(&tbl_entry, 0, sizeof(tbl_entry));
@@ -2449,15 +2463,6 @@ static void dce110_program_front_end_for_pipe(

         program_scaler(dc, pipe_ctx);

-       /* fbc not applicable on Underlay pipe */
-       if (dc->fbc_compressor && old_pipe->stream &&
-           pipe_ctx->pipe_idx != underlay_idx) {
-               if (plane_state->tiling_info.gfx8.array_mode == DC_ARRAY_LINEAR_GENERAL)
-                       dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
-               else
-                       enable_fbc(dc, dc->current_state);
-       }
-
         mi->funcs->mem_input_program_surface_config(
                         mi,
                         plane_state->format,
@@ -2534,6 +2539,9 @@ static void dce110_apply_ctx_for_surface(
         if (num_planes == 0)
                 return;

+       if (dc->fbc_compressor)
+               dc->fbc_compressor->funcs->disable_fbc(dc->fbc_compressor);
+
         for (i = 0; i < dc->res_pool->pipe_count; i++) {
                 struct pipe_ctx *pipe_ctx = &context->res_ctx.pipe_ctx[i];
                 struct pipe_ctx *old_pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
@@ -2576,6 +2584,9 @@ static void dce110_apply_ctx_for_surface(
                         (pipe_ctx->plane_state || old_pipe_ctx->plane_state))
                         dc->hwss.pipe_control_lock(dc, pipe_ctx, false);
         }
+
+       if (dc->fbc_compressor)
+               enable_fbc(dc, dc->current_state);
 }

 static void dce110_power_down_fe(struct dc *dc, struct pipe_ctx *pipe_ctx)
diff --git a/drivers/gpu/drm/amd/display/dc/inc/compressor.h b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
index bcb18f5..7a147a9 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/compressor.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/compressor.h
@@ -77,6 +77,7 @@ struct compressor_funcs {
 };
 struct compressor {
         struct dc_context *ctx;
+       /* CONTROLLER_ID_D0 + instance, CONTROLLER_ID_UNDEFINED = 0 */
         uint32_t attached_inst;
         bool is_enabled;
         const struct compressor_funcs *funcs;
--
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 36968 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

[PATCH 10/16] drm/amd/display: Re-arrange GFX9 fields

[sunpeng.li-5C7GfCeVMHo]

From: Nevenko Stupar <Nevenko.Stupar@amd.com>

For more clear usage in future

Signed-off-by: Nevenko Stupar <Nevenko.Stupar@amd.com>
Reviewed-by: Tony Cheng <Tony.Cheng@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/dc_hw_types.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
index 7825e4b..9ddfe4c 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_hw_types.h
@@ -358,15 +358,16 @@ union dc_tiling_info {
 	} gfx8;
 
 	struct {
+		enum swizzle_mode_values swizzle;
 		unsigned int num_pipes;
-		unsigned int num_banks;
+		unsigned int max_compressed_frags;
 		unsigned int pipe_interleave;
+
+		unsigned int num_banks;
 		unsigned int num_shader_engines;
 		unsigned int num_rb_per_se;
-		unsigned int max_compressed_frags;
 		bool shaderEnable;
 
-		enum swizzle_mode_values swizzle;
 		bool meta_linear;
 		bool rb_aligned;
 		bool pipe_aligned;
-- 
2.7.4

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

[PATCH 11/16] drm/amd/display: Add customizable tracing event

[sunpeng.li-5C7GfCeVMHo]

From: Chiawen Huang <chiawen.huang@amd.com>

[why]
add customizable log with a message input, which is for adding
test log in debugging as printf function in ETW.

[Usage]
EVENT_LOG_CUST_MSG1("TestLog","Hello World %d=0x%x", 123, pDC);

Signed-off-by: Chiawen Huang <chiawen.huang@amd.com>
Reviewed-by: Tony Cheng <Tony.Cheng@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/dm_event_log.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dm_event_log.h b/drivers/gpu/drm/amd/display/dc/dm_event_log.h
index 34a701c..65663f4 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_event_log.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_event_log.h
@@ -33,6 +33,7 @@
 
 #define EVENT_LOG_AUX_REQ(ddc, type, action, address, len, data)
 #define EVENT_LOG_AUX_REP(ddc, type, replyStatus, len, data)
+#define EVENT_LOG_CUST_MSG(tag, a, ...)
 
 #endif
 
-- 
2.7.4

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

[PATCH 12/16] drm/amd/display: Copy crc_enabled when duplicating dm_crtc_state

[sunpeng.li-5C7GfCeVMHo]

From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

[Why]
When running igt@kms_plane@pixel-format-pipe-* tests the CRC read will
time out and the test will fail.

This is because the CRTC is duplicated but the crc_enabled parameter
isn't copied over to the new dm_crtc_state. CRC reads will time out
because amdgpu_dm_crtc_handle_crc_irq will no longer call
drm_crtc_add_crc_entry.

[How]
Copy crc_enabled when duplicating the state.

Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: David Francis <David.Francis@amd.com>
Reviewed-by: Sun peng Li <Sunpeng.Li@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
 1 file changed, 1 insertion(+)

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 6c865ce..249239e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3007,6 +3007,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
 	state->abm_level = cur->abm_level;
 	state->vrr_supported = cur->vrr_supported;
 	state->freesync_config = cur->freesync_config;
+	state->crc_enabled = cur->crc_enabled;
 
 	/* TODO Duplicate dc_stream after objects are stream object is flattened */
 
-- 
2.7.4

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

[PATCH 13/16] drm/amd/display: Program dithering if requested

[sunpeng.li-5C7GfCeVMHo]

From: SivapiriyanKumarasamy <sivapiriyan.kumarasamy@amd.com>

Dithering needs to be enabled or disabled as requested. If
dc_stream_update->dither_option is non-null, program the FMT blocks.

Signed-off-by: SivapiriyanKumarasamy <sivapiriyan.kumarasamy@amd.com>
Reviewed-by: Anthony Koo <Anthony.Koo@amd.com>
Reviewed-by: Krunoslav Kovac <Krunoslav.Kovac@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc.c   | 8 ++++++++
 drivers/gpu/drm/amd/display/dc/dc_stream.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index dffd083..ad67b98 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1483,6 +1483,14 @@ static void commit_planes_do_stream_update(struct dc *dc,
 			if (stream_update->output_csc_transform)
 				dc_stream_program_csc_matrix(dc, stream);
 
+			if (stream_update->dither_option) {
+				resource_build_bit_depth_reduction_params(pipe_ctx->stream,
+									&pipe_ctx->stream->bit_depth_params);
+				pipe_ctx->stream_res.opp->funcs->opp_program_fmt(pipe_ctx->stream_res.opp,
+						&stream->bit_depth_params,
+						&stream->clamping);
+			}
+
 			/* Full fe update*/
 			if (update_type == UPDATE_TYPE_FAST)
 				continue;
diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
index 0c42418..be34d63 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
@@ -136,6 +136,7 @@ struct dc_stream_update {
 
 	struct colorspace_transform *gamut_remap;
 	enum dc_color_space *output_color_space;
+	enum dc_dither_option *dither_option;
 
 	struct dc_csc_transform *output_csc_transform;
 
-- 
2.7.4

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

[PATCH 14/16] drm/amd/display: Allow clock lower on dce100

[sunpeng.li-5C7GfCeVMHo]

From: David Francis <David.Francis@amd.com>

dce100 was set to always pass safe_to_lower = false
to the clock manager

Thus, on suspend the clocks were not being set to 0
which is incorrect behaviour

This was causing s3 resume to blackscreen on intel
CPUs with dce100 GPUs attached

(Note that the hash in this Fixes: tag is the hash on Alex's tree)
Fixes: ae7d8aeb38d7 ("drm/amd/display: remove safe_to_lower flag from dc, use 2 functions instead")

Signed-off-by: David Francis <David.Francis@amd.com>
Reviewed-by: Harry Wentland <Harry.Wentland@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 .../gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c    | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c
index bc50a8e..8777167 100644
--- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c
@@ -117,6 +117,18 @@ void dce100_prepare_bandwidth(
 			false);
 }
 
+void dce100_optimize_bandwidth(
+		struct dc *dc,
+		struct dc_state *context)
+{
+	dce110_set_safe_displaymarks(&context->res_ctx, dc->res_pool);
+
+	dc->res_pool->clk_mgr->funcs->update_clocks(
+			dc->res_pool->clk_mgr,
+			context,
+			true);
+}
+
 /**************************************************************************/
 
 void dce100_hw_sequencer_construct(struct dc *dc)
@@ -125,6 +137,6 @@ void dce100_hw_sequencer_construct(struct dc *dc)
 
 	dc->hwss.enable_display_power_gating = dce100_enable_display_power_gating;
 	dc->hwss.prepare_bandwidth = dce100_prepare_bandwidth;
-	dc->hwss.optimize_bandwidth = dce100_prepare_bandwidth;
+	dc->hwss.optimize_bandwidth = dce100_optimize_bandwidth;
 }
 
-- 
2.7.4

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

[PATCH 15/16] drm/amd/display: 3.2.08

[sunpeng.li-5C7GfCeVMHo]

From: Steven Chiu <steven.chiu@amd.com>

Signed-off-by: Steven Chiu <steven.chiu@amd.com>
Reviewed-by: Fatemeh Darbehani <Fatemeh.Darbehani@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/amd/display/dc/dc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 70873d2..4b5bbb1 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -39,7 +39,7 @@
 #include "inc/hw/dmcu.h"
 #include "dml/display_mode_lib.h"
 
-#define DC_VER "3.2.07"
+#define DC_VER "3.2.08"
 
 #define MAX_SURFACES 3
 #define MAX_STREAMS 6
-- 
2.7.4

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

[PATCH 16/16] drm/amd/display: Clean up for DCN1 clock debug logging

[sunpeng.li-5C7GfCeVMHo]

From: Fatemeh Darbehani <fatemeh.darbehani@amd.com>

[Why]
To prepare for clock debug logging. With the exception of removing
max_supported_dppclk_khz from logs, there are no functional changes.

[How]
Add clk_bypass struct and clean up buffer logic

Signed-off-by: Fatemeh Darbehani <fatemeh.darbehani@amd.com>
Reviewed-by: Yongqiang Sun <yongqiang.sun@amd.com>
Acked-by: Su Chung <Su.Chung@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
---
 .../gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.c   |  4 +--
 .../gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.h   |  6 ++++
 .../display/dc/dcn10/dcn10_hw_sequencer_debug.c    | 39 +++++++++++++---------
 .../gpu/drm/amd/display/dc/dcn10/dcn10_resource.c  |  4 ---
 drivers/gpu/drm/amd/display/dc/dm_pp_smu.h         |  2 +-
 5 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.c
index f9d7d2c..54abedb 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.c
@@ -328,12 +328,10 @@ static void dcn1_update_clocks(struct clk_mgr *clk_mgr,
 
 	*smu_req_cur = smu_req;
 }
-
 static const struct clk_mgr_funcs dcn1_funcs = {
 	.get_dp_ref_clk_frequency = dce12_get_dp_ref_freq_khz,
 	.update_clocks = dcn1_update_clocks
 };
-
 struct clk_mgr *dcn1_clk_mgr_create(struct dc_context *ctx)
 {
 	struct dc_debug_options *debug = &ctx->dc->debug;
@@ -373,3 +371,5 @@ struct clk_mgr *dcn1_clk_mgr_create(struct dc_context *ctx)
 
 	return &clk_mgr_dce->base;
 }
+
+
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.h b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.h
index 9dbaf65..a995eda 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.h
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_clk_mgr.h
@@ -28,6 +28,12 @@
 
 #include "../dce/dce_clk_mgr.h"
 
+struct clk_bypass {
+	uint32_t dcfclk_bypass;
+	uint32_t dispclk_pypass;
+	uint32_t dprefclk_bypass;
+};
+
 void dcn1_pplib_apply_display_requirements(
 	struct dc *dc,
 	struct dc_state *context);
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer_debug.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer_debug.c
index 211bb24..cd46901 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer_debug.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer_debug.c
@@ -44,6 +44,7 @@
 #include "dcn10_hubp.h"
 #include "dcn10_hubbub.h"
 #include "dcn10_cm_common.h"
+#include "dcn10_clk_mgr.h"
 
 static unsigned int snprintf_count(char *pBuf, unsigned int bufSize, char *fmt, ...)
 {
@@ -463,19 +464,22 @@ static unsigned int dcn10_get_otg_states(struct dc *dc, char *pBuf, unsigned int
 static unsigned int dcn10_get_clock_states(struct dc *dc, char *pBuf, unsigned int bufSize)
 {
 	unsigned int chars_printed = 0;
+	unsigned int remaining_buffer = bufSize;
 
-	chars_printed = snprintf_count(pBuf, bufSize, "dcfclk_khz,dcfclk_deep_sleep_khz,dispclk_khz,"
-		"dppclk_khz,max_supported_dppclk_khz,fclk_khz,socclk_khz\n"
-		"%d,%d,%d,%d,%d,%d,%d\n",
+	chars_printed = snprintf_count(pBuf, bufSize, "dcfclk,dcfclk_deep_sleep,dispclk,"
+		"dppclk,fclk,socclk\n"
+		"%d,%d,%d,%d,%d,%d\n",
 		dc->current_state->bw.dcn.clk.dcfclk_khz,
 		dc->current_state->bw.dcn.clk.dcfclk_deep_sleep_khz,
 		dc->current_state->bw.dcn.clk.dispclk_khz,
 		dc->current_state->bw.dcn.clk.dppclk_khz,
-		dc->current_state->bw.dcn.clk.max_supported_dppclk_khz,
 		dc->current_state->bw.dcn.clk.fclk_khz,
 		dc->current_state->bw.dcn.clk.socclk_khz);
 
-	return chars_printed;
+	remaining_buffer -= chars_printed;
+	pBuf += chars_printed;
+
+	return bufSize - remaining_buffer;
 }
 
 static void dcn10_clear_otpc_underflow(struct dc *dc)
@@ -538,16 +542,16 @@ void dcn10_get_hw_state(struct dc *dc, char *pBuf, unsigned int bufSize, unsigne
 	 *  Bit 0 - 15: Hardware block mask
 	 *  Bit 15: 1 = Invariant Only, 0 = All
 	 */
-	const unsigned int DC_HW_STATE_MASK_HUBBUB 	= 0x1;
-	const unsigned int DC_HW_STATE_MASK_HUBP 	= 0x2;
-	const unsigned int DC_HW_STATE_MASK_RQ 		= 0x4;
-	const unsigned int DC_HW_STATE_MASK_DLG 	= 0x8;
-	const unsigned int DC_HW_STATE_MASK_TTU 	= 0x10;
-	const unsigned int DC_HW_STATE_MASK_CM 		= 0x20;
-	const unsigned int DC_HW_STATE_MASK_MPCC 	= 0x40;
-	const unsigned int DC_HW_STATE_MASK_OTG 	= 0x80;
-	const unsigned int DC_HW_STATE_MASK_CLOCKS 	= 0x100;
-	const unsigned int DC_HW_STATE_INVAR_ONLY	= 0x8000;
+	const unsigned int DC_HW_STATE_MASK_HUBBUB			= 0x1;
+	const unsigned int DC_HW_STATE_MASK_HUBP			= 0x2;
+	const unsigned int DC_HW_STATE_MASK_RQ				= 0x4;
+	const unsigned int DC_HW_STATE_MASK_DLG				= 0x8;
+	const unsigned int DC_HW_STATE_MASK_TTU				= 0x10;
+	const unsigned int DC_HW_STATE_MASK_CM				= 0x20;
+	const unsigned int DC_HW_STATE_MASK_MPCC			= 0x40;
+	const unsigned int DC_HW_STATE_MASK_OTG				= 0x80;
+	const unsigned int DC_HW_STATE_MASK_CLOCKS			= 0x100;
+	const unsigned int DC_HW_STATE_INVAR_ONLY			= 0x8000;
 
 	unsigned int chars_printed = 0;
 	unsigned int remaining_buf_size = bufSize;
@@ -603,6 +607,9 @@ void dcn10_get_hw_state(struct dc *dc, char *pBuf, unsigned int bufSize, unsigne
 		remaining_buf_size -= chars_printed;
 	}
 
-	if ((mask & DC_HW_STATE_MASK_CLOCKS) && remaining_buf_size > 0)
+	if ((mask & DC_HW_STATE_MASK_CLOCKS) && remaining_buf_size > 0) {
 		chars_printed = dcn10_get_clock_states(dc, pBuf, remaining_buf_size);
+		pBuf += chars_printed;
+		remaining_buf_size -= chars_printed;
+	}
 }
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index 47dbe4b..5d4772d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -202,7 +202,6 @@ enum dcn10_clk_src_array_id {
 #define MMHUB_SR(reg_name)\
 		.reg_name = MMHUB_BASE(mm ## reg_name ## _BASE_IDX) +  \
 					mm ## reg_name
-
 /* macros to expend register list macro defined in HW object header file
  * end *********************/
 
@@ -436,7 +435,6 @@ static const struct dcn_optc_mask tg_mask = {
 	TG_COMMON_MASK_SH_LIST_DCN1_0(_MASK)
 };
 
-
 static const struct bios_registers bios_regs = {
 		NBIO_SR(BIOS_SCRATCH_0),
 		NBIO_SR(BIOS_SCRATCH_3),
@@ -497,7 +495,6 @@ static const struct dce110_clk_src_mask cs_mask = {
 		CS_COMMON_MASK_SH_LIST_DCN1_0(_MASK)
 };
 
-
 static const struct resource_caps res_cap = {
 		.num_timing_generator = 4,
 		.num_opp = 4,
@@ -1277,7 +1274,6 @@ static bool construct(
 			goto fail;
 		}
 	}
-
 	pool->base.clk_mgr = dcn1_clk_mgr_create(ctx);
 	if (pool->base.clk_mgr == NULL) {
 		dm_error("DC: failed to create display clock!\n");
diff --git a/drivers/gpu/drm/amd/display/dc/dm_pp_smu.h b/drivers/gpu/drm/amd/display/dc/dm_pp_smu.h
index beb08fd..0029a39 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_pp_smu.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_pp_smu.h
@@ -102,7 +102,7 @@ struct pp_smu_funcs_rv {
 	 */
 	void (*set_display_count)(struct pp_smu *pp, int count);
 
-	/* which SMU message?  are reader and writer WM separate SMU msg? */
+	/* reader and writer WM's are sent together as part of one table*/
 	/*
 	 * PPSMC_MSG_SetDriverDramAddrHigh
 	 * PPSMC_MSG_SetDriverDramAddrLow
-- 
2.7.4

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