[PATCH v3 0/5] drm: rcar-du: DSI fixes

[PATCH v3 0/5] drm: rcar-du: DSI fixes

[Tomi Valkeinen]

Changes to v2:

- Move the LVDS patch to the beginning and improve the patch desc.
- Write 0 to registers in rcar_mipi_dsi_shutdown instead of just
  clearing a bit.
- Fix function names in "fix DSI enable & disable sequence" patch desc.
- Add "drm: rcar-du: dsi: Fix VCLKSET write"

 Tomi

Tomi Valkeinen (5):
  drm: rcar-du: lvds: Rename pclk enable/disable functions
  drm: rcar-du: dsi: Properly stop video mode TX
  drm: rcar-du: dsi: Improve DSI shutdown
  drm: rcar-du: fix DSI enable & disable sequence
  drm: rcar-du: dsi: Fix VCLKSET write

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c    | 30 +++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h     |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c |  4 ++
 drivers/gpu/drm/rcar-du/rcar_lvds.c       |  4 +-
 drivers/gpu/drm/rcar-du/rcar_lvds.h       | 10 ++--
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c   | 71 ++++++++++++++++++++---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h   | 31 ++++++++++
 7 files changed, 135 insertions(+), 17 deletions(-)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h

-- 
2.34.1

[PATCH v3 1/5] drm: rcar-du: lvds: Rename pclk enable/disable functions

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

The DU driver uses the rcar_lvds_clk_enable() and
rcar_lvds_clk_disable() functions enable or disable the pixel clock
generated by the LVDS encoder, as it requires that clock for proper DU
operation. Rename the functions by replacing "clk" with "pclk" to make
it clearer that they related to the pixel clock.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  4 ++--
 drivers/gpu/drm/rcar-du/rcar_lvds.c    |  4 ++--
 drivers/gpu/drm/rcar-du/rcar_lvds.h    | 10 +++++-----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index fd3b94649a01..d38092d85df7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -744,7 +744,7 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 		const struct drm_display_mode *mode =
 			&crtc->state->adjusted_mode;
 
-		rcar_lvds_clk_enable(bridge, mode->clock * 1000);
+		rcar_lvds_pclk_enable(bridge, mode->clock * 1000);
 	}
 
 	rcar_du_crtc_start(rcrtc);
@@ -777,7 +777,7 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
 		 * Disable the LVDS clock output, see
 		 * rcar_du_crtc_atomic_enable().
 		 */
-		rcar_lvds_clk_disable(bridge);
+		rcar_lvds_pclk_disable(bridge);
 	}
 
 	spin_lock_irq(&crtc->dev->event_lock);
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index d85aa4bc7f84..f438d7d858c7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -306,7 +306,7 @@ static void rcar_lvds_pll_setup_d3_e3(struct rcar_lvds *lvds, unsigned int freq)
  * Clock - D3/E3 only
  */
 
-int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq)
+int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 	int ret;
@@ -326,7 +326,7 @@ int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq)
 }
 EXPORT_SYMBOL_GPL(rcar_lvds_clk_enable);
 
-void rcar_lvds_clk_disable(struct drm_bridge *bridge)
+void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h
index 3097bf749bec..bee7033b60d6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.h
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h
@@ -13,17 +13,17 @@
 struct drm_bridge;
 
 #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
-int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq);
-void rcar_lvds_clk_disable(struct drm_bridge *bridge);
+int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq);
+void rcar_lvds_pclk_disable(struct drm_bridge *bridge);
 bool rcar_lvds_dual_link(struct drm_bridge *bridge);
 bool rcar_lvds_is_connected(struct drm_bridge *bridge);
 #else
-static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge,
-				       unsigned long freq)
+static inline int rcar_lvds_pclk_enable(struct drm_bridge *bridge,
+					unsigned long freq)
 {
 	return -ENOSYS;
 }
-static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { }
+static inline void rcar_lvds_pclk_disable(struct drm_bridge *bridge) { }
 static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge)
 {
 	return false;
-- 
2.34.1

[PATCH v3 2/5] drm: rcar-du: dsi: Properly stop video mode TX

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

The driver does not explicitly stop the video mode transmission when
disabling the output. While this doesn't seem to be causing any issues,
lets follow the steps described in the documentation and add a
rcar_mipi_dsi_stop_video() which stop the video mode transmission. This
function will also be used in later patches to stop the video
transmission even if the DSI IP is not shut down.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 29 +++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 62f7eb84ab01..7f2be490fcf8 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -542,6 +542,34 @@ static int rcar_mipi_dsi_start_video(struct rcar_mipi_dsi *dsi)
 	return 0;
 }
 
+static void rcar_mipi_dsi_stop_video(struct rcar_mipi_dsi *dsi)
+{
+	u32 status;
+	int ret;
+
+	/* Disable transmission in video mode. */
+	rcar_mipi_dsi_clr(dsi, TXVMCR, TXVMCR_EN_VIDEO);
+
+	ret = read_poll_timeout(rcar_mipi_dsi_read, status,
+				!(status & TXVMSR_ACT),
+				2000, 100000, false, dsi, TXVMSR);
+	if (ret < 0) {
+		dev_err(dsi->dev, "Failed to disable video transmission\n");
+		return;
+	}
+
+	/* Assert video FIFO clear. */
+	rcar_mipi_dsi_set(dsi, TXVMCR, TXVMCR_VFCLR);
+
+	ret = read_poll_timeout(rcar_mipi_dsi_read, status,
+				!(status & TXVMSR_VFRDY),
+				2000, 100000, false, dsi, TXVMSR);
+	if (ret < 0) {
+		dev_err(dsi->dev, "Failed to assert video FIFO clear\n");
+		return;
+	}
+}
+
 /* -----------------------------------------------------------------------------
  * Bridge
  */
@@ -601,6 +629,7 @@ static void rcar_mipi_dsi_atomic_disable(struct drm_bridge *bridge,
 {
 	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
 
+	rcar_mipi_dsi_stop_video(dsi);
 	rcar_mipi_dsi_shutdown(dsi);
 	rcar_mipi_dsi_clk_disable(dsi);
 }
-- 
2.34.1

[PATCH v3 3/5] drm: rcar-du: dsi: Improve DSI shutdown

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Improve the DSI shutdown procedure by clearing various bits that were
set while enabling the DSI output. There has been no clear issues caused
by these, but it's safer to ensure that the features are disabled at the
start of the next DSI enable.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 7f2be490fcf8..9c79fe2fc70b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -441,9 +441,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 
 static void rcar_mipi_dsi_shutdown(struct rcar_mipi_dsi *dsi)
 {
+	/* Disable VCLKEN */
+	rcar_mipi_dsi_write(dsi, VCLKSET, 0);
+
+	/* Disable DOT clock */
+	rcar_mipi_dsi_write(dsi, VCLKSET, 0);
+
 	rcar_mipi_dsi_clr(dsi, PHYSETUP, PHYSETUP_RSTZ);
 	rcar_mipi_dsi_clr(dsi, PHYSETUP, PHYSETUP_SHUTDOWNZ);
 
+	/* CFGCLK disable */
+	rcar_mipi_dsi_clr(dsi, CFGCLKSET, CFGCLKSET_CKEN);
+
+	/* LPCLK disable */
+	rcar_mipi_dsi_clr(dsi, LPCLKSET, LPCLKSET_CKEN);
+
 	dev_dbg(dsi->dev, "DSI device is shutdown\n");
 }
 
-- 
2.34.1

[PATCH v3 4/5] drm: rcar-du: fix DSI enable & disable sequence

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

The rcar crtc depends on the clock provided from the rcar DSI bridge.
When the DSI bridge is disabled, the clock is stopped, which causes the
crtc disable to timeout.

Also, while I have no issue with the enable, the documentation suggests
to enable the DSI before the crtc so that the crtc has its clock enabled
at enable time. This is also not done by the current driver.

To fix this, we need to keep the DSI bridge enabled until the crtc has
disabled itself, and enable the DSI bridge before crtc enables itself.

Add functions rcar_mipi_dsi_pclk_enable and rcar_mipi_dsi_pclk_disable
to the rcar DSI bridge driver which the rcar driver can use to
enable/disable the DSI clock when needed.  This is similar to what is
already done with the rcar LVDS bridge.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c    | 26 +++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_drv.h     |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c |  4 +++
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c   | 25 +++++++++++++-----
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h   | 31 +++++++++++++++++++++++
 5 files changed, 82 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index d38092d85df7..3619e1ddeb62 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -29,6 +29,7 @@
 #include "rcar_du_regs.h"
 #include "rcar_du_vsp.h"
 #include "rcar_lvds.h"
+#include "rcar_mipi_dsi.h"
 
 static u32 rcar_du_crtc_read(struct rcar_du_crtc *rcrtc, u32 reg)
 {
@@ -747,6 +748,18 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 		rcar_lvds_pclk_enable(bridge, mode->clock * 1000);
 	}
 
+	/*
+	 * Similarly to LVDS, on V3U the dot clock is provided by the DSI
+	 * encoder, and we need to enable the DSI clocks before enabling the CRTC.
+	 */
+	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
+	    (rstate->outputs &
+	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
+		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
+
+		rcar_mipi_dsi_pclk_enable(bridge, state);
+	}
+
 	rcar_du_crtc_start(rcrtc);
 
 	/*
@@ -780,6 +793,19 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
 		rcar_lvds_pclk_disable(bridge);
 	}
 
+	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
+	    (rstate->outputs &
+	     (BIT(RCAR_DU_OUTPUT_DSI0) | BIT(RCAR_DU_OUTPUT_DSI1)))) {
+		struct drm_bridge *bridge = rcdu->dsi[rcrtc->index];
+
+		/*
+		 * Disable the DSI clock output, see
+		 * rcar_du_crtc_atomic_enable().
+		 */
+
+		rcar_mipi_dsi_pclk_disable(bridge);
+	}
+
 	spin_lock_irq(&crtc->dev->event_lock);
 	if (crtc->state->event) {
 		drm_crtc_send_vblank_event(crtc, crtc->state->event);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index bfad7775d9a1..9367c2f59431 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -91,6 +91,7 @@ struct rcar_du_device_info {
 #define RCAR_DU_MAX_GROUPS		DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
 #define RCAR_DU_MAX_VSPS		4
 #define RCAR_DU_MAX_LVDS		2
+#define RCAR_DU_MAX_DSI			2
 
 struct rcar_du_device {
 	struct device *dev;
@@ -107,6 +108,7 @@ struct rcar_du_device {
 	struct platform_device *cmms[RCAR_DU_MAX_CRTCS];
 	struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
 	struct drm_bridge *lvds[RCAR_DU_MAX_LVDS];
+	struct drm_bridge *dsi[RCAR_DU_MAX_DSI];
 
 	struct {
 		struct drm_property *colorkey;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 60d6be78323b..ac93e08e8af4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -84,6 +84,10 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 		if (output == RCAR_DU_OUTPUT_LVDS0 ||
 		    output == RCAR_DU_OUTPUT_LVDS1)
 			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge;
+
+		if (output == RCAR_DU_OUTPUT_DSI0 ||
+		    output == RCAR_DU_OUTPUT_DSI1)
+			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = bridge;
 	}
 
 	/*
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 9c79fe2fc70b..06250f2f3499 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -598,7 +598,22 @@ static int rcar_mipi_dsi_attach(struct drm_bridge *bridge,
 static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
 					struct drm_bridge_state *old_bridge_state)
 {
-	struct drm_atomic_state *state = old_bridge_state->base.state;
+	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
+
+	rcar_mipi_dsi_start_video(dsi);
+}
+
+static void rcar_mipi_dsi_atomic_disable(struct drm_bridge *bridge,
+					 struct drm_bridge_state *old_bridge_state)
+{
+	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
+
+	rcar_mipi_dsi_stop_video(dsi);
+}
+
+void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_state *state)
+{
 	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
 	const struct drm_display_mode *mode;
 	struct drm_connector *connector;
@@ -626,8 +641,6 @@ static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
 	if (ret < 0)
 		goto err_dsi_start_hs;
 
-	rcar_mipi_dsi_start_video(dsi);
-
 	return;
 
 err_dsi_start_hs:
@@ -635,16 +648,16 @@ static void rcar_mipi_dsi_atomic_enable(struct drm_bridge *bridge,
 err_dsi_startup:
 	rcar_mipi_dsi_clk_disable(dsi);
 }
+EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_enable);
 
-static void rcar_mipi_dsi_atomic_disable(struct drm_bridge *bridge,
-					 struct drm_bridge_state *old_bridge_state)
+void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
 {
 	struct rcar_mipi_dsi *dsi = bridge_to_rcar_mipi_dsi(bridge);
 
-	rcar_mipi_dsi_stop_video(dsi);
 	rcar_mipi_dsi_shutdown(dsi);
 	rcar_mipi_dsi_clk_disable(dsi);
 }
+EXPORT_SYMBOL_GPL(rcar_mipi_dsi_pclk_disable);
 
 static enum drm_mode_status
 rcar_mipi_dsi_bridge_mode_valid(struct drm_bridge *bridge,
diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h
new file mode 100644
index 000000000000..ff759fc5edc7
--- /dev/null
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * R-Car DSI Encoder
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ *
+ * Contact: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
+ */
+
+#ifndef __RCAR_MIPI_DSI_H__
+#define __RCAR_MIPI_DSI_H__
+
+struct drm_atomic_state;
+struct drm_bridge;
+
+#if IS_ENABLED(CONFIG_DRM_RCAR_MIPI_DSI)
+void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
+			       struct drm_atomic_state *state);
+void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge);
+#else
+static inline void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
+					     struct drm_atomic_state *state)
+{
+}
+
+void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
+{
+}
+#endif /* CONFIG_DRM_RCAR_MIPI_DSI */
+
+#endif /* __RCAR_MIPI_DSI_H__ */
-- 
2.34.1

[PATCH v3 5/5] drm: rcar-du: dsi: Fix VCLKSET write

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses
or-operation to add the new values to the current value in the register,
it should first make sure the fields are cleared.

Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to
VCLKSET before the rest of the VCLKSET configuration.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 06250f2f3499..b60a6d4a5d46 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 			return ret;
 	}
 
