From: Harry Wentland <harry.wentland@amd.com>
Date: Tue, 8 May 2018 16:28:31 -0400
Subject: drm/amd/display: Break out function to simply read aux reply
Git-commit: 899e2aaddbfa0ff96fbaf31f0d9e91427e87dd88
Patch-mainline: v4.19-rc1
References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166
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>
Acked-by: Petr Tesarik <ptesarik@suse.com>
---
drivers/gpu/drm/amd/display/dc/i2caux/aux_engine.h | 6
drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c | 123 +++++-----
2 files changed, 78 insertions(+), 51 deletions(-)
--- a/drivers/gpu/drm/amd/display/dc/i2caux/aux_engine.h
+++ b/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);
--- a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
+++ b/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);
+ uint32_t bytes_replied;
+ uint32_t reply_result_32;
- /* 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. */
+ *sw_status = REG_GET(AUX_SW_STATUS, AUX_SW_REPLY_BYTE_COUNT,
+ &bytes_replied);
- uint32_t bytes_replied;
- uint32_t value;
+ /* In case HPD is LOW, exit AUX transaction */
+ if ((*sw_status & AUX_SW_STATUS__AUX_SW_HPD_DISCON_MASK))
+ return -1;
- value = REG_GET(AUX_SW_STATUS,
- AUX_SW_REPLY_BYTE_COUNT, &bytes_replied);
+ /* Need at least the status byte */
+ if (!bytes_replied)
+ return -1;
- /* 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;
+ 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_32);
+ *reply_result = (uint8_t)reply_result_32;
+
+ if (reply_result_32 >> 4 == 0) { /* ACK */
+ uint32_t i = 0;
+
+ /* 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);
+ buffer[i] = aux_sw_data_val;
+ ++i;
+ }
+
+ return i;
}
- if (bytes_replied) {
- uint32_t reply_result;
+ return 0;
+}
- REG_UPDATE_1BY1_3(AUX_SW_DATA,
- AUX_SW_INDEX, 0,
- AUX_SW_AUTOINCREMENT_DISABLE, 1,
- AUX_SW_DATA_RW, 1);
+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);
- REG_GET(AUX_SW_DATA,
- AUX_SW_DATA, &reply_result);
+ /* 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 */ {
- uint32_t i = 0;
-
- /* first byte was already used
- * to get the command status */
- --bytes_replied;
-
- 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;
- ++i;
- }
-
+ 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
.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,
};