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 <[email protected]>
> > > > > > > > > 
> > > > > > > > > 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 <[email protected]>
> > > > > > > > > Signed-off-by: Yongxing Mou <[email protected]>
> > > > > > > > > ---
> > > > > > > > >      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?

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

-- 
With best wishes
Dmitry

Reply via email to