diff mbox series

[v3,1/3] vop2: Add clock resets support

Message ID 20241108185212.198603-2-detlev.casanova@collabora.com (mailing list archive)
State New
Headers show
Series drm: rockchip: vop2: Add VP clock resets support | expand

Commit Message

Detlev Casanova Nov. 8, 2024, 6:50 p.m. UTC
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.

This issue can be reproduced by switching modes multiple times.
Depending on the setup, after about 10 mode switches, the signal will be
lost and the value in register 0x890 (VSYNCWIDTH + VFRONT) will take the value
`0x0000018c`.
That makes VSYNCWIDTH=0, which is wrong.

Adding the clock resets after the VOP configuration fixes the issue.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 30 ++++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

Heiko Stuebner Nov. 15, 2024, 8:34 a.m. UTC | #1
Hi Detlev,

Am Freitag, 8. November 2024, 19:50:39 CET schrieb Detlev Casanova:
> 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.
> 
> This issue can be reproduced by switching modes multiple times.
> Depending on the setup, after about 10 mode switches, the signal will be
> lost and the value in register 0x890 (VSYNCWIDTH + VFRONT) will take the value
> `0x0000018c`.
> That makes VSYNCWIDTH=0, which is wrong.
> 
> Adding the clock resets after the VOP configuration fixes the issue.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>

patch subject should be "drm/rockchip: vop2: ...." please


>  static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>  				    struct drm_atomic_state *state)
>  {
> @@ -2057,6 +2079,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);

this conflicts with the merge gamma lut support, can you please rebase on
top of drm-misc-next? Or alternatively just tell me if it should be

	vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);

+	vop2_clk_reset(vp);
+
	vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);

	drm_crtc_vblank_on(crtc);

----- or ----
	vop2_vp_write(vp, RK3568_VP_DSP_CTRL, dsp_ctrl);

	vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);

+	vop2_clk_reset(vp);
+
	drm_crtc_vblank_on(crtc);


Thanks a lot
Heiko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 9873172e3fd3..6122eb18e6c9 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;
@@ -1917,6 +1919,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;
+
+	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)
 {
@@ -2057,6 +2079,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);
@@ -2708,6 +2732,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);