drm/amd/display: Break out function to simply read aux reply

DRM's DP helpers take care of dealing with the error code for us. In
order not to step on each other's toes we'll need to be able to simply
read auch channel replies without further logic based on return values.
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Sun peng Li <Sunpeng.Li@amd.com>
Acked-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

drivers/gpu/drm/amd/display/dc/i2caux/aux_engine.h

@@ -44,6 +44,12 @@ struct aux_engine_funcs {
 	void (*process_channel_reply)(
 		struct aux_engine *engine,
 		struct aux_reply_transaction_data *reply);
+	int (*read_channel_reply)(
+		struct aux_engine *engine,
+		uint32_t size,
+		uint8_t *buffer,
+		uint8_t *reply_result,
+		uint32_t *sw_status);
 	enum aux_channel_operation_result (*get_channel_status)(
 		struct aux_engine *engine,
 		uint8_t *returned_bytes);

drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c

@@ -275,61 +275,92 @@ static void submit_channel_request(
 	REG_UPDATE(AUX_SW_CONTROL, AUX_SW_GO, 1);
 }
 
-static void process_channel_reply(
-	struct aux_engine *engine,
-	struct aux_reply_transaction_data *reply)
+static int read_channel_reply(struct aux_engine *engine, uint32_t size,
+			      uint8_t *buffer, uint8_t *reply_result,
+			      uint32_t *sw_status)
 {
 	struct aux_engine_dce110 *aux110 = FROM_AUX_ENGINE(engine);
-
-	/* Need to do a read to get the number of bytes to process
-	 * Alternatively, this information can be passed -
-	 * but that causes coupling which isn't good either. */
-
 	uint32_t bytes_replied;
-	uint32_t value;
+	uint32_t reply_result_32;
 
-	value = REG_GET(AUX_SW_STATUS,
-			AUX_SW_REPLY_BYTE_COUNT, &bytes_replied);
+	*sw_status = REG_GET(AUX_SW_STATUS, AUX_SW_REPLY_BYTE_COUNT,
+			     &bytes_replied);
 
-	/* in case HPD is LOW, exit AUX transaction */
-	if ((value & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
-		reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;
-		return;
-	}
+	/* In case HPD is LOW, exit AUX transaction */
+	if ((*sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK))
+		return -1;
 
-	if (bytes_replied) {
-		uint32_t reply_result;
+	/* Need at least the status byte */
+	if (!bytes_replied)
+		return -1;
 
 	REG_UPDATE_1BY1_3(AUX_SW_DATA,
 			  AUX_SW_INDEX, 0,
 			  AUX_SW_AUTOINCREMENT_DISABLE, 1,
 			  AUX_SW_DATA_RW, 1);
 
-		REG_GET(AUX_SW_DATA,
-				AUX_SW_DATA, &reply_result);
-
-		reply_result = reply_result >> 4;
+	REG_GET(AUX_SW_DATA, AUX_SW_DATA, &reply_result_32);
+	*reply_result = (uint8_t)reply_result_32;
 
-		switch (reply_result) {
-		case 0: /* ACK */ {
+	if (reply_result_32 >> 4 == 0) { /* ACK */
 		uint32_t i = 0;
 
-			/* first byte was already used
-			 * to get the command status */
+		/* First byte was already used to get the command status */
 		--bytes_replied;
 
+		/* Do not overflow buffer */
+		if (bytes_replied > size)
+			return -1;
+
 		while (i < bytes_replied) {
 			uint32_t aux_sw_data_val;
 
-				REG_GET(AUX_SW_DATA,
-						AUX_SW_DATA, &aux_sw_data_val);
-
-				reply->data[i] = aux_sw_data_val;
+			REG_GET(AUX_SW_DATA, AUX_SW_DATA, &aux_sw_data_val);
+			buffer[i] = aux_sw_data_val;
 			++i;
 		}
 
-			reply->status = AUX_TRANSACTION_REPLY_AUX_ACK;
+		return i;
+	}
+
+	return 0;
+}
+
+static void process_channel_reply(
+	struct aux_engine *engine,
+	struct aux_reply_transaction_data *reply)
+{
+	int bytes_replied;
+	uint8_t reply_result;
+	uint32_t sw_status;
+
+	bytes_replied = read_channel_reply(engine, reply->length, reply->data,
+					   &reply_result, &sw_status);
+
+	/* in case HPD is LOW, exit AUX transaction */
+	if ((sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
+		reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
+		return;
+	}
+
+	if (bytes_replied < 0) {
+		/* Need to handle an error case...
+		 * Hopefully, upper layer function won't call this function if
+		 * the number of bytes in the reply was 0, because there was
+		 * surely an error that was asserted that should have been
+		 * handled for hot plug case, this could happens
+		 */
+		if (!(sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
+			reply->status = AUX_TRANSACTION_REPLY_INVALID;
+			ASSERT_CRITICAL(false);
+			return;
 		}
+	} else {
+		reply_result = reply_result >> 4;
+
+		switch (reply_result) {
+		case 0: /* ACK */
+			reply->status = AUX_TRANSACTION_REPLY_AUX_ACK;
 		break;
 		case 1: /* NACK */
 			reply->status = AUX_TRANSACTION_REPLY_AUX_NACK;
@@ -346,17 +377,6 @@ static void process_channel_reply(
 		default:
 			reply->status = AUX_TRANSACTION_REPLY_INVALID;
 		}
-	} else {
-		/* Need to handle an error case...
-		 * hopefully, upper layer function won't call this function
-		 * if the number of bytes in the reply was 0
-		 * because there was surely an error that was asserted
-		 * that should have been handled
-		 * for hot plug case, this could happens*/
-		if (!(value & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK)) {
-			reply->status = AUX_TRANSACTION_REPLY_INVALID;
-			ASSERT_CRITICAL(false);
-		}
 	}
 }
 
@@ -427,6 +447,7 @@ static const struct aux_engine_funcs aux_engine_funcs = {
 	.acquire_engine = acquire_engine,
 	.submit_channel_request = submit_channel_request,
 	.process_channel_reply = process_channel_reply,
+	.read_channel_reply = read_channel_reply,
 	.get_channel_status = get_channel_status,
 	.is_engine_available = is_engine_available,
 };