Hi Dmitry,
On 5/13/2026 8:19 PM, Dmitry Baryshkov wrote:
> If the DisplayPort drivers use display-connector for the HPD detection,
> the internal HPD state machine might be not active and thus the hardware
> might be not able to handle cable detection correctly. Instead it will
> depend on the externall HPD notifications to set the cable state,
> bypassing the internal HPD state machine (for example this is the case
> for the msm DP driver).
>
> However if the cable has been plugged before the HPD IRQ has been
> enabled, there will be no HPD event coming. The drivers might fail
> detection in such a case. Trigger the HPD notification after enabling
> the HPD IRQ, propagating the cable insertion state.
>
> Note, thia issue only affects drivers which set OP_HPD but not OP_DETECT
> (like dp-connector). Here DP differs from HDMI. For HDMI there is no
> additional state or extra "bridge with no sinks plugged" cases. The HPD
> pin state is equalc to the display plugged state. Nor do we have a an
> AUX bus with timeouts, etc.
>
> Fixes: 2e2bf3a5584d ("drm/bridge: display-connector: add DP support")
> Reported-by: Yongxing Mou <[email protected]>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/bridge/display-connector.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/display-connector.c
> b/drivers/gpu/drm/bridge/display-connector.c
> index 6bb1134f75c3..f7a5bfd9c075 100644
> --- a/drivers/gpu/drm/bridge/display-connector.c
> +++ b/drivers/gpu/drm/bridge/display-connector.c
> @@ -12,6 +12,7 @@
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/workqueue.h>
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_bridge.h>
> @@ -25,6 +26,8 @@ struct display_connector {
>
> struct regulator *supply;
> struct gpio_desc *ddc_en;
> +
> + struct work_struct hpd_work;
> };
>
> static inline struct display_connector *
> @@ -92,15 +95,29 @@ static void display_connector_hpd_enable(struct
> drm_bridge *bridge)
> struct display_connector *conn = to_display_connector(bridge);
>
> enable_irq(conn->hpd_irq);
> +
> + if (conn->bridge.type == DRM_MODE_CONNECTOR_DisplayPort)
> + schedule_work(&conn->hpd_work);
> }
>
> static void display_connector_hpd_disable(struct drm_bridge *bridge)
> {
> struct display_connector *conn = to_display_connector(bridge);
>
> + if (conn->bridge.type == DRM_MODE_CONNECTOR_DisplayPort)
> + cancel_work_sync(&conn->hpd_work);
Could use of the sync() variant cause a deadlock here?
Both drm_bridge_hpd_disable() that calls into this func and the
drm_bridge_hpd_notify() called from the hpd_work both hold the
bridge->hpd_mutex.
Sashiko pointed out a similar deadlock question when I tried to do
something very similar to this for the dw-hdmi bridge, and I think
it theoretically could happen?
- hpd_work is scheduled
- drm_bridge_hpd_disable() calls hpd_enable() under lock
- hpd_work starts and calls drm_bridge_hpd_notify()
- drm_bridge_hpd_notify() waits on lock to be released
- hpd_enable() calls cancel_work_sync() and waits
- deadlock
Ahh, looks like Sashiko found same for your patch as it did for mine :-)
https://sashiko.dev/#/patchset/20260513-dp-connector-hpd-v2-0-42f757bfcbf9%40oss.qualcomm.com
Regards,
Jonas
> +
> disable_irq(conn->hpd_irq);
> }
>
> +static void display_connector_hpd_work(struct work_struct *work)
> +{
> + struct display_connector *conn = container_of(work, struct
> display_connector, hpd_work);
> + struct drm_bridge *bridge = &conn->bridge;
> +
> + drm_bridge_hpd_notify(bridge, display_connector_detect(bridge));
> +}
> +
> static const struct drm_edid *display_connector_edid_read(struct drm_bridge
> *bridge,
> struct drm_connector
> *connector)
> {
> @@ -395,6 +412,8 @@ static int display_connector_probe(struct platform_device
> *pdev)
> conn->bridge.ops |= DRM_BRIDGE_OP_DETECT;
> if (conn->hpd_irq >= 0)
> conn->bridge.ops |= DRM_BRIDGE_OP_HPD;
> + if (conn->hpd_irq >= 0 && type == DRM_MODE_CONNECTOR_DisplayPort)
> + INIT_WORK(&conn->hpd_work, display_connector_hpd_work);
>
> dev_dbg(&pdev->dev,
> "Found %s display connector '%s' %s DDC bus and %s HPD GPIO
> (ops 0x%x)\n",
>
