[PATCH 20/54] drm/amd/display: Return aux replies directly to DRM

Harry Wentland harry.wentland at amd.com
Tue Jul 10 00:36:58 UTC 2018


Currently we still go through DC code that does error checking, retries,
etc. There's no need for that since DRM already does that for us. This
simplifies the code a bit and makes it easier to debug.

This also ensures we correctly tell DRM how many bytes have actually
been read, as we should. This allows DRM to correctly read the EDID on
the Chamelium DP port.

Signed-off-by: Harry Wentland <harry.wentland at amd.com>
Reviewed-by: Tony Cheng <Tony.Cheng at amd.com>
Acked-by: Harry Wentland <harry.wentland at amd.com>
---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  89 ++++++++-----
 .../gpu/drm/amd/display/dc/core/dc_link_ddc.c | 120 ++++++++----------
 .../gpu/drm/amd/display/dc/inc/dc_link_ddc.h  |  22 +---
 3 files changed, 117 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index db669c427dab..00b44d0eda32 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -80,52 +80,72 @@ static void log_dpcd(uint8_t type,
 static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
 				  struct drm_dp_aux_msg *msg)
 {
-	enum i2c_mot_mode mot = (msg->request & DP_AUX_I2C_MOT) ?
-		I2C_MOT_TRUE : I2C_MOT_FALSE;
-	enum ddc_result res;
+	ssize_t result = 0;
+	enum i2caux_transaction_action action;
+	enum aux_transaction_type type;
 
 	if (WARN_ON(msg->size > 16))
 		return -E2BIG;
 
 	switch (msg->request & ~DP_AUX_I2C_MOT) {
 	case DP_AUX_NATIVE_READ:
-		res = dal_ddc_service_read_dpcd_data(
-				TO_DM_AUX(aux)->ddc_service,
-				false,
-				I2C_MOT_UNDEF,
-				msg->address,
-				msg->buffer,
-				msg->size);
+		type = AUX_TRANSACTION_TYPE_DP;
+		action = I2CAUX_TRANSACTION_ACTION_DP_READ;
+
+		result = dc_link_aux_transfer(TO_DM_AUX(aux)->ddc_service,
+					      msg->address,
+					      &msg->reply,
+					      msg->buffer,
+					      msg->size,
+					      type,
+					      action);
 		break;
 	case DP_AUX_NATIVE_WRITE:
-		res = dal_ddc_service_write_dpcd_data(
-				TO_DM_AUX(aux)->ddc_service,
-				false,
-				I2C_MOT_UNDEF,
-				msg->address,
-				msg->buffer,
-				msg->size);
+		type = AUX_TRANSACTION_TYPE_DP;
+		action = I2CAUX_TRANSACTION_ACTION_DP_WRITE;
+
+		dc_link_aux_transfer(TO_DM_AUX(aux)->ddc_service,
+				     msg->address,
+				     &msg->reply,
+				     msg->buffer,
+				     msg->size,
+				     type,
+				     action);
+		result = msg->size;
 		break;
 	case DP_AUX_I2C_READ:
-		res = dal_ddc_service_read_dpcd_data(
-				TO_DM_AUX(aux)->ddc_service,
-				true,
-				mot,
-				msg->address,
-				msg->buffer,
-				msg->size);
+		type = AUX_TRANSACTION_TYPE_I2C;
+		if (msg->request & DP_AUX_I2C_MOT)
+			action = I2CAUX_TRANSACTION_ACTION_I2C_READ_MOT;
+		else
+			action = I2CAUX_TRANSACTION_ACTION_I2C_READ;
+
+		result = dc_link_aux_transfer(TO_DM_AUX(aux)->ddc_service,
+					      msg->address,
+					      &msg->reply,
+					      msg->buffer,
+					      msg->size,
+					      type,
+					      action);
 		break;
 	case DP_AUX_I2C_WRITE:
-		res = dal_ddc_service_write_dpcd_data(
-				TO_DM_AUX(aux)->ddc_service,
-				true,
-				mot,
-				msg->address,
-				msg->buffer,
-				msg->size);
+		type = AUX_TRANSACTION_TYPE_I2C;
+		if (msg->request & DP_AUX_I2C_MOT)
+			action = I2CAUX_TRANSACTION_ACTION_I2C_WRITE_MOT;
+		else
+			action = I2CAUX_TRANSACTION_ACTION_I2C_WRITE;
+
+		dc_link_aux_transfer(TO_DM_AUX(aux)->ddc_service,
+				     msg->address,
+				     &msg->reply,
+				     msg->buffer,
+				     msg->size,
+				     type,
+				     action);
+		result = msg->size;
 		break;
 	default:
-		return 0;
+		return -EINVAL;
 	}
 
 #ifdef TRACE_DPCD
@@ -136,7 +156,10 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
 		 r == DDC_RESULT_SUCESSFULL);
 #endif
 
-	return msg->size;
+	if (result < 0) /* DC doesn't know about kernel error codes */
+		result = -EIO;
+
+	return result;
 }
 
 static enum drm_connector_status
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
index d5294798b0a5..d108ccfc5cf9 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
@@ -33,6 +33,10 @@
 #include "include/vector.h"
 #include "core_types.h"
 #include "dc_link_ddc.h"
+#include "i2caux/engine.h"
+#include "i2caux/i2c_engine.h"
+#include "i2caux/aux_engine.h"
+#include "i2caux/i2caux.h"
 
 #define AUX_POWER_UP_WA_DELAY 500
 #define I2C_OVER_AUX_DEFER_WA_DELAY 70
@@ -629,78 +633,62 @@ bool dal_ddc_service_query_ddc_data(
 	return ret;
 }
 
