drm/amd/display: Respect aux return values

[Why]
The new aux implementation was not up to spec. This caused us to fail DP
compliance as well as introduced serious delays during system resume.

[How]
Make dce_aux_transfer_raw return the operation result

Make dce_aux_transfer_with_retries delay with udelay instead
of msleep, and only on invalid reply.  Also fail on the second
invalid reply, third timeout, or first of any other error

Convert return values to drm error codes in amdgpu_dm

As the two aux transfer functions are now noticeably
different, change the names to better reflect their
functionality and document.

There was one last call to dc_link_aux_transfer that
should have retries, fix that
Signed-off-by: David Francis <David.Francis@amd.com>
Signed-off-by: Thomas Lim <Thomas.Lim@amd.com>
Reviewed-by: David Francis <David.Francis@amd.com>
Acked-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Acked-by: Eric Yang <eric.yang2@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c

@@ -84,6 +84,7 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
 {
 	ssize_t result = 0;
 	struct aux_payload payload;
+	enum aux_channel_operation_result operation_result;
 
 	if (WARN_ON(msg->size > 16))
 		return -E2BIG;
@@ -97,13 +98,27 @@ static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,
 	payload.mot = (msg->request & DP_AUX_I2C_MOT) != 0;
 	payload.defer_delay = 0;
 
-	result = dc_link_aux_transfer(TO_DM_AUX(aux)->ddc_service, &payload);
+	result = dc_link_aux_transfer_raw(TO_DM_AUX(aux)->ddc_service, &payload,
+				      &operation_result);
 
 	if (payload.write)
 		result = msg->size;
 
-	if (result < 0) /* DC doesn't know about kernel error codes */
+	if (result < 0)
+		switch (operation_result) {
+		case AUX_CHANNEL_OPERATION_SUCCEEDED:
+			break;
+		case AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON:
+		case AUX_CHANNEL_OPERATION_FAILED_REASON_UNKNOWN:
 			result = -EIO;
+			break;
+		case AUX_CHANNEL_OPERATION_FAILED_INVALID_REPLY:
+			result = -EBUSY;
+			break;
+		case AUX_CHANNEL_OPERATION_FAILED_TIMEOUT:
+			result = -ETIMEDOUT;
+			break;
+		}
 
 	return result;
 }

drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c

@@ -573,12 +573,28 @@ bool dal_ddc_service_query_ddc_data(
 	return ret;
 }
 
-int dc_link_aux_transfer(struct ddc_service *ddc,
-		struct aux_payload *payload)
+/* 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
+ * in the payload->reply and the result through
+ * *operation_result.  Returns the number of bytes transferred,
+ * or -1 on a failure.
+ */
+int dc_link_aux_transfer_raw(struct ddc_service *ddc,
+		struct aux_payload *payload,
+		enum aux_channel_operation_result *operation_result)
 {
-	return dce_aux_transfer(ddc, payload);
+	return dce_aux_transfer_raw(ddc, payload, operation_result);
 }
 
