Re: [PATCH 2/5] drm/bridge_connector: preserve connector status for IRQ-only HPD events
Yongxing Mou <[email protected]>
| Newsgroups | org.infradead.lists.linux-amlogic,org.freedesktop.lists.dri-devel,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/18/2026 11:18 AM, Dmitry Baryshkov wrote: > 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? > So my understanding is that,IRQ_HPD is primarily used to trigger internal status revalidation. A hotplug event is only generated if that processing concludes that there has been an actual connector or topology state 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. > Got it. > 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 yes, it still an issue if each irq . The SST connector should not be reported as connected, even momentarily. When the connector status is changed from connected back to disconnected, this state transition is treated as a hotplug event and trigger hotplug, introducing unnecessary userspace polling and re-probing. > HPD events in the drm_sysfs_connector_hotplug_event(), making sure that > we don't send duplicate disconnected events? > I think driver should confirm it really send a hotplug when we call drm_sysfs_connector_hotplug_event() > What is the expected behaviour of drm_client's? > - Genuine connect/disconnect: the client is woken up, re-probes and follows the new state. - IRQ_HPD with no actual change: the client must not be woken up at all, otherwise every short pulse triggers a full re-probe of all connectors. - IRQ_HPD where the driver does conclude that something changed: the driver escalates it explicitly into a real status notification. - The SST connector while MST is active: permanently 'disconnected', and the client must never select it. The real outputs are the MST port connectors created by the topology manager, which get their own hotplug events from it. >>>> + */ >>>> + 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 > _______________________________________________ linux-amlogic mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-amlogic