diff mbox series

[v9,28/61] drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init

Message ID 20260723-dw-hdmi-qp-scramb-v9-28-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 
Start with HPD masked, matching RK3588, so the bridge framework controls
when interrupts are armed via hpd_enable().

Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

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

New issues:
- [Medium] Incomplete/Defeated HPD Interrupt Deferral Implementation

Pre-existing issues:
- [High] Use-After-Free and resource leak due to mismatched drmm/devm component lifecycles
--

--- Patch [28]: [PATCH v9 28/61] drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init ---
commit 14fe26855150a57c0302feac3b2461064deead56
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Date:   Thu Jul 23 01:35:17 2026 +0000

    drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init
    
    Start with HPD masked, matching RK3588, so the bridge framework controls
    when interrupts are armed via hpd_enable().

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 7e3e86d056116..12cd8db7cc50b 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -432,7 +432,7 @@ static void dw_hdmi_qp_rk3576_io_init(struct rockchip_hdmi_qp *hdmi)
>  
>  	regmap_write(hdmi->vo_regmap, RK3576_VO0_GRF_SOC_CON14, val);
>  
> -	val = FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 0);
> +	val = FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 1);
>  	regmap_write(hdmi->regmap, RK3576_IOC_MISC_CON0, val);
>  }

[Severity: Medium]
The commit message states the goal is for the bridge framework to control
when interrupts are armed, but does this actually happen?

It looks like the required .enable_hpd and .disable_hpd callbacks are
not implemented in rk3576_hdmi_phy_ops:

static const struct dw_hdmi_qp_phy_ops rk3576_hdmi_phy_ops = {
	.init		= dw_hdmi_qp_rk3588_phy_init,
	.disable	= dw_hdmi_qp_rk3588_phy_disable,
	.read_hpd	= dw_hdmi_qp_rk3576_read_hpd,
	.setup_hpd	= dw_hdmi_qp_rk3576_setup_hpd,
};

Without these callbacks, the bridge framework's attempts to unmask HPD
will be no-ops.

Furthermore, dw_hdmi_qp_init_hw() in drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
unconditionally calls .setup_hpd() during bind:

	if (hdmi->phy.ops->setup_hpd)
		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);

And dw_hdmi_qp_rk3576_setup_hpd() writes 0 to the mask bit, overriding
the mask set here in io_init:

	val = (FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_CLR, 1) |
	       FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 0));

Does this completely bypass the intended deferral and leave the interrupt
armed immediately during bind?

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk during
unbind due to mismatched lifecycles?

The context structure is allocated tied to the DRM master's lifecycle using
drmm_kzalloc() in dw_hdmi_qp_rockchip_bind():

	hdmi = drmm_kzalloc(drm, sizeof(*hdmi), GFP_KERNEL);

However, the HPD interrupt is requested using devm_request_threaded_irq()
on the component device, which outlives the DRM master across unbinds or
probe failures:

	ret = devm_request_threaded_irq(dev, irq,
					cfg->ctrl_ops->hardirq_callback,
					cfg->ctrl_ops->irq_callback,
					IRQF_SHARED, "dw-hdmi-qp-hpd",
					hdmi);

The component unbind function fails to explicitly free the IRQ:

static void dw_hdmi_qp_rockchip_unbind(struct device *dev,
				       struct device *master,
				       void *data)
{
	struct rockchip_hdmi_qp *hdmi = dev_get_drvdata(dev);

	cancel_delayed_work_sync(&hdmi->hpd_work);
}

If the DRM master unbinds, the DRM device is destroyed and hdmi is freed.
Will the IRQ remain active on the component device and dereference the
freed hdmi pointer if the interrupt fires?
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 7e3e86d05611..12cd8db7cc50 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -432,7 +432,7 @@  static void dw_hdmi_qp_rk3576_io_init(struct rockchip_hdmi_qp *hdmi)
 
 	regmap_write(hdmi->vo_regmap, RK3576_VO0_GRF_SOC_CON14, val);
 
-	val = FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 0);
+	val = FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 1);
 	regmap_write(hdmi->regmap, RK3576_IOC_MISC_CON0, val);
 }