Re: [PATCH v4 37/39] drm/msm/dp: add HPD callback for dp MST

[Yongxing Mou <yongxing.mou@oss.qualcomm.com>]

On 4/12/2026 6:00 AM, Dmitry Baryshkov wrote:
> On Fri, Apr 10, 2026 at 05:34:12PM +0800, Yongxing Mou wrote:
>> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> Add HPD callback for the MST module which shall be invoked from the
>> dp_display's HPD handler to perform MST specific operations in case
>> of HPD. In MST case, route the HPD messages to MST module.
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c | 23 +++++++++++++++++++----
>>   drivers/gpu/drm/msm/dp/dp_mst_drm.c | 34 ++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/msm/dp/dp_mst_drm.h |  1 +
>>   3 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 919767945ba5..ca89e20b7563 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -454,6 +454,9 @@ static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
>>   			dp->msm_dp_display.connector_type,
>>   			dp->link->sink_count);
>>   
>> +	if (dp->plugged)
>> +		return 0;
>> +
>>   	mutex_lock(&dp->plugged_lock);
>>   
>>   	ret = pm_runtime_resume_and_get(&pdev->dev);
>> @@ -556,12 +559,19 @@ static int msm_dp_irq_hpd_handle(struct msm_dp_display_private *dp)
>>   {
>>   	u32 sink_request;
>>   	int rc = 0;
>> +	struct msm_dp *msm_dp_display = &dp->msm_dp_display;
>>   
>>   	/* irq_hpd can happen at either connected or disconnected state */
>>   	drm_dbg_dp(dp->drm_dev, "Before, type=%d, sink_count=%d\n",
>>   			dp->msm_dp_display.connector_type,
>>   			dp->link->sink_count);
>>   
>> +	if (msm_dp_display->mst_active) {
>> +		if (msm_dp_aux_is_link_connected(dp->aux) != ISR_DISCONNECTED)
> 
> Will this work for USB-C?
> 
Hmm not work for USB-C. We can remove this check here, as the IRQ thread 
can handle the disconnect case itself.
>> +			msm_dp_mst_display_hpd_irq(&dp->msm_dp_display);
>> +		return 0;
>> +	}
>> +
>>   	/* check for any test request issued by sink */
>>   	rc = msm_dp_link_process_request(dp->link);
>>   	if (!rc) {
>> @@ -1125,9 +1135,13 @@ static irqreturn_t msm_dp_display_irq_thread(int irq, void *dev_id)
>>   				      connector_status_connected);
>>   
>>   	/* Send HPD as connected and distinguish it in the notifier */
>> -	if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK)
>> -		drm_bridge_hpd_notify(dp->msm_dp_display.bridge,
>> -				      connector_status_connected);
>> +	if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) {
>> +		if (dp->msm_dp_display.mst_active)
>> +			msm_dp_irq_hpd_handle(dp);
> 
> No, don't touch this code. HPD notifications might be coming from the
> other entities. This IRQ thread can only send the HPD notification.
> There rest should be handled in the notifier.
> 
Ok. From my understanding, after this series 
(https://patchwork.freedesktop.org/series/164954/#rev5) is rebased, we 
should use drm_aux_hpd_bridge_notify_extra() here to notify the IRQ?
>> +		else
>> +			drm_bridge_hpd_notify(dp->msm_dp_display.bridge,
>> +					      connector_status_connected);
>> +	}
>>   
>>   	ret = IRQ_HANDLED;
>>   
>> @@ -1793,7 +1807,8 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
>>   			msm_dp_hpd_plug_handle(dp);
>>   		}
>>   	} else {
>> -		msm_dp_hpd_unplug_handle(dp);
>> +		if (hpd_link_status == ISR_DISCONNECTED)
> 
> Why?
> 
Let me explain this in more detail here.
Currently, MST hotplug and IRQ events are handled through the SST bridge.
This guards against spurious unplug handling caused by 
msm_dp_bridge_hpd_notify() being invoked from non-HPD contexts where 
status == connector_status_disconnected does not actually mean the cable 
is gone.

In addition to the real HPD IRQ path, drm_bridge_connector_detect() also 
calls drm_bridge_connector_hpd_notify() to broadcast the detect result 
to all bridges in the chain. So a single physical plug-in produces 
multiple msm_dp_bridge_hpd_notify() calls — one from the real IRQ, then 
several more from various probe/poll paths. Stack traces from a single 
insertion on QCS8300:

   1. msm_dp_display_irq_thread → real HPD plug, status=connected
   2. fbdev probe triggered by (1) → drm_bridge_connector_detect → 
status=disconnected (link not ready yet)
   3. output_poll_execute worker → same path → status=disconnected
   4. drm_dp_mst_link_probe_work → same path → status=disconnected
   5. output_poll_execute again → status=disconnected

Here not work for USB-C case yet, I’d like to switch to using 
drm_dp_read_sink_count to detect whether the sink is actually 
disconnected or no sink devices.

>> +			msm_dp_hpd_unplug_handle(dp);
>>   	}
>>   
>>   	pm_runtime_put_sync(&msm_dp_display->pdev->dev);
>> diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.c b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
>> index bb3898b1f6b1..71d3f63973e6 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_mst_drm.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
>> @@ -490,6 +490,40 @@ int msm_dp_mst_display_set_mgr_state(struct msm_dp *dp_display, bool state)
>>   	return rc;
>>   }
>>   
>> +/* DP MST HPD IRQ callback */
> 
> Useless comment.
> 
Got it.
>> +void msm_dp_mst_display_hpd_irq(struct msm_dp *dp_display)
>> +{
>> +	int rc;
>> +	struct msm_dp_mst *mst = dp_display->msm_dp_mst;
>> +	u8 ack[8] = {};
>> +	u8 esi[4];
>> +	unsigned int esi_res = DP_SINK_COUNT_ESI + 1;
>> +	bool handled;
>> +
>> +	rc = drm_dp_dpcd_read_data(mst->dp_aux, DP_SINK_COUNT_ESI, esi, 4);
>> +	if (rc < 0) {
>> +		DRM_ERROR("DPCD sink status read failed, rlen=%d\n", rc);
>> +		return;
>> +	}
>> +
>> +	drm_dbg_dp(dp_display->drm_dev, "MST irq: esi1[0x%x] esi2[0x%x] esi3[%x]\n",
>> +		   esi[1], esi[2], esi[3]);
>> +
>> +	rc = drm_dp_mst_hpd_irq_handle_event(&mst->mst_mgr, esi, ack, &handled);
>> +
>> +	/* ack the request */
>> +	if (handled) {
>> +		rc = drm_dp_dpcd_write_byte(mst->dp_aux, esi_res, ack[1]);
>> +		if (rc < 0) {
>> +			DRM_ERROR("DPCD esi_res failed. rc=%d\n", rc);
>> +			return;
>> +		}
>> +
>> +		drm_dp_mst_hpd_irq_send_new_request(&mst->mst_mgr);
>> +	}
>> +	drm_dbg_dp(dp_display->drm_dev, "MST display hpd_irq handled:%d rc:%d\n", handled, rc);
>> +}
>> +
>>   /* DP MST Connector OPs */
>>   static int
>>   msm_dp_mst_connector_detect(struct drm_connector *connector,
>> diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.h b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
>> index 5d411529f681..08e145399cfc 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_mst_drm.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.h
>> @@ -9,5 +9,6 @@
>>   
>>   int msm_dp_mst_init(struct msm_dp *dp_display, u32 max_streams, struct drm_dp_aux *drm_aux);
>>   int msm_dp_mst_display_set_mgr_state(struct msm_dp *dp_display, bool state);
>> +void msm_dp_mst_display_hpd_irq(struct msm_dp *dp_display);
>>   
>>   #endif /* _DP_MST_DRM_H_ */
>>
>> -- 
>> 2.43.0
>>
>