mbox series

[v2,0/3] drm: rockchip: vop2: Add VP clock resets support

Message ID 20240522185924.461742-1-detlev.casanova@collabora.com
Headers show
Series drm: rockchip: vop2: Add VP clock resets support | expand

Message

Detlev Casanova May 22, 2024, 6:57 p.m. UTC 
The clock reset must be used when the VOP is configured. Skipping it can
put the VOP in an unknown state where the HDMI signal is either lost or
not matching the selected mode.

This adds support for rk3588(s) based SoCs.

Changes since v1:
- Add AXI and AHB clock resets
- Set maxItems for !rk3588 in vop2 bindings

Detlev Casanova (3):
  vop2: Add clock resets support
  arm64: dts: rockchip: Add VOP clock resets for rk3588s
  dt-bindings: display: vop2: Add VP clock resets

 .../display/rockchip/rockchip-vop2.yaml       | 40 +++++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 12 ++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c  | 30 ++++++++++++++
 3 files changed, 82 insertions(+)

Comments

Andy Yan May 24, 2024, 3:09 a.m. UTC | #1 
Hi Detlev，


At 2024-05-23 02:57:48, "Detlev Casanova" <detlev.casanova@collabora.com> 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@collabora.com>
>---
> 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@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel