Re: [PATCH v3 09/13] drm: bridge: dw_hdmi: Use delayed_work to debounce hotplug event

Dmitry Baryshkov
2026-04-28T12:38:37Z

On Fri, Apr 03, 2026 at 06:52:55PM +0000, Jonas Karlman wrote:
> HDMI Specification Version 1.4b chapter 8.5 mentions:
>
> An HDMI Sink shall not assert high voltage level on its Hot Plug
> Detect pin when the E-EDID is not available for reading.
>
> A Source may use a high voltage level Hot Plug Detect signal to
> initiate the reading of E-EDID data.
>
> An HDMI Sink shall indicate any change to the contents of the E-EDID
> by driving a low voltage level pulse on the Hot Plug Detect pin. This
> pulse shall be at least 100 msec.
>
> Use a work queue to debounce reacting on HPD events to better handle a
> HPD low voltage level pulse when a sink changes the EDID.
>
> The 1100 msec hotplug debounce timeout was arbitrarily picked to match
> other drivers using same const, and testing using a Raspberry Pi Monitor
> seem to use a 200-300 msec pulse when going from standby to power on
> state.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v3: New patch
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 24 +++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index f3f8144ae98e..4d079b689b3b 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -51,6 +51,8 @@
>
> #define HDMI14_MAX_TMDSCLK 340000000
>
> +#define HOTPLUG_DEBOUNCE_MS 1100
> +
> static const u16 csc_coeff_default[3][4] = {
> { 0x2000, 0x0000, 0x0000, 0x0000 },
> { 0x0000, 0x2000, 0x0000, 0x0000 },
> @@ -192,6 +194,7 @@ struct dw_hdmi {
> hdmi_codec_plugged_cb plugged_cb;
> struct device *codec_dev;
> enum drm_connector_status last_connector_result;
> + struct delayed_work hpd_work;
> };
>
> const struct dw_hdmi_plat_data *dw_hdmi_to_plat_data(struct dw_hdmi *hdmi)
> @@ -2528,6 +2531,7 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>
> mutex_lock(&hdmi->mutex);
> hdmi->force = connector->force;
> + hdmi->last_connector_result = connector->status;
> dw_hdmi_update_phy_mask(hdmi);
> mutex_unlock(&hdmi->mutex);
> }
> @@ -3046,6 +3050,16 @@ void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>
> +static void dw_hdmi_hpd_work(struct work_struct *work)
> +{
> + struct dw_hdmi *hdmi = container_of(work, struct dw_hdmi, hpd_work.work);
> +
> + if (hdmi->bridge.dev) {
> + drm_helper_hpd_irq_event(hdmi->bridge.dev);

Nit: please consider this for the followup cleanup. The
drm_helper_hpd_irq_event() is not very effective: we already know which
bridge (and thus connector) caused the HPD event.

Moreover, the call should be redundant as drm_bridge_hpd_notify will
call the registered callback, which normally would be
drm_bridge_connector_hpd_cb(). Internally, this function calls
drm_kms_helper_connector_hotplug_event().

> + drm_bridge_hpd_notify(&hdmi->bridge, hdmi->last_connector_result);
> + }
> +}
> +
> static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> {
> struct dw_hdmi *hdmi = dev_id;
> @@ -3097,10 +3111,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> status == connector_status_connected ?
> "plugin" : "plugout");
>
> - if (hdmi->bridge.dev) {
> - drm_helper_hpd_irq_event(hdmi->bridge.dev);
> - drm_bridge_hpd_notify(&hdmi->bridge, status);
> - }
> + mod_delayed_work(system_percpu_wq, &hdmi->hpd_work,
> + msecs_to_jiffies(HOTPLUG_DEBOUNCE_MS));
> }
>
> hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
> @@ -3420,6 +3432,8 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> goto err_res;
> }
>
> + INIT_DELAYED_WORK(&hdmi->hpd_work, dw_hdmi_hpd_work);
> +
> ret = devm_request_threaded_irq(dev, irq, dw_hdmi_hardirq,
> dw_hdmi_irq, IRQF_SHARED,
> dev_name(dev), hdmi);
> @@ -3552,6 +3566,8 @@ EXPORT_SYMBOL_GPL(dw_hdmi_probe);
>
> void dw_hdmi_remove(struct dw_hdmi *hdmi)
> {
> + disable_delayed_work_sync(&hdmi->hpd_work);
> +
> drm_bridge_remove(&hdmi->bridge);
>
> if (hdmi->audio && !IS_ERR(hdmi->audio))
> --
> 2.53.0
>

--
With best wishes
Dmitry