+/* dc_link_aux_transfer_with_retries() - Attempt to submit an
+ * aux payload, retrying on timeouts, defers, and busy states
+ * as outlined in the DP spec.  Returns true if the request
+ * was successful.
+ *
+ * Unless you want to implement your own retry semantics, this
+ * is probably the one you want.
+ */
 bool dc_link_aux_transfer_with_retries(struct ddc_service *ddc,
 		struct aux_payload *payload)
 {

drivers/gpu/drm/amd/display/dc/dce/dce_aux.c

@@ -438,12 +438,12 @@ static enum i2caux_transaction_action i2caux_action_from_payload(struct aux_payl
 	return I2CAUX_TRANSACTION_ACTION_DP_READ;
 }
 
-int dce_aux_transfer(struct ddc_service *ddc,
-		struct aux_payload *payload)
+int dce_aux_transfer_raw(struct ddc_service *ddc,
+		struct aux_payload *payload,
+		enum aux_channel_operation_result *operation_result)
 {
 	struct ddc *ddc_pin = ddc->ddc_pin;
 	struct dce_aux *aux_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;
@@ -470,28 +470,26 @@ int dce_aux_transfer(struct ddc_service *ddc,
 	aux_req.data = payload->data;
 
 	submit_channel_request(aux_engine, &aux_req);
-	operation_result = get_channel_status(aux_engine, &returned_bytes);
+	*operation_result = get_channel_status(aux_engine, &returned_bytes);
 
-	switch (operation_result) {
-	case AUX_CHANNEL_OPERATION_SUCCEEDED:
-		res = read_channel_reply(aux_engine, payload->length,
+	if (*operation_result == AUX_CHANNEL_OPERATION_SUCCEEDED) {
+		read_channel_reply(aux_engine, payload->length,
 					 payload->data, payload->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 = returned_bytes;
+	} else {
 		res = -1;
-		break;
 	}
+
 	release_engine(aux_engine);
 	return res;
 }
 
-#define AUX_RETRY_MAX 7
+#define AUX_MAX_RETRIES 7
+#define AUX_MAX_DEFER_RETRIES 7
+#define AUX_MAX_I2C_DEFER_RETRIES 7
+#define AUX_MAX_INVALID_REPLY_RETRIES 2
+#define AUX_MAX_TIMEOUT_RETRIES 3
 
 bool dce_aux_transfer_with_retries(struct ddc_service *ddc,
 		struct aux_payload *payload)
@@ -499,24 +497,83 @@ bool dce_aux_transfer_with_retries(struct ddc_service *ddc,
 	int i, ret = 0;
 	uint8_t reply;
 	bool payload_reply = true;
+	enum aux_channel_operation_result operation_result;
+	int aux_ack_retries = 0,
+		aux_defer_retries = 0,
+		aux_i2c_defer_retries = 0,
+		aux_timeout_retries = 0,
+		aux_invalid_reply_retries = 0;
 
 	if (!payload->reply) {
 		payload_reply = false;
 		payload->reply = &reply;
 	}
 
-	for (i = 0; i < AUX_RETRY_MAX; i++) {
-		ret = dce_aux_transfer(ddc, payload);
-
-		if (ret >= 0) {
-			if (*payload->reply == 0) {
-				if (!payload_reply)
-					payload->reply = NULL;
+	for (i = 0; i < AUX_MAX_RETRIES; i++) {
+		ret = dce_aux_transfer_raw(ddc, payload, &operation_result);
+		switch (operation_result) {
+		case AUX_CHANNEL_OPERATION_SUCCEEDED:
+			aux_timeout_retries = 0;
+			aux_invalid_reply_retries = 0;
+
+			switch (*payload->reply) {
+			case AUX_TRANSACTION_REPLY_AUX_ACK:
+				if (!payload->write && payload->length != ret) {
+					if (++aux_ack_retries >= AUX_MAX_RETRIES)
+						goto fail;
+					else
+						udelay(300);
+				} else
 					return true;
+			break;
+
+			case AUX_TRANSACTION_REPLY_AUX_DEFER:
+				if (++aux_defer_retries >= AUX_MAX_DEFER_RETRIES)
+					goto fail;
+				break;
+
+			case AUX_TRANSACTION_REPLY_I2C_DEFER:
+				aux_defer_retries = 0;
+				if (++aux_i2c_defer_retries >= AUX_MAX_I2C_DEFER_RETRIES)
+					goto fail;
+				break;
+
+			case AUX_TRANSACTION_REPLY_AUX_NACK:
+			case AUX_TRANSACTION_REPLY_HPD_DISCON:
+			default:
+				goto fail;
 			}
+			break;
+
+		case AUX_CHANNEL_OPERATION_FAILED_INVALID_REPLY:
+			if (++aux_invalid_reply_retries >= AUX_MAX_INVALID_REPLY_RETRIES)
+				goto fail;
+			else
+				udelay(400);
+			break;
+
+		case AUX_CHANNEL_OPERATION_FAILED_TIMEOUT:
+			if (++aux_timeout_retries >= AUX_MAX_TIMEOUT_RETRIES)
+				goto fail;
+			else {
+				/*
+				 * DP 1.4, 2.8.2:  AUX Transaction Response/Reply Timeouts
+				 * According to the DP spec there should be 3 retries total
+				 * with a 400us wait inbetween each. Hardware already waits
+				 * for 550us therefore no wait is required here.
+				 */
 			}
+			break;
 
-		udelay(1000);
+		case AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON:
+		case AUX_CHANNEL_OPERATION_FAILED_REASON_UNKNOWN:
+		default:
+			goto fail;
+		}
 	}
+
+fail:
+	if (!payload_reply)
+		payload->reply = NULL;
 	return false;
 }

drivers/gpu/drm/amd/display/dc/dce/dce_aux.h

@@ -123,8 +123,9 @@ bool dce110_aux_engine_acquire(
 	struct dce_aux *aux_engine,
 	struct ddc *ddc);
 
-int dce_aux_transfer(struct ddc_service *ddc,
-		struct aux_payload *cmd);
+int dce_aux_transfer_raw(struct ddc_service *ddc,
+		struct aux_payload *cmd,
+		enum aux_channel_operation_result *operation_result);
 
 bool dce_aux_transfer_with_retries(struct ddc_service *ddc,
 		struct aux_payload *cmd);

drivers/gpu/drm/amd/display/dc/inc/dc_link_ddc.h

@@ -95,8 +95,9 @@ bool dal_ddc_service_query_ddc_data(
 		uint8_t *read_buf,
 		uint32_t read_size);
 
-int dc_link_aux_transfer(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);
 
 bool dc_link_aux_transfer_with_retries(struct ddc_service *ddc,
 		struct aux_payload *payload);