Re: [PATCH v2 3/3] drm/msm/dp: continue link training downgrade when cable is still plugged

[Naidu Nama <naidu.nama@oss.qualcomm.com>]

On 11-11-2026 19:38, Dmitry Baryshkov wrote:
> On Wed, Nov 11, 2026 at 04:13:55PM +0530, naidu nama wrote:
>> From: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
>>
>> During a Type-C reconnect the AUX channel may report link-disconnected
>> transiently while the physical cable is still present. The link training
>> retry loop in msm_dp_ctrl_on_link() was aborting immediately on any
>> msm_dp_aux_is_link_connected() failure, preventing the rate/lane downgrade
>> path from running.
>>
>> When the display is known to be plugged (msm_dp_ctrl->plugged), an AUX
>> link-disconnected status is likely a transient glitch rather than a true
>> unplug. Allow the downgrade loop to continue in that case by requiring both
>> conditions before breaking out of the retry loop: AUX reports disconnected
>> and the display is not plugged.
> Is there a reason to assume that DP is not plugged at this point? I
> guess not. But the other issue is: what if the DP gets unplugged in the
> middle of link training.


dp is plugged at this point, the issue we have is the 
msm_dp_aux_is_link_connected is giving the return status as 0 in 
sometimes , due to this the link training
retry loop in msm_dp_ctrl_on_link() was aborting immediately. To avoid 
this we are checking the plugged status as well.

Similar check  was made at dp_dispaly.c in function msm_dp_bridge_detect.

Here as well we are getting the ret as 0 ,as there is one more check 
plugged which is true , which is avoiding to go to if statement and 
continue further .
ret = msm_dp_aux_is_link_connected(priv->aux);
         DRM_DEBUG_DP("aux link status: %x\n", ret);
         if (!priv->plugged && !ret) {
                 DRM_DEBUG_DP("aux not connected\n");
                 priv->plugged = false;
                 goto end;
         }

if dp gets unplugged in the middle of link training,but the call started 
with plugged=true, that particular check won’t stop training 
immediately. It may continue retrying or fail later when AUX/link 
operations fail.
>
>> Signed-off-by: Saurabh Anand <saurabh.anand@oss.qualcomm.com>
> SoB


Sure, I will update in v3.
>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 15 ++++++++-------
>>   drivers/gpu/drm/msm/dp/dp_ctrl.h    |  4 ++--
>>   drivers/gpu/drm/msm/dp/dp_display.c |  7 +++----
>>   3 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index 6b9d9a76ba05..f8a6bd7b0d10 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -2274,7 +2274,8 @@ void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl, enum msm_dp_stre
>>   }
>>   
>>   static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl,
>> -						struct msm_dp_panel *panel)
>> +						struct msm_dp_panel *panel,
>> +						bool plugged)
> Can it ever be false here with your patch in place?


it will be always true, this is set to true before the link training.
>
>>   {
>>   	int ret;
>>   	unsigned long pixel_rate;
>> @@ -2293,7 +2294,7 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
>>   	msm_dp_ctrl_off_pixel_clk(&ctrl->msm_dp_ctrl, panel->stream_id);
>>   	msm_dp_ctrl_off_link(&ctrl->msm_dp_ctrl, panel);
>>   
>> -	ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl, panel);
>> +	ret = msm_dp_ctrl_on_link(&ctrl->msm_dp_ctrl, panel, plugged);
>>   	if (ret) {
>>   		DRM_ERROR("failed to enable DP link controller\n");
>>   		return ret;
>> @@ -2310,7 +2311,7 @@ static int msm_dp_ctrl_process_phy_test_request(struct msm_dp_ctrl_private *ctrl
>>   }
>>   
>>   void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
>> -				     struct msm_dp_panel *panel)
>> +				     struct msm_dp_panel *panel, bool plugged)
>>   {
>>   	struct msm_dp_ctrl_private *ctrl;
>>   	u32 sink_request = 0x0;
>> @@ -2325,7 +2326,7 @@ void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
>>   
>>   	if (sink_request & DP_TEST_LINK_PHY_TEST_PATTERN) {
>>   		drm_dbg_dp(ctrl->drm_dev, "PHY_TEST_PATTERN request\n");
>> -		if (msm_dp_ctrl_process_phy_test_request(ctrl, panel)) {
>> +		if (msm_dp_ctrl_process_phy_test_request(ctrl, panel, plugged)) {
>>   			DRM_ERROR("process phy_test_req failed\n");
>>   			return;
>>   		}
>> @@ -2377,7 +2378,7 @@ static bool msm_dp_ctrl_channel_eq_ok(struct msm_dp_ctrl_private *ctrl)
>>   }
>>   
>>   int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
>> -			struct msm_dp_panel *panel)
>> +			struct msm_dp_panel *panel, bool plugged)
>>   {
>>   	int rc = 0;
>>   	struct msm_dp_ctrl_private *ctrl;
>> @@ -2427,7 +2428,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
>>   			break;
>>   		} else if (training_step == DP_TRAINING_1) {
>>   			/* link train_1 failed */
>> -			if (!msm_dp_aux_is_link_connected(ctrl->aux))
>> +			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
>>   				break;
>>   
>>   			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
>> @@ -2452,7 +2453,7 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
>>   			}
>>   		} else if (training_step == DP_TRAINING_2) {
>>   			/* link train_2 failed */
>> -			if (!msm_dp_aux_is_link_connected(ctrl->aux))
>> +			if (!msm_dp_aux_is_link_connected(ctrl->aux) && !plugged)
>>   				break;
>>   
>>   			drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
>> index fc884038182f..3edf954cc805 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
>> @@ -17,7 +17,7 @@ struct msm_dp_ctrl {
>>   struct phy;
>>   
>>   int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl,
>> -			struct msm_dp_panel *panel);
>> +			struct msm_dp_panel *panel, bool plugged);
>>   int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *panel);
>>   int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl,
>>   				  struct msm_dp_panel *panel);
>> @@ -28,7 +28,7 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl);
>>   irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl,
>>   			    struct msm_dp_panel *panel);
>>   void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl,
>> -				     struct msm_dp_panel *panel);
>> +				     struct msm_dp_panel *panel, bool plugged);
>>   struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev,
>>   				    struct msm_dp_link *link,
>>   				    struct drm_dp_aux *aux,
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 169938ba515e..551d2c63ef8e 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -399,7 +399,7 @@ static int msm_dp_display_handle_irq_hpd(struct msm_dp_display_private *dp)
>>   
>>   	drm_dbg_dp(dp->drm_dev, "%d\n", sink_request);
>>   
>> -	msm_dp_ctrl_handle_sink_request(dp->ctrl, dp->panel);
>> +	msm_dp_ctrl_handle_sink_request(dp->ctrl, dp->panel, dp->plugged);
>>   
>>   	if (sink_request & DP_TEST_LINK_VIDEO_PATTERN)
>>   		msm_dp_display_handle_video_request(dp);
>> @@ -648,11 +648,10 @@ static int msm_dp_display_prepare_link(struct msm_dp_display_private *dp)
>>   	if (dp->link->sink_count == 0)
>>   		return -ENOTCONN;
>>   
>> -	if (!msm_dp_display->power_on) {
>> +	if (!msm_dp_display->power_on)
>>   		msm_dp_display_host_phy_init(dp);
>> -	}
>>   
>> -	rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel);
>> +	rc = msm_dp_ctrl_on_link(dp->ctrl, dp->panel, dp->plugged);
>>   	if (rc) {
>>   		DRM_ERROR("Failed link training (rc=%d)\n", rc);
>>   		// TODO: schedule drm_connector_set_link_status_property()
>> -- 
>> 2.43.0
>>