Re: [PATCH v3 31/38] drm/msm/dp: add HPD callback for dp MST

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

On 5/19/2026 12:36 AM, Dmitry Baryshkov wrote:
> On Thu, May 14, 2026 at 03:12:17PM +0800, Yongxing Mou wrote:
>>
>>
>> On 4/19/2026 8:29 AM, Dmitry Baryshkov wrote:
>>> On Wed, Apr 15, 2026 at 06:32:29PM +0800, Yongxing Mou wrote:
>>>>
>>>>
>>>> On 4/15/2026 2:43 AM, Dmitry Baryshkov wrote:
>>>>> On Tue, Apr 14, 2026 at 05:51:51PM +0800, Yongxing Mou wrote:
>>>>>>
>>>>>>
>>>>>> On 3/25/2026 3:30 AM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, Mar 24, 2026 at 09:04:24PM +0800, Yongxing Mou wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 8/27/2025 2:40 AM, Dmitry Baryshkov wrote:
>>>>>>>>> On Mon, Aug 25, 2025 at 10:16:17PM +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 | 15 ++++++++++++---
>>>>>>>>>>       drivers/gpu/drm/msm/dp/dp_mst_drm.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>>>>>       drivers/gpu/drm/msm/dp/dp_mst_drm.h |  2 ++
>>>>>>>>>>       3 files changed, 48 insertions(+), 3 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>>>>>>> index abcab3ed43b6da5ef898355cf9b7561cd9fe0404..59720e1ad4b1193e33a4fc6aad0c401eaf9cbec8 100644
>>>>>>>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>>>>>>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>>>>>>> @@ -500,9 +500,16 @@ static int msm_dp_display_handle_irq_hpd(struct msm_dp_display_private *dp)
>>>>>>>>>>       static int msm_dp_display_usbpd_attention_cb(struct device *dev)
>>>>>>>>>>       {
>>>>>>>>>> -	int rc = 0;
>>>>>>>>>> -	u32 sink_request;
>>>>>>>>>>       	struct msm_dp_display_private *dp = dev_get_dp_display_private(dev);
>>>>>>>>>> +	struct msm_dp *msm_dp_display = &dp->msm_dp_display;
>>>>>>>>>> +	u32 sink_request;
>>>>>>>>>> +	int rc = 0;
>>>>>>>>>> +
>>>>>>>>>> +	if (msm_dp_display->mst_active) {
>>>>>>>>>> +		if (msm_dp_aux_is_link_connected(dp->aux) != ISR_DISCONNECTED)
>>>>>>>>>> +			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);
>>>>>>>>>> @@ -1129,8 +1136,10 @@ static irqreturn_t msm_dp_display_irq_thread(int irq, void *dev_id)
>>>>>>>>>>       	if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK)
>>>>>>>>>>       		msm_dp_display_send_hpd_notification(dp, false);
>>>>>>>>>> -	if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK)
>>>>>>>>>> +	if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) {
>>>>>>>>>>       		msm_dp_display_send_hpd_notification(dp, true);
>>>>>>>>>> +		msm_dp_irq_hpd_handle(dp, 0);
>>>>>>>>>
>>>>>>>>> Why is it a part of this patch?? It has nothing to do with MST.
>>>>>>>>>
>>>>>>>> Emm ... maybe here we can directly call msm_dp_mst_display_hpd_irq..
>>>>>>>> I tried an alternative approach by calling the MST IRQ handler from
>>>>>>>> msm_dp_bridge_hpd_notify(). I expected that when hpd_isr_status ==
>>>>>>>> DP_DP_IRQ_HPD_INT_MASK, the hpd_link_status read in
>>>>>>>> msm_dp_bridge_hpd_notify() would be ISR_IRQ_HPD_PULSE_COUNT. That way, we
>>>>>>>> could handle both SST and MST interrupt paths in msm_dp_irq_hpd_handle().
>>>>>>>> However, hpd_link_status only reports ISR_CONNECTED. So I had to move the
>>>>>>>> MST IRQ handling into the IRQ thread. Do you have any suggestions on this?
>>>>>>>
>>>>>>> When are the link status bits updated? Please remember, we need to
>>>>>>> support all three cases:
>>>>>>>
>>>>>>> - Native DP, native DP HPD pin handling
>>>>>>> - Native DP, DP HPD pin not handled by the controller
>>>>>>> - DP AltMode, DP HPD pin not used at all
>>>>>>>
>>>>>>> In the second and the third cases we will not be getting the IRQs.
>>>>>>> Instead one of the next bridges (connector, EC, AltMode, etc.) will send
>>>>>>> the HPD event, which lands in the .hpd_notify() callback.
>>>>>>>
>>>>>> I added some logs and did some testing. I think
>>>>>> msm_dp_aux_is_link_connected() only shows the current HPD state. Since IRQ
>>>>>> HPD Pulse Count is very short, by the time we read REG_DP_DP_HPD_INT_STATUS
>>>>>> in the IRQ flow, the HPD state machine has usually already finished pulse
>>>>>> classification and returned to Connected.
>>>>>
>>>>> But the IRQ should be sticky and it should be readable from the status
>>>>> bits.
>>>>>
>>>> Yes... I’m not sure how this is handled on other platforms, but on LeMans
>>>> can not get IRQ status from msm_dp_aux_is_link_connected().
>>>
>>> Can we clarify that somehow? Maybe with the hardware team if it is
>>> uncear from the HPG?
>>>
>>>>> Note, in the USB-C AltMode case the HPD machine is not used at all.
>>>>>
>>>>>>
>>>>>> Because of that, the condition hpd_link_status == ISR_IRQ_HPD_PULSE_COUNT
>>>>>> will usually not be hit.
>>>>>>
>>>>>> do you have any suggestion that in how to distinguish between an IRQ event
>>>>>> and a plug event in .hpd_notify() better? We probably don’t want to
>>>>>> introduce another state machine.
>>>>>
>>>>> Then, I assume, currently there is no way to actually distinguish those.
>>>>> The easiest way to handle the replug would be to store the current
>>>>> "connected" status and verify if we are receiving "connected" while
>>>>> being connected or if it is a disconnected -> connected change.
>>>>>
>>>> Emm.. Currently, regardless of whether it is the native DP HPD (on LeMans)
>>>> or DP over Type‑C Alt Mode(test on Hamoa), a single plug event always
>>>> results in two or more identical .hpd_notify() callbacks.
>>>
>>> Could you please check, why? On Hamoa it might be because of the LTTPRs.
>>>
>>>> In other words, after the transition from disconnected → connected is
>>>> completed, there is still one more .hpd_notify() with connected → connected.
>>>> So it still can store "connected" to distinguish between an IRQ event and a
>>>> plug event from .hpd_notify()?
>>>
>>> I've sent a series, which explicitly tracks the IRQ events. Hope that
>>> helps.
>>>
>> Very thanks for sending the HPD IRQ series
>> https://patchwork.freedesktop.org/series/151522/. it very helpful for TYPE-C
>> MST.
>>
>> I’ve been testing it locally based on HPD refator series, and TYPE-C basic
>> plug case works on my side (although with some local modify, maybe now it is
>> workaround). At least the IRQ is being delivered correctly now and the
>> simplest case works. It still need to do some additional testing.
>>
>> There is a small question:
>> When do you plan to merge the HPD refactor series?
> 
> Would you or your colleagues review it? Or at least T-B it?
> 
  I’ve given the T-B. Could Konrad help review this series?
>> and could you please
>> rebase the irq series patch to HPD refactor series ?
> 
> Once it is merged.
> 
>> so that i can keep MST
>> depend on those 2 series.
>>> Thoug storing of the "connected" state should help us to identify the
>>> long HPD pulse (wich should be treated as unplug & replug).
>>>
>>>> This is my current understanding. If this is incorrect, please feel free to
>>>> correct me. Thanks.
>>>> As an additional note, msm_dp_hpd_plug_handle runs through its full flow
>>>> twice for a single plug event. Also, the behavior I described above does not
>>>> include any MST-specific filtering codes.
>>>>> For a longer term (and granted that HDMI also has a notion of HPD pulse
>>>>> events) we might want to extend the DRM HPD API to pass through the "IRQ
>>>>> pulse" events as is (instead of converting those to
>>>>> connected-whilec-connected events).
>>>>>
>>>>> Let me sketch a draft for that.
>>>>>
>>>>
>>>
>>
>