[PATCH 02/17] drm/amd/display: refine i2c over aux

Bhawanpreet Lakha Bhawanpreet.Lakha at amd.com
Wed Aug 28 21:03:39 UTC 2019


From: Lewis Huang <Lewis.Huang at amd.com>

[Why]
When user mode use i2c over aux through ADL or DDI, the function
dal_ddc_service_query_ddc_data will be called. There are two issues.

1. When read/write length > 16byte, current always return false because
the DEFAULT_AUX_MAX_DATA_SIZE != length.
2. When usermode only need to read data through i2c, driver will write
mot = true at the same address and cause i2c sink confused. Therefore
the following read command will get garbage.

[How]
1. Add function dal_dcc_submit_aux_command to handle length > 16 byte.
2. Check read size and write size when query ddc data.

Signed-off-by: Lewis Huang <Lewis.Huang at amd.com>
Reviewed-by: Charlene Liu <Charlene.Liu at amd.com>
Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha at amd.com>
---
 .../gpu/drm/amd/display/dc/core/dc_link_ddc.c | 87 +++++++++++++------
 .../gpu/drm/amd/display/dc/inc/dc_link_ddc.h  |  3 +
 2 files changed, 63 insertions(+), 27 deletions(-)

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 7fd2d1358f1b..f70137d67c82 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
@@ -494,7 +494,7 @@ bool dal_ddc_service_query_ddc_data(
 	uint8_t *read_buf,
 	uint32_t read_size)
 {
-	bool ret;
+	bool ret = false;
 	uint32_t payload_size =
 		dal_ddc_service_is_in_aux_transaction_mode(ddc) ?
 			DEFAULT_AUX_MAX_DATA_SIZE : EDID_SEGMENT_SIZE;
@@ -513,34 +513,32 @@ bool dal_ddc_service_query_ddc_data(
 	/*TODO: len of payload data for i2c and aux is uint8!!!!,
 	 *  but we want to read 256 over i2c!!!!*/
 	if (dal_ddc_service_is_in_aux_transaction_mode(ddc)) {
-		struct aux_payload write_payload = {
-			.i2c_over_aux = true,
-			.write = true,
-			.mot = true,
-			.address = address,
-			.length = write_size,
-			.data = write_buf,
-			.reply = NULL,
-			.defer_delay = get_defer_delay(ddc),
-		};
-
-		struct aux_payload read_payload = {
-			.i2c_over_aux = true,
-			.write = false,
-			.mot = false,
-			.address = address,
-			.length = read_size,
-			.data = read_buf,
-			.reply = NULL,
-			.defer_delay = get_defer_delay(ddc),
-		};
-
-		ret = dc_link_aux_transfer_with_retries(ddc, &write_payload);
+		struct aux_payload payload;
+		bool read_available = true;
+
+		payload.i2c_over_aux = true;
+		payload.address = address;
+		payload.reply = NULL;
+		payload.defer_delay = get_defer_delay(ddc);
+
+		if (write_size != 0) {
+			payload.write = true;
+			payload.mot = true;
+			payload.length = write_size;
+			payload.data = write_buf;
+
+			ret = dal_ddc_submit_aux_command(ddc, &payload);
+			read_available = ret;
+		}
 
-		if (!ret)
-			return false;
+		if (read_size != 0 && read_available) {
+			payload.write = false;
+			payload.mot = false;
+			payload.length = read_size;
+			payload.data = read_buf;
 
-		ret = dc_link_aux_transfer_with_retries(ddc, &read_payload);
+			ret = dal_ddc_submit_aux_command(ddc, &payload);
+		}
 	} else {
 		struct i2c_payloads *payloads =
 			dal_ddc_i2c_payloads_create(ddc->ctx, payloads_num);
@@ -571,6 +569,41 @@ bool dal_ddc_service_query_ddc_data(
 	return ret;
 }
 
+bool dal_ddc_submit_aux_command(struct ddc_service *ddc,
+		struct aux_payload *payload)
+{
+	uint8_t retrieved = 0;
+	bool ret = 0;
+
+	if (!ddc)
+		return false;
+
+	if (!payload)
+		return false;
+
+	do {
+		struct aux_payload current_payload;
+		bool is_end_of_payload = (retrieved + DEFAULT_AUX_MAX_DATA_SIZE) >
+			payload->length ? true : false;
+
+		current_payload.address = payload->address;
+		current_payload.data = &payload->data[retrieved];
+		current_payload.defer_delay = payload->defer_delay;
+		current_payload.i2c_over_aux = payload->i2c_over_aux;
+		current_payload.length = is_end_of_payload ?
+			payload->length - retrieved : DEFAULT_AUX_MAX_DATA_SIZE;
+		current_payload.mot = payload->mot ? payload->mot : !is_end_of_payload;
+		current_payload.reply = payload->reply;
+		current_payload.write = payload->write;
+
+		ret = dc_link_aux_transfer_with_retries(ddc, &current_payload);
+
+		retrieved += current_payload.length;
+	} while (retrieved < payload->length && ret == true);
+
+	return ret;
+}
+
 /* dc_link_aux_transfer_raw() - Attempt to transfer
  * the given aux payload.  This function does not perform
  * retries or handle error states.  The reply is returned
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 b1fab251c09b..7d35d03a2d43 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
@@ -95,6 +95,9 @@ bool dal_ddc_service_query_ddc_data(
 		uint8_t *read_buf,
 		uint32_t read_size);
 
+bool dal_ddc_submit_aux_command(struct ddc_service *ddc,
+		struct aux_payload *payload);
+
 int dc_link_aux_transfer_raw(struct ddc_service *ddc,
 		struct aux_payload *payload,
 		enum aux_channel_operation_result *operation_result);
-- 
2.17.1



More information about the amd-gfx mailing list