Re: [PATCH v6 07/15] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/rockchip: vop2: missing of_node_put() on error in vop2_create_crtcs()
- [Medium] drm/rockchip: vop2: axi_rst pulsed in vop2_bind() without enabling aclk or PM
--

commit 7b3ce842063d5aafe025d15a4339ddc7bddb6893
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

This commit adds optional AXI and VP DCLK reset controls to the Rockchip VOP2
driver, pulsing the AXI reset during bind and CRTC disable and the VP DCLK
reset during CRTC enable to leave the hardware in a clean state.

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index b85cd94d1e9d9..98430587a7abe 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

[ ... ]

> @@ -2585,6 +2610,12 @@ static int vop2_create_crtcs(struct vop2 *vop2)
>  			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk),
>  					     "failed to get %s\n", dclk_name);
>  
> +		vp->dclk_rst = devm_reset_control_get_optional_exclusive(vop2->dev,
> +									 dclk_name);
> +		if (IS_ERR(vp->dclk_rst))
> +			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk_rst),
> +					     "failed to get %s reset\n", dclk_name);

[Severity: Medium]
Does returning directly here leak the port device_node references acquired in
earlier iterations of this loop?

In vop2_create_crtcs(), each iteration for a connected video port acquires a
port reference at the end of the loop body:

drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:vop2_create_crtcs() {
	for (i = 0; i < vop2_data->nr_vps; i++) {
		...
		port = of_graph_get_port_by_id(dev->of_node, i);
		if (!port)
			return dev_err_probe(drm->dev, -ENOENT,
					     "no port node found for video_port%d\n", i);
		vp->crtc.port = port;
		nvps++;
	}
}

If devm_reset_control_get_optional_exclusive() fails on a subsequent iteration
(i > 0), vop2_create_crtcs() returns immediately to vop2_bind(), which also
returns the error without calling of_node_put() on vop2->vps[0 .. i - 1].crtc.port
(a pre-existing issue also shared with the other early returns in
vop2_create_crtcs()).

> @@ -2944,6 +2975,13 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
>  		return dev_err_probe(drm->dev, PTR_ERR(vop2->pll_hdmiphy1),
>  				     "failed to get pll_hdmiphy1\n");
>  
> +	vop2->axi_rst = devm_reset_control_get_optional_exclusive(vop2->dev, "axi");
> +	if (IS_ERR(vop2->axi_rst))
> +		return dev_err_probe(drm->dev, PTR_ERR(vop2->axi_rst),
> +				     "failed to get axi reset\n");
> +
> +	vop2_reset_assert_deassert(vop2, vop2->axi_rst);

[Severity: Medium]
As reported by Sashiko on v5 [1], can pulsing vop2->axi_rst here in
vop2_bind() take effect if the VOP2 power domain is powered down or
vop2->aclk is gated?

In vop2_bind(), vop2_reset_assert_deassert() is called before
pm_runtime_enable(&pdev->dev) and without resuming runtime PM or enabling
vop2->aclk. After vop2_probe() completes, genpd_dev_pm_sync() schedules
power-off for the RK3588_PD_VOP domain (which uses GENPD_FLAG_PM_CLK |
GENPD_FLAG_NO_STAY_ON), and the core clocks are only prepared and enabled
later in vop2_enable():

drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:vop2_enable() {
	ret = pm_runtime_resume_and_get(vop2->dev);
	if (ret < 0) {
		drm_err(vop2->drm, "failed to get pm runtime: %d\n", ret);
		return ret;
	}

	ret = vop2_core_clks_prepare_enable(vop2);
	if (ret)
		goto err_put_pm;
}

By contrast, vop2_disable() pulses vop2->axi_rst while runtime PM and the core
clocks are still active, and vop_initial() in rockchip_drm_vop.c resumes
runtime PM and enables hclk and aclk before pulsing resets. Should this reset
be performed while the power domain and aclk are enabled?

[1] https://lore.kernel.org/r/20261014-dw-hdmi-qp-yuv-v5-7-6fab0fe43a9b@collabora.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-dw-hdmi-qp-yuv-v6-0-1bb63ef5766e@collabora.com?part=7