Re: [PATCH v7 23/23] drm: bridge: dw_hdmi: Merge top and bottom half IRQ handlers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clearing the HPD interrupt status after modifying the interrupt polarity creates a race window where an edge transition is missed, breaking hotplug detection.
--

commit b4c743935eb9892c0e2a6a962a52440a37e985de
Author: Jonas Karlman <jonas@kwiboo.se>
drm: bridge: dw_hdmi: Merge top and bottom half IRQ handlers

This commit simplifies IRQ handling by merging the top and bottom half IRQ
handlers into a single handler. It transitions HPD event handling to use a
delayed work queue instead of an IRQ thread.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2993,30 +2993,18 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
[ ... ]
> -	intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0);
> -	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
> +	intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0) & HDMI_IH_PHY_STAT0_HPD;
> +	if (intr_stat) {
>  		enum drm_connector_status status;
>  
> +		hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
> +
>  		/* Set HPD interrupt polarity based on current HPD status. */
>  		status = dw_hdmi_phy_read_hpd(hdmi, hdmi->phy.data);
>  		hdmi_modb(hdmi, status == connector_status_connected ?
> @@ -3028,12 +3016,13 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>  
>  		mod_delayed_work(system_percpu_wq, &hdmi->hpd_work,
>  				 msecs_to_jiffies(HOTPLUG_DEBOUNCE_MS));
> -	}
>  
> -	hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
> -	hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);
> +		hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);

Does clearing the HPD interrupt status after modifying the interrupt polarity
create a race window where an edge transition is missed?

While this isn't a new regression introduced by this patch, it persists in
the new merged handler:

1. The ISR reads HDMI_IH_PHY_STAT0 and finds the HPD bit set.

2. It reads the current PHY state (e.g. CONNECTED) and updates the polarity
HDMI_PHY_POL0 to look for the opposite edge (e.g. PLUGOUT).

3. If a fast bounce occurs right here (the cable is unplugged), the hardware
detects the edge and sets the HPD sticky bit to 1 again.

4. The ISR then writes the original intr_stat (with the HPD bit set) back to
HDMI_IH_PHY_STAT0 (a write-1-to-clear register). This clears the newly latched
unplug event.

This leaves the hardware waiting for a PLUGOUT event, but the cable is already
disconnected. When the cable is plugged back in, the hardware ignores the
PLUGIN edge because it is still looking for a PLUGOUT.

Could this lead to a regression where hotplug interrupts break until the next
reboot? Should the W1C register be cleared before reading the PHY status and
updating the polarity?

> +		hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);
> +		ret = IRQ_HANDLED;
> +	}
>  
> -	return IRQ_HANDLED;
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518180206.2480119-1-jonas@kwiboo.se?part=23

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic