[PATCH 17/24] drm/amd/display: Enforce DPCD Address ranges

Anson Jacob Anson.Jacob at amd.com
Thu Jun 10 16:28:31 UTC 2021


From: Wesley Chalmers <Wesley.Chalmers at amd.com>

[WHY]
Some DPCD addresses, notably LTTPR Capability registers, are expected to
be read all together in a single DPCD transaction. Rather than force callers to
read registers they don't need, we want to quietly extend the addresses
read, and only return back the values the caller asked for.
This does not affect DPCD writes.

[HOW]
Create an additional layer above AUX to perform 'checked' DPCD
transactions.
Iterate through an array of DPCD address ranges that are marked as being
contiguous. If a requested read falls within one of those ranges, extend
the read to include the entire range.
After DPCD has been queried, copy the requested bytes into the caller's
data buffer, and deallocate all resources used.

Signed-off-by: Wesley Chalmers <Wesley.Chalmers at amd.com>
Reviewed-by: Jun Lei <Jun.Lei at amd.com>
Acked-by: Anson Jacob <Anson.Jacob at amd.com>
---
 drivers/gpu/drm/amd/display/dc/Makefile       |   2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link.c |   1 +
 .../gpu/drm/amd/display/dc/core/dc_link_dp.c  |   2 +
 .../drm/amd/display/dc/core/dc_link_dpcd.c    | 135 ++++++++++++++++++
 .../drm/amd/display/dc/core/dc_link_hwss.c    |  31 +---
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |   1 +
 .../drm/amd/display/dc/dcn30/dcn30_hwseq.c    |   1 +
 .../drm/amd/display/dc/dcn31/dcn31_hwseq.c    |   1 +
 .../gpu/drm/amd/display/dc/hdcp/hdcp_msg.c    |   1 +
 .../gpu/drm/amd/display/dc/inc/link_dpcd.h    |  18 +++
 .../gpu/drm/amd/display/dc/inc/link_hwss.h    |  14 --
 11 files changed, 162 insertions(+), 45 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/dc/core/dc_link_dpcd.c
 create mode 100644 drivers/gpu/drm/amd/display/dc/inc/link_dpcd.h

diff --git a/drivers/gpu/drm/amd/display/dc/Makefile b/drivers/gpu/drm/amd/display/dc/Makefile
index 95aca9b0ef7f..34fc36e77595 100644
--- a/drivers/gpu/drm/amd/display/dc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/Makefile
@@ -60,7 +60,7 @@ include $(AMD_DC)
 
 DISPLAY_CORE = dc.o  dc_stat.o dc_link.o dc_resource.o dc_hw_sequencer.o dc_sink.o \
 dc_surface.o dc_link_hwss.o dc_link_dp.o dc_link_ddc.o dc_debug.o dc_stream.o \
-dc_link_enc_cfg.o
+dc_link_enc_cfg.o dc_link_dpcd.o
 
 ifdef CONFIG_DRM_AMD_DC_DCN
 DISPLAY_CORE += dc_vm_helper.o
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 05c963a5b789..9058e45add92 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -49,6 +49,7 @@
 #include "dmub/dmub_srv.h"
 #include "inc/hw/panel_cntl.h"
 #include "inc/link_enc_cfg.h"
+#include "inc/link_dpcd.h"
 
 #define DC_LOGGER_INIT(logger)
 
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 7e52bb3047bc..1b28b4a40f62 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
@@ -25,6 +25,8 @@ static const uint8_t DP_VGA_LVDS_CONVERTER_ID_3[] = "dnomlA";
 	link->ctx->logger
 #define DC_TRACE_LEVEL_MESSAGE(...) /* do nothing */
 