-enum ddc_result dal_ddc_service_read_dpcd_data(
-	struct ddc_service *ddc,
-	bool i2c,
-	enum i2c_mot_mode mot,
-	uint32_t address,
-	uint8_t *data,
-	uint32_t len)
+int dc_link_aux_transfer(struct ddc_service *ddc,
+			     unsigned int address,
+			     uint8_t *reply,
+			     void *buffer,
+			     unsigned int size,
+			     enum aux_transaction_type type,
+			     enum i2caux_transaction_action action)
 {
-	struct aux_payload read_payload = {
-		.i2c_over_aux = i2c,
-		.write = false,
-		.address = address,
-		.length = len,
-		.data = data,
-	};
-	struct aux_command command = {
-		.payloads = &read_payload,
-		.number_of_payloads = 1,
-		.defer_delay = 0,
-		.max_defer_write_retry = 0,
-		.mot = mot
-	};
-
-	if (len > DEFAULT_AUX_MAX_DATA_SIZE) {
-		BREAK_TO_DEBUGGER();
-		return DDC_RESULT_FAILED_INVALID_OPERATION;
-	}
+	struct i2caux *i2caux = ddc->ctx->i2caux;
+	struct ddc *ddc_pin = ddc->ddc_pin;
+	struct aux_engine *engine;
+	enum aux_channel_operation_result operation_result;
+	struct aux_request_transaction_data aux_req;
+	struct aux_reply_transaction_data aux_rep;
+	uint8_t returned_bytes = 0;
+	int res = -1;
+	uint32_t status;
 
-	if (dal_i2caux_submit_aux_command(
-		ddc->ctx->i2caux,
-		ddc->ddc_pin,
-		&command))
-		return DDC_RESULT_SUCESSFULL;
+	memset(&aux_req, 0, sizeof(aux_req));
+	memset(&aux_rep, 0, sizeof(aux_rep));
 
-	return DDC_RESULT_FAILED_OPERATION;
-}
+	engine = i2caux->funcs->acquire_aux_engine(i2caux, ddc_pin);
 
-enum ddc_result dal_ddc_service_write_dpcd_data(
-	struct ddc_service *ddc,
-	bool i2c,
-	enum i2c_mot_mode mot,
-	uint32_t address,
-	const uint8_t *data,
-	uint32_t len)
-{
-	struct aux_payload write_payload = {
-		.i2c_over_aux = i2c,
-		.write = true,
-		.address = address,
-		.length = len,
-		.data = (uint8_t *)data,
-	};
-	struct aux_command command = {
-		.payloads = &write_payload,
-		.number_of_payloads = 1,
-		.defer_delay = 0,
-		.max_defer_write_retry = 0,
-		.mot = mot
-	};
-
-	if (len > DEFAULT_AUX_MAX_DATA_SIZE) {
-		BREAK_TO_DEBUGGER();
-		return DDC_RESULT_FAILED_INVALID_OPERATION;
-	}
+	aux_req.type = type;
+	aux_req.action = action;
+
+	aux_req.address = address;
+	aux_req.delay = 0;
+	aux_req.length = size;
+	aux_req.data = buffer;
+
+	engine->funcs->submit_channel_request(engine, &aux_req);
+	operation_result = engine->funcs->get_channel_status(engine, &returned_bytes);
 
-	if (dal_i2caux_submit_aux_command(
-		ddc->ctx->i2caux,
-		ddc->ddc_pin,
-		&command))
-		return DDC_RESULT_SUCESSFULL;
+	switch (operation_result) {
+	case AUX_CHANNEL_OPERATION_SUCCEEDED:
+		res = returned_bytes;
+
+		if (res <= size && res > 0)
+			res = engine->funcs->read_channel_reply(engine, size,
+								buffer, reply,
+								&status);
+
+		break;
+	case AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON:
+		res = 0;
+		break;
+	case AUX_CHANNEL_OPERATION_FAILED_REASON_UNKNOWN:
+	case AUX_CHANNEL_OPERATION_FAILED_INVALID_REPLY:
+	case AUX_CHANNEL_OPERATION_FAILED_TIMEOUT:
+		res = -1;
+		break;
+	}
 
-	return DDC_RESULT_FAILED_OPERATION;
+	i2caux->funcs->release_engine(i2caux, &engine->base);
+	return res;
 }
 
 /*test only function*/
diff --git a/drivers/gpu/drm/amd/display/dc/inc/dc_link_ddc.h b/drivers/gpu/drm/amd/display/dc/inc/dc_link_ddc.h
index 0bf73b742f1f..538b83303b86 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/dc_link_ddc.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/dc_link_ddc.h
@@ -102,21 +102,13 @@ bool dal_ddc_service_query_ddc_data(
 		uint8_t *read_buf,
 		uint32_t read_size);
 
-enum ddc_result dal_ddc_service_read_dpcd_data(
-		struct ddc_service *ddc,
-		bool i2c,
-		enum i2c_mot_mode mot,
-		uint32_t address,
-		uint8_t *data,
-		uint32_t len);
-
-enum ddc_result dal_ddc_service_write_dpcd_data(
-		struct ddc_service *ddc,
-		bool i2c,
-		enum i2c_mot_mode mot,
-		uint32_t address,
-		const uint8_t *data,
-		uint32_t len);
+int dc_link_aux_transfer(struct ddc_service *ddc,
+			     unsigned int address,
+			     uint8_t *reply,
+			     void *buffer,
+			     unsigned int size,
+			     enum aux_transaction_type type,
+			     enum i2caux_transaction_action action);
 
 void dal_ddc_service_write_scdc_data(
 		struct ddc_service *ddc_service,
-- 
2.17.1



More information about the amd-gfx mailing list