On Mon, Jun 15, 2026 at 06:05:07PM +0800, Yongxing Mou wrote:
>
>
> 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 <[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 | 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.
Please. Start testing with USB-C too.
> > > + 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?
No. There is no aux bridge here. But yes, I'd need to call a different
function in that series.
> > > + 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)
This should not be happening. We don't use link status anymore to return
connected status.
> 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.
drm_dp_read_sink_count() isn't enough here. See the plugged flag. Maybe
we need more flags here.
>
> > > + msm_dp_hpd_unplug_handle(dp);
> > > }
> > > pm_runtime_put_sync(&msm_dp_display->pdev->dev);
--
With best wishes
Dmitry