-	/* Enable DOT clock */
-	vclkset = VCLKSET_CKEN;
-	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
+	/* Clear VCLKSET and enable DOT clock */
+	rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN);
+
+	vclkset = 0;
 
 	if (dsi_format == 24)
 		vclkset |= VCLKSET_BPP_24;
-- 
2.34.1

Re: [PATCH v3 5/5] drm: rcar-du: dsi: Fix VCLKSET write

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Mon, Aug 22, 2022 at 05:34:01PM +0300, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses
> or-operation to add the new values to the current value in the register,
> it should first make sure the fields are cleared.
> 
> Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to
> VCLKSET before the rest of the VCLKSET configuration.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 06250f2f3499..b60a6d4a5d46 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>  			return ret;
>  	}
>  
> -	/* Enable DOT clock */
> -	vclkset = VCLKSET_CKEN;
> -	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
> +	/* Clear VCLKSET and enable DOT clock */
> +	rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN);
> +
> +	vclkset = 0;
>  
>  	if (dsi_format == 24)
>  		vclkset |= VCLKSET_BPP_24;

You can replace one more set() with a write():

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 06250f2f3499..2744ea23e6f6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,

 	/* Enable DOT clock */
 	vclkset = VCLKSET_CKEN;
-	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
+	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);

 	if (dsi_format == 24)
 		vclkset |= VCLKSET_BPP_24;
@@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 	vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
 		|  VCLKSET_LANE(dsi->lanes - 1);

-	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
+	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);

 	/* After setting VCLKSET register, enable VCLKEN */
 	rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN);

I'll apply patches 1/5 to 4/5 to my tree already.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 5/5] drm: rcar-du: dsi: Fix VCLKSET write

[Tomi Valkeinen]

On 22/08/2022 17:45, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Aug 22, 2022 at 05:34:01PM +0300, Tomi Valkeinen wrote:
>> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>
>> rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses
>> or-operation to add the new values to the current value in the register,
>> it should first make sure the fields are cleared.
>>
>> Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to
>> VCLKSET before the rest of the VCLKSET configuration.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> index 06250f2f3499..b60a6d4a5d46 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
>> @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>>   			return ret;
>>   	}
>>   
>> -	/* Enable DOT clock */
>> -	vclkset = VCLKSET_CKEN;
>> -	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
>> +	/* Clear VCLKSET and enable DOT clock */
>> +	rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN);
>> +
>> +	vclkset = 0;
>>   
>>   	if (dsi_format == 24)
>>   		vclkset |= VCLKSET_BPP_24;
> 
> You can replace one more set() with a write():

That's true. I'll send a new one.

> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 06250f2f3499..2744ea23e6f6 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
> 
>   	/* Enable DOT clock */
>   	vclkset = VCLKSET_CKEN;
> -	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
> +	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
> 
>   	if (dsi_format == 24)
>   		vclkset |= VCLKSET_BPP_24;
> @@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>   	vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
>   		|  VCLKSET_LANE(dsi->lanes - 1);
> 
> -	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
> +	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
> 
>   	/* After setting VCLKSET register, enable VCLKEN */
>   	rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN);
> 
> I'll apply patches 1/5 to 4/5 to my tree already.

I'm not sure if 4 works correctly without 2.

  Tomi

[PATCH v3.1 5/5] drm: rcar-du: dsi: Fix VCLKSET write

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses
or-operation to add the new values to the current value in the register,
it should first make sure the fields are cleared.

Do this by using rcar_mipi_dsi_write() to write the VCLKSET register
with a variable that has all the unused bits zeroed.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 06250f2f3499..90128d5e3d17 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 
 	/* Enable DOT clock */
 	vclkset = VCLKSET_CKEN;
-	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
+	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
 
 	if (dsi_format == 24)
 		vclkset |= VCLKSET_BPP_24;
@@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 	vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
 		|  VCLKSET_LANE(dsi->lanes - 1);
 
