[Why] Notice AUX request format of I2C-over-AUX with Write_Status_Update_Request flag set is incorrect. It should be address only request without length and data like: "SYNC->COM3:0 (= 0110)|0000-> 0000|0000-> 0|7-bit I2C address (the same as the last)-> STOP->".
[How] Refer to DP v2.1 Table 2-178, correct the Write_Status_Update_Request to be address only request.
Note that we might receive 0 returned by aux->transfer() when receive reply I2C_ACK|AUX_ACK of Write_Status_Update_Request transaction. Which indicating all data bytes get written. We should avoid to return 0 bytes get transferred under this case.
Fixes: 68ec2a2a2481 ("drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests") Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Mario Limonciello mario.limonciello@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: stable@vger.kernel.org Signed-off-by: Wayne Lin Wayne.Lin@amd.com --- drivers/gpu/drm/display/drm_dp_helper.c | 45 +++++++++++++++++++++---- 1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index 57828f2b7b5a..0c8cba7ed875 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -1857,6 +1857,12 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) I2C_FUNC_10BIT_ADDR; }
+static inline bool +drm_dp_i2c_msg_is_write_status_update(struct drm_dp_aux_msg *msg) +{ + return ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE_STATUS_UPDATE); +} + static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) { /* @@ -1965,6 +1971,7 @@ MODULE_PARM_DESC(dp_aux_i2c_speed_khz, static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) { unsigned int retry, defer_i2c; + struct drm_dp_aux_msg orig_msg = *msg; int ret; /* * DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device @@ -1976,6 +1983,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) int max_retries = max(7, drm_dp_i2c_retry_count(msg, dp_aux_i2c_speed_khz));
for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c); retry++) { + if (drm_dp_i2c_msg_is_write_status_update(msg)) { + /* Address only transaction */ + msg->buffer = NULL; + msg->size = 0; + } + ret = aux->transfer(aux, msg); if (ret < 0) { if (ret == -EBUSY) @@ -1993,7 +2006,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) else drm_dbg_kms(aux->drm_dev, "%s: transaction failed: %d\n", aux->name, ret); - return ret; + goto out; }
@@ -2008,7 +2021,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) case DP_AUX_NATIVE_REPLY_NACK: drm_dbg_kms(aux->drm_dev, "%s: native nack (result=%d, size=%zu)\n", aux->name, ret, msg->size); - return -EREMOTEIO; + ret = -EREMOTEIO; + goto out;
case DP_AUX_NATIVE_REPLY_DEFER: drm_dbg_kms(aux->drm_dev, "%s: native defer\n", aux->name); @@ -2027,24 +2041,35 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) default: drm_err(aux->drm_dev, "%s: invalid native reply %#04x\n", aux->name, msg->reply); - return -EREMOTEIO; + ret = -EREMOTEIO; + goto out; }
switch (msg->reply & DP_AUX_I2C_REPLY_MASK) { case DP_AUX_I2C_REPLY_ACK: + /* + * When I2C write firstly get defer and get ack after + * retries by wirte_status_update, we have to return + * all data bytes get transferred instead of 0. + */ + if (drm_dp_i2c_msg_is_write_status_update(msg) && ret == 0) + ret = orig_msg.size; + /* * Both native ACK and I2C ACK replies received. We * can assume the transfer was successful. */ if (ret != msg->size) drm_dp_i2c_msg_write_status_update(msg); - return ret; + + goto out;
case DP_AUX_I2C_REPLY_NACK: drm_dbg_kms(aux->drm_dev, "%s: I2C nack (result=%d, size=%zu)\n", aux->name, ret, msg->size); aux->i2c_nack_count++; - return -EREMOTEIO; + ret = -EREMOTEIO; + goto out;
case DP_AUX_I2C_REPLY_DEFER: drm_dbg_kms(aux->drm_dev, "%s: I2C defer\n", aux->name); @@ -2063,12 +2088,18 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) default: drm_err(aux->drm_dev, "%s: invalid I2C reply %#04x\n", aux->name, msg->reply); - return -EREMOTEIO; + ret = -EREMOTEIO; + goto out; } }
drm_dbg_kms(aux->drm_dev, "%s: Too many retries, giving up\n", aux->name); - return -EREMOTEIO; + ret = -EREMOTEIO; +out: + /* In case we change original msg by Write_Status_Update*/ + msg->buffer = orig_msg.buffer; + msg->size = orig_msg.size; + return ret; }
static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg,
On 4/27/2025 4:50 AM, Wayne Lin wrote:
[Why] Notice AUX request format of I2C-over-AUX with Write_Status_Update_Request flag set is incorrect. It should be address only request without length and data like: "SYNC->COM3:0 (= 0110)|0000-> 0000|0000-> 0|7-bit I2C address (the same as the last)-> STOP->".
[How] Refer to DP v2.1 Table 2-178, correct the Write_Status_Update_Request to be address only request.
Note that we might receive 0 returned by aux->transfer() when receive reply I2C_ACK|AUX_ACK of Write_Status_Update_Request transaction. Which indicating all data bytes get written. We should avoid to return 0 bytes get transferred under this case.
Fixes: 68ec2a2a2481 ("drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests") Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Mario Limonciello mario.limonciello@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: stable@vger.kernel.org Signed-off-by: Wayne Lin Wayne.Lin@amd.com
drivers/gpu/drm/display/drm_dp_helper.c | 45 +++++++++++++++++++++---- 1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index 57828f2b7b5a..0c8cba7ed875 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -1857,6 +1857,12 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) I2C_FUNC_10BIT_ADDR; } +static inline bool +drm_dp_i2c_msg_is_write_status_update(struct drm_dp_aux_msg *msg) +{
- return ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE_STATUS_UPDATE);
+}
- static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) { /*
@@ -1965,6 +1971,7 @@ MODULE_PARM_DESC(dp_aux_i2c_speed_khz, static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) { unsigned int retry, defer_i2c;
- struct drm_dp_aux_msg orig_msg = *msg; int ret; /*
- DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device
@@ -1976,6 +1983,12 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) int max_retries = max(7, drm_dp_i2c_retry_count(msg, dp_aux_i2c_speed_khz)); for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c); retry++) {
if (drm_dp_i2c_msg_is_write_status_update(msg)) {/* Address only transaction */msg->buffer = NULL;msg->size = 0;}- ret = aux->transfer(aux, msg); if (ret < 0) { if (ret == -EBUSY)
@@ -1993,7 +2006,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) else drm_dbg_kms(aux->drm_dev, "%s: transaction failed: %d\n", aux->name, ret);
return ret;
goto out;@@ -2008,7 +2021,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) case DP_AUX_NATIVE_REPLY_NACK: drm_dbg_kms(aux->drm_dev, "%s: native nack (result=%d, size=%zu)\n", aux->name, ret, msg->size);
return -EREMOTEIO;
ret = -EREMOTEIO;goto out;case DP_AUX_NATIVE_REPLY_DEFER: drm_dbg_kms(aux->drm_dev, "%s: native defer\n", aux->name); @@ -2027,24 +2041,35 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) default: drm_err(aux->drm_dev, "%s: invalid native reply %#04x\n", aux->name, msg->reply);
return -EREMOTEIO;
ret = -EREMOTEIO;goto out;switch (msg->reply & DP_AUX_I2C_REPLY_MASK) { case DP_AUX_I2C_REPLY_ACK:
/** When I2C write firstly get defer and get ack after* retries by wirte_status_update, we have to return* all data bytes get transferred instead of 0.*/if (drm_dp_i2c_msg_is_write_status_update(msg) && ret == 0)ret = orig_msg.size;/* * Both native ACK and I2C ACK replies received. We * can assume the transfer was successful. */ if (ret != msg->size) drm_dp_i2c_msg_write_status_update(msg);
return ret;
goto out;case DP_AUX_I2C_REPLY_NACK: drm_dbg_kms(aux->drm_dev, "%s: I2C nack (result=%d, size=%zu)\n", aux->name, ret, msg->size); aux->i2c_nack_count++;
return -EREMOTEIO;
ret = -EREMOTEIO;goto out;case DP_AUX_I2C_REPLY_DEFER: drm_dbg_kms(aux->drm_dev, "%s: I2C defer\n", aux->name); @@ -2063,12 +2088,18 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) default: drm_err(aux->drm_dev, "%s: invalid I2C reply %#04x\n", aux->name, msg->reply);
return -EREMOTEIO;
ret = -EREMOTEIO;goto out;drm_dbg_kms(aux->drm_dev, "%s: Too many retries, giving up\n", aux->name);
- return -EREMOTEIO;
- ret = -EREMOTEIO;
+out:
- /* In case we change original msg by Write_Status_Update*/
As there are multiple cases that jump to the "out" label, would it be clearer to use:
if (drm_dp_i2c_msg_is_write_status_update(msg)) { msg->buffer = orig_msg.buffer; msg->size = orig_msg.size; }
return ret;
- msg->buffer = orig_msg.buffer;
- msg->size = orig_msg.size;
- return ret; }
static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg,
[AMD Official Use Only - AMD Internal Distribution Only]
Thanks for the feedback! I'll adjust it and give v2 soon.
Thanks, Wayne
-----Original Message----- From: Limonciello, Mario Mario.Limonciello@amd.com Sent: Tuesday, May 6, 2025 4:22 AM To: Lin, Wayne Wayne.Lin@amd.com; dri-devel@lists.freedesktop.org Cc: ville.syrjala@linux.intel.com; jani.nikula@intel.com; Wentland, Harry Harry.Wentland@amd.com; stable@vger.kernel.org Subject: Re: [PATCH] drm/dp: Fix Write_Status_Update_Request AUX request format
On 4/27/2025 4:50 AM, Wayne Lin wrote:
[Why] Notice AUX request format of I2C-over-AUX with Write_Status_Update_Request flag set is incorrect. It should be address only request without length and data like: "SYNC->COM3:0 (= 0110)|0000-> 0000|0000-> 0|7-bit I2C address (the same as the last)-> STOP->".
[How] Refer to DP v2.1 Table 2-178, correct the Write_Status_Update_Request to be address only request.
Note that we might receive 0 returned by aux->transfer() when receive reply I2C_ACK|AUX_ACK of Write_Status_Update_Request transaction. Which indicating all data bytes get written. We should avoid to return 0 bytes get transferred under this case.
Fixes: 68ec2a2a2481 ("drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests") Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Jani Nikula jani.nikula@intel.com Cc: Mario Limonciello mario.limonciello@amd.com Cc: Harry Wentland harry.wentland@amd.com Cc: stable@vger.kernel.org Signed-off-by: Wayne Lin Wayne.Lin@amd.com
drivers/gpu/drm/display/drm_dp_helper.c | 45 +++++++++++++++++++++---- 1 file changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index 57828f2b7b5a..0c8cba7ed875 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -1857,6 +1857,12 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter
*adapter)
I2C_FUNC_10BIT_ADDR;}
+static inline bool +drm_dp_i2c_msg_is_write_status_update(struct drm_dp_aux_msg *msg) {
- return ((msg->request & ~DP_AUX_I2C_MOT) ==
+DP_AUX_I2C_WRITE_STATUS_UPDATE); }
- static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) { /*
@@ -1965,6 +1971,7 @@ MODULE_PARM_DESC(dp_aux_i2c_speed_khz, static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg
*msg)
{ unsigned int retry, defer_i2c;
- struct drm_dp_aux_msg orig_msg = *msg; int ret; /*
- DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source
device @@ -1976,6 +1983,12 @@ static int drm_dp_i2c_do_msg(struct
drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
int max_retries = max(7, drm_dp_i2c_retry_count(msg,dp_aux_i2c_speed_khz));
for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c);retry++) {
if (drm_dp_i2c_msg_is_write_status_update(msg)) {/* Address only transaction */msg->buffer = NULL;msg->size = 0;}ret = aux->transfer(aux, msg); if (ret < 0) { if (ret == -EBUSY)@@ -1993,7 +2006,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
else drm_dbg_kms(aux->drm_dev, "%s: transaction failed:%d\n",
aux->name, ret);
return ret;
goto out; }@@ -2008,7 +2021,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
case DP_AUX_NATIVE_REPLY_NACK: drm_dbg_kms(aux->drm_dev, "%s: native nack (result=%d,size=%zu)\n",
aux->name, ret, msg->size);
return -EREMOTEIO;
ret = -EREMOTEIO;goto out; case DP_AUX_NATIVE_REPLY_DEFER: drm_dbg_kms(aux->drm_dev, "%s: native defer\n", aux-name); @@ -2027,24 +2041,35 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
default: drm_err(aux->drm_dev, "%s: invalid native reply %#04x\n", aux->name, msg->reply);
return -EREMOTEIO;
ret = -EREMOTEIO;goto out; } switch (msg->reply & DP_AUX_I2C_REPLY_MASK) { case DP_AUX_I2C_REPLY_ACK:/** When I2C write firstly get defer and get ack after* retries by wirte_status_update, we have to return* all data bytes get transferred instead of 0.*/if (drm_dp_i2c_msg_is_write_status_update(msg) && ret ==
ret = orig_msg.size;/* * Both native ACK and I2C ACK replies received. We * can assume the transfer was successful. */ if (ret != msg->size) drm_dp_i2c_msg_write_status_update(msg);
return ret;
goto out; case DP_AUX_I2C_REPLY_NACK: drm_dbg_kms(aux->drm_dev, "%s: I2C nack (result=%d,size=%zu)\n",
aux->name, ret, msg->size); aux->i2c_nack_count++;
return -EREMOTEIO;
ret = -EREMOTEIO;goto out; case DP_AUX_I2C_REPLY_DEFER: drm_dbg_kms(aux->drm_dev, "%s: I2C defer\n", aux->name);@@
-2063,12 +2088,18 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
default: drm_err(aux->drm_dev, "%s: invalid I2C reply %#04x\n", aux->name, msg->reply);
return -EREMOTEIO;
ret = -EREMOTEIO;goto out; }}
drm_dbg_kms(aux->drm_dev, "%s: Too many retries, giving up\n", aux-
name);
- return -EREMOTEIO;
- ret = -EREMOTEIO;
+out:
- /* In case we change original msg by Write_Status_Update*/
As there are multiple cases that jump to the "out" label, would it be clearer to use:
if (drm_dp_i2c_msg_is_write_status_update(msg)) { msg->buffer = orig_msg.buffer; msg->size = orig_msg.size; }
return ret;
msg->buffer = orig_msg.buffer;
msg->size = orig_msg.size;
return ret; }
static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg,
On Sun, 27 Apr 2025, Wayne Lin Wayne.Lin@amd.com wrote:
/** When I2C write firstly get defer and get ack after* retries by wirte_status_update, we have to return* all data bytes get transferred instead of 0.*/
My brain gives me syntax and parse error here. ;)
BR, Jani.
[Public]
-----Original Message----- From: Jani Nikula jani.nikula@intel.com Sent: Thursday, May 8, 2025 4:19 PM To: Lin, Wayne Wayne.Lin@amd.com; dri-devel@lists.freedesktop.org Cc: ville.syrjala@linux.intel.com; Limonciello, Mario Mario.Limonciello@amd.com; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com; stable@vger.kernel.org Subject: Re: [PATCH] drm/dp: Fix Write_Status_Update_Request AUX request format
On Sun, 27 Apr 2025, Wayne Lin Wayne.Lin@amd.com wrote:
/** When I2C write firstly get defer and get ack after* retries by wirte_status_update, we have to return* all data bytes get transferred instead of 0.*/My brain gives me syntax and parse error here. ;)
Appreciate for the feedback, Jani. Could you elaborate more on your concerns please?
Since Write_Status_Update_Request is address only request. Data length is 0. When I2C write request completes, reply for Write_Status_Update_Request from DPRx will be ACK only (i.e. data length is 0).
Is your concern about returning 0 from aux->transfer? My thoughts is drm_dp_i2c_do_msg() is designed to handle I2C-Over-Aux reply data, and aux->transfer() is handling hw specific manipulation and return transferred bytes. For Write_Status_Update_Request request itself, nothing new to be transferred. I think drm_dp_i2c_do_msg() should be responsible for determining the correct transferred data bytes under this case. Or do you expect aux->transfer to memorize the data length of write request?
Thanks!
BR, Jani.
-- Jani Nikula, Intel
-- Wayne Lin
On Thu, 08 May 2025, "Lin, Wayne" Wayne.Lin@amd.com wrote:
[Public]
-----Original Message----- From: Jani Nikula jani.nikula@intel.com Sent: Thursday, May 8, 2025 4:19 PM To: Lin, Wayne Wayne.Lin@amd.com; dri-devel@lists.freedesktop.org Cc: ville.syrjala@linux.intel.com; Limonciello, Mario Mario.Limonciello@amd.com; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com; stable@vger.kernel.org Subject: Re: [PATCH] drm/dp: Fix Write_Status_Update_Request AUX request format
On Sun, 27 Apr 2025, Wayne Lin Wayne.Lin@amd.com wrote:
/** When I2C write firstly get defer and get ack after* retries by wirte_status_update, we have to return* all data bytes get transferred instead of 0.*/My brain gives me syntax and parse error here. ;)
Appreciate for the feedback, Jani. Could you elaborate more on your concerns please?
Since Write_Status_Update_Request is address only request. Data length is 0. When I2C write request completes, reply for Write_Status_Update_Request from DPRx will be ACK only (i.e. data length is 0).
Is your concern about returning 0 from aux->transfer? My thoughts is drm_dp_i2c_do_msg() is designed to handle I2C-Over-Aux reply data, and aux->transfer() is handling hw specific manipulation and return transferred bytes. For Write_Status_Update_Request request itself, nothing new to be transferred. I think drm_dp_i2c_do_msg() should be responsible for determining the correct transferred data bytes under this case. Or do you expect aux->transfer to memorize the data length of write request?
My concern is that I don't understand what the comment is trying to say.
"when i2c write firstly get defer" - what does it mean?
"wirte_status_update" - typo
"we have to" - why?
"return all data bytes get transferred" - what does it mean?
Thanks!
BR, Jani.
-- Jani Nikula, Intel
-- Wayne Lin
[Public]
-----Original Message----- From: Jani Nikula jani.nikula@intel.com Sent: Thursday, May 8, 2025 8:16 PM To: Lin, Wayne Wayne.Lin@amd.com; dri-devel@lists.freedesktop.org Cc: ville.syrjala@linux.intel.com; Limonciello, Mario Mario.Limonciello@amd.com; Wentland, Harry Harry.Wentland@amd.com; stable@vger.kernel.org Subject: RE: [PATCH] drm/dp: Fix Write_Status_Update_Request AUX request format
On Thu, 08 May 2025, "Lin, Wayne" Wayne.Lin@amd.com wrote:
[Public]
-----Original Message----- From: Jani Nikula jani.nikula@intel.com Sent: Thursday, May 8, 2025 4:19 PM To: Lin, Wayne Wayne.Lin@amd.com; dri-devel@lists.freedesktop.org Cc: ville.syrjala@linux.intel.com; Limonciello, Mario Mario.Limonciello@amd.com; Wentland, Harry Harry.Wentland@amd.com; Lin, Wayne Wayne.Lin@amd.com; stable@vger.kernel.org Subject: Re: [PATCH] drm/dp: Fix Write_Status_Update_Request AUX request format
On Sun, 27 Apr 2025, Wayne Lin Wayne.Lin@amd.com wrote:
/** When I2C write firstly get defer and get ack after* retries by wirte_status_update, we have to return* all data bytes get transferred instead of 0.*/My brain gives me syntax and parse error here. ;)
Appreciate for the feedback, Jani. Could you elaborate more on your concerns please?
Since Write_Status_Update_Request is address only request. Data length is 0. When I2C write request completes, reply for Write_Status_Update_Request from DPRx will be ACK only (i.e. data length is 0).
Is your concern about returning 0 from aux->transfer? My thoughts is drm_dp_i2c_do_msg() is designed to handle I2C-Over-Aux reply data, and aux->transfer() is handling hw specific manipulation and return transferred bytes. For Write_Status_Update_Request request itself, nothing new to be transferred. I think drm_dp_i2c_do_msg() should be responsible for determining the correct transferred data bytes under this case. Or do you expect aux->transfer to memorize the data length of write request?
My concern is that I don't understand what the comment is trying to say.
"when i2c write firstly get defer" - what does it mean?
"wirte_status_update" - typo
"we have to" - why?
"return all data bytes get transferred" - what does it mean?
I see. Thanks! We can't reply 0 since drm_dp_i2c_drain_msg() take 0 returned from drm_dp_i2c_do_msg() as error. And it actually completes transferring all data bytes.
I'll refine the comment. Thanks again.
Thanks!
BR, Jani.
-- Jani Nikula, Intel
-- Wayne Lin
-- Jani Nikula, Intel
-- Wayne Lin
linux-stable-mirror@lists.linaro.org
-
Jani Nikula -
Lin, Wayne -
Mario Limonciello -
Wayne Lin