On Thu, Apr 24, 2025 at 11:07:33AM +0800, Wayne Lin wrote:

[Why] Notice few problems under I2C-write-over-Aux with Write_Status_Update_Request flag set cases:

• I2C-write-over-Aux request with Write_Status_Update_Request flag set won't get sent upon the reply of I2C_ACK|AUX_ACK followed by “M” Value. Now just set the flag but won't send out

drm_dp_i2c_drain_msg() should keep going until an error or the full message got transferred.

• The I2C-over-Aux request command with Write_Status_Update_Request flag set is incorrect. Should be "SYNC->COM3:0 (= 0110)|0000-> 0000|0000-> 0|7-bit I2C address (the same as the last)-> STOP->". Address only transaction without length and data.

This looks like a real issue.

• Upon I2C_DEFER|AUX_ACK Reply for I2C-read-over-Aux, soure should repeat the identical I2C-read-over-AUX transaction with the same LEN value. Not with Write_Status_Update_Request set.

drm_dp_i2c_msg_write_status_update() already checks the request type.

[How] Refer to DP v2.1: 2.11.7.1.5.3 & 2.11.7.1.5.4

• Clean aux msg buffer and size when constructing write status update request.

• Send out Write_Status_Update_Request upon reply of I2C_ACK|AUX_ACK followed by “M”

• Send Write_Status_Update_Request upon I2C_DEFER|AUX_ACK reply only when the request is I2C-write-over-Aux.

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 | 27 +++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c index 6ee51003de3c..28f0708c3b27 100644 --- a/drivers/gpu/drm/display/drm_dp_helper.c +++ b/drivers/gpu/drm/display/drm_dp_helper.c @@ -1631,6 +1631,12 @@ static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) msg->request &= DP_AUX_I2C_MOT; msg->request |= DP_AUX_I2C_WRITE_STATUS_UPDATE; }

• /*

• * Address only transaction
  

• */
  

• msg->buffer = NULL;
• msg->size = 0;

} #define AUX_PRECHARGE_LEN 10 /* 10 to 16 */ @@ -1797,10 +1803,22 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) case DP_AUX_I2C_REPLY_ACK: /* * Both native ACK and I2C ACK replies received. We

• 	 * can assume the transfer was successful.
  

• 	 * can't assume the transfer was completed. Both I2C
  

• 	 * WRITE/READ request may get I2C ack reply with partially
  

• 	 * completion. We have to continue to poll for the
  

• 	 * completion of the request.
   */
  

• 	if (ret != msg->size)
  

• 		drm_dp_i2c_msg_write_status_update(msg);
  

• 	if (ret != msg->size) {
  

• 		drm_dbg_kms(aux->drm_dev,
  

• 			    "%s: I2C partially ack (result=%d, size=%zu)\n",
  

• 			    aux->name, ret, msg->size);
  

• 		if (!(msg->request & DP_AUX_I2C_READ)) {
  

• 			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
  

• 			drm_dp_i2c_msg_write_status_update(msg);
  

• 		}
  

• 		continue;
  

• 	}
  return ret;
  

case DP_AUX_I2C_REPLY_NACK: @@ -1819,7 +1837,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) if (defer_i2c < 7) defer_i2c++; usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);

• 	drm_dp_i2c_msg_write_status_update(msg);
  

• 	if (!(msg->request & DP_AUX_I2C_READ))
  

• 		drm_dp_i2c_msg_write_status_update(msg);
  

continue; -- 2.43.0