diff mbox series

[v10,34/69] drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init

Message ID 20260731-dw-hdmi-qp-scramb-v10-34-294364b2cf15@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 31, 2026, 4:19 p.m. UTC 
Start with HPD masked, matching RK3588, so the bridge framework controls
when interrupts are armed via hpd_enable().

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 31, 2026, 9:14 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Logic error: HPD interrupt is unconditionally unmasked during `setup_hpd`, defeating the patch's intent to defer arming to the bridge framework.

Pre-existing issues:
- [High] High-severity Use-After-Free and resource leaks due to improper use of `devm_*` APIs in the component `bind` callback.
- [High] RK3588 HPD IRQ handler clears the interrupt status a second time when unmasking, causing potential loss of hotplug events.
--

--- Patch [34]: [PATCH v10 34/69] drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init ---
commit 1e56140d6c0038883d14ac31b8ac46f871e3f59e
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Mask RK3576 HPD IRQ in io_init

This commit masks the HPD interrupt during I/O initialization for RK3576,
intending to defer interrupt arming to the bridge framework.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> --- 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]
Does this change actually defer the interrupt arming?

Looking at dw_hdmi_qp_rk3576_setup_hpd() in the same file, the interrupt
appears to be unconditionally unmasked later during hardware initialization:

drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rk3576_setup_hpd() {
    ...
    val = (FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_CLR, 1) | FIELD_PREP_WM16(RK3576_HDMI_HPD_INT_MSK, 0));
    ...
}

Additionally, it appears that rk3576_hdmi_phy_ops lacks the .enable_hpd and
.disable_hpd callbacks. Can the bridge framework actually control the
interrupt without these callbacks implemented?

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
in the component bind/unbind lifecycle.

In dw_hdmi_qp_rockchip_bind(), devm_request_threaded_irq() is used to
allocate the IRQ handler on the platform device:

drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_bind() {
    ...
    ret = devm_request_threaded_irq(dev, irq, ...);
    ...
}

However, the dev_id passed is hdmi, which is allocated via
drmm_kzalloc(). Since devm_ resources are tied to the device probe/remove
lifecycle rather than the component bind/unbind lifecycle, the IRQ is not
freed in dw_hdmi_qp_rockchip_unbind().

If the master unbinds, the drmm memory is freed, but the IRQ handler remains
active. Could an interrupt firing after unbind cause a use-after-free when
it accesses the freed hdmi pointer?

[Severity: High]
This is a pre-existing issue, but does dw_hdmi_qp_rk3588_irq() accidentally
clear the interrupt a second time when unmasking?

drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rk3588_irq() {
    ...
    val |= FIELD_PREP_WM16(RK3588_HDMI0_HPD_INT_MSK, 0);
    regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val);
    ...
}

Because this uses a bitwise OR, val retains the RK3588_HDMI0_HPD_INT_CLR
write-enable mask and data bit from earlier in the function. Does this
second regmap_write() clear the interrupt again, potentially losing hotplug
events that occurred between the two writes?
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 f5342c5f457b..64e8e4c12cbd 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);
 }