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>

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 c4ea3a91f17aa44910e13a92932727b3f4388f96..6e205ee36ac3b02aeb0e1aa8a88ae2e5ef18adc1 100644 (file)
--- 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
@@ -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 */
-               result = -EIO;
+       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;
 }

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 b7ee63cd8dc7df4296b98d98c4f5dbbfd255a14a..f02092a0dc76a176efb4e68b0569857dc18d69e1 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
+++ b/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)
 {

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
index 2f50be33ab154882ae77aa8c47b33b345547977a..65b290d80143014ebd3b66f04abc28bf4bbe1618 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
+++ b/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);
-
-       switch (operation_result) {
-       case AUX_CHANNEL_OPERATION_SUCCEEDED:
-               res = 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:
+       *operation_result = get_channel_status(aux_engine, &returned_bytes);
+
+       if (*operation_result == AUX_CHANNEL_OPERATION_SUCCEEDED) {
+               read_channel_reply(aux_engine, payload->length,
+                                        payload->data, payload->reply,
+                                        &status);
+               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;
-                               return true;
+       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;
 }

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
index d27f22c05e4b5abd0085fb7252fccba5296f6831..aab5f0c34584c1587f344f1ee2bff3a9a98c2020 100644 (file)
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.h
+++ b/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);

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 16fd4dc6c4dd73377fbe0c239be135475919f726..b1fab251c09bede95ca2783110d94c056e6cfb11 100644 (file)
--- 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,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);