Hi Dmitry,
Thank you for the patch.
On Sat, Mar 14, 2026 at 02:43:12AM +0200, 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.
The explanation makes sense, but it sounds like to could apply the same
way to HDMI. What am I missing ?
Also, wouldn't it be better if the DRM core interrogated the state of
the HPD signal when enabling HPD, instead of relying on all drivers to
implement something similar as this patch ?
> 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);
> +
> 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",
>
--
Regards,
Laurent Pinchart
