Re: [PATCH v10 30/69] drm/rockchip: dw_hdmi_qp: Cancel pending HPD work on suspend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Deadlock during suspend: disabling the main IRQ before cancelling the HPD work causes I2C timeouts.

Pre-existing issues:
- [High] Incomplete fix: HPD IRQ is not actually disabled, allowing delayed work to be re-armed during suspend.
- [High] Use-After-Free of `hdmi` due to devres/drmm mismatch on component unbind.
--

--- Patch [30]: [PATCH v10 30/69] drm/rockchip: dw_hdmi_qp: Cancel pending HPD work on suspend ---
commit 9b084d07cd142a4d5581877b274f1d064cd6ebd9
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Cancel pending HPD work on suspend

This commit cancels the pending HPD work during the suspend callback.
The intent is to prevent the work from probing the connector and reading
the EDID after the HDMI controller's power domain has been powered off.

Closes: https://lore.kernel.org/all/20260723015909.7F79A1F000E9@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 646a4cb968309..e9fda67b8ca94 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -728,8 +728,12 @@ static int __maybe_unused dw_hdmi_qp_rockchip_suspend(struct device *dev)
>  {
>  	struct rockchip_hdmi_qp *hdmi = dev_get_drvdata(dev);
>  
> -	if (hdmi)
> -		dw_hdmi_qp_suspend(dev, hdmi->hdmi);
> +	if (!hdmi)
> +		return 0;
> +
> +	dw_hdmi_qp_suspend(dev, hdmi->hdmi);
> +
> +	cancel_delayed_work_sync(&hdmi->hpd_work);

[Severity: High]
Does this code cause a deadlock during suspend if the HPD work is actively
running?

Because dw_hdmi_qp_suspend() is called first, it explicitly disables the
main IRQ.

If the HPD work is running and reads the EDID over DDC, the read relies on
dw_hdmi_qp_i2c_read(), which waits for a completion signaled by the main IRQ
handler.

Since the main IRQ is already disabled, will the I2C transfers time out
repeatedly, stalling cancel_delayed_work_sync() and completely hanging the
suspend process?

[Severity: High]
This isn't a bug introduced by this patch, but does this actually prevent the
work from running on unpowered hardware?

The commit message assumes drm_kms_helper_poll_disable() disables the HPD IRQ
via the bridge .hpd_disable() op.

However, rk3588_hdmi_phy_ops does not implement .disable_hpd:

static const struct dw_hdmi_qp_phy_ops rk3588_hdmi_phy_ops = {
	.init		= dw_hdmi_qp_rk3588_phy_init,
	.disable	= dw_hdmi_qp_rk3588_phy_disable,
	.read_hpd	= dw_hdmi_qp_rk3588_read_hpd,
	.setup_hpd	= dw_hdmi_qp_rk3588_setup_hpd,
};

This makes the DRM core's attempt to mask the IRQ a no-op, and the separate
HPD IRQ requested in dw_hdmi_qp_rockchip_bind() remains unmasked.

If a hotplug event occurs after cancel_delayed_work_sync() returns, but
before suspend_noirq disables all IRQs, will the handler fire and re-arm the
work on system_percpu_wq?

Since this workqueue is not freezable, could the work still execute after the
power domain is off?

[Severity: High]
This is a pre-existing issue, but does the component unbind path have a
use-after-free issue?

In dw_hdmi_qp_rockchip_bind(), the hdmi context is tied to the DRM device's
lifetime:

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

However, the HPD IRQ is tied to the platform device's lifetime:

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

When the component is unbound (e.g., driver removal or DRM teardown), the
DRM device frees all drmm memory, including the hdmi structure.

Because the platform device itself isn't removed, its devm resources are not
released, leaving the IRQ active.

If an HPD interrupt fires subsequently, will the handler dereference the
freed hdmi pointer?

>  
>  	return 0;
>  }

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