-	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
+	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
 
 	/* After setting VCLKSET register, enable VCLKEN */
 	rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN);
-- 
2.34.1

Re: [PATCH v3 4/5] drm: rcar-du: fix DSI enable & disable sequence

[Tomi Valkeinen]

On 22/08/2022 17:34, Tomi Valkeinen wrote:

> +struct drm_atomic_state;
> +struct drm_bridge;
> +
> +#if IS_ENABLED(CONFIG_DRM_RCAR_MIPI_DSI)
> +void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
> +			       struct drm_atomic_state *state);
> +void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge);
> +#else
> +static inline void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
> +					     struct drm_atomic_state *state)
> +{
> +}
> +
> +void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
> +{
> +}

This one is missing static inline.

Laurent, do you want me to re-send the series, or do you already have 
this applied to your branch?

  Tomi

Re: [PATCH v3 5/5] drm: rcar-du: dsi: Fix VCLKSET write

[Laurent Pinchart]

On Tue, Aug 23, 2022 at 01:52:38PM +0300, Tomi Valkeinen wrote:
> On 22/08/2022 17:45, Laurent Pinchart wrote:
> > Hi Tomi,
> > 
> > Thank you for the patch.
> > 
> > On Mon, Aug 22, 2022 at 05:34:01PM +0300, Tomi Valkeinen wrote:
> >> From: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >>
> >> rcar_mipi_dsi_startup() writes correct values to VCLKSET, but as it uses
> >> or-operation to add the new values to the current value in the register,
> >> it should first make sure the fields are cleared.
> >>
> >> Do this by using rcar_mipi_dsi_write() to write the VCLKSET_CKEN bit to
> >> VCLKSET before the rest of the VCLKSET configuration.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> >> ---
> >>   drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 7 ++++---
> >>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> >> index 06250f2f3499..b60a6d4a5d46 100644
> >> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> >> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> >> @@ -412,9 +412,10 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
> >>   			return ret;
> >>   	}
> >>   
> >> -	/* Enable DOT clock */
> >> -	vclkset = VCLKSET_CKEN;
> >> -	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
> >> +	/* Clear VCLKSET and enable DOT clock */
> >> +	rcar_mipi_dsi_write(dsi, VCLKSET, VCLKSET_CKEN);
> >> +
> >> +	vclkset = 0;
> >>   
> >>   	if (dsi_format == 24)
> >>   		vclkset |= VCLKSET_BPP_24;
> > 
> > You can replace one more set() with a write():
> 
> That's true. I'll send a new one.
> 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > index 06250f2f3499..2744ea23e6f6 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > @@ -414,7 +414,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
> > 
> >   	/* Enable DOT clock */
> >   	vclkset = VCLKSET_CKEN;
> > -	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
> > +	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
> > 
> >   	if (dsi_format == 24)
> >   		vclkset |= VCLKSET_BPP_24;
> > @@ -429,7 +429,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
> >   	vclkset |= VCLKSET_COLOR_RGB | VCLKSET_DIV(setup_info.div)
> >   		|  VCLKSET_LANE(dsi->lanes - 1);
> > 
> > -	rcar_mipi_dsi_set(dsi, VCLKSET, vclkset);
> > +	rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
> > 
> >   	/* After setting VCLKSET register, enable VCLKEN */
> >   	rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN);
> > 
> > I'll apply patches 1/5 to 4/5 to my tree already.
> 
> I'm not sure if 4 works correctly without 2.

I meant 1/5 to 4/5, not 1/5 and 4/5 :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 4/5] drm: rcar-du: fix DSI enable & disable sequence

[Laurent Pinchart]

On Tue, Aug 23, 2022 at 02:04:21PM +0300, Tomi Valkeinen wrote:
> On 22/08/2022 17:34, Tomi Valkeinen wrote:
> 
> > +struct drm_atomic_state;
> > +struct drm_bridge;
> > +
> > +#if IS_ENABLED(CONFIG_DRM_RCAR_MIPI_DSI)
> > +void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
> > +			       struct drm_atomic_state *state);
> > +void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge);
> > +#else
> > +static inline void rcar_mipi_dsi_pclk_enable(struct drm_bridge *bridge,
> > +					     struct drm_atomic_state *state)
> > +{
> > +}
> > +
> > +void rcar_mipi_dsi_pclk_disable(struct drm_bridge *bridge)
> > +{
> > +}
> 
> This one is missing static inline.
> 
> Laurent, do you want me to re-send the series, or do you already have 
> this applied to your branch?

I'll fix it locally.

-- 
Regards,

Laurent Pinchart