diff mbox series

[v9,27/61] drm/rockchip: dw_hdmi_qp: Avoid spurious HPD IRQ thread wakeups

Message ID 20260723-dw-hdmi-qp-scramb-v9-27-4fb12ea22ac9@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 23, 2026, 1:35 a.m. UTC 
The RK3576 and RK3588 HPD hardirq handlers read shared status registers
and currently wake the threaded handler whenever any bit in those
registers is set.  This can wake the HDMI IRQ thread for unrelated
status bits, and on RK3588 can also wake the thread for the other HDMI
port.

Mask the status value with the SoC-specific HPD interrupt bit before
deciding whether to wake the threaded handler.  On RK3588, select the
HDMI0 or HDMI1 HPD bit according to the instance port id.

No stable backport is needed.  The issue may only cause spurious IRQ
thread wakeups and does not cause functional breakage.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260702151346.5A4EC1F000E9@smtp.kernel.org/
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

sashiko-bot@kernel.org July 23, 2026, 1:51 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free and resource leaks in component bind due to mismatched devm_ and drmm_ lifecycles.
--

--- Patch [27]: [PATCH v9 27/61] drm/rockchip: dw_hdmi_qp: Avoid spurious HPD IRQ thread wakeups ---
commit fe0521a96f0ae3fca18406e54bcdc6ddbecf6e85
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Avoid spurious HPD IRQ thread wakeups

The RK3576 and RK3588 HPD hardirq handlers read shared status registers
and currently wake the threaded handler whenever any bit in those
registers is set. This commit masks the status value with the SoC-specific
HPD interrupt bit to avoid spurious IRQ thread wakeups.

Link: https://lore.kernel.org/all/20260702151346.5A4EC1F000E9@smtp.kernel.org/

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 2aceff11d14b5..7e3e86d056116 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c

[ ... ]

> @@ -381,6 +384,8 @@ static irqreturn_t dw_hdmi_qp_rk3588_hardirq(int irq, void *dev_id)
>  
>  	regmap_read(hdmi->regmap, RK3588_GRF_SOC_STATUS1, &intr_stat);
>  
> +	intr_stat &= hdmi->port_id ? RK3588_HDMI1_OHPD_INT : RK3588_HDMI0_OHPD_INT;
> +

[Severity: High]
This is a pre-existing issue, but could this handler trigger a use-after-free
after the component is unbound?

It looks like dw_hdmi_qp_rockchip_bind() allocates the hdmi context using
drmm_kzalloc() (tied to the DRM aggregate device), but registers this IRQ
handler using devm_request_threaded_irq() (tied to the platform device):

drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_bind() {
    ...
    ret = devm_request_threaded_irq(dev, irq,
                                    cfg->ctrl_ops->hardirq_callback,
                                    cfg->ctrl_ops->irq_callback,
                                    IRQF_SHARED, "dw-hdmi-qp-hpd",
                                    hdmi);
    ...
}

When the DRM component unbinds, drmm_ frees the hdmi struct. However, since
the platform device itself isn't removed, the devm_ allocated IRQ handler
remains active.

If an interrupt fires after unbind, wouldn't dw_hdmi_qp_rk3588_hardirq()
dereference the freed hdmi pointer when calling regmap_read()?

Should dw_hdmi_qp_rockchip_unbind() manually free the IRQ and other resources,
or should the IRQ be registered with a drmm_ wrapper instead?

>  	if (intr_stat) {
>  		if (hdmi->port_id)
>  			val = FIELD_PREP_WM16(RK3588_HDMI1_HPD_INT_MSK, 1);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 2aceff11d14b..7e3e86d05611 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -34,6 +34,7 @@ 
 
 #define RK3576_IOC_HDMI_HPD_STATUS	0xa440
 #define RK3576_HDMI_LEVEL_INT		BIT(3)
+#define RK3576_HDMI_OHPD_INT		BIT(4)
 
 #define RK3576_VO0_GRF_SOC_CON1		0x0004
 #define RK3576_HDMI_FRL_MOD		BIT(0)
@@ -64,7 +65,9 @@ 
 #define RK3588_HPD_HDMI1_IO_EN_MASK	BIT(13)
 #define RK3588_GRF_SOC_STATUS1		0x0384
 #define RK3588_HDMI0_LEVEL_INT		BIT(16)
+#define RK3588_HDMI0_OHPD_INT		BIT(17)
 #define RK3588_HDMI1_LEVEL_INT		BIT(24)
+#define RK3588_HDMI1_OHPD_INT		BIT(25)
 #define RK3588_GRF_VO1_CON3		0x000c
 #define RK3588_GRF_VO1_CON6		0x0018
 #define RK3588_COLOR_DEPTH_MASK		GENMASK(7, 4)
@@ -348,7 +351,7 @@  static irqreturn_t dw_hdmi_qp_rk3576_hardirq(int irq, void *dev_id)
 	u32 intr_stat, val;
 
 	regmap_read(hdmi->regmap, RK3576_IOC_HDMI_HPD_STATUS, &intr_stat);
-	if (intr_stat) {
+	if (intr_stat & RK3576_HDMI_OHPD_INT) {
 		val = FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 1);
 
 		regmap_write(hdmi->regmap, RK3576_IOC_MISC_CON0, val);
@@ -381,6 +384,8 @@  static irqreturn_t dw_hdmi_qp_rk3588_hardirq(int irq, void *dev_id)
 
 	regmap_read(hdmi->regmap, RK3588_GRF_SOC_STATUS1, &intr_stat);
 
+	intr_stat &= hdmi->port_id ? RK3588_HDMI1_OHPD_INT : RK3588_HDMI0_OHPD_INT;
+
 	if (intr_stat) {
 		if (hdmi->port_id)
 			val = FIELD_PREP_WM16(RK3588_HDMI1_HPD_INT_MSK, 1);