Re: [PATCH 2/5] drm/bridge_connector: preserve connector status for IRQ-only HPD events

On Mon, Aug 17, 2026 at 04:02:31PM +0800, Yongxing Mou wrote:
> 
> 
> On 7/12/2026 6:33 PM, Dmitry Baryshkov wrote:
> > On Mon, Jun 29, 2026 at 10:48:04PM +0800, Yongxing Mou wrote:
> > > The bridge connector HPD handling path currently updates
> > > connector->status for every hpd_notify() invocation.
> > > 
> > > This does not work well for IRQ-only notifications where the event being
> > > reported is carried by extra_status and no connector status transition is
> > > associated with it.
> > > 
> > > One example is DP MST. HPD IRQs are propagated through
> > > drm_bridge_hpd_notify_*() so that bridge drivers can process the
> > > notification. During MST operation, however, the SST connector attached
> > > to the bridge connector is intentionally kept disconnected while the MST
> > > topology manager handles all connector creation, removal and hotplug
> > > processing.
> > > 
> > > Updating connector->status for an IRQ-only MST notification may cause
> > > the SST connector state to oscillate between connected and disconnected
> > > depending on the notification path. These artificial state transitions
> > > can later be detected by the polling logic and result in unnecessary
> > > hotplug events being generated. Userspace then re-probes connector
> > > status, potentially triggering the same sequence again.
> > 
> > Then the API might need to be adjusted.
> > 
> > Remember, we have two usecases, which we must be able to interpret
> > correctly:
> > - The driver gets separate HPD and IRQ_HPD events.
> > - The driver gets HPD and IRQ_HPD at the same time.
> > 
> Ohh yes, here need to rework.
> > > 
> > > Treat notifications with status == connector_status_unknown and a valid
> > > extra_status as IRQ-only events. Forward the notification to bridge
> > > drivers without modifying connector->status.
> > > 
> > > This keeps IRQ delivery working while leaving connector state management
> > > to the component that actually owns it, such as the DP MST topology
> > > framework.
> > 
> > How is it handled by other drivers (i915, amd, nouveau)?
> > 
> i915, amdgpu, and nouveau don't go through the drm_bridge_hpd_notify()
> bridge chain -- their DP controllers are integrated into the SoC, and
> HPD interrupts are handled directly in their own encoder code.
> 
> MSM DP is different in that HPD comes from Type-C / pmic_glink altmode
> via aux-hpd-bridge, so it has to go through the bridge chain, which
> means this semantic needs to be extended to the bridge API.

Still, when do those drivers send the HPD event in case of IRQ_HPD? Or
is it that in their case IRQ_HPD just triggers inner logic to reread the
status registers and then the driver sends the HPD if there is any
actual change?

> > > Signed-off-by: Yongxing Mou <[email protected]>
> > > ---
> > >   drivers/gpu/drm/display/drm_bridge_connector.c | 12 ++++++++++++
> > >   1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> > > index 5edca47a025f..7334d6677604 100644
> > > --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> > > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> > > @@ -163,6 +163,18 @@ static void drm_bridge_connector_handle_hpd(struct drm_bridge_connector *drm_bri
> > >   	struct drm_connector *connector = &drm_bridge_connector->base;
> > >   	struct drm_device *dev = connector->dev;
> > > +	/*
> > > +	 * IRQ-only notification: extra_status carries the event but
> > > +	 * status is unknown — do not overwrite connector->status.
> > 
> > But it's not unknown at this point. The connector status is reported
> > following the HPD status.
> > 
> You are right, in the SST case the bridge_connector status does follow
> HPD / link status -- because the bridge_connector itself represents
> that SST connector, so its status naturally is the link status.
> 
> The MST case has a key difference though: the SST connector must be
> explicitly marked disconnected (to prevent the DRM framework from
> enabling it), consistent with what i915, amdgpu and nouveau do. In
> other words, once MST is enabled, the SST connector that the
> bridge_connector represents no longer equates to the link status --
> the real link is managed by the MST topology, and the SST connector
> is just a placeholder at that point.

This is fine.

> 
> The issue is that IRQ_HPD still travels through the bridge_connector
> chain and takes the old "update the SST connector's status -> emit
> hotplug" path. Under MST that runs into two constraints -- and this
> is exactly what this series is trying to address:
> 
> 1. The SST connector represented by bridge_connector no longer stands
>    for the link, so its status must not be overwritten by IRQ_HPD
>    events.
> 2. IRQ_HPD needs a clean path that does not trigger a hotplug on the
>    bridge_connector -- the MST framework already manages hotplugs
>    independently.
> 
> Using connector_status_unknown as a sentinel here does feel a bit odd;
> let me think about whether there is a cleaner approach.

The status here must represent the status reported by the corresponding
layer: be it DP ALtMode, the dp-connector driver handling the HPD GPIO
or the DP driver itself handling the HPD pin via the state machine.

Is it still an issue if the bridge's hpd_notify() callback determines
that we should not be reporting the event and drops connector->status to
'disconnected' again? Why is it an issue? Should we instead filter the
HPD events in the drm_sysfs_connector_hotplug_event(), making sure that
we don't send duplicate disconnected events?

What is the expected behaviour of drm_client's?

> > > +	 */
> > > +	if (status == connector_status_unknown &&
> > > +	    extra_status != DRM_CONNECTOR_NO_EXTRA_STATUS) {
> > > +		drm_bridge_connector_hpd_notify(connector,
> > > +						connector->status,
> > > +						extra_status, NULL);
> > > +		return;
> > > +	}
> > > +
> > >   	mutex_lock(&dev->mode_config.mutex);
> > >   	connector->status = status;
> > >   	mutex_unlock(&dev->mode_config.mutex);
> > > 
> > > -- 
> > > 2.43.0
> > > 
> > 
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
With best wishes
Dmitry