+#include "link_dpcd.h"
+
 	/* maximum pre emphasis level allowed for each voltage swing level*/
 	static const enum dc_pre_emphasis
 	voltage_swing_to_pre_emphasis[] = { PRE_EMPHASIS_LEVEL3,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dpcd.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dpcd.c
new file mode 100644
index 000000000000..8957565f87bc
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dpcd.c
@@ -0,0 +1,135 @@
+#include <inc/core_status.h>
+#include <dc_link.h>
+#include <inc/link_hwss.h>
+#include <inc/link_dpcd.h>
+#include "drm/drm_dp_helper.h"
+#include <dc_dp_types.h>
+#include "dm_helpers.h"
+
+#define END_ADDRESS(start, size) (start + size - 1)
+#define ADDRESS_RANGE_SIZE(start, end) (end - start + 1)
+struct dpcd_address_range {
+	uint32_t start;
+	uint32_t end;
+};
+
+static enum dc_status internal_link_read_dpcd(
+	struct dc_link *link,
+	uint32_t address,
+	uint8_t *data,
+	uint32_t size)
+{
+	if (!link->aux_access_disabled &&
+			!dm_helpers_dp_read_dpcd(link->ctx,
+			link, address, data, size)) {
+		return DC_ERROR_UNEXPECTED;
+	}
+
+	return DC_OK;
+}
+
+static enum dc_status internal_link_write_dpcd(
+	struct dc_link *link,
+	uint32_t address,
+	const uint8_t *data,
+	uint32_t size)
+{
+	if (!link->aux_access_disabled &&
+			!dm_helpers_dp_write_dpcd(link->ctx,
+			link, address, data, size)) {
+		return DC_ERROR_UNEXPECTED;
+	}
+
+	return DC_OK;
+}
+
+/*
+ * Ranges of DPCD addresses that must be read in a single transaction
+ * XXX: Do not allow any two address ranges in this array to overlap
+ */
+static const struct dpcd_address_range mandatory_dpcd_blocks[] = {
+	{ DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV, DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT }};
+
+/*
+ * extend addresses to read all mandatory blocks together
+ */
+static void dpcd_extend_address_range(
+		const uint32_t in_address,
+		uint8_t * const in_data,
+		const uint32_t in_size,
+		uint32_t *out_address,
+		uint8_t **out_data,
+		uint32_t *out_size)
+{
+	const uint32_t end_address = END_ADDRESS(in_address, in_size);
+	const struct dpcd_address_range *addr_range;
+	struct dpcd_address_range new_addr_range;
+	uint32_t i;
+
+	new_addr_range.start = in_address;
+	new_addr_range.end = end_address;
+	for (i = 0; i < ARRAY_SIZE(mandatory_dpcd_blocks); i++) {
+		addr_range = &mandatory_dpcd_blocks[i];
+		if (addr_range->start <= in_address && addr_range->end >= in_address)
+			new_addr_range.start = addr_range->start;
+
+		if (addr_range->start <= end_address && addr_range->end >= end_address)
+			new_addr_range.end = addr_range->end;
+	}
+	*out_address = in_address;
+	*out_size = in_size;
+	*out_data = in_data;
+	if (new_addr_range.start != in_address || new_addr_range.end != end_address) {
+		*out_address = new_addr_range.start;
+		*out_size = ADDRESS_RANGE_SIZE(new_addr_range.start, new_addr_range.end);
+		*out_data = kzalloc(*out_size * sizeof(**out_data), GFP_KERNEL);
+	}
+}
+
+/*
+ * Reduce the AUX reply down to the values the caller requested
+ */
+static void dpcd_reduce_address_range(
+		const uint32_t extended_address,
+		uint8_t * const extended_data,
+		const uint32_t extended_size,
+		const uint32_t reduced_address,
+		uint8_t * const reduced_data,
+		const uint32_t reduced_size)
+{
+	const uint32_t reduced_end_address = END_ADDRESS(reduced_address, reduced_size);
+	const uint32_t extended_end_address = END_ADDRESS(reduced_address, extended_size);
+	const uint32_t offset = reduced_address - extended_address;
+
+	if (extended_end_address == reduced_end_address && extended_address == reduced_address)
+		return; /* extended and reduced address ranges point to the same data */
+
+	memcpy(&extended_data[offset], reduced_data, reduced_size);
+	kfree(extended_data);
+}
+
+enum dc_status core_link_read_dpcd(
+	struct dc_link *link,
+	uint32_t address,
+	uint8_t *data,
+	uint32_t size)
+{
+	uint32_t extended_address;
+	uint8_t *extended_data;
+	uint32_t extended_size;
+	enum dc_status status;
+
+	dpcd_extend_address_range(address, data, size, &extended_address, &extended_data, &extended_size);
+	status = internal_link_read_dpcd(link, extended_address, extended_data, extended_size);
+	dpcd_reduce_address_range(extended_address, extended_data, extended_size, address, data, size);
+	return status;
+}
+
+enum dc_status core_link_write_dpcd(
+	struct dc_link *link,
+	uint32_t address,
+	const uint8_t *data,
+	uint32_t size)
+{
+	return internal_link_write_dpcd(link, address, data, size);
+}
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
index f7dfc8fefdfa..9c51cd09dcf1 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
@@ -16,6 +16,7 @@
 #include "resource.h"
 #include "link_enc_cfg.h"
 #include "clk_mgr.h"
+#include "inc/link_dpcd.h"
 
 static uint8_t convert_to_count(uint8_t lttpr_repeater_count)
 {
@@ -47,36 +48,6 @@ static inline bool is_immediate_downstream(struct dc_link *link, uint32_t offset
 	return (convert_to_count(link->dpcd_caps.lttpr_caps.phy_repeater_cnt) == offset);
 }
 
-enum dc_status core_link_read_dpcd(
-	struct dc_link *link,
-	uint32_t address,
-	uint8_t *data,
-	uint32_t size)
-{
-	if (!link->aux_access_disabled &&
-			!dm_helpers_dp_read_dpcd(link->ctx,
-			link, address, data, size)) {
-		return DC_ERROR_UNEXPECTED;
-	}
-
-	return DC_OK;
-}
-
-enum dc_status core_link_write_dpcd(
-	struct dc_link *link,
-	uint32_t address,
-	const uint8_t *data,
-	uint32_t size)
-{
-	if (!link->aux_access_disabled &&
-			!dm_helpers_dp_write_dpcd(link->ctx,
-			link, address, data, size)) {
-		return DC_ERROR_UNEXPECTED;
-	}
-
-	return DC_OK;
-}
-
 void dp_receiver_power_ctrl(struct dc_link *link, bool on)
 {
 	uint8_t state;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index 711ba953a99b..5d54900f7b61 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -55,6 +55,7 @@
 #include "dc_trace.h"
 #include "dce/dmub_outbox.h"
 #include "inc/dc_link_dp.h"
+#include "inc/link_dpcd.h"
 
 #define DC_LOGGER_INIT(logger)
 
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
index 6c88c5f236a7..5642172e0df8 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
@@ -49,6 +49,7 @@
 #include "link_hwss.h"
 #include "dpcd_defs.h"
 #include "inc/dc_link_dp.h"
+#include "inc/link_dpcd.h"
 
 
 
diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
index 10070515d29d..cf1779588f96 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hwseq.c
@@ -46,6 +46,7 @@
 #include "dpcd_defs.h"
 #include "dce/dmub_outbox.h"
 #include "dc_link_dp.h"
+#include "inc/link_dpcd.h"
 
 #define DC_LOGGER_INIT(logger)
 
diff --git a/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c b/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c
index 51855a2624cf..4233955e3c47 100644
--- a/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c
+++ b/drivers/gpu/drm/amd/display/dc/hdcp/hdcp_msg.c
@@ -33,6 +33,7 @@
 #include "core_types.h"
 #include "dc_link_ddc.h"
 #include "link_hwss.h"
+#include "inc/link_dpcd.h"
 
 #define DC_LOGGER \
 	link->ctx->logger
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link_dpcd.h b/drivers/gpu/drm/amd/display/dc/inc/link_dpcd.h
new file mode 100644
index 000000000000..d4d52ef1b165
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/dc/inc/link_dpcd.h
@@ -0,0 +1,18 @@
+#ifndef __LINK_DPCD_H__
+#define __LINK_DPCD_H__
+#include <inc/core_status.h>
+#include <dc_link.h>
+#include <inc/link_hwss.h>
+
+enum dc_status core_link_read_dpcd(
+		struct dc_link *link,
+		uint32_t address,
+		uint8_t *data,
+		uint32_t size);
+
+enum dc_status core_link_write_dpcd(
+		struct dc_link *link,
+		uint32_t address,
+		const uint8_t *data,
+		uint32_t size);
+#endif
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h b/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h
index 33590a728fc5..fc1d289bb9fe 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link_hwss.h
@@ -26,20 +26,6 @@
 #ifndef __DC_LINK_HWSS_H__
 #define __DC_LINK_HWSS_H__
 
-#include "inc/core_status.h"
-
-enum dc_status core_link_read_dpcd(
-	struct dc_link *link,
-	uint32_t address,
-	uint8_t *data,
-	uint32_t size);
-
-enum dc_status core_link_write_dpcd(
-	struct dc_link *link,
-	uint32_t address,
-	const uint8_t *data,
-	uint32_t size);
-
 struct gpio *get_hpd_gpio(struct dc_bios *dcb,
 		struct graphics_object_id link_id,
 		struct gpio_service *gpio_service);
-- 
2.25.1



More information about the amd-gfx mailing list