Re: [PATCH RESEND v5 25/25] drm/msm/dp: add HPD callback for dp MST

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

On 7/13/2026 8:54 AM, Dmitry Baryshkov wrote:
> On Mon, Jun 29, 2026 at 10:14:46PM +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 | 22 ++++++++++++++----
>>   drivers/gpu/drm/msm/dp/dp_mst_drm.c | 46 +++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/msm/dp/dp_mst_drm.h |  1 +
>>   3 files changed, 65 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 6eac390af2e0..49a7bc5e031e 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -455,6 +455,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 && dp->msm_dp_display.mst_active)
>> +		return 0;
> 
> Why?
> 
We need to filter out unnecessary plug events. A single physical HPD 
event may result in multiple hpd_notify() calls. The first call comes 
from the actual IRQ, while subsequent hotplug processing may trigger 
connector re-detection, which in turn causes additional hpd_notify() calls.
>> +
>>   	guard(mutex)(&dp->plugged_lock);
>>   
>>   	ret = pm_runtime_resume_and_get(&pdev->dev);
>> @@ -550,12 +553,18 @@ 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) {
>> +		msm_dp_mst_display_hpd_irq(&dp->msm_dp_display);
> 
> This should be routed through the HPD / notifications API so that there
> is always a single path for handling the IRQ_HPD interrupts.
> 
Got it. The reason for having two paths in the native DP case today is 
that the current DRM IRQ API may result in unnecessary hotplug 
notifications.

Once the API adaptation is completed, both native DP and Type-C DP are 
expected to use the same IRQ handling path.
>> +		return 0;
>> +	}
>> +
>>   	/* check for any test request issued by sink */
>>   	rc = msm_dp_link_process_request(dp->link);
>>   	if (!rc) {
>> @@ -1111,9 +1120,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)
> 
> Locking? What if MST is being enabled or disabled on the other core?
> 
In the future, we plan to switch this over to the DRM IRQ API. At that 
point, SST and MST will no longer be handled through separate paths, so 
this issue should naturally disappear.
>> +			msm_dp_irq_hpd_handle(dp);
>> +		else
>> +			drm_bridge_hpd_notify(dp->msm_dp_display.bridge,
>> +					      connector_status_connected);
>> +	}
>>   
>>   	ret = IRQ_HANDLED;
>>   
>