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

[sashiko-bot@kernel.org]

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-dw-hdmi-qp-scramb-v10-0-294364b2cf15@collabora.com?part=34