All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] drm: rcar-du: Misc patches
@ 2023-01-20  8:50 ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Tomi Valkeinen

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

Hi,

v2 of the series. Diff to v1 can be found below.

The main changes are:
- Runtime PM support for LVDS
- Skip rcar_du_group_setup() for gen4+

 Tomi

Koji Matsuoka (1):
  drm: rcar-du: lvds: Fix stop sequence

Tomi Valkeinen (6):
  drm: rcar-du: dsi: add 'select RESET_CONTROLLER'
  drm: rcar-du: lvds: Add runtime PM
  drm: rcar-du: lvsd: Add reset control
  drm: rcar-du: Add quirk for H3 ES1.x pclk workaround
  drm: rcar-du: Fix setting a reserved bit in DPLLCR
  drm: rcar-du: Stop accessing non-existant registers on gen4

 drivers/gpu/drm/rcar-du/Kconfig         |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 39 +++++++----
 drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 49 ++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 +++-
 drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  8 +--
 drivers/gpu/drm/rcar-du/rcar_lvds.c     | 86 +++++++++++++++++++++++--
 7 files changed, 169 insertions(+), 28 deletions(-)

Interdiff against v1:
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 56b23333993c..008e172ed43b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -249,15 +249,24 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 			dpllcr |= DPLLCR_PLCS1
 			       |  DPLLCR_INCS_DOTCLKIN1;
 		} else {
-			dpllcr |= DPLLCR_PLCS0
+			dpllcr |= DPLLCR_PLCS0_PLL
 			       |  DPLLCR_INCS_DOTCLKIN0;
+
 			/*
-			 * On H3 ES1.x, in addition to setting bit 21 (PLCS0),
-			 * also bit 20 has to be set to select PLL1 as the
-			 * clock source.
+			 * On ES2.x we have a single mux controlled via bit 21,
+			 * which selects between DCLKIN source (bit 21 = 0) and
+			 * a PLL source (bit 21 = 1), where the PLL is always
+			 * PLL1.
+			 *
+			 * On ES1.x we have an additional mux, controlled
+			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
+			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
+			 * so on ES1.x, in addition to setting bit 21, we need
+			 * to set the bit 20.
 			 */
+
 			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
-				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1_SEL;
+				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
 		}
 
 		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index d689f2510081..82f9718ff500 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -710,10 +710,10 @@ static void rcar_du_shutdown(struct platform_device *pdev)
 
 static int rcar_du_probe(struct platform_device *pdev)
 {
+	const struct soc_device_attribute *soc_attr;
 	struct rcar_du_device *rcdu;
 	unsigned int mask;
 	int ret;
-	const struct soc_device_attribute *soc_attr;
 
 	if (drm_firmware_drivers_only())
 		return -ENODEV;
@@ -726,13 +726,12 @@ static int rcar_du_probe(struct platform_device *pdev)
 
 	rcdu->dev = &pdev->dev;
 
+	rcdu->info = of_device_get_match_data(rcdu->dev);
+
 	soc_attr = soc_device_match(rcar_du_soc_table);
 	if (soc_attr)
 		rcdu->info = soc_attr->data;
 
-	if (!rcdu->info)
-		rcdu->info = of_device_get_match_data(rcdu->dev);
-
 	platform_set_drvdata(pdev, rcdu);
 
 	/* I/O resources */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index c236e2aa8a01..c2209d427bb7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -194,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
  */
 int rcar_du_group_get(struct rcar_du_group *rgrp)
 {
+	struct rcar_du_device *rcdu = rgrp->dev;
+
 	if (rgrp->use_count)
 		goto done;
 
-	rcar_du_group_setup(rgrp);
+	if (rcdu->info->gen < 4)
+		rcar_du_group_setup(rgrp);
 
 done:
 	rgrp->use_count++;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
index 94d913f66c8f..789ae9285108 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
@@ -283,13 +283,8 @@
 #define DPLLCR			0x20044
 #define DPLLCR_CODE		(0x95 << 24)
 #define DPLLCR_PLCS1		(1 << 23)
-/*
- * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
- * isn't implemented by other SoC in the Gen3 family it can safely be set
- * unconditionally.
- */
-#define DPLLCR_PLCS0		(1 << 21)
-#define DPLLCR_PLCS0_H3ES1X_PLL1_SEL	(1 << 20)
+#define DPLLCR_PLCS0_PLL	(1 << 21)
+#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
 #define DPLLCR_CLKE		(1 << 18)
 #define DPLLCR_FDPLL(n)		((n) << 12)
 #define DPLLCR_N(n)		((n) << 5)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 01800cef1c33..8fa5f7400179 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -16,6 +16,7 @@
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
@@ -82,14 +83,14 @@ struct rcar_lvds {
 #define bridge_to_rcar_lvds(b) \
 	container_of(b, struct rcar_lvds, bridge)
 
-static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
+static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
 {
-	iowrite32(data, lvds->mmio + reg);
+	return ioread32(lvds->mmio + reg);
 }
 
-static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
+static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
 {
-	return ioread32(lvds->mmio + reg);
+	iowrite32(data, lvds->mmio + reg);
 }
 
 /* -----------------------------------------------------------------------------
@@ -323,10 +324,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
 
 	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
 
-	reset_control_deassert(lvds->rstc);
-
-	ret = clk_prepare_enable(lvds->clocks.mod);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
 		return ret;
 
 	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
@@ -346,9 +345,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
 
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
-	clk_disable_unprepare(lvds->clocks.mod);
-
-	reset_control_assert(lvds->rstc);
+	pm_runtime_put(lvds->dev);
 }
 EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
 
@@ -407,10 +404,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
 	u32 lvdcr0;
 	int ret;
 
-	reset_control_deassert(lvds->rstc);
-
-	ret = clk_prepare_enable(lvds->clocks.mod);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
 		return;
 
 	/* Enable the companion LVDS encoder in dual-link mode. */
@@ -576,6 +571,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 	}
 
+	rcar_lvds_write(lvds, LVDCR0, 0);
 	rcar_lvds_write(lvds, LVDCR1, 0);
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
@@ -584,8 +580,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 		lvds->companion->funcs->atomic_disable(lvds->companion,
 						       old_bridge_state);
 
-	clk_disable_unprepare(lvds->clocks.mod);
-	reset_control_assert(lvds->rstc);
+	pm_runtime_put(lvds->dev);
 }
 
 static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
@@ -878,11 +873,11 @@ static int rcar_lvds_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	lvds->rstc = devm_reset_control_get(&pdev->dev, NULL);
-	if (IS_ERR(lvds->rstc)) {
-		dev_err(&pdev->dev, "failed to get cpg reset\n");
-		return PTR_ERR(lvds->rstc);
-	}
+	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(lvds->rstc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
+				     "failed to get cpg reset\n");
+	pm_runtime_enable(&pdev->dev);
 
 	drm_bridge_add(&lvds->bridge);
 
@@ -895,6 +890,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
 
 	drm_bridge_remove(&lvds->bridge);
 
+	pm_runtime_disable(&pdev->dev);
+
 	return 0;
 }
 
@@ -953,11 +950,48 @@ static const struct of_device_id rcar_lvds_of_table[] = {
 
 MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
 
+static int rcar_lvds_runtime_suspend(struct device *dev)
+{
+	struct rcar_lvds *lvds = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(lvds->clocks.mod);
+
+	reset_control_assert(lvds->rstc);
+
+	return 0;
+}
+
+static int rcar_lvds_runtime_resume(struct device *dev)
+{
+	struct rcar_lvds *lvds = dev_get_drvdata(dev);
+	int ret;
+
+	ret = reset_control_deassert(lvds->rstc);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(lvds->clocks.mod);
+	if (ret < 0)
+		goto err_reset_assert;
+
+	return 0;
+
+err_reset_assert:
+	reset_control_assert(lvds->rstc);
+
+	return ret;
+}
+
+static const struct dev_pm_ops rcar_lvds_pm_ops = {
+	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
+};
+
 static struct platform_driver rcar_lvds_platform_driver = {
 	.probe		= rcar_lvds_probe,
 	.remove		= rcar_lvds_remove,
 	.driver		= {
 		.name	= "rcar-lvds",
+		.pm	= &rcar_lvds_pm_ops,
 		.of_match_table = rcar_lvds_of_table,
 	},
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 0/7] drm: rcar-du: Misc patches
@ 2023-01-20  8:50 ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Tomi Valkeinen, Geert Uytterhoeven

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

Hi,

v2 of the series. Diff to v1 can be found below.

The main changes are:
- Runtime PM support for LVDS
- Skip rcar_du_group_setup() for gen4+

 Tomi

Koji Matsuoka (1):
  drm: rcar-du: lvds: Fix stop sequence

Tomi Valkeinen (6):
  drm: rcar-du: dsi: add 'select RESET_CONTROLLER'
  drm: rcar-du: lvds: Add runtime PM
  drm: rcar-du: lvsd: Add reset control
  drm: rcar-du: Add quirk for H3 ES1.x pclk workaround
  drm: rcar-du: Fix setting a reserved bit in DPLLCR
  drm: rcar-du: Stop accessing non-existant registers on gen4

 drivers/gpu/drm/rcar-du/Kconfig         |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 39 +++++++----
 drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 49 ++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  2 +
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 +++-
 drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  8 +--
 drivers/gpu/drm/rcar-du/rcar_lvds.c     | 86 +++++++++++++++++++++++--
 7 files changed, 169 insertions(+), 28 deletions(-)

Interdiff against v1:
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 56b23333993c..008e172ed43b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -249,15 +249,24 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 			dpllcr |= DPLLCR_PLCS1
 			       |  DPLLCR_INCS_DOTCLKIN1;
 		} else {
-			dpllcr |= DPLLCR_PLCS0
+			dpllcr |= DPLLCR_PLCS0_PLL
 			       |  DPLLCR_INCS_DOTCLKIN0;
+
 			/*
-			 * On H3 ES1.x, in addition to setting bit 21 (PLCS0),
-			 * also bit 20 has to be set to select PLL1 as the
-			 * clock source.
+			 * On ES2.x we have a single mux controlled via bit 21,
+			 * which selects between DCLKIN source (bit 21 = 0) and
+			 * a PLL source (bit 21 = 1), where the PLL is always
+			 * PLL1.
+			 *
+			 * On ES1.x we have an additional mux, controlled
+			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
+			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
+			 * so on ES1.x, in addition to setting bit 21, we need
+			 * to set the bit 20.
 			 */
+
 			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
-				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1_SEL;
+				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
 		}
 
 		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index d689f2510081..82f9718ff500 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -710,10 +710,10 @@ static void rcar_du_shutdown(struct platform_device *pdev)
 
 static int rcar_du_probe(struct platform_device *pdev)
 {
+	const struct soc_device_attribute *soc_attr;
 	struct rcar_du_device *rcdu;
 	unsigned int mask;
 	int ret;
-	const struct soc_device_attribute *soc_attr;
 
 	if (drm_firmware_drivers_only())
 		return -ENODEV;
@@ -726,13 +726,12 @@ static int rcar_du_probe(struct platform_device *pdev)
 
 	rcdu->dev = &pdev->dev;
 
+	rcdu->info = of_device_get_match_data(rcdu->dev);
+
 	soc_attr = soc_device_match(rcar_du_soc_table);
 	if (soc_attr)
 		rcdu->info = soc_attr->data;
 
-	if (!rcdu->info)
-		rcdu->info = of_device_get_match_data(rcdu->dev);
-
 	platform_set_drvdata(pdev, rcdu);
 
 	/* I/O resources */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index c236e2aa8a01..c2209d427bb7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -194,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
  */
 int rcar_du_group_get(struct rcar_du_group *rgrp)
 {
+	struct rcar_du_device *rcdu = rgrp->dev;
+
 	if (rgrp->use_count)
 		goto done;
 
-	rcar_du_group_setup(rgrp);
+	if (rcdu->info->gen < 4)
+		rcar_du_group_setup(rgrp);
 
 done:
 	rgrp->use_count++;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
index 94d913f66c8f..789ae9285108 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
@@ -283,13 +283,8 @@
 #define DPLLCR			0x20044
 #define DPLLCR_CODE		(0x95 << 24)
 #define DPLLCR_PLCS1		(1 << 23)
-/*
- * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
- * isn't implemented by other SoC in the Gen3 family it can safely be set
- * unconditionally.
- */
-#define DPLLCR_PLCS0		(1 << 21)
-#define DPLLCR_PLCS0_H3ES1X_PLL1_SEL	(1 << 20)
+#define DPLLCR_PLCS0_PLL	(1 << 21)
+#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
 #define DPLLCR_CLKE		(1 << 18)
 #define DPLLCR_FDPLL(n)		((n) << 12)
 #define DPLLCR_N(n)		((n) << 5)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 01800cef1c33..8fa5f7400179 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -16,6 +16,7 @@
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
@@ -82,14 +83,14 @@ struct rcar_lvds {
 #define bridge_to_rcar_lvds(b) \
 	container_of(b, struct rcar_lvds, bridge)
 
-static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
+static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
 {
-	iowrite32(data, lvds->mmio + reg);
+	return ioread32(lvds->mmio + reg);
 }
 
-static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
+static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
 {
-	return ioread32(lvds->mmio + reg);
+	iowrite32(data, lvds->mmio + reg);
 }
 
 /* -----------------------------------------------------------------------------
@@ -323,10 +324,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
 
 	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
 
-	reset_control_deassert(lvds->rstc);
-
-	ret = clk_prepare_enable(lvds->clocks.mod);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
 		return ret;
 
 	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
@@ -346,9 +345,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
 
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
-	clk_disable_unprepare(lvds->clocks.mod);
-
-	reset_control_assert(lvds->rstc);
+	pm_runtime_put(lvds->dev);
 }
 EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
 
@@ -407,10 +404,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
 	u32 lvdcr0;
 	int ret;
 
-	reset_control_deassert(lvds->rstc);
-
-	ret = clk_prepare_enable(lvds->clocks.mod);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
 		return;
 
 	/* Enable the companion LVDS encoder in dual-link mode. */
@@ -576,6 +571,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 	}
 
+	rcar_lvds_write(lvds, LVDCR0, 0);
 	rcar_lvds_write(lvds, LVDCR1, 0);
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
@@ -584,8 +580,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 		lvds->companion->funcs->atomic_disable(lvds->companion,
 						       old_bridge_state);
 
-	clk_disable_unprepare(lvds->clocks.mod);
-	reset_control_assert(lvds->rstc);
+	pm_runtime_put(lvds->dev);
 }
 
 static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
@@ -878,11 +873,11 @@ static int rcar_lvds_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	lvds->rstc = devm_reset_control_get(&pdev->dev, NULL);
-	if (IS_ERR(lvds->rstc)) {
-		dev_err(&pdev->dev, "failed to get cpg reset\n");
-		return PTR_ERR(lvds->rstc);
-	}
+	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(lvds->rstc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
+				     "failed to get cpg reset\n");
+	pm_runtime_enable(&pdev->dev);
 
 	drm_bridge_add(&lvds->bridge);
 
@@ -895,6 +890,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
 
 	drm_bridge_remove(&lvds->bridge);
 
+	pm_runtime_disable(&pdev->dev);
+
 	return 0;
 }
 
@@ -953,11 +950,48 @@ static const struct of_device_id rcar_lvds_of_table[] = {
 
 MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
 
+static int rcar_lvds_runtime_suspend(struct device *dev)
+{
+	struct rcar_lvds *lvds = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(lvds->clocks.mod);
+
+	reset_control_assert(lvds->rstc);
+
+	return 0;
+}
+
+static int rcar_lvds_runtime_resume(struct device *dev)
+{
+	struct rcar_lvds *lvds = dev_get_drvdata(dev);
+	int ret;
+
+	ret = reset_control_deassert(lvds->rstc);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(lvds->clocks.mod);
+	if (ret < 0)
+		goto err_reset_assert;
+
+	return 0;
+
+err_reset_assert:
+	reset_control_assert(lvds->rstc);
+
+	return ret;
+}
+
+static const struct dev_pm_ops rcar_lvds_pm_ops = {
+	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
+};
+
 static struct platform_driver rcar_lvds_platform_driver = {
 	.probe		= rcar_lvds_probe,
 	.remove		= rcar_lvds_remove,
 	.driver		= {
 		.name	= "rcar-lvds",
+		.pm	= &rcar_lvds_pm_ops,
 		.of_match_table = rcar_lvds_of_table,
 	},
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 1/7] drm: rcar-du: dsi: add 'select RESET_CONTROLLER'
  2023-01-20  8:50 ` Tomi Valkeinen
@ 2023-01-20  8:50   ` Tomi Valkeinen
  -1 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Tomi Valkeinen, Laurent Pinchart

The RCAR DSI driver uses reset controller, so we should select it in the
Kconfig.

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

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index fd2c2eaee26b..a8f862c68b4f 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -55,6 +55,7 @@ config DRM_RCAR_MIPI_DSI
 	def_tristate DRM_RCAR_DU
 	depends on DRM_RCAR_USE_MIPI_DSI
 	select DRM_MIPI_DSI
+	select RESET_CONTROLLER
 
 config DRM_RCAR_VSP
 	bool "R-Car DU VSP Compositor Support" if ARM
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 1/7] drm: rcar-du: dsi: add 'select RESET_CONTROLLER'
@ 2023-01-20  8:50   ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Laurent Pinchart, Geert Uytterhoeven, Tomi Valkeinen

The RCAR DSI driver uses reset controller, so we should select it in the
Kconfig.

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

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index fd2c2eaee26b..a8f862c68b4f 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -55,6 +55,7 @@ config DRM_RCAR_MIPI_DSI
 	def_tristate DRM_RCAR_DU
 	depends on DRM_RCAR_USE_MIPI_DSI
 	select DRM_MIPI_DSI
+	select RESET_CONTROLLER
 
 config DRM_RCAR_VSP
 	bool "R-Car DU VSP Compositor Support" if ARM
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM
  2023-01-20  8:50 ` Tomi Valkeinen
  (?)
  (?)
@ 2023-01-20  8:50 ` Tomi Valkeinen
  2023-01-20 16:16     ` Laurent Pinchart
  -1 siblings, 1 reply; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Tomi Valkeinen

Add simple runtime PM suspend and resume functionality.

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

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 81a060c2fe3f..8e1be51fbee6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -16,6 +16,7 @@
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
 
@@ -316,8 +317,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
 
 	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
 
-	ret = clk_prepare_enable(lvds->clocks.mod);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
 		return ret;
 
 	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
@@ -337,7 +338,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
 
 	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
-	clk_disable_unprepare(lvds->clocks.mod);
+	pm_runtime_put(lvds->dev);
 }
 EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
 
@@ -396,8 +397,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
 	u32 lvdcr0;
 	int ret;
 
-	ret = clk_prepare_enable(lvds->clocks.mod);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
 		return;
 
 	/* Enable the companion LVDS encoder in dual-link mode. */
@@ -551,7 +552,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 		lvds->companion->funcs->atomic_disable(lvds->companion,
 						       old_bridge_state);
 
-	clk_disable_unprepare(lvds->clocks.mod);
+	pm_runtime_put(lvds->dev);
 }
 
 static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
@@ -844,6 +845,8 @@ static int rcar_lvds_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	pm_runtime_enable(&pdev->dev);
+
 	drm_bridge_add(&lvds->bridge);
 
 	return 0;
@@ -855,6 +858,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
 
 	drm_bridge_remove(&lvds->bridge);
 
+	pm_runtime_disable(&pdev->dev);
+
 	return 0;
 }
 
@@ -913,11 +918,37 @@ static const struct of_device_id rcar_lvds_of_table[] = {
 
 MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
 
+static int rcar_lvds_runtime_suspend(struct device *dev)
+{
+	struct rcar_lvds *lvds = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(lvds->clocks.mod);
+
+	return 0;
+}
+
+static int rcar_lvds_runtime_resume(struct device *dev)
+{
+	struct rcar_lvds *lvds = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(lvds->clocks.mod);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static const struct dev_pm_ops rcar_lvds_pm_ops = {
+	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
+};
+
 static struct platform_driver rcar_lvds_platform_driver = {
 	.probe		= rcar_lvds_probe,
 	.remove		= rcar_lvds_remove,
 	.driver		= {
 		.name	= "rcar-lvds",
+		.pm	= &rcar_lvds_pm_ops,
 		.of_match_table = rcar_lvds_of_table,
 	},
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control
  2023-01-20  8:50 ` Tomi Valkeinen
                   ` (2 preceding siblings ...)
  (?)
@ 2023-01-20  8:50 ` Tomi Valkeinen
  2023-01-20 16:18     ` Laurent Pinchart
  -1 siblings, 1 reply; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Tomi Valkeinen

Reset LVDS using the reset control as CPG reset/release is required in
the hardware manual sequence.

Based on a BSP patch from Koji Matsuoka <koji.matsuoka.xm@renesas.com>.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/Kconfig     |  1 +
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index a8f862c68b4f..151e400b996d 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
 	select DRM_PANEL
 	select OF_FLATTREE
 	select OF_OVERLAY
+	select RESET_CONTROLLER
 
 config DRM_RCAR_USE_MIPI_DSI
 	bool "R-Car DU MIPI DSI Encoder Support"
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 8e1be51fbee6..668604616bfd 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -17,6 +17,7 @@
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
 
@@ -61,6 +62,7 @@ struct rcar_lvds_device_info {
 struct rcar_lvds {
 	struct device *dev;
 	const struct rcar_lvds_device_info *info;
+	struct reset_control *rstc;
 
 	struct drm_bridge bridge;
 
@@ -845,6 +847,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(lvds->rstc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
+				     "failed to get cpg reset\n");
 	pm_runtime_enable(&pdev->dev);
 
 	drm_bridge_add(&lvds->bridge);
@@ -924,6 +930,8 @@ static int rcar_lvds_runtime_suspend(struct device *dev)
 
 	clk_disable_unprepare(lvds->clocks.mod);
 
+	reset_control_assert(lvds->rstc);
+
 	return 0;
 }
 
@@ -932,11 +940,20 @@ static int rcar_lvds_runtime_resume(struct device *dev)
 	struct rcar_lvds *lvds = dev_get_drvdata(dev);
 	int ret;
 
+	ret = reset_control_deassert(lvds->rstc);
+	if (ret)
+		return ret;
+
 	ret = clk_prepare_enable(lvds->clocks.mod);
 	if (ret < 0)
-		return ret;
+		goto err_reset_assert;
 
 	return 0;
+
+err_reset_assert:
+	reset_control_assert(lvds->rstc);
+
+	return ret;
 }
 
 static const struct dev_pm_ops rcar_lvds_pm_ops = {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 4/7] drm: rcar-du: lvds: Fix stop sequence
  2023-01-20  8:50 ` Tomi Valkeinen
@ 2023-01-20  8:50   ` Tomi Valkeinen
  -1 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Koji Matsuoka, LUU HOAI, Tomi Valkeinen

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

According to hardware manual, LVDCR0 register must be cleared bit by bit
when disabling LVDS.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: LUU HOAI <hoai.luu.ub@renesas.com>
[tomi.valkeinen: simplified the code a bit]
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 668604616bfd..8fa5f7400179 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -83,6 +83,11 @@ struct rcar_lvds {
 #define bridge_to_rcar_lvds(b) \
 	container_of(b, struct rcar_lvds, bridge)
 
+static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
+{
+	return ioread32(lvds->mmio + reg);
+}
+
 static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
 {
 	iowrite32(data, lvds->mmio + reg);
@@ -544,6 +549,27 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 				     struct drm_bridge_state *old_bridge_state)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+	u32 lvdcr0;
+
+	lvdcr0 = rcar_lvds_read(lvds, LVDCR0);
+
+	lvdcr0 &= ~LVDCR0_LVRES;
+	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+
+	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
+		lvdcr0 &= ~LVDCR0_LVEN;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
+
+	if (lvds->info->quirks & RCAR_LVDS_QUIRK_PWD) {
+		lvdcr0 &= ~LVDCR0_PWD;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
+
+	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
+		lvdcr0 &= ~LVDCR0_PLLON;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
 
 	rcar_lvds_write(lvds, LVDCR0, 0);
 	rcar_lvds_write(lvds, LVDCR1, 0);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 4/7] drm: rcar-du: lvds: Fix stop sequence
@ 2023-01-20  8:50   ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Tomi Valkeinen, Geert Uytterhoeven, Koji Matsuoka, LUU HOAI

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

According to hardware manual, LVDCR0 register must be cleared bit by bit
when disabling LVDS.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: LUU HOAI <hoai.luu.ub@renesas.com>
[tomi.valkeinen: simplified the code a bit]
Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 668604616bfd..8fa5f7400179 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -83,6 +83,11 @@ struct rcar_lvds {
 #define bridge_to_rcar_lvds(b) \
 	container_of(b, struct rcar_lvds, bridge)
 
+static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
+{
+	return ioread32(lvds->mmio + reg);
+}
+
 static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
 {
 	iowrite32(data, lvds->mmio + reg);
@@ -544,6 +549,27 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 				     struct drm_bridge_state *old_bridge_state)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+	u32 lvdcr0;
+
+	lvdcr0 = rcar_lvds_read(lvds, LVDCR0);
+
+	lvdcr0 &= ~LVDCR0_LVRES;
+	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+
+	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
+		lvdcr0 &= ~LVDCR0_LVEN;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
+
+	if (lvds->info->quirks & RCAR_LVDS_QUIRK_PWD) {
+		lvdcr0 &= ~LVDCR0_PWD;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
+
+	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
+		lvdcr0 &= ~LVDCR0_PLLON;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
 
 	rcar_lvds_write(lvds, LVDCR0, 0);
 	rcar_lvds_write(lvds, LVDCR1, 0);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 5/7] drm: rcar-du: Add quirk for H3 ES1.x pclk workaround
  2023-01-20  8:50 ` Tomi Valkeinen
@ 2023-01-20  8:50   ` Tomi Valkeinen
  -1 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Tomi Valkeinen, Laurent Pinchart

rcar_du_crtc.c does a soc_device_match() in
rcar_du_crtc_set_display_timing() to find out if the SoC is H3 ES1.x, and
if so, apply a workaround.

We will need another H3 ES1.x check in the following patch, so rather than
adding more soc_device_match() calls, let's add a rcar_du_device_info
entry for the ES1, and a quirk flag,
RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, for the workaround.

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

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 3619e1ddeb62..f2d3266509cc 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -10,7 +10,6 @@
 #include <linux/clk.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
-#include <linux/sys_soc.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -204,11 +203,6 @@ static void rcar_du_escr_divider(struct clk *clk, unsigned long target,
 	}
 }
 
-static const struct soc_device_attribute rcar_du_r8a7795_es1[] = {
-	{ .soc_id = "r8a7795", .revision = "ES1.*" },
-	{ /* sentinel */ }
-};
-
 static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 {
 	const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode;
@@ -238,7 +232,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 		 * no post-divider when a display PLL is present (as shown by
 		 * the workaround breaking HDMI output on M3-W during testing).
 		 */
-		if (soc_device_match(rcar_du_r8a7795_es1)) {
+		if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY) {
 			target *= 2;
 			div = 1;
 		}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index c7c5217cfc1a..ea3a8cff74b7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
+#include <linux/sys_soc.h>
 #include <linux/wait.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -386,6 +387,42 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = {
 	.dpll_mask =  BIT(2) | BIT(1),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = {
+	.gen = 3,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ
+		  | RCAR_DU_FEATURE_CRTC_CLOCK
+		  | RCAR_DU_FEATURE_VSP1_SOURCE
+		  | RCAR_DU_FEATURE_INTERLACED
+		  | RCAR_DU_FEATURE_TVM_SYNC,
+	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY,
+	.channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0),
+	.routes = {
+		/*
+		 * R8A7795 has one RGB output, two HDMI outputs and one
+		 * LVDS output.
+		 */
+		[RCAR_DU_OUTPUT_DPAD0] = {
+			.possible_crtcs = BIT(3),
+			.port = 0,
+		},
+		[RCAR_DU_OUTPUT_HDMI0] = {
+			.possible_crtcs = BIT(1),
+			.port = 1,
+		},
+		[RCAR_DU_OUTPUT_HDMI1] = {
+			.possible_crtcs = BIT(2),
+			.port = 2,
+		},
+		[RCAR_DU_OUTPUT_LVDS0] = {
+			.possible_crtcs = BIT(0),
+			.port = 3,
+		},
+	},
+	.num_lvds = 1,
+	.num_rpf = 5,
+	.dpll_mask =  BIT(2) | BIT(1),
+};
+
 static const struct rcar_du_device_info rcar_du_r8a7796_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ
@@ -576,6 +613,11 @@ static const struct of_device_id rcar_du_of_table[] = {
 
 MODULE_DEVICE_TABLE(of, rcar_du_of_table);
 
+static const struct soc_device_attribute rcar_du_soc_table[] = {
+	{ .soc_id = "r8a7795", .revision = "ES1.*", .data = &rcar_du_r8a7795_es1_info },
+	{ /* sentinel */ }
+};
+
 const char *rcar_du_output_name(enum rcar_du_output output)
 {
 	static const char * const names[] = {
@@ -667,6 +709,7 @@ static void rcar_du_shutdown(struct platform_device *pdev)
 
 static int rcar_du_probe(struct platform_device *pdev)
 {
+	const struct soc_device_attribute *soc_attr;
 	struct rcar_du_device *rcdu;
 	unsigned int mask;
 	int ret;
@@ -681,8 +724,13 @@ static int rcar_du_probe(struct platform_device *pdev)
 		return PTR_ERR(rcdu);
 
 	rcdu->dev = &pdev->dev;
+
 	rcdu->info = of_device_get_match_data(rcdu->dev);
 
+	soc_attr = soc_device_match(rcar_du_soc_table);
+	if (soc_attr)
+		rcdu->info = soc_attr->data;
+
 	platform_set_drvdata(pdev, rcdu);
 
 	/* I/O resources */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 5cfa2bb7ad93..df87ccab146f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -34,6 +34,7 @@ struct rcar_du_device;
 #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
 
 #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
+#define RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY BIT(1)	/* H3 ES1 has pclk stability issue */
 
 enum rcar_du_output {
 	RCAR_DU_OUTPUT_DPAD0,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 5/7] drm: rcar-du: Add quirk for H3 ES1.x pclk workaround
@ 2023-01-20  8:50   ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Laurent Pinchart, Geert Uytterhoeven, Tomi Valkeinen

rcar_du_crtc.c does a soc_device_match() in
rcar_du_crtc_set_display_timing() to find out if the SoC is H3 ES1.x, and
if so, apply a workaround.

We will need another H3 ES1.x check in the following patch, so rather than
adding more soc_device_match() calls, let's add a rcar_du_device_info
entry for the ES1, and a quirk flag,
RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY, for the workaround.

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

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 3619e1ddeb62..f2d3266509cc 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -10,7 +10,6 @@
 #include <linux/clk.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
-#include <linux/sys_soc.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -204,11 +203,6 @@ static void rcar_du_escr_divider(struct clk *clk, unsigned long target,
 	}
 }
 
-static const struct soc_device_attribute rcar_du_r8a7795_es1[] = {
-	{ .soc_id = "r8a7795", .revision = "ES1.*" },
-	{ /* sentinel */ }
-};
-
 static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 {
 	const struct drm_display_mode *mode = &rcrtc->crtc.state->adjusted_mode;
@@ -238,7 +232,7 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 		 * no post-divider when a display PLL is present (as shown by
 		 * the workaround breaking HDMI output on M3-W during testing).
 		 */
-		if (soc_device_match(rcar_du_r8a7795_es1)) {
+		if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY) {
 			target *= 2;
 			div = 1;
 		}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index c7c5217cfc1a..ea3a8cff74b7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
+#include <linux/sys_soc.h>
 #include <linux/wait.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -386,6 +387,42 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = {
 	.dpll_mask =  BIT(2) | BIT(1),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = {
+	.gen = 3,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ
+		  | RCAR_DU_FEATURE_CRTC_CLOCK
+		  | RCAR_DU_FEATURE_VSP1_SOURCE
+		  | RCAR_DU_FEATURE_INTERLACED
+		  | RCAR_DU_FEATURE_TVM_SYNC,
+	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY,
+	.channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0),
+	.routes = {
+		/*
+		 * R8A7795 has one RGB output, two HDMI outputs and one
+		 * LVDS output.
+		 */
+		[RCAR_DU_OUTPUT_DPAD0] = {
+			.possible_crtcs = BIT(3),
+			.port = 0,
+		},
+		[RCAR_DU_OUTPUT_HDMI0] = {
+			.possible_crtcs = BIT(1),
+			.port = 1,
+		},
+		[RCAR_DU_OUTPUT_HDMI1] = {
+			.possible_crtcs = BIT(2),
+			.port = 2,
+		},
+		[RCAR_DU_OUTPUT_LVDS0] = {
+			.possible_crtcs = BIT(0),
+			.port = 3,
+		},
+	},
+	.num_lvds = 1,
+	.num_rpf = 5,
+	.dpll_mask =  BIT(2) | BIT(1),
+};
+
 static const struct rcar_du_device_info rcar_du_r8a7796_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ
@@ -576,6 +613,11 @@ static const struct of_device_id rcar_du_of_table[] = {
 
 MODULE_DEVICE_TABLE(of, rcar_du_of_table);
 
+static const struct soc_device_attribute rcar_du_soc_table[] = {
+	{ .soc_id = "r8a7795", .revision = "ES1.*", .data = &rcar_du_r8a7795_es1_info },
+	{ /* sentinel */ }
+};
+
 const char *rcar_du_output_name(enum rcar_du_output output)
 {
 	static const char * const names[] = {
@@ -667,6 +709,7 @@ static void rcar_du_shutdown(struct platform_device *pdev)
 
 static int rcar_du_probe(struct platform_device *pdev)
 {
+	const struct soc_device_attribute *soc_attr;
 	struct rcar_du_device *rcdu;
 	unsigned int mask;
 	int ret;
@@ -681,8 +724,13 @@ static int rcar_du_probe(struct platform_device *pdev)
 		return PTR_ERR(rcdu);
 
 	rcdu->dev = &pdev->dev;
+
 	rcdu->info = of_device_get_match_data(rcdu->dev);
 
+	soc_attr = soc_device_match(rcar_du_soc_table);
+	if (soc_attr)
+		rcdu->info = soc_attr->data;
+
 	platform_set_drvdata(pdev, rcdu);
 
 	/* I/O resources */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 5cfa2bb7ad93..df87ccab146f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -34,6 +34,7 @@ struct rcar_du_device;
 #define RCAR_DU_FEATURE_NO_BLENDING	BIT(5)	/* PnMR.SPIM does not have ALP nor EOR bits */
 
 #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
+#define RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY BIT(1)	/* H3 ES1 has pclk stability issue */
 
 enum rcar_du_output {
 	RCAR_DU_OUTPUT_DPAD0,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 6/7] drm: rcar-du: Fix setting a reserved bit in DPLLCR
  2023-01-20  8:50 ` Tomi Valkeinen
                   ` (5 preceding siblings ...)
  (?)
@ 2023-01-20  8:50 ` Tomi Valkeinen
  2023-01-20 17:03     ` Laurent Pinchart
  -1 siblings, 1 reply; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Tomi Valkeinen

On H3 ES1.x two bits in DPLLCR are used to select the DU input dot clock
source. These are bits 20 and 21 for DU2, and bits 22 and 23 for DU1. On
non-ES1.x, only the higher bits are used (bits 21 and 23), and the lower
bits are reserved and should be set to 0.

The current code always sets the lower bits, even on non-ES1.x.

For both DU1 and DU2, on all SoC versions, when writing zeroes to those
bits the input clock is DCLKIN, and thus there's no difference between
ES1.x and non-ES1.x.

For DU1, writing 0b10 to the bits (or only writing the higher bit)
results in using PLL0 as the input clock, so in this case there's also
no difference between ES1.x and non-ES1.x.

However, for DU2, writing 0b10 to the bits results in using PLL0 as the
input clock on ES1.x, whereas on non-ES1.x it results in using PLL1. On
ES1.x you need to write 0b11 to select PLL1.

The current code always writes 0b11 to PLCS0 field to select PLL1 on all
SoC versions, which works but causes an illegal (in the sense of not
allowed by the documentation) write to a reserved bit field.

To remove the illegal bit write on PLSC0 we need to handle the input dot
clock selection differently for ES1.x and non-ES1.x.

Add a new quirk, RCAR_DU_QUIRK_H3_ES1_PLL, for this. This way we can
always set the bit 21 on PLSC0 when choosing the PLL as the source
clock, and additionally set the bit 20 when on ES1.x.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 23 ++++++++++++++++++++---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  3 ++-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h  |  1 +
 drivers/gpu/drm/rcar-du/rcar_du_regs.h |  8 ++------
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index f2d3266509cc..b7dd59fe119e 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -245,13 +245,30 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 		       | DPLLCR_N(dpll.n) | DPLLCR_M(dpll.m)
 		       | DPLLCR_STBY;
 
-		if (rcrtc->index == 1)
+		if (rcrtc->index == 1) {
 			dpllcr |= DPLLCR_PLCS1
 			       |  DPLLCR_INCS_DOTCLKIN1;
-		else
-			dpllcr |= DPLLCR_PLCS0
+		} else {
+			dpllcr |= DPLLCR_PLCS0_PLL
 			       |  DPLLCR_INCS_DOTCLKIN0;
 
+			/*
+			 * On ES2.x we have a single mux controlled via bit 21,
+			 * which selects between DCLKIN source (bit 21 = 0) and
+			 * a PLL source (bit 21 = 1), where the PLL is always
+			 * PLL1.
+			 *
+			 * On ES1.x we have an additional mux, controlled
+			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
+			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
+			 * so on ES1.x, in addition to setting bit 21, we need
+			 * to set the bit 20.
+			 */
+
+			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
+				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
+		}
+
 		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
 
 		escr = ESCR_DCLKSEL_DCLKIN | div;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index ea3a8cff74b7..82f9718ff500 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -394,7 +394,8 @@ static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = {
 		  | RCAR_DU_FEATURE_VSP1_SOURCE
 		  | RCAR_DU_FEATURE_INTERLACED
 		  | RCAR_DU_FEATURE_TVM_SYNC,
-	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY,
+	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY
+		| RCAR_DU_QUIRK_H3_ES1_PLL,
 	.channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0),
 	.routes = {
 		/*
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index df87ccab146f..acc3673fefe1 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -35,6 +35,7 @@ struct rcar_du_device;
 
 #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
 #define RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY BIT(1)	/* H3 ES1 has pclk stability issue */
+#define RCAR_DU_QUIRK_H3_ES1_PLL	BIT(2)	/* H3 ES1 PLL setup differs from non-ES1 */
 
 enum rcar_du_output {
 	RCAR_DU_OUTPUT_DPAD0,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
index c1bcb0e8b5b4..789ae9285108 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
@@ -283,12 +283,8 @@
 #define DPLLCR			0x20044
 #define DPLLCR_CODE		(0x95 << 24)
 #define DPLLCR_PLCS1		(1 << 23)
-/*
- * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
- * isn't implemented by other SoC in the Gen3 family it can safely be set
- * unconditionally.
- */
-#define DPLLCR_PLCS0		(3 << 20)
+#define DPLLCR_PLCS0_PLL	(1 << 21)
+#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
 #define DPLLCR_CLKE		(1 << 18)
 #define DPLLCR_FDPLL(n)		((n) << 12)
 #define DPLLCR_N(n)		((n) << 5)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 7/7] drm: rcar-du: Stop accessing non-existant registers on gen4
  2023-01-20  8:50 ` Tomi Valkeinen
@ 2023-01-20  8:50   ` Tomi Valkeinen
  -1 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Geert Uytterhoeven, Tomi Valkeinen, Laurent Pinchart

The following registers do not exist on gen4, so we should not write
them: DEF6Rm, DEF7Rm, DEF8Rm, ESCRn, OTARn.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  8 +++++---
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 ++++++++---
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index b7dd59fe119e..008e172ed43b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -298,10 +298,12 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 		escr = params.escr;
 	}
 
-	dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
+	if (rcdu->info->gen < 4) {
+		dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
 
-	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
-	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
+		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
+		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
+	}
 
 	/* Signal polarities */
 	dsmr = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 6da01760ede5..c2209d427bb7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -148,7 +148,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 	}
 	rcar_du_group_write(rgrp, DEFR5, DEFR5_CODE | DEFR5_DEFE5);
 
-	rcar_du_group_setup_pins(rgrp);
+	if (rcdu->info->gen < 4)
+		rcar_du_group_setup_pins(rgrp);
 
 	/*
 	 * TODO: Handle routing of the DU output to CMM dynamically, as we
@@ -160,7 +161,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 	rcar_du_group_write(rgrp, DEFR7, defr7);
 
 	if (rcdu->info->gen >= 2) {
-		rcar_du_group_setup_defr8(rgrp);
+		if (rcdu->info->gen < 4)
+			rcar_du_group_setup_defr8(rgrp);
 		rcar_du_group_setup_didsr(rgrp);
 	}
 
@@ -192,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
  */
 int rcar_du_group_get(struct rcar_du_group *rgrp)
 {
+	struct rcar_du_device *rcdu = rgrp->dev;
+
 	if (rgrp->use_count)
 		goto done;
 
-	rcar_du_group_setup(rgrp);
+	if (rcdu->info->gen < 4)
+		rcar_du_group_setup(rgrp);
 
 done:
 	rgrp->use_count++;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 7/7] drm: rcar-du: Stop accessing non-existant registers on gen4
@ 2023-01-20  8:50   ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20  8:50 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, dri-devel, linux-renesas-soc
  Cc: Laurent Pinchart, Geert Uytterhoeven, Tomi Valkeinen

The following registers do not exist on gen4, so we should not write
them: DEF6Rm, DEF7Rm, DEF8Rm, ESCRn, OTARn.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  8 +++++---
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 ++++++++---
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index b7dd59fe119e..008e172ed43b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -298,10 +298,12 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 		escr = params.escr;
 	}
 
-	dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
+	if (rcdu->info->gen < 4) {
+		dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
 
-	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
-	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
+		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
+		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
+	}
 
 	/* Signal polarities */
 	dsmr = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 6da01760ede5..c2209d427bb7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -148,7 +148,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 	}
 	rcar_du_group_write(rgrp, DEFR5, DEFR5_CODE | DEFR5_DEFE5);
 
-	rcar_du_group_setup_pins(rgrp);
+	if (rcdu->info->gen < 4)
+		rcar_du_group_setup_pins(rgrp);
 
 	/*
 	 * TODO: Handle routing of the DU output to CMM dynamically, as we
@@ -160,7 +161,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 	rcar_du_group_write(rgrp, DEFR7, defr7);
 
 	if (rcdu->info->gen >= 2) {
-		rcar_du_group_setup_defr8(rgrp);
+		if (rcdu->info->gen < 4)
+			rcar_du_group_setup_defr8(rgrp);
 		rcar_du_group_setup_didsr(rgrp);
 	}
 
@@ -192,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
  */
 int rcar_du_group_get(struct rcar_du_group *rgrp)
 {
+	struct rcar_du_device *rcdu = rgrp->dev;
+
 	if (rgrp->use_count)
 		goto done;
 
-	rcar_du_group_setup(rgrp);
+	if (rcdu->info->gen < 4)
+		rcar_du_group_setup(rgrp);
 
 done:
 	rgrp->use_count++;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM
  2023-01-20  8:50 ` [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM Tomi Valkeinen
@ 2023-01-20 16:16     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:16 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:04AM +0200, Tomi Valkeinen wrote:
> Add simple runtime PM suspend and resume functionality.

I think you need to depend on PM in Kconfig. That's not a compile-time
dependency but a runtime-dependency, with runtime PM support the
suspend/resume handler will never be called.

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 43 +++++++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 81a060c2fe3f..8e1be51fbee6 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
>  
> @@ -316,8 +317,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
>  
>  	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>  
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return ret;
>  
>  	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
> @@ -337,7 +338,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>  
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> +	pm_runtime_put(lvds->dev);

Should we use pm_runtime_put_sync() here, to make sure the clock gets
disabled right away ? The DU hardware may depend on the exact sequencing
of events. I would then do the same in rcar_lvds_atomic_disable().

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  }
>  EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
>  
> @@ -396,8 +397,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
>  	u32 lvdcr0;
>  	int ret;
>  
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return;
>  
>  	/* Enable the companion LVDS encoder in dual-link mode. */
> @@ -551,7 +552,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		lvds->companion->funcs->atomic_disable(lvds->companion,
>  						       old_bridge_state);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> +	pm_runtime_put(lvds->dev);
>  }
>  
>  static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> @@ -844,6 +845,8 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> +	pm_runtime_enable(&pdev->dev);
> +
>  	drm_bridge_add(&lvds->bridge);
>  
>  	return 0;
> @@ -855,6 +858,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
>  
>  	drm_bridge_remove(&lvds->bridge);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	return 0;
>  }
>  
> @@ -913,11 +918,37 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  
>  MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
>  
> +static int rcar_lvds_runtime_suspend(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(lvds->clocks.mod);
> +
> +	return 0;
> +}
> +
> +static int rcar_lvds_runtime_resume(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(lvds->clocks.mod);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops rcar_lvds_pm_ops = {
> +	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver rcar_lvds_platform_driver = {
>  	.probe		= rcar_lvds_probe,
>  	.remove		= rcar_lvds_remove,
>  	.driver		= {
>  		.name	= "rcar-lvds",
> +		.pm	= &rcar_lvds_pm_ops,
>  		.of_match_table = rcar_lvds_of_table,
>  	},
>  };

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM
@ 2023-01-20 16:16     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:16 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:04AM +0200, Tomi Valkeinen wrote:
> Add simple runtime PM suspend and resume functionality.

I think you need to depend on PM in Kconfig. That's not a compile-time
dependency but a runtime-dependency, with runtime PM support the
suspend/resume handler will never be called.

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 43 +++++++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 81a060c2fe3f..8e1be51fbee6 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
>  
> @@ -316,8 +317,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
>  
>  	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>  
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return ret;
>  
>  	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
> @@ -337,7 +338,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>  
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> +	pm_runtime_put(lvds->dev);

Should we use pm_runtime_put_sync() here, to make sure the clock gets
disabled right away ? The DU hardware may depend on the exact sequencing
of events. I would then do the same in rcar_lvds_atomic_disable().

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  }
>  EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
>  
> @@ -396,8 +397,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
>  	u32 lvdcr0;
>  	int ret;
>  
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return;
>  
>  	/* Enable the companion LVDS encoder in dual-link mode. */
> @@ -551,7 +552,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		lvds->companion->funcs->atomic_disable(lvds->companion,
>  						       old_bridge_state);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> +	pm_runtime_put(lvds->dev);
>  }
>  
>  static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> @@ -844,6 +845,8 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> +	pm_runtime_enable(&pdev->dev);
> +
>  	drm_bridge_add(&lvds->bridge);
>  
>  	return 0;
> @@ -855,6 +858,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
>  
>  	drm_bridge_remove(&lvds->bridge);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	return 0;
>  }
>  
> @@ -913,11 +918,37 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  
>  MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
>  
> +static int rcar_lvds_runtime_suspend(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(lvds->clocks.mod);
> +
> +	return 0;
> +}
> +
> +static int rcar_lvds_runtime_resume(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(lvds->clocks.mod);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops rcar_lvds_pm_ops = {
> +	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver rcar_lvds_platform_driver = {
>  	.probe		= rcar_lvds_probe,
>  	.remove		= rcar_lvds_remove,
>  	.driver		= {
>  		.name	= "rcar-lvds",
> +		.pm	= &rcar_lvds_pm_ops,
>  		.of_match_table = rcar_lvds_of_table,
>  	},
>  };

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control
  2023-01-20  8:50 ` [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control Tomi Valkeinen
@ 2023-01-20 16:18     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:18 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:05AM +0200, Tomi Valkeinen wrote:
> Reset LVDS using the reset control as CPG reset/release is required in
> the hardware manual sequence.
> 
> Based on a BSP patch from Koji Matsuoka <koji.matsuoka.xm@renesas.com>.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig     |  1 +
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ++++++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index a8f862c68b4f..151e400b996d 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
>  	select DRM_PANEL
>  	select OF_FLATTREE
>  	select OF_OVERLAY
> +	select RESET_CONTROLLER
>  
>  config DRM_RCAR_USE_MIPI_DSI
>  	bool "R-Car DU MIPI DSI Encoder Support"
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 8e1be51fbee6..668604616bfd 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -17,6 +17,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
>  
> @@ -61,6 +62,7 @@ struct rcar_lvds_device_info {
>  struct rcar_lvds {
>  	struct device *dev;
>  	const struct rcar_lvds_device_info *info;
> +	struct reset_control *rstc;
>  
>  	struct drm_bridge bridge;
>  
> @@ -845,6 +847,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(lvds->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
> +				     "failed to get cpg reset\n");

Missing blank line.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	pm_runtime_enable(&pdev->dev);
>  
>  	drm_bridge_add(&lvds->bridge);
> @@ -924,6 +930,8 @@ static int rcar_lvds_runtime_suspend(struct device *dev)
>  
>  	clk_disable_unprepare(lvds->clocks.mod);
>  
> +	reset_control_assert(lvds->rstc);
> +
>  	return 0;
>  }
>  
> @@ -932,11 +940,20 @@ static int rcar_lvds_runtime_resume(struct device *dev)
>  	struct rcar_lvds *lvds = dev_get_drvdata(dev);
>  	int ret;
>  
> +	ret = reset_control_deassert(lvds->rstc);
> +	if (ret)
> +		return ret;
> +
>  	ret = clk_prepare_enable(lvds->clocks.mod);
>  	if (ret < 0)
> -		return ret;
> +		goto err_reset_assert;
>  
>  	return 0;
> +
> +err_reset_assert:
> +	reset_control_assert(lvds->rstc);
> +
> +	return ret;
>  }
>  
>  static const struct dev_pm_ops rcar_lvds_pm_ops = {

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control
@ 2023-01-20 16:18     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:18 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:05AM +0200, Tomi Valkeinen wrote:
> Reset LVDS using the reset control as CPG reset/release is required in
> the hardware manual sequence.
> 
> Based on a BSP patch from Koji Matsuoka <koji.matsuoka.xm@renesas.com>.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig     |  1 +
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ++++++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index a8f862c68b4f..151e400b996d 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
>  	select DRM_PANEL
>  	select OF_FLATTREE
>  	select OF_OVERLAY
> +	select RESET_CONTROLLER
>  
>  config DRM_RCAR_USE_MIPI_DSI
>  	bool "R-Car DU MIPI DSI Encoder Support"
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 8e1be51fbee6..668604616bfd 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -17,6 +17,7 @@
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
>  
> @@ -61,6 +62,7 @@ struct rcar_lvds_device_info {
>  struct rcar_lvds {
>  	struct device *dev;
>  	const struct rcar_lvds_device_info *info;
> +	struct reset_control *rstc;
>  
>  	struct drm_bridge bridge;
>  
> @@ -845,6 +847,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(lvds->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
> +				     "failed to get cpg reset\n");

Missing blank line.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	pm_runtime_enable(&pdev->dev);
>  
>  	drm_bridge_add(&lvds->bridge);
> @@ -924,6 +930,8 @@ static int rcar_lvds_runtime_suspend(struct device *dev)
>  
>  	clk_disable_unprepare(lvds->clocks.mod);
>  
> +	reset_control_assert(lvds->rstc);
> +
>  	return 0;
>  }
>  
> @@ -932,11 +940,20 @@ static int rcar_lvds_runtime_resume(struct device *dev)
>  	struct rcar_lvds *lvds = dev_get_drvdata(dev);
>  	int ret;
>  
> +	ret = reset_control_deassert(lvds->rstc);
> +	if (ret)
> +		return ret;
> +
>  	ret = clk_prepare_enable(lvds->clocks.mod);
>  	if (ret < 0)
> -		return ret;
> +		goto err_reset_assert;
>  
>  	return 0;
> +
> +err_reset_assert:
> +	reset_control_assert(lvds->rstc);
> +
> +	return ret;
>  }
>  
>  static const struct dev_pm_ops rcar_lvds_pm_ops = {

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 4/7] drm: rcar-du: lvds: Fix stop sequence
  2023-01-20  8:50   ` Tomi Valkeinen
@ 2023-01-20 16:19     ` Laurent Pinchart
  -1 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:19 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven,
	Koji Matsuoka, LUU HOAI

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:06AM +0200, Tomi Valkeinen wrote:
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> According to hardware manual, LVDCR0 register must be cleared bit by bit
> when disabling LVDS.
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: LUU HOAI <hoai.luu.ub@renesas.com>
> [tomi.valkeinen: simplified the code a bit]
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 668604616bfd..8fa5f7400179 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -83,6 +83,11 @@ struct rcar_lvds {
>  #define bridge_to_rcar_lvds(b) \
>  	container_of(b, struct rcar_lvds, bridge)
>  
> +static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
> +{
> +	return ioread32(lvds->mmio + reg);
> +}
> +
>  static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
>  {
>  	iowrite32(data, lvds->mmio + reg);
> @@ -544,6 +549,27 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  				     struct drm_bridge_state *old_bridge_state)
>  {
>  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> +	u32 lvdcr0;
> +
> +	lvdcr0 = rcar_lvds_read(lvds, LVDCR0);
> +
> +	lvdcr0 &= ~LVDCR0_LVRES;
> +	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +
> +	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
> +		lvdcr0 &= ~LVDCR0_LVEN;
> +		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	}
> +
> +	if (lvds->info->quirks & RCAR_LVDS_QUIRK_PWD) {
> +		lvdcr0 &= ~LVDCR0_PWD;
> +		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	}
> +
> +	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
> +		lvdcr0 &= ~LVDCR0_PLLON;
> +		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	}
>  
>  	rcar_lvds_write(lvds, LVDCR0, 0);
>  	rcar_lvds_write(lvds, LVDCR1, 0);

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 4/7] drm: rcar-du: lvds: Fix stop sequence
@ 2023-01-20 16:19     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:19 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Koji Matsuoka, dri-devel, linux-renesas-soc, Kieran Bingham,
	Geert Uytterhoeven, LUU HOAI

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:06AM +0200, Tomi Valkeinen wrote:
> From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> 
> According to hardware manual, LVDCR0 register must be cleared bit by bit
> when disabling LVDS.
> 
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: LUU HOAI <hoai.luu.ub@renesas.com>
> [tomi.valkeinen: simplified the code a bit]
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 668604616bfd..8fa5f7400179 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -83,6 +83,11 @@ struct rcar_lvds {
>  #define bridge_to_rcar_lvds(b) \
>  	container_of(b, struct rcar_lvds, bridge)
>  
> +static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
> +{
> +	return ioread32(lvds->mmio + reg);
> +}
> +
>  static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
>  {
>  	iowrite32(data, lvds->mmio + reg);
> @@ -544,6 +549,27 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  				     struct drm_bridge_state *old_bridge_state)
>  {
>  	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> +	u32 lvdcr0;
> +
> +	lvdcr0 = rcar_lvds_read(lvds, LVDCR0);
> +
> +	lvdcr0 &= ~LVDCR0_LVRES;
> +	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +
> +	if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
> +		lvdcr0 &= ~LVDCR0_LVEN;
> +		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	}
> +
> +	if (lvds->info->quirks & RCAR_LVDS_QUIRK_PWD) {
> +		lvdcr0 &= ~LVDCR0_PWD;
> +		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	}
> +
> +	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
> +		lvdcr0 &= ~LVDCR0_PLLON;
> +		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> +	}
>  
>  	rcar_lvds_write(lvds, LVDCR0, 0);
>  	rcar_lvds_write(lvds, LVDCR1, 0);

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 7/7] drm: rcar-du: Stop accessing non-existant registers on gen4
  2023-01-20  8:50   ` Tomi Valkeinen
@ 2023-01-20 16:21     ` Laurent Pinchart
  -1 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:21 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

On Fri, Jan 20, 2023 at 10:50:09AM +0200, Tomi Valkeinen wrote:
> The following registers do not exist on gen4, so we should not write
> them: DEF6Rm, DEF7Rm, DEF8Rm, ESCRn, OTARn.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  8 +++++---
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 ++++++++---
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index b7dd59fe119e..008e172ed43b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -298,10 +298,12 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  		escr = params.escr;
>  	}
>  
> -	dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
> +	if (rcdu->info->gen < 4) {
> +		dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
>  
> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
> +	}
>  
>  	/* Signal polarities */
>  	dsmr = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 6da01760ede5..c2209d427bb7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -148,7 +148,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  	}
>  	rcar_du_group_write(rgrp, DEFR5, DEFR5_CODE | DEFR5_DEFE5);
>  
> -	rcar_du_group_setup_pins(rgrp);
> +	if (rcdu->info->gen < 4)
> +		rcar_du_group_setup_pins(rgrp);
>  
>  	/*
>  	 * TODO: Handle routing of the DU output to CMM dynamically, as we
> @@ -160,7 +161,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  	rcar_du_group_write(rgrp, DEFR7, defr7);
>  
>  	if (rcdu->info->gen >= 2) {
> -		rcar_du_group_setup_defr8(rgrp);
> +		if (rcdu->info->gen < 4)
> +			rcar_du_group_setup_defr8(rgrp);
>  		rcar_du_group_setup_didsr(rgrp);
>  	}
>  
> @@ -192,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>   */
>  int rcar_du_group_get(struct rcar_du_group *rgrp)
>  {
> +	struct rcar_du_device *rcdu = rgrp->dev;
> +
>  	if (rgrp->use_count)
>  		goto done;
>  
> -	rcar_du_group_setup(rgrp);
> +	if (rcdu->info->gen < 4)
> +		rcar_du_group_setup(rgrp);

This doesn't look right, you're disabling way more than necessary.

>  
>  done:
>  	rgrp->use_count++;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 7/7] drm: rcar-du: Stop accessing non-existant registers on gen4
@ 2023-01-20 16:21     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 16:21 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

On Fri, Jan 20, 2023 at 10:50:09AM +0200, Tomi Valkeinen wrote:
> The following registers do not exist on gen4, so we should not write
> them: DEF6Rm, DEF7Rm, DEF8Rm, ESCRn, OTARn.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  8 +++++---
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 ++++++++---
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index b7dd59fe119e..008e172ed43b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -298,10 +298,12 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  		escr = params.escr;
>  	}
>  
> -	dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
> +	if (rcdu->info->gen < 4) {
> +		dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
>  
> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
> +	}
>  
>  	/* Signal polarities */
>  	dsmr = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 6da01760ede5..c2209d427bb7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -148,7 +148,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  	}
>  	rcar_du_group_write(rgrp, DEFR5, DEFR5_CODE | DEFR5_DEFE5);
>  
> -	rcar_du_group_setup_pins(rgrp);
> +	if (rcdu->info->gen < 4)
> +		rcar_du_group_setup_pins(rgrp);
>  
>  	/*
>  	 * TODO: Handle routing of the DU output to CMM dynamically, as we
> @@ -160,7 +161,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>  	rcar_du_group_write(rgrp, DEFR7, defr7);
>  
>  	if (rcdu->info->gen >= 2) {
> -		rcar_du_group_setup_defr8(rgrp);
> +		if (rcdu->info->gen < 4)
> +			rcar_du_group_setup_defr8(rgrp);
>  		rcar_du_group_setup_didsr(rgrp);
>  	}
>  
> @@ -192,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>   */
>  int rcar_du_group_get(struct rcar_du_group *rgrp)
>  {
> +	struct rcar_du_device *rcdu = rgrp->dev;
> +
>  	if (rgrp->use_count)
>  		goto done;
>  
> -	rcar_du_group_setup(rgrp);
> +	if (rcdu->info->gen < 4)
> +		rcar_du_group_setup(rgrp);

This doesn't look right, you're disabling way more than necessary.

>  
>  done:
>  	rgrp->use_count++;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 7/7] drm: rcar-du: Stop accessing non-existant registers on gen4
  2023-01-20 16:21     ` Laurent Pinchart
@ 2023-01-20 16:28       ` Tomi Valkeinen
  -1 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20 16:28 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

On 20/01/2023 18:21, Laurent Pinchart wrote:
> On Fri, Jan 20, 2023 at 10:50:09AM +0200, Tomi Valkeinen wrote:
>> The following registers do not exist on gen4, so we should not write
>> them: DEF6Rm, DEF7Rm, DEF8Rm, ESCRn, OTARn.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  8 +++++---
>>   drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 ++++++++---
>>   2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> index b7dd59fe119e..008e172ed43b 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> @@ -298,10 +298,12 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>>   		escr = params.escr;
>>   	}
>>   
>> -	dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
>> +	if (rcdu->info->gen < 4) {
>> +		dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
>>   
>> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
>> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
>> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
>> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
>> +	}
>>   
>>   	/* Signal polarities */
>>   	dsmr = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> index 6da01760ede5..c2209d427bb7 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> @@ -148,7 +148,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>   	}
>>   	rcar_du_group_write(rgrp, DEFR5, DEFR5_CODE | DEFR5_DEFE5);
>>   
>> -	rcar_du_group_setup_pins(rgrp);
>> +	if (rcdu->info->gen < 4)
>> +		rcar_du_group_setup_pins(rgrp);
>>   
>>   	/*
>>   	 * TODO: Handle routing of the DU output to CMM dynamically, as we
>> @@ -160,7 +161,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>   	rcar_du_group_write(rgrp, DEFR7, defr7);
>>   
>>   	if (rcdu->info->gen >= 2) {
>> -		rcar_du_group_setup_defr8(rgrp);
>> +		if (rcdu->info->gen < 4)
>> +			rcar_du_group_setup_defr8(rgrp);
>>   		rcar_du_group_setup_didsr(rgrp);
>>   	}
>>   
>> @@ -192,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>    */
>>   int rcar_du_group_get(struct rcar_du_group *rgrp)
>>   {
>> +	struct rcar_du_device *rcdu = rgrp->dev;
>> +
>>   	if (rgrp->use_count)
>>   		goto done;
>>   
>> -	rcar_du_group_setup(rgrp);
>> +	if (rcdu->info->gen < 4)
>> +		rcar_du_group_setup(rgrp);
> 
> This doesn't look right, you're disabling way more than necessary.

You're right, doesn't look even remotely correct. A morning patch, 
obviously.

  Tomi


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 7/7] drm: rcar-du: Stop accessing non-existant registers on gen4
@ 2023-01-20 16:28       ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-20 16:28 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

On 20/01/2023 18:21, Laurent Pinchart wrote:
> On Fri, Jan 20, 2023 at 10:50:09AM +0200, Tomi Valkeinen wrote:
>> The following registers do not exist on gen4, so we should not write
>> them: DEF6Rm, DEF7Rm, DEF8Rm, ESCRn, OTARn.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  8 +++++---
>>   drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 ++++++++---
>>   2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> index b7dd59fe119e..008e172ed43b 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> @@ -298,10 +298,12 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>>   		escr = params.escr;
>>   	}
>>   
>> -	dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
>> +	if (rcdu->info->gen < 4) {
>> +		dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
>>   
>> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
>> -	rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
>> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
>> +		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
>> +	}
>>   
>>   	/* Signal polarities */
>>   	dsmr = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> index 6da01760ede5..c2209d427bb7 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> @@ -148,7 +148,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>   	}
>>   	rcar_du_group_write(rgrp, DEFR5, DEFR5_CODE | DEFR5_DEFE5);
>>   
>> -	rcar_du_group_setup_pins(rgrp);
>> +	if (rcdu->info->gen < 4)
>> +		rcar_du_group_setup_pins(rgrp);
>>   
>>   	/*
>>   	 * TODO: Handle routing of the DU output to CMM dynamically, as we
>> @@ -160,7 +161,8 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>   	rcar_du_group_write(rgrp, DEFR7, defr7);
>>   
>>   	if (rcdu->info->gen >= 2) {
>> -		rcar_du_group_setup_defr8(rgrp);
>> +		if (rcdu->info->gen < 4)
>> +			rcar_du_group_setup_defr8(rgrp);
>>   		rcar_du_group_setup_didsr(rgrp);
>>   	}
>>   
>> @@ -192,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>>    */
>>   int rcar_du_group_get(struct rcar_du_group *rgrp)
>>   {
>> +	struct rcar_du_device *rcdu = rgrp->dev;
>> +
>>   	if (rgrp->use_count)
>>   		goto done;
>>   
>> -	rcar_du_group_setup(rgrp);
>> +	if (rcdu->info->gen < 4)
>> +		rcar_du_group_setup(rgrp);
> 
> This doesn't look right, you're disabling way more than necessary.

You're right, doesn't look even remotely correct. A morning patch, 
obviously.

  Tomi


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/7] drm: rcar-du: Fix setting a reserved bit in DPLLCR
  2023-01-20  8:50 ` [PATCH v2 6/7] drm: rcar-du: Fix setting a reserved bit in DPLLCR Tomi Valkeinen
@ 2023-01-20 17:03     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 17:03 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:08AM +0200, Tomi Valkeinen wrote:
> On H3 ES1.x two bits in DPLLCR are used to select the DU input dot clock
> source. These are bits 20 and 21 for DU2, and bits 22 and 23 for DU1. On
> non-ES1.x, only the higher bits are used (bits 21 and 23), and the lower
> bits are reserved and should be set to 0.
> 
> The current code always sets the lower bits, even on non-ES1.x.
> 
> For both DU1 and DU2, on all SoC versions, when writing zeroes to those
> bits the input clock is DCLKIN, and thus there's no difference between
> ES1.x and non-ES1.x.
> 
> For DU1, writing 0b10 to the bits (or only writing the higher bit)
> results in using PLL0 as the input clock, so in this case there's also
> no difference between ES1.x and non-ES1.x.
> 
> However, for DU2, writing 0b10 to the bits results in using PLL0 as the
> input clock on ES1.x, whereas on non-ES1.x it results in using PLL1. On
> ES1.x you need to write 0b11 to select PLL1.
> 
> The current code always writes 0b11 to PLCS0 field to select PLL1 on all
> SoC versions, which works but causes an illegal (in the sense of not
> allowed by the documentation) write to a reserved bit field.
> 
> To remove the illegal bit write on PLSC0 we need to handle the input dot
> clock selection differently for ES1.x and non-ES1.x.
> 
> Add a new quirk, RCAR_DU_QUIRK_H3_ES1_PLL, for this. This way we can
> always set the bit 21 on PLSC0 when choosing the PLL as the source
> clock, and additionally set the bit 20 when on ES1.x.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 23 ++++++++++++++++++++---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  3 ++-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h  |  1 +
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h |  8 ++------
>  4 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index f2d3266509cc..b7dd59fe119e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -245,13 +245,30 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  		       | DPLLCR_N(dpll.n) | DPLLCR_M(dpll.m)
>  		       | DPLLCR_STBY;
>  
> -		if (rcrtc->index == 1)
> +		if (rcrtc->index == 1) {
>  			dpllcr |= DPLLCR_PLCS1
>  			       |  DPLLCR_INCS_DOTCLKIN1;
> -		else
> -			dpllcr |= DPLLCR_PLCS0
> +		} else {
> +			dpllcr |= DPLLCR_PLCS0_PLL
>  			       |  DPLLCR_INCS_DOTCLKIN0;
>  
> +			/*
> +			 * On ES2.x we have a single mux controlled via bit 21,
> +			 * which selects between DCLKIN source (bit 21 = 0) and
> +			 * a PLL source (bit 21 = 1), where the PLL is always
> +			 * PLL1.
> +			 *
> +			 * On ES1.x we have an additional mux, controlled
> +			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
> +			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
> +			 * so on ES1.x, in addition to setting bit 21, we need
> +			 * to set the bit 20.
> +			 */
> +
> +			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
> +				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
> +		}
> +
>  		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
>  
>  		escr = ESCR_DCLKSEL_DCLKIN | div;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index ea3a8cff74b7..82f9718ff500 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -394,7 +394,8 @@ static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = {
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
> -	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY,
> +	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY
> +		| RCAR_DU_QUIRK_H3_ES1_PLL,
>  	.channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0),
>  	.routes = {
>  		/*
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index df87ccab146f..acc3673fefe1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -35,6 +35,7 @@ struct rcar_du_device;
>  
>  #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
>  #define RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY BIT(1)	/* H3 ES1 has pclk stability issue */
> +#define RCAR_DU_QUIRK_H3_ES1_PLL	BIT(2)	/* H3 ES1 PLL setup differs from non-ES1 */
>  
>  enum rcar_du_output {
>  	RCAR_DU_OUTPUT_DPAD0,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index c1bcb0e8b5b4..789ae9285108 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -283,12 +283,8 @@
>  #define DPLLCR			0x20044
>  #define DPLLCR_CODE		(0x95 << 24)
>  #define DPLLCR_PLCS1		(1 << 23)
> -/*
> - * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
> - * isn't implemented by other SoC in the Gen3 family it can safely be set
> - * unconditionally.
> - */
> -#define DPLLCR_PLCS0		(3 << 20)
> +#define DPLLCR_PLCS0_PLL	(1 << 21)
> +#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
>  #define DPLLCR_CLKE		(1 << 18)
>  #define DPLLCR_FDPLL(n)		((n) << 12)
>  #define DPLLCR_N(n)		((n) << 5)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/7] drm: rcar-du: Fix setting a reserved bit in DPLLCR
@ 2023-01-20 17:03     ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 17:03 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:08AM +0200, Tomi Valkeinen wrote:
> On H3 ES1.x two bits in DPLLCR are used to select the DU input dot clock
> source. These are bits 20 and 21 for DU2, and bits 22 and 23 for DU1. On
> non-ES1.x, only the higher bits are used (bits 21 and 23), and the lower
> bits are reserved and should be set to 0.
> 
> The current code always sets the lower bits, even on non-ES1.x.
> 
> For both DU1 and DU2, on all SoC versions, when writing zeroes to those
> bits the input clock is DCLKIN, and thus there's no difference between
> ES1.x and non-ES1.x.
> 
> For DU1, writing 0b10 to the bits (or only writing the higher bit)
> results in using PLL0 as the input clock, so in this case there's also
> no difference between ES1.x and non-ES1.x.
> 
> However, for DU2, writing 0b10 to the bits results in using PLL0 as the
> input clock on ES1.x, whereas on non-ES1.x it results in using PLL1. On
> ES1.x you need to write 0b11 to select PLL1.
> 
> The current code always writes 0b11 to PLCS0 field to select PLL1 on all
> SoC versions, which works but causes an illegal (in the sense of not
> allowed by the documentation) write to a reserved bit field.
> 
> To remove the illegal bit write on PLSC0 we need to handle the input dot
> clock selection differently for ES1.x and non-ES1.x.
> 
> Add a new quirk, RCAR_DU_QUIRK_H3_ES1_PLL, for this. This way we can
> always set the bit 21 on PLSC0 when choosing the PLL as the source
> clock, and additionally set the bit 20 when on ES1.x.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 23 ++++++++++++++++++++---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c  |  3 ++-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h  |  1 +
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h |  8 ++------
>  4 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index f2d3266509cc..b7dd59fe119e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -245,13 +245,30 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  		       | DPLLCR_N(dpll.n) | DPLLCR_M(dpll.m)
>  		       | DPLLCR_STBY;
>  
> -		if (rcrtc->index == 1)
> +		if (rcrtc->index == 1) {
>  			dpllcr |= DPLLCR_PLCS1
>  			       |  DPLLCR_INCS_DOTCLKIN1;
> -		else
> -			dpllcr |= DPLLCR_PLCS0
> +		} else {
> +			dpllcr |= DPLLCR_PLCS0_PLL
>  			       |  DPLLCR_INCS_DOTCLKIN0;
>  
> +			/*
> +			 * On ES2.x we have a single mux controlled via bit 21,
> +			 * which selects between DCLKIN source (bit 21 = 0) and
> +			 * a PLL source (bit 21 = 1), where the PLL is always
> +			 * PLL1.
> +			 *
> +			 * On ES1.x we have an additional mux, controlled
> +			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
> +			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
> +			 * so on ES1.x, in addition to setting bit 21, we need
> +			 * to set the bit 20.
> +			 */
> +
> +			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
> +				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
> +		}
> +
>  		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
>  
>  		escr = ESCR_DCLKSEL_DCLKIN | div;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index ea3a8cff74b7..82f9718ff500 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -394,7 +394,8 @@ static const struct rcar_du_device_info rcar_du_r8a7795_es1_info = {
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE
>  		  | RCAR_DU_FEATURE_INTERLACED
>  		  | RCAR_DU_FEATURE_TVM_SYNC,
> -	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY,
> +	.quirks = RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY
> +		| RCAR_DU_QUIRK_H3_ES1_PLL,
>  	.channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0),
>  	.routes = {
>  		/*
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index df87ccab146f..acc3673fefe1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -35,6 +35,7 @@ struct rcar_du_device;
>  
>  #define RCAR_DU_QUIRK_ALIGN_128B	BIT(0)	/* Align pitches to 128 bytes */
>  #define RCAR_DU_QUIRK_H3_ES1_PCLK_STABILITY BIT(1)	/* H3 ES1 has pclk stability issue */
> +#define RCAR_DU_QUIRK_H3_ES1_PLL	BIT(2)	/* H3 ES1 PLL setup differs from non-ES1 */
>  
>  enum rcar_du_output {
>  	RCAR_DU_OUTPUT_DPAD0,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index c1bcb0e8b5b4..789ae9285108 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -283,12 +283,8 @@
>  #define DPLLCR			0x20044
>  #define DPLLCR_CODE		(0x95 << 24)
>  #define DPLLCR_PLCS1		(1 << 23)
> -/*
> - * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
> - * isn't implemented by other SoC in the Gen3 family it can safely be set
> - * unconditionally.
> - */
> -#define DPLLCR_PLCS0		(3 << 20)
> +#define DPLLCR_PLCS0_PLL	(1 << 21)
> +#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
>  #define DPLLCR_CLKE		(1 << 18)
>  #define DPLLCR_FDPLL(n)		((n) << 12)
>  #define DPLLCR_N(n)		((n) << 5)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control
  2023-01-20 16:18     ` Laurent Pinchart
@ 2023-01-20 17:05       ` Laurent Pinchart
  -1 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 17:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

On Fri, Jan 20, 2023 at 06:18:07PM +0200, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.

Another small comment: in the commit message, s/lvsd/lvds/

> On Fri, Jan 20, 2023 at 10:50:05AM +0200, Tomi Valkeinen wrote:
> > Reset LVDS using the reset control as CPG reset/release is required in
> > the hardware manual sequence.
> > 
> > Based on a BSP patch from Koji Matsuoka <koji.matsuoka.xm@renesas.com>.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/rcar-du/Kconfig     |  1 +
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ++++++++++++++++++-
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> > index a8f862c68b4f..151e400b996d 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
> >  	select DRM_PANEL
> >  	select OF_FLATTREE
> >  	select OF_OVERLAY
> > +	select RESET_CONTROLLER
> >  
> >  config DRM_RCAR_USE_MIPI_DSI
> >  	bool "R-Car DU MIPI DSI Encoder Support"
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 8e1be51fbee6..668604616bfd 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> >  #include <linux/slab.h>
> >  #include <linux/sys_soc.h>
> >  
> > @@ -61,6 +62,7 @@ struct rcar_lvds_device_info {
> >  struct rcar_lvds {
> >  	struct device *dev;
> >  	const struct rcar_lvds_device_info *info;
> > +	struct reset_control *rstc;
> >  
> >  	struct drm_bridge bridge;
> >  
> > @@ -845,6 +847,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > +	if (IS_ERR(lvds->rstc))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
> > +				     "failed to get cpg reset\n");
> 
> Missing blank line.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >  	pm_runtime_enable(&pdev->dev);
> >  
> >  	drm_bridge_add(&lvds->bridge);
> > @@ -924,6 +930,8 @@ static int rcar_lvds_runtime_suspend(struct device *dev)
> >  
> >  	clk_disable_unprepare(lvds->clocks.mod);
> >  
> > +	reset_control_assert(lvds->rstc);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -932,11 +940,20 @@ static int rcar_lvds_runtime_resume(struct device *dev)
> >  	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> >  	int ret;
> >  
> > +	ret = reset_control_deassert(lvds->rstc);
> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = clk_prepare_enable(lvds->clocks.mod);
> >  	if (ret < 0)
> > -		return ret;
> > +		goto err_reset_assert;
> >  
> >  	return 0;
> > +
> > +err_reset_assert:
> > +	reset_control_assert(lvds->rstc);
> > +
> > +	return ret;
> >  }
> >  
> >  static const struct dev_pm_ops rcar_lvds_pm_ops = {
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control
@ 2023-01-20 17:05       ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 17:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

On Fri, Jan 20, 2023 at 06:18:07PM +0200, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.

Another small comment: in the commit message, s/lvsd/lvds/

> On Fri, Jan 20, 2023 at 10:50:05AM +0200, Tomi Valkeinen wrote:
> > Reset LVDS using the reset control as CPG reset/release is required in
> > the hardware manual sequence.
> > 
> > Based on a BSP patch from Koji Matsuoka <koji.matsuoka.xm@renesas.com>.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > ---
> >  drivers/gpu/drm/rcar-du/Kconfig     |  1 +
> >  drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ++++++++++++++++++-
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> > index a8f862c68b4f..151e400b996d 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
> >  	select DRM_PANEL
> >  	select OF_FLATTREE
> >  	select OF_OVERLAY
> > +	select RESET_CONTROLLER
> >  
> >  config DRM_RCAR_USE_MIPI_DSI
> >  	bool "R-Car DU MIPI DSI Encoder Support"
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 8e1be51fbee6..668604616bfd 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> >  #include <linux/slab.h>
> >  #include <linux/sys_soc.h>
> >  
> > @@ -61,6 +62,7 @@ struct rcar_lvds_device_info {
> >  struct rcar_lvds {
> >  	struct device *dev;
> >  	const struct rcar_lvds_device_info *info;
> > +	struct reset_control *rstc;
> >  
> >  	struct drm_bridge bridge;
> >  
> > @@ -845,6 +847,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > +	if (IS_ERR(lvds->rstc))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
> > +				     "failed to get cpg reset\n");
> 
> Missing blank line.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >  	pm_runtime_enable(&pdev->dev);
> >  
> >  	drm_bridge_add(&lvds->bridge);
> > @@ -924,6 +930,8 @@ static int rcar_lvds_runtime_suspend(struct device *dev)
> >  
> >  	clk_disable_unprepare(lvds->clocks.mod);
> >  
> > +	reset_control_assert(lvds->rstc);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -932,11 +940,20 @@ static int rcar_lvds_runtime_resume(struct device *dev)
> >  	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> >  	int ret;
> >  
> > +	ret = reset_control_deassert(lvds->rstc);
> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = clk_prepare_enable(lvds->clocks.mod);
> >  	if (ret < 0)
> > -		return ret;
> > +		goto err_reset_assert;
> >  
> >  	return 0;
> > +
> > +err_reset_assert:
> > +	reset_control_assert(lvds->rstc);
> > +
> > +	return ret;
> >  }
> >  
> >  static const struct dev_pm_ops rcar_lvds_pm_ops = {
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 0/7] drm: rcar-du: Misc patches
  2023-01-20  8:50 ` Tomi Valkeinen
@ 2023-01-20 23:36   ` Laurent Pinchart
  -1 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 23:36 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Tomi Valkeinen,
	Geert Uytterhoeven

Hi Tomi,

On Fri, Jan 20, 2023 at 10:50:02AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> Hi,
> 
> v2 of the series. Diff to v1 can be found below.
> 
> The main changes are:
> - Runtime PM support for LVDS
> - Skip rcar_du_group_setup() for gen4+
> 
>  Tomi
> 
> Koji Matsuoka (1):
>   drm: rcar-du: lvds: Fix stop sequence
> 
> Tomi Valkeinen (6):
>   drm: rcar-du: dsi: add 'select RESET_CONTROLLER'
>   drm: rcar-du: lvds: Add runtime PM
>   drm: rcar-du: lvsd: Add reset control
>   drm: rcar-du: Add quirk for H3 ES1.x pclk workaround
>   drm: rcar-du: Fix setting a reserved bit in DPLLCR
>   drm: rcar-du: Stop accessing non-existant registers on gen4

For patches 1/7 to 6/7,

Tested-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

on D3, H3 ES2 and M3N.

I wanted to test this on H3 ES1.x too, but the boot loader on that board
is ancient and doesn't fit with my workflow anymore. I fear updating
them would be a deep rabbit hole. If I find spare time, I'll give it a
try.

>  drivers/gpu/drm/rcar-du/Kconfig         |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 39 +++++++----
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 49 ++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 +++-
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  8 +--
>  drivers/gpu/drm/rcar-du/rcar_lvds.c     | 86 +++++++++++++++++++++++--
>  7 files changed, 169 insertions(+), 28 deletions(-)
> 
> Interdiff against v1:
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 56b23333993c..008e172ed43b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -249,15 +249,24 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  			dpllcr |= DPLLCR_PLCS1
>  			       |  DPLLCR_INCS_DOTCLKIN1;
>  		} else {
> -			dpllcr |= DPLLCR_PLCS0
> +			dpllcr |= DPLLCR_PLCS0_PLL
>  			       |  DPLLCR_INCS_DOTCLKIN0;
> +
>  			/*
> -			 * On H3 ES1.x, in addition to setting bit 21 (PLCS0),
> -			 * also bit 20 has to be set to select PLL1 as the
> -			 * clock source.
> +			 * On ES2.x we have a single mux controlled via bit 21,
> +			 * which selects between DCLKIN source (bit 21 = 0) and
> +			 * a PLL source (bit 21 = 1), where the PLL is always
> +			 * PLL1.
> +			 *
> +			 * On ES1.x we have an additional mux, controlled
> +			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
> +			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
> +			 * so on ES1.x, in addition to setting bit 21, we need
> +			 * to set the bit 20.
>  			 */
> +
>  			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
> -				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1_SEL;
> +				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
>  		}
>  
>  		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index d689f2510081..82f9718ff500 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -710,10 +710,10 @@ static void rcar_du_shutdown(struct platform_device *pdev)
>  
>  static int rcar_du_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *soc_attr;
>  	struct rcar_du_device *rcdu;
>  	unsigned int mask;
>  	int ret;
> -	const struct soc_device_attribute *soc_attr;
>  
>  	if (drm_firmware_drivers_only())
>  		return -ENODEV;
> @@ -726,13 +726,12 @@ static int rcar_du_probe(struct platform_device *pdev)
>  
>  	rcdu->dev = &pdev->dev;
>  
> +	rcdu->info = of_device_get_match_data(rcdu->dev);
> +
>  	soc_attr = soc_device_match(rcar_du_soc_table);
>  	if (soc_attr)
>  		rcdu->info = soc_attr->data;
>  
> -	if (!rcdu->info)
> -		rcdu->info = of_device_get_match_data(rcdu->dev);
> -
>  	platform_set_drvdata(pdev, rcdu);
>  
>  	/* I/O resources */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index c236e2aa8a01..c2209d427bb7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -194,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>   */
>  int rcar_du_group_get(struct rcar_du_group *rgrp)
>  {
> +	struct rcar_du_device *rcdu = rgrp->dev;
> +
>  	if (rgrp->use_count)
>  		goto done;
>  
> -	rcar_du_group_setup(rgrp);
> +	if (rcdu->info->gen < 4)
> +		rcar_du_group_setup(rgrp);
>  
>  done:
>  	rgrp->use_count++;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index 94d913f66c8f..789ae9285108 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -283,13 +283,8 @@
>  #define DPLLCR			0x20044
>  #define DPLLCR_CODE		(0x95 << 24)
>  #define DPLLCR_PLCS1		(1 << 23)
> -/*
> - * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
> - * isn't implemented by other SoC in the Gen3 family it can safely be set
> - * unconditionally.
> - */
> -#define DPLLCR_PLCS0		(1 << 21)
> -#define DPLLCR_PLCS0_H3ES1X_PLL1_SEL	(1 << 20)
> +#define DPLLCR_PLCS0_PLL	(1 << 21)
> +#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
>  #define DPLLCR_CLKE		(1 << 18)
>  #define DPLLCR_FDPLL(n)		((n) << 12)
>  #define DPLLCR_N(n)		((n) << 5)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 01800cef1c33..8fa5f7400179 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
> @@ -82,14 +83,14 @@ struct rcar_lvds {
>  #define bridge_to_rcar_lvds(b) \
>  	container_of(b, struct rcar_lvds, bridge)
>  
> -static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
> +static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
>  {
> -	iowrite32(data, lvds->mmio + reg);
> +	return ioread32(lvds->mmio + reg);
>  }
>  
> -static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
> +static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
>  {
> -	return ioread32(lvds->mmio + reg);
> +	iowrite32(data, lvds->mmio + reg);
>  }
>  
>  /* -----------------------------------------------------------------------------
> @@ -323,10 +324,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
>  
>  	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>  
> -	reset_control_deassert(lvds->rstc);
> -
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return ret;
>  
>  	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
> @@ -346,9 +345,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>  
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> -
> -	reset_control_assert(lvds->rstc);
> +	pm_runtime_put(lvds->dev);
>  }
>  EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
>  
> @@ -407,10 +404,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
>  	u32 lvdcr0;
>  	int ret;
>  
> -	reset_control_deassert(lvds->rstc);
> -
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return;
>  
>  	/* Enable the companion LVDS encoder in dual-link mode. */
> @@ -576,6 +571,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>  	}
>  
> +	rcar_lvds_write(lvds, LVDCR0, 0);
>  	rcar_lvds_write(lvds, LVDCR1, 0);
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> @@ -584,8 +580,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		lvds->companion->funcs->atomic_disable(lvds->companion,
>  						       old_bridge_state);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> -	reset_control_assert(lvds->rstc);
> +	pm_runtime_put(lvds->dev);
>  }
>  
>  static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> @@ -878,11 +873,11 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	lvds->rstc = devm_reset_control_get(&pdev->dev, NULL);
> -	if (IS_ERR(lvds->rstc)) {
> -		dev_err(&pdev->dev, "failed to get cpg reset\n");
> -		return PTR_ERR(lvds->rstc);
> -	}
> +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(lvds->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
> +				     "failed to get cpg reset\n");
> +	pm_runtime_enable(&pdev->dev);
>  
>  	drm_bridge_add(&lvds->bridge);
>  
> @@ -895,6 +890,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
>  
>  	drm_bridge_remove(&lvds->bridge);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	return 0;
>  }
>  
> @@ -953,11 +950,48 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  
>  MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
>  
> +static int rcar_lvds_runtime_suspend(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(lvds->clocks.mod);
> +
> +	reset_control_assert(lvds->rstc);
> +
> +	return 0;
> +}
> +
> +static int rcar_lvds_runtime_resume(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = reset_control_deassert(lvds->rstc);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(lvds->clocks.mod);
> +	if (ret < 0)
> +		goto err_reset_assert;
> +
> +	return 0;
> +
> +err_reset_assert:
> +	reset_control_assert(lvds->rstc);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops rcar_lvds_pm_ops = {
> +	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver rcar_lvds_platform_driver = {
>  	.probe		= rcar_lvds_probe,
>  	.remove		= rcar_lvds_remove,
>  	.driver		= {
>  		.name	= "rcar-lvds",
> +		.pm	= &rcar_lvds_pm_ops,
>  		.of_match_table = rcar_lvds_of_table,
>  	},
>  };

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 0/7] drm: rcar-du: Misc patches
@ 2023-01-20 23:36   ` Laurent Pinchart
  0 siblings, 0 replies; 35+ messages in thread
From: Laurent Pinchart @ 2023-01-20 23:36 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-renesas-soc, Tomi Valkeinen, Kieran Bingham,
	Geert Uytterhoeven, dri-devel

Hi Tomi,

On Fri, Jan 20, 2023 at 10:50:02AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> 
> Hi,
> 
> v2 of the series. Diff to v1 can be found below.
> 
> The main changes are:
> - Runtime PM support for LVDS
> - Skip rcar_du_group_setup() for gen4+
> 
>  Tomi
> 
> Koji Matsuoka (1):
>   drm: rcar-du: lvds: Fix stop sequence
> 
> Tomi Valkeinen (6):
>   drm: rcar-du: dsi: add 'select RESET_CONTROLLER'
>   drm: rcar-du: lvds: Add runtime PM
>   drm: rcar-du: lvsd: Add reset control
>   drm: rcar-du: Add quirk for H3 ES1.x pclk workaround
>   drm: rcar-du: Fix setting a reserved bit in DPLLCR
>   drm: rcar-du: Stop accessing non-existant registers on gen4

For patches 1/7 to 6/7,

Tested-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

on D3, H3 ES2 and M3N.

I wanted to test this on H3 ES1.x too, but the boot loader on that board
is ancient and doesn't fit with my workflow anymore. I fear updating
them would be a deep rabbit hole. If I find spare time, I'll give it a
try.

>  drivers/gpu/drm/rcar-du/Kconfig         |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 39 +++++++----
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 49 ++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  2 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 11 +++-
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  |  8 +--
>  drivers/gpu/drm/rcar-du/rcar_lvds.c     | 86 +++++++++++++++++++++++--
>  7 files changed, 169 insertions(+), 28 deletions(-)
> 
> Interdiff against v1:
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 56b23333993c..008e172ed43b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -249,15 +249,24 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
>  			dpllcr |= DPLLCR_PLCS1
>  			       |  DPLLCR_INCS_DOTCLKIN1;
>  		} else {
> -			dpllcr |= DPLLCR_PLCS0
> +			dpllcr |= DPLLCR_PLCS0_PLL
>  			       |  DPLLCR_INCS_DOTCLKIN0;
> +
>  			/*
> -			 * On H3 ES1.x, in addition to setting bit 21 (PLCS0),
> -			 * also bit 20 has to be set to select PLL1 as the
> -			 * clock source.
> +			 * On ES2.x we have a single mux controlled via bit 21,
> +			 * which selects between DCLKIN source (bit 21 = 0) and
> +			 * a PLL source (bit 21 = 1), where the PLL is always
> +			 * PLL1.
> +			 *
> +			 * On ES1.x we have an additional mux, controlled
> +			 * via bit 20, for choosing between PLL0 (bit 20 = 0)
> +			 * and PLL1 (bit 20 = 1). We always want to use PLL1,
> +			 * so on ES1.x, in addition to setting bit 21, we need
> +			 * to set the bit 20.
>  			 */
> +
>  			if (rcdu->info->quirks & RCAR_DU_QUIRK_H3_ES1_PLL)
> -				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1_SEL;
> +				dpllcr |= DPLLCR_PLCS0_H3ES1X_PLL1;
>  		}
>  
>  		rcar_du_group_write(rcrtc->group, DPLLCR, dpllcr);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index d689f2510081..82f9718ff500 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -710,10 +710,10 @@ static void rcar_du_shutdown(struct platform_device *pdev)
>  
>  static int rcar_du_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *soc_attr;
>  	struct rcar_du_device *rcdu;
>  	unsigned int mask;
>  	int ret;
> -	const struct soc_device_attribute *soc_attr;
>  
>  	if (drm_firmware_drivers_only())
>  		return -ENODEV;
> @@ -726,13 +726,12 @@ static int rcar_du_probe(struct platform_device *pdev)
>  
>  	rcdu->dev = &pdev->dev;
>  
> +	rcdu->info = of_device_get_match_data(rcdu->dev);
> +
>  	soc_attr = soc_device_match(rcar_du_soc_table);
>  	if (soc_attr)
>  		rcdu->info = soc_attr->data;
>  
> -	if (!rcdu->info)
> -		rcdu->info = of_device_get_match_data(rcdu->dev);
> -
>  	platform_set_drvdata(pdev, rcdu);
>  
>  	/* I/O resources */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index c236e2aa8a01..c2209d427bb7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -194,10 +194,13 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>   */
>  int rcar_du_group_get(struct rcar_du_group *rgrp)
>  {
> +	struct rcar_du_device *rcdu = rgrp->dev;
> +
>  	if (rgrp->use_count)
>  		goto done;
>  
> -	rcar_du_group_setup(rgrp);
> +	if (rcdu->info->gen < 4)
> +		rcar_du_group_setup(rgrp);
>  
>  done:
>  	rgrp->use_count++;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index 94d913f66c8f..789ae9285108 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -283,13 +283,8 @@
>  #define DPLLCR			0x20044
>  #define DPLLCR_CODE		(0x95 << 24)
>  #define DPLLCR_PLCS1		(1 << 23)
> -/*
> - * PLCS0 is bit 21, but H3 ES1.x requires bit 20 to be set as well. As bit 20
> - * isn't implemented by other SoC in the Gen3 family it can safely be set
> - * unconditionally.
> - */
> -#define DPLLCR_PLCS0		(1 << 21)
> -#define DPLLCR_PLCS0_H3ES1X_PLL1_SEL	(1 << 20)
> +#define DPLLCR_PLCS0_PLL	(1 << 21)
> +#define DPLLCR_PLCS0_H3ES1X_PLL1	(1 << 20)
>  #define DPLLCR_CLKE		(1 << 18)
>  #define DPLLCR_FDPLL(n)		((n) << 12)
>  #define DPLLCR_N(n)		((n) << 5)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 01800cef1c33..8fa5f7400179 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
> @@ -82,14 +83,14 @@ struct rcar_lvds {
>  #define bridge_to_rcar_lvds(b) \
>  	container_of(b, struct rcar_lvds, bridge)
>  
> -static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
> +static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
>  {
> -	iowrite32(data, lvds->mmio + reg);
> +	return ioread32(lvds->mmio + reg);
>  }
>  
> -static u32 rcar_lvds_read(struct rcar_lvds *lvds, u32 reg)
> +static void rcar_lvds_write(struct rcar_lvds *lvds, u32 reg, u32 data)
>  {
> -	return ioread32(lvds->mmio + reg);
> +	iowrite32(data, lvds->mmio + reg);
>  }
>  
>  /* -----------------------------------------------------------------------------
> @@ -323,10 +324,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
>  
>  	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>  
> -	reset_control_deassert(lvds->rstc);
> -
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return ret;
>  
>  	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
> @@ -346,9 +345,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>  
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> -
> -	reset_control_assert(lvds->rstc);
> +	pm_runtime_put(lvds->dev);
>  }
>  EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
>  
> @@ -407,10 +404,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
>  	u32 lvdcr0;
>  	int ret;
>  
> -	reset_control_deassert(lvds->rstc);
> -
> -	ret = clk_prepare_enable(lvds->clocks.mod);
> -	if (ret < 0)
> +	ret = pm_runtime_resume_and_get(lvds->dev);
> +	if (ret)
>  		return;
>  
>  	/* Enable the companion LVDS encoder in dual-link mode. */
> @@ -576,6 +571,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>  	}
>  
> +	rcar_lvds_write(lvds, LVDCR0, 0);
>  	rcar_lvds_write(lvds, LVDCR1, 0);
>  	rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
> @@ -584,8 +580,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
>  		lvds->companion->funcs->atomic_disable(lvds->companion,
>  						       old_bridge_state);
>  
> -	clk_disable_unprepare(lvds->clocks.mod);
> -	reset_control_assert(lvds->rstc);
> +	pm_runtime_put(lvds->dev);
>  }
>  
>  static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> @@ -878,11 +873,11 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	lvds->rstc = devm_reset_control_get(&pdev->dev, NULL);
> -	if (IS_ERR(lvds->rstc)) {
> -		dev_err(&pdev->dev, "failed to get cpg reset\n");
> -		return PTR_ERR(lvds->rstc);
> -	}
> +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(lvds->rstc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
> +				     "failed to get cpg reset\n");
> +	pm_runtime_enable(&pdev->dev);
>  
>  	drm_bridge_add(&lvds->bridge);
>  
> @@ -895,6 +890,8 @@ static int rcar_lvds_remove(struct platform_device *pdev)
>  
>  	drm_bridge_remove(&lvds->bridge);
>  
> +	pm_runtime_disable(&pdev->dev);
> +
>  	return 0;
>  }
>  
> @@ -953,11 +950,48 @@ static const struct of_device_id rcar_lvds_of_table[] = {
>  
>  MODULE_DEVICE_TABLE(of, rcar_lvds_of_table);
>  
> +static int rcar_lvds_runtime_suspend(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(lvds->clocks.mod);
> +
> +	reset_control_assert(lvds->rstc);
> +
> +	return 0;
> +}
> +
> +static int rcar_lvds_runtime_resume(struct device *dev)
> +{
> +	struct rcar_lvds *lvds = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = reset_control_deassert(lvds->rstc);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(lvds->clocks.mod);
> +	if (ret < 0)
> +		goto err_reset_assert;
> +
> +	return 0;
> +
> +err_reset_assert:
> +	reset_control_assert(lvds->rstc);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops rcar_lvds_pm_ops = {
> +	SET_RUNTIME_PM_OPS(rcar_lvds_runtime_suspend, rcar_lvds_runtime_resume, NULL)
> +};
> +
>  static struct platform_driver rcar_lvds_platform_driver = {
>  	.probe		= rcar_lvds_probe,
>  	.remove		= rcar_lvds_remove,
>  	.driver		= {
>  		.name	= "rcar-lvds",
> +		.pm	= &rcar_lvds_pm_ops,
>  		.of_match_table = rcar_lvds_of_table,
>  	},
>  };

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM
  2023-01-20 16:16     ` Laurent Pinchart
@ 2023-01-23  8:05       ` Tomi Valkeinen
  -1 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-23  8:05 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

On 20/01/2023 18:16, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, Jan 20, 2023 at 10:50:04AM +0200, Tomi Valkeinen wrote:
>> Add simple runtime PM suspend and resume functionality.
> 
> I think you need to depend on PM in Kconfig. That's not a compile-time
> dependency but a runtime-dependency, with runtime PM support the
> suspend/resume handler will never be called.

Yes, LVDS won't work without runtime PM after this patch.

>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_lvds.c | 43 +++++++++++++++++++++++++----
>>   1 file changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>> index 81a060c2fe3f..8e1be51fbee6 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/of_device.h>
>>   #include <linux/of_graph.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>>   #include <linux/slab.h>
>>   #include <linux/sys_soc.h>
>>   
>> @@ -316,8 +317,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
>>   
>>   	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>>   
>> -	ret = clk_prepare_enable(lvds->clocks.mod);
>> -	if (ret < 0)
>> +	ret = pm_runtime_resume_and_get(lvds->dev);
>> +	if (ret)
>>   		return ret;
>>   
>>   	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
>> @@ -337,7 +338,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>>   
>>   	rcar_lvds_write(lvds, LVDPLLCR, 0);
>>   
>> -	clk_disable_unprepare(lvds->clocks.mod);
>> +	pm_runtime_put(lvds->dev);
> 
> Should we use pm_runtime_put_sync() here, to make sure the clock gets
> disabled right away ? The DU hardware may depend on the exact sequencing
> of events. I would then do the same in rcar_lvds_atomic_disable().

That's perhaps a good idea. I think I saw some of the docs saying that 
startup sequences must begin with the reset. If we don't use _sync, we 
could end up not resetting at all.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

  Tomi


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM
@ 2023-01-23  8:05       ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-23  8:05 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

On 20/01/2023 18:16, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, Jan 20, 2023 at 10:50:04AM +0200, Tomi Valkeinen wrote:
>> Add simple runtime PM suspend and resume functionality.
> 
> I think you need to depend on PM in Kconfig. That's not a compile-time
> dependency but a runtime-dependency, with runtime PM support the
> suspend/resume handler will never be called.

Yes, LVDS won't work without runtime PM after this patch.

>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_lvds.c | 43 +++++++++++++++++++++++++----
>>   1 file changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>> index 81a060c2fe3f..8e1be51fbee6 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/of_device.h>
>>   #include <linux/of_graph.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>>   #include <linux/slab.h>
>>   #include <linux/sys_soc.h>
>>   
>> @@ -316,8 +317,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
>>   
>>   	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>>   
>> -	ret = clk_prepare_enable(lvds->clocks.mod);
>> -	if (ret < 0)
>> +	ret = pm_runtime_resume_and_get(lvds->dev);
>> +	if (ret)
>>   		return ret;
>>   
>>   	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
>> @@ -337,7 +338,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>>   
>>   	rcar_lvds_write(lvds, LVDPLLCR, 0);
>>   
>> -	clk_disable_unprepare(lvds->clocks.mod);
>> +	pm_runtime_put(lvds->dev);
> 
> Should we use pm_runtime_put_sync() here, to make sure the clock gets
> disabled right away ? The DU hardware may depend on the exact sequencing
> of events. I would then do the same in rcar_lvds_atomic_disable().

That's perhaps a good idea. I think I saw some of the docs saying that 
startup sequences must begin with the reset. If we don't use _sync, we 
could end up not resetting at all.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

  Tomi


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control
  2023-01-20 17:05       ` Laurent Pinchart
@ 2023-01-23  8:07         ` Tomi Valkeinen
  -1 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-23  8:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, dri-devel, linux-renesas-soc, Geert Uytterhoeven

On 20/01/2023 19:05, Laurent Pinchart wrote:
> On Fri, Jan 20, 2023 at 06:18:07PM +0200, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> Thank you for the patch.
> 
> Another small comment: in the commit message, s/lvsd/lvds/

Yep.

>> On Fri, Jan 20, 2023 at 10:50:05AM +0200, Tomi Valkeinen wrote:
>>> Reset LVDS using the reset control as CPG reset/release is required in
>>> the hardware manual sequence.
>>>
>>> Based on a BSP patch from Koji Matsuoka <koji.matsuoka.xm@renesas.com>.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>> ---
>>>   drivers/gpu/drm/rcar-du/Kconfig     |  1 +
>>>   drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ++++++++++++++++++-
>>>   2 files changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
>>> index a8f862c68b4f..151e400b996d 100644
>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
>>>   	select DRM_PANEL
>>>   	select OF_FLATTREE
>>>   	select OF_OVERLAY
>>> +	select RESET_CONTROLLER
>>>   
>>>   config DRM_RCAR_USE_MIPI_DSI
>>>   	bool "R-Car DU MIPI DSI Encoder Support"
>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>>> index 8e1be51fbee6..668604616bfd 100644
>>> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
>>> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>>> @@ -17,6 +17,7 @@
>>>   #include <linux/of_graph.h>
>>>   #include <linux/platform_device.h>
>>>   #include <linux/pm_runtime.h>
>>> +#include <linux/reset.h>
>>>   #include <linux/slab.h>
>>>   #include <linux/sys_soc.h>
>>>   
>>> @@ -61,6 +62,7 @@ struct rcar_lvds_device_info {
>>>   struct rcar_lvds {
>>>   	struct device *dev;
>>>   	const struct rcar_lvds_device_info *info;
>>> +	struct reset_control *rstc;
>>>   
>>>   	struct drm_bridge bridge;
>>>   
>>> @@ -845,6 +847,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>>>   	if (ret < 0)
>>>   		return ret;
>>>   
>>> +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>>> +	if (IS_ERR(lvds->rstc))
>>> +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
>>> +				     "failed to get cpg reset\n");
>>
>> Missing blank line.
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

  Tomi


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control
@ 2023-01-23  8:07         ` Tomi Valkeinen
  0 siblings, 0 replies; 35+ messages in thread
From: Tomi Valkeinen @ 2023-01-23  8:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-renesas-soc, Kieran Bingham, Geert Uytterhoeven, dri-devel

On 20/01/2023 19:05, Laurent Pinchart wrote:
> On Fri, Jan 20, 2023 at 06:18:07PM +0200, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> Thank you for the patch.
> 
> Another small comment: in the commit message, s/lvsd/lvds/

Yep.

>> On Fri, Jan 20, 2023 at 10:50:05AM +0200, Tomi Valkeinen wrote:
>>> Reset LVDS using the reset control as CPG reset/release is required in
>>> the hardware manual sequence.
>>>
>>> Based on a BSP patch from Koji Matsuoka <koji.matsuoka.xm@renesas.com>.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>>> ---
>>>   drivers/gpu/drm/rcar-du/Kconfig     |  1 +
>>>   drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ++++++++++++++++++-
>>>   2 files changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
>>> index a8f862c68b4f..151e400b996d 100644
>>> --- a/drivers/gpu/drm/rcar-du/Kconfig
>>> +++ b/drivers/gpu/drm/rcar-du/Kconfig
>>> @@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
>>>   	select DRM_PANEL
>>>   	select OF_FLATTREE
>>>   	select OF_OVERLAY
>>> +	select RESET_CONTROLLER
>>>   
>>>   config DRM_RCAR_USE_MIPI_DSI
>>>   	bool "R-Car DU MIPI DSI Encoder Support"
>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>>> index 8e1be51fbee6..668604616bfd 100644
>>> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
>>> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
>>> @@ -17,6 +17,7 @@
>>>   #include <linux/of_graph.h>
>>>   #include <linux/platform_device.h>
>>>   #include <linux/pm_runtime.h>
>>> +#include <linux/reset.h>
>>>   #include <linux/slab.h>
>>>   #include <linux/sys_soc.h>
>>>   
>>> @@ -61,6 +62,7 @@ struct rcar_lvds_device_info {
>>>   struct rcar_lvds {
>>>   	struct device *dev;
>>>   	const struct rcar_lvds_device_info *info;
>>> +	struct reset_control *rstc;
>>>   
>>>   	struct drm_bridge bridge;
>>>   
>>> @@ -845,6 +847,10 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>>>   	if (ret < 0)
>>>   		return ret;
>>>   
>>> +	lvds->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>>> +	if (IS_ERR(lvds->rstc))
>>> +		return dev_err_probe(&pdev->dev, PTR_ERR(lvds->rstc),
>>> +				     "failed to get cpg reset\n");
>>
>> Missing blank line.
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

  Tomi


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM
  2023-01-20 16:16     ` Laurent Pinchart
@ 2023-01-23 14:31       ` Geert Uytterhoeven
  -1 siblings, 0 replies; 35+ messages in thread
From: Geert Uytterhoeven @ 2023-01-23 14:31 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tomi Valkeinen, Kieran Bingham, dri-devel, linux-renesas-soc

Hi Laurent,

On Fri, Jan 20, 2023 at 5:16 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Jan 20, 2023 at 10:50:04AM +0200, Tomi Valkeinen wrote:
> > Add simple runtime PM suspend and resume functionality.
>
> I think you need to depend on PM in Kconfig. That's not a compile-time
> dependency but a runtime-dependency, with runtime PM support the
> suspend/resume handler will never be called.

While technically that is correct, you'll have a hard time getting here
with CONFIG_PM=n, as both ARCH_RCAR_GEN2 and ARCH_RCAR_GEN3
do select PM.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM
@ 2023-01-23 14:31       ` Geert Uytterhoeven
  0 siblings, 0 replies; 35+ messages in thread
From: Geert Uytterhoeven @ 2023-01-23 14:31 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-renesas-soc, Kieran Bingham, dri-devel, Tomi Valkeinen

Hi Laurent,

On Fri, Jan 20, 2023 at 5:16 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Jan 20, 2023 at 10:50:04AM +0200, Tomi Valkeinen wrote:
> > Add simple runtime PM suspend and resume functionality.
>
> I think you need to depend on PM in Kconfig. That's not a compile-time
> dependency but a runtime-dependency, with runtime PM support the
> suspend/resume handler will never be called.

While technically that is correct, you'll have a hard time getting here
with CONFIG_PM=n, as both ARCH_RCAR_GEN2 and ARCH_RCAR_GEN3
do select PM.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2023-01-23 14:32 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-20  8:50 [PATCH v2 0/7] drm: rcar-du: Misc patches Tomi Valkeinen
2023-01-20  8:50 ` Tomi Valkeinen
2023-01-20  8:50 ` [PATCH v2 1/7] drm: rcar-du: dsi: add 'select RESET_CONTROLLER' Tomi Valkeinen
2023-01-20  8:50   ` Tomi Valkeinen
2023-01-20  8:50 ` [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM Tomi Valkeinen
2023-01-20 16:16   ` Laurent Pinchart
2023-01-20 16:16     ` Laurent Pinchart
2023-01-23  8:05     ` Tomi Valkeinen
2023-01-23  8:05       ` Tomi Valkeinen
2023-01-23 14:31     ` Geert Uytterhoeven
2023-01-23 14:31       ` Geert Uytterhoeven
2023-01-20  8:50 ` [PATCH v2 3/7] drm: rcar-du: lvsd: Add reset control Tomi Valkeinen
2023-01-20 16:18   ` Laurent Pinchart
2023-01-20 16:18     ` Laurent Pinchart
2023-01-20 17:05     ` Laurent Pinchart
2023-01-20 17:05       ` Laurent Pinchart
2023-01-23  8:07       ` Tomi Valkeinen
2023-01-23  8:07         ` Tomi Valkeinen
2023-01-20  8:50 ` [PATCH v2 4/7] drm: rcar-du: lvds: Fix stop sequence Tomi Valkeinen
2023-01-20  8:50   ` Tomi Valkeinen
2023-01-20 16:19   ` Laurent Pinchart
2023-01-20 16:19     ` Laurent Pinchart
2023-01-20  8:50 ` [PATCH v2 5/7] drm: rcar-du: Add quirk for H3 ES1.x pclk workaround Tomi Valkeinen
2023-01-20  8:50   ` Tomi Valkeinen
2023-01-20  8:50 ` [PATCH v2 6/7] drm: rcar-du: Fix setting a reserved bit in DPLLCR Tomi Valkeinen
2023-01-20 17:03   ` Laurent Pinchart
2023-01-20 17:03     ` Laurent Pinchart
2023-01-20  8:50 ` [PATCH v2 7/7] drm: rcar-du: Stop accessing non-existant registers on gen4 Tomi Valkeinen
2023-01-20  8:50   ` Tomi Valkeinen
2023-01-20 16:21   ` Laurent Pinchart
2023-01-20 16:21     ` Laurent Pinchart
2023-01-20 16:28     ` Tomi Valkeinen
2023-01-20 16:28       ` Tomi Valkeinen
2023-01-20 23:36 ` [PATCH v2 0/7] drm: rcar-du: Misc patches Laurent Pinchart
2023-01-20 23:36   ` Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.