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

[Dmitry Baryshkov]

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@xxxxxxxxxxx>
> > > > > > > 
> > > > > > > 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@xxxxxxxxxxx>
> > > > > > > Signed-off-by: Yongxing Mou <yongxing.mou@xxxxxxxxxxxxxxxx>
> > > > > > > ---
> > > > > > >     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.

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.
> > 
> 

-- 
With best wishes
Dmitry