Re:[PATCH v2 1/3] vop2: Add clock resets support

[Andy Yan]

Hi Detlev，


At 2024-05-23 02:57:48, "Detlev Casanova" <detlev.casanova@xxxxxxxxxxxxx> wrote:
>At the end of initialization, each VP clock needs to be reset before
>they can be used.
>
>Failing to do so can put the VOP in an undefined state where the
>generated HDMI signal is either lost or not matching the selected mode.

Would you please provide a detailed description of your test case?


>
>Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx>
>---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 30 ++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>index fdd768bbd487c..e81a67161d29a 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>@@ -17,6 +17,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
>+#include <linux/reset.h>
> #include <linux/swab.h>
> 
> #include <drm/drm.h>
>@@ -157,6 +158,7 @@ struct vop2_win {
> struct vop2_video_port {
> 	struct drm_crtc crtc;
> 	struct vop2 *vop2;
>+	struct reset_control *dclk_rst;
> 	struct clk *dclk;
> 	unsigned int id;
> 	const struct vop2_video_port_data *data;
>@@ -1915,6 +1917,26 @@ static int us_to_vertical_line(struct drm_display_mode *mode, int us)
> 	return us * mode->clock / mode->htotal / 1000;
> }
> 
>+static int vop2_clk_reset(struct vop2_video_port *vp)
>+{
>+	struct reset_control *rstc = vp->dclk_rst;
>+	struct vop2 *vop2 = vp->vop2;
>+	int ret;
>+
>+	if (!rstc)
>+		return 0;


In fact, this check is not necessary here.  The following reset control api will check for NULL pointer。

>+
>+	ret = reset_control_assert(rstc);
>+	if (ret < 0)
>+		drm_warn(vop2->drm, "failed to assert reset\n");
>+	udelay(10);
>+	ret = reset_control_deassert(rstc);
>+	if (ret < 0)
>+		drm_warn(vop2->drm, "failed to deassert reset\n");
>+
>+	return ret;
>+}
>+
> static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
> 				    struct drm_atomic_state *state)
> {
>@@ -2055,6 +2077,8 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
> 
> 	vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);
> 
>+	vop2_clk_reset(vp);
>+
> 	drm_crtc_vblank_on(crtc);
> 
> 	vop2_unlock(vop2);
>@@ -2706,6 +2730,12 @@ static int vop2_create_crtcs(struct vop2 *vop2)
> 		vp->data = vp_data;
> 
> 		snprintf(dclk_name, sizeof(dclk_name), "dclk_vp%d", vp->id);
>+		vp->dclk_rst = devm_reset_control_get_optional(vop2->dev, dclk_name);
>+		if (IS_ERR(vp->dclk_rst)) {
>+		        drm_err(vop2->drm, "failed to get %s reset\n", dclk_name);
>+		        return PTR_ERR(vp->dclk_rst);
>+		}
>+
> 		vp->dclk = devm_clk_get(vop2->dev, dclk_name);
> 		if (IS_ERR(vp->dclk)) {
> 			drm_err(vop2->drm, "failed to get %s\n", dclk_name);
>-- 
>2.44.1
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel