drm: rcar-du: add R8A77970 support

PATCH In review Sergei Shtylyov 2 participants 5 messages Jan 11, 2018 – Jan 12, 2018

Subsystems (4)

Device tree bindings DRM drivers DRM misc Arm/RISC-V/Renesas architecture

lore

Share & tools

lore

Sergei Shtylyov

author

2018-01-11 16:54 UTC #0 lore raw

PATCH 3/3 drm: rcar-du: add R8A77970 support

Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC has only 1 display port). Note that there are some differences with the other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same layout as on the R-Car gen2 SoCs...

Signed-off-by Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/[email protected]>

---
 Documentation/devicetree/bindings/display/renesas,du.txt |    1 
 drivers/gpu/drm/rcar-du/rcar_du_drv.c                    |   23 +++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_drv.h                    |    1 
 drivers/gpu/drm/rcar-du/rcar_du_group.c                  |   10 +++---
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c                |   20 +++++++++----
 5 files changed, 45 insertions(+), 10 deletions(-)

Documentation/devicetree/bindings/display/renesas,du.txt +1 -0

Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt
===================================================================
@@ -13,6 +13,7 @@ Required Properties:
     - "renesas,du-r8a7794" for R8A7794 (R-Car E2) compatible DU
     - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
     - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
+    - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU
 
   - reg: A list of base address and length of each memory resource, one for
     each entry in the reg-names property.

drivers/gpu/drm/rcar-du/rcar_du_drv.c +23 -0

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
===================================================================
@@ -258,6 +258,28 @@ static const struct rcar_du_device_info
 	.dpll_ch =  BIT(1),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a77970_info = {
+	.gen = 3,
+	.model = R8A77970,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
+	.num_crtcs = 1,
+	.routes = {
+		/* R8A77970 has one RGB output and one LVDS output. */
+		[RCAR_DU_OUTPUT_DPAD0] = {
+			.possible_crtcs = BIT(0),
+			.port = 1,
+		},
+		[RCAR_DU_OUTPUT_LVDS0] = {
+			.possible_crtcs = BIT(0),
+			.port = 0,
+		},
+	},
+	.num_lvds = 1,
+	.dpll_ch  = BIT(1),
+};
+
 static const struct of_device_id rcar_du_of_table[] = {
 	{ .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
 	{ .compatible = "renesas,du-r8a7745", .data = &rzg1_du_r8a7745_info },

@@ -269,6 +291,7 @@ static const struct of_device_id rcar_du
 	{ .compatible = "renesas,du-r8a7794", .data = &rcar_du_r8a7794_info },
 	{ .compatible = "renesas,du-r8a7795", .data = &rcar_du_r8a7795_info },
 	{ .compatible = "renesas,du-r8a7796", .data = &rcar_du_r8a7796_info },
+	{ .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info },
 	{ }
 };

drivers/gpu/drm/rcar-du/rcar_du_drv.h +1 -0

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
===================================================================
@@ -59,6 +59,7 @@ enum rcar_du_model {
 	R8A7794,
 	R8A7795,
 	R8A7796,
+	R8A77970,
 };
 
 /*

drivers/gpu/drm/rcar-du/rcar_du_group.c +6 -4

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
===================================================================
@@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r
 	rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS);
 
 	/* Apply planes to CRTCs association. */
-	mutex_lock(&rgrp->lock);
-	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
-			    rgrp->dptsr_planes);
-	mutex_unlock(&rgrp->lock);
+	if (rcdu->info->num_crtcs > 1) {
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}
 }
 
 /*

drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +14 -6

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===================================================================
@@ -71,8 +71,10 @@ static int rcar_du_lvdsenc_start(struct
 				 struct rcar_du_crtc *rcrtc)
 {
 	u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
+	const struct rcar_du_device_info *info = lvds->dev->info;
 	const struct drm_display_mode *mode = &rcrtc->crtc.mode;
-	unsigned int gen = lvds->dev->info->gen;
+	unsigned int gen = info->gen;
+	enum rcar_du_model model = info->model;
 	unsigned int freq = mode->clock;
 	int ret;

@@ -104,7 +106,7 @@ static int rcar_du_lvdsenc_start(struct
 	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
 
 	/* PLL clock configuration */
-	if (gen < 3)
+	if (gen < 3 || model == R8A77970)
 		lvdpllcr = rcar_lvds_gen2_lvdpllcr(freq);
 	else
 		lvdpllcr = rcar_lvds_gen3_lvdpllcr(freq);

@@ -136,6 +138,12 @@ static int rcar_du_lvdsenc_start(struct
 		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 	}
 
+	/* Turn on the LVDS PHY on R-Car V3M. */
+	if (model == R8A77970) {
+		lvdcr0 |= LVDCR0_LVEN;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
+
 	/* Wait for the startup delay. */
 	usleep_range(100, 150);

@@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d
 void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
 				  struct drm_display_mode *mode)
 {
-	struct rcar_du_device *rcdu = lvds->dev;
+	const struct rcar_du_device_info *info = lvds->dev->info;
 
 	/*
 	 * The internal LVDS encoder has a restricted clock frequency operating
-	 * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). Clamp
-	 * the clock accordingly.
+	 * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz
+	 * on Gen3). Clamp the clock accordingly.
 	 */
-	if (rcdu->info->gen < 3)
+	if (info->gen < 3 || info->model == R8A77970)
 		mode->clock = clamp(mode->clock, 30000, 150000);
 	else
 		mode->clock = clamp(mode->clock, 25175, 148500);

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Laurent Pinchart

2018-01-12 01:13 UTC #1 lore raw

Hi Sergei,

Thank you for the patch.

Quoted 6 lines · On Thursday, 11 January 2018 18:54:35 EET Sergei Shtylyov wrote:

Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC has only 1 display port). Note that there are some differences with the other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same layout as on the R-Car gen2 SoCs...
Signed-off-by Sergei Shtylyov <[email protected]>

Could you please rebase this series on top of the LVDS rework posted as "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https://
www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier to implement support for V3M. Please then split the DU and LVDS drivers changes in two patches.

Quoted 2 lines

---
 Documentation/devicetree/bindings/display/renesas,du.txt |    1

Please split the DT bindings changes to a separate patch.

Quoted 17 lines

 drivers/gpu/drm/rcar-du/rcar_du_drv.c                    |   23 +++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_drv.h                    |    1
 drivers/gpu/drm/rcar-du/rcar_du_group.c                  |   10 +++---
 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c                |   20 +++++++----
 5 files changed, 45 insertions(+), 10 deletions(-)

Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt
===================================================================

Documentation/devicetree/bindings/display/renesas,du.txt +1 -0

@@ -13,6 +13,7 @@ Required Properties:
     - "renesas,du-r8a7794" for R8A7794 (R-Car E2) compatible DU
     - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
     - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
+    - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU
 
   - reg: A list of base address and length of each memory resource, one for

Quoted 1 line

each entry in the reg-names property.

You also need to update the ports table further down in this file.

Quoted 15 lines

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
===================================================================

drivers/gpu/drm/rcar-du/rcar_du_drv.c +7 -0

@@ -258,6 +258,28 @@ static const struct rcar_du_device_info
 	.dpll_ch =  BIT(1),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a77970_info = {
+	.gen = 3,
+	.model = R8A77970,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
+	.num_crtcs = 1,

drivers/gpu/drm/rcar-du/rcar_du_drv.c static const struct rcar_du_device_info · line 277

Quoted 10 lines

drivers/gpu/drm/rcar-du/rcar_du_drv.c +10 -0

+	.routes = {
+		/* R8A77970 has one RGB output and one LVDS output. */
+		[RCAR_DU_OUTPUT_DPAD0] = {
+			.possible_crtcs = BIT(0),
+			.port = 1,
+		},
+		[RCAR_DU_OUTPUT_LVDS0] = {
+			.possible_crtcs = BIT(0),
+			.port = 0,
+		},

All the other SoCs have DPAD0 as port 0. Unless there's a specific need for a different implementation with V3M I'd keep the same order.

drivers/gpu/drm/rcar-du/rcar_du_drv.c static const struct rcar_du_device_info · line 280

Quoted 3 lines

drivers/gpu/drm/rcar-du/rcar_du_drv.c +3 -0
+	},
+	.num_lvds = 1,
+	.dpll_ch  = BIT(1),

This doesn't seem to be correct, there's no DPLL in V3M.

Quoted 32 lines

drivers/gpu/drm/rcar-du/rcar_du_drv.c +3 -0

+};
+
 static const struct of_device_id rcar_du_of_table[] = {
 	{ .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info },
 	{ .compatible = "renesas,du-r8a7745", .data = &rzg1_du_r8a7745_info },

@@ -269,6 +291,7 @@ static const struct of_device_id rcar_du
 	{ .compatible = "renesas,du-r8a7794", .data = &rcar_du_r8a7794_info },
 	{ .compatible = "renesas,du-r8a7795", .data = &rcar_du_r8a7795_info },
 	{ .compatible = "renesas,du-r8a7796", .data = &rcar_du_r8a7796_info },
+	{ .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info },
 	{ }
 };

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
===================================================================

drivers/gpu/drm/rcar-du/rcar_du_drv.h +1 -0

@@ -59,6 +59,7 @@ enum rcar_du_model {
 	R8A7794,
 	R8A7795,
 	R8A7796,
+	R8A77970,
 };
 
 /*

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
===================================================================

drivers/gpu/drm/rcar-du/rcar_du_group.c +0 -0

@@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r
 	rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS);
 
 	/* Apply planes to CRTCs association. */

drivers/gpu/drm/rcar-du/rcar_du_group.c static void rcar_du_group_setup(struct r · line 141

Quoted 10 lines

drivers/gpu/drm/rcar-du/rcar_du_group.c +6 -4

-	mutex_lock(&rgrp->lock);
-	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
-			    rgrp->dptsr_planes);
-	mutex_unlock(&rgrp->lock);
+	if (rcdu->info->num_crtcs > 1) {
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}

Shouldn't you skip writing to the DPTSR register if there's a single DPTSR in the group ? That would then apply to M3-W as well, which doesn't have the DPTSR2 register. I'd split this change to a separate patch.

Quoted 48 lines

drivers/gpu/drm/rcar-du/rcar_du_group.c +0 -0

 }
 
 /*

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===================================================================

drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +11 -3

@@ -71,8 +71,10 @@ static int rcar_du_lvdsenc_start(struct
 				 struct rcar_du_crtc *rcrtc)
 {
 	u32 lvdhcr, lvdpllcr, lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
+	const struct rcar_du_device_info *info = lvds->dev->info;
 	const struct drm_display_mode *mode = &rcrtc->crtc.mode;
-	unsigned int gen = lvds->dev->info->gen;
+	unsigned int gen = info->gen;
+	enum rcar_du_model model = info->model;
 	unsigned int freq = mode->clock;
 	int ret;

@@ -104,7 +106,7 @@ static int rcar_du_lvdsenc_start(struct
 	rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
 
 	/* PLL clock configuration */
-	if (gen < 3)
+	if (gen < 3 || model == R8A77970)
 		lvdpllcr = rcar_lvds_gen2_lvdpllcr(freq);
 	else
 		lvdpllcr = rcar_lvds_gen3_lvdpllcr(freq);

@@ -136,6 +138,12 @@ static int rcar_du_lvdsenc_start(struct
 		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 	}
 
+	/* Turn on the LVDS PHY on R-Car V3M. */
+	if (model == R8A77970) {
+		lvdcr0 |= LVDCR0_LVEN;
+		rcar_lvds_write(lvds, LVDCR0, lvdcr0);
+	}
+
 	/* Wait for the startup delay. */
 	usleep_range(100, 150);

@@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d
 void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
 				  struct drm_display_mode *mode)
 {
-	struct rcar_du_device *rcdu = lvds->dev;
+	const struct rcar_du_device_info *info = lvds->dev->info;

drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c int rcar_du_lvdsenc_enable(struct rcar_d · line 196

Quoted 10 lines

drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +3 -3

 	/*
 	 * The internal LVDS encoder has a restricted clock frequency operating
-	 * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). Clamp
-	 * the clock accordingly.
+	 * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz
+	 * on Gen3). Clamp the clock accordingly.
 	 */
-	if (rcdu->info->gen < 3)
+	if (info->gen < 3 || info->model == R8A77970)
 		mode->clock = clamp(mode->clock, 30000, 150000);

According to the datasheet the frequency range for V3M is the same as for the H3 and M3 SoCs. The range seems to have changed starting in datasheet version 0.52. I would fix the range in a separate patch first.

If you want I can send patches to fix this issue and the previous one, or you can write them and include them in v2. Let me know what you'd prefer.

Quoted 2 lines

drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +0 -0
 	else
 		mode->clock = clamp(mode->clock, 25175, 148500);

Signature · 3 lines

Regards,

Laurent Pinchart

Sergei Shtylyov

author

2018-01-12 09:23 UTC #2 lore raw

Quoted 13 lines · Laurent Pinchart

Quoted 6 lines
Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC has only 1 display port). Note that there are some differences with the other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same layout as on the R-Car gen2 SoCs...
Signed-off-by Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/[email protected]>
Could you please rebase this series on top of the LVDS rework posted as "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https://
www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier to implement support for V3M. Please then split the DU and LVDS drivers changes in two patches.

Quoted 4 lines

Quoted 2 lines
---
  Documentation/devicetree/bindings/display/renesas,du.txt |    1
Please split the DT bindings changes to a separate patch.

I don't like putting a one-line chnage into a separate bindings patch...

Quoted 16 lines

  drivers/gpu/drm/rcar-du/rcar_du_drv.c                    |   23 +++++++++++
  drivers/gpu/drm/rcar-du/rcar_du_drv.h                    |    1
  drivers/gpu/drm/rcar-du/rcar_du_group.c                  |   10 +++---
  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c                |   20 +++++++----
  5 files changed, 45 insertions(+), 10 deletions(-)

Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt
===================================================================

Documentation/devicetree/bindings/display/renesas,du.txt +1 -0

@@ -13,6 +13,7 @@ Required Properties:
      - "renesas,du-r8a7794" for R8A7794 (R-Car E2) compatible DU
      - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU
      - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU
+    - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU

Quoted 4 lines

Quoted 2 lines
Documentation/devicetree/bindings/display/renesas,du.txt +0 -0
    - reg: A list of base address and length of each memory resource, one for
each entry in the reg-names property.
You also need to update the ports table further down in this file.

... but this one seems to justify dpoing it that way. :-)

Quoted 20 lines

Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
===================================================================

drivers/gpu/drm/rcar-du/rcar_du_drv.c +12 -0

@@ -258,6 +258,28 @@ static const struct rcar_du_device_info
  	.dpll_ch =  BIT(1),
  };
 
+static const struct rcar_du_device_info rcar_du_r8a77970_info = {
+	.gen = 3,
+	.model = R8A77970,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
+	.num_crtcs = 1,
+	.routes = {
+		/* R8A77970 has one RGB output and one LVDS output. */
+		[RCAR_DU_OUTPUT_DPAD0] = {
+			.possible_crtcs = BIT(0),
+			.port = 1,

Quoted 8 lines

drivers/gpu/drm/rcar-du/rcar_du_drv.c static const struct rcar_du_device_info · line 277
Quoted 5 lines
drivers/gpu/drm/rcar-du/rcar_du_drv.c +5 -0
+		},
+		[RCAR_DU_OUTPUT_LVDS0] = {
+			.possible_crtcs = BIT(0),
+			.port = 0,
+		},
All the other SoCs have DPAD0 as port 0. Unless there's a specific need for a different implementation with V3M I'd keep the same order.

I'll look into this..

Quoted 5 lines

drivers/gpu/drm/rcar-du/rcar_du_drv.c static const struct rcar_du_device_info · line 280
Quoted 3 lines
drivers/gpu/drm/rcar-du/rcar_du_drv.c +3 -0
+	},
+	.num_lvds = 1,
+	.dpll_ch  = BIT(1),
This doesn't seem to be correct, there's no DPLL in V3M.

Indeed, thanks!

[...]

Quoted 7 lines

Quoted 7 lines
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
===================================================================
drivers/gpu/drm/rcar-du/rcar_du_group.c +0 -0
@@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r
  	rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS);
 

Quoted 15 lines

drivers/gpu/drm/rcar-du/rcar_du_group.c static void rcar_du_group_setup(struct r · line 141
Quoted 11 lines
drivers/gpu/drm/rcar-du/rcar_du_group.c +6 -4
  	/* Apply planes to CRTCs association. */
-	mutex_lock(&rgrp->lock);
-	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
-			    rgrp->dptsr_planes);
-	mutex_unlock(&rgrp->lock);
+	if (rcdu->info->num_crtcs > 1) {
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}
Shouldn't you skip writing to the DPTSR register if there's a single DPTSR in the group ? That would then apply to M3-W as well, which doesn't have the DPTSR2 register. I'd split this change to a separate patch.

OK, I guess you know this stuff better -- I didn't know DPTSR2 is used at
all... :-)

[...]

Quoted 4 lines

Quoted 4 lines
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===================================================================
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +0 -0

[...]

Quoted 7 lines

drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +1 -1

@@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d
  void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
  				  struct drm_display_mode *mode)
  {
-	struct rcar_du_device *rcdu = lvds->dev;
+	const struct rcar_du_device_info *info = lvds->dev->info;

Quoted 13 lines

drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c int rcar_du_lvdsenc_enable(struct rcar_d · line 196
Quoted 10 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +3 -3
  	/*
  	 * The internal LVDS encoder has a restricted clock frequency operating
-	 * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3). Clamp
-	 * the clock accordingly.
+	 * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz
+	 * on Gen3). Clamp the clock accordingly.
  	 */
-	if (rcdu->info->gen < 3)
+	if (info->gen < 3 || info->model == R8A77970)
  		mode->clock = clamp(mode->clock, 30000, 150000);
According to the datasheet the frequency range for V3M is the same as for the H3 and M3 SoCs.

Indeed! I thought it's determined by the LVDPLLCR layout but it's not...

Quoted 2 lines

The range seems to have changed starting in datasheet version 0.52. I would fix the range in a separate patch first.

Yes.

Quoted 1 line

If you want I can send patches to fix this issue

Yes, please. You clearly know about DU more than me. :-)

Quoted 5 lines

and the previous one, or you
can write them and include them in v2. Let me know what you'd prefer.
Quoted 2 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +0 -0
  	else
  		mode->clock = clamp(mode->clock, 25175, 148500);

The lower bound documented on gen3 is 31 MHz indeed...

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Laurent Pinchart

2018-01-12 14:30 UTC #3 lore raw

Hi Sergei,

Quoted 26 lines · On Friday, 12 January 2018 11:23:00 EET Sergei Shtylyov wrote:

Quoted 13 lines · Laurent Pinchart
Quoted 6 lines
Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC has only 1 display port). Note that there are some differences with the other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same layout as on the R-Car gen2 SoCs...
Signed-off-by Sergei Shtylyov <[email protected]>
Could you please rebase this series on top of the LVDS rework posted as "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https://
www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier to implement support for V3M. Please then split the DU and LVDS drivers changes in two patches.
Quoted 5 lines
Quoted 3 lines
---
  Documentation/devicetree/bindings/display/renesas,du.txt |    1
Please split the DT bindings changes to a separate patch.
I don't like putting a one-line chnage into a separate bindings patch...
Quoted 5 lines
Quoted 5 lines
  drivers/gpu/drm/rcar-du/rcar_du_drv.c                    |   23 +++++++
  drivers/gpu/drm/rcar-du/rcar_du_drv.h                    |    1
  drivers/gpu/drm/rcar-du/rcar_du_group.c                  |   10 +++---
  drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c                |   20 ++++---
  5 files changed, 45 insertions(+), 10 deletions(-)

[snip]

Quoted 25 lines

Quoted 10 lines
Quoted 10 lines
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
===================================================================
drivers/gpu/drm/rcar-du/rcar_du_group.c +0 -0
@@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r
 
  	rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS);
 
  	/* Apply planes to CRTCs association. */
 
Quoted 14 lines
drivers/gpu/drm/rcar-du/rcar_du_group.c static void rcar_du_group_setup(struct r · line 141
Quoted 10 lines
drivers/gpu/drm/rcar-du/rcar_du_group.c +6 -4
-	mutex_lock(&rgrp->lock);
-	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
-			    rgrp->dptsr_planes);
-	mutex_unlock(&rgrp->lock);
+	if (rcdu->info->num_crtcs > 1) {
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}
Shouldn't you skip writing to the DPTSR register if there's a single DPTSR in the group ? That would then apply to M3-W as well, which doesn't have the DPTSR2 register. I'd split this change to a separate patch.

Quoted 2 lines

OK, I guess you know this stuff better -- I didn't know DPTSR2 is used at all... :-)

Should I send a patch for this as well ?

Quoted 34 lines

[...]
Quoted 4 lines
Quoted 4 lines
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===================================================================
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +0 -0
[...]
Quoted 7 lines
Quoted 7 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +1 -1
@@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d
  void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
  				  struct drm_display_mode *mode)
  {
-	struct rcar_du_device *rcdu = lvds->dev;
+	const struct rcar_du_device_info *info = lvds->dev->info;
 
Quoted 15 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c int rcar_du_lvdsenc_enable(struct rcar_d · line 196
Quoted 12 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +0 -1
  /*
   * The internal LVDS encoder has a restricted clock frequency
  	 operating
-	 * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3).
Clamp
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +3 -2
-	 * the clock accordingly.
+	 * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz
+	 * on Gen3). Clamp the clock accordingly.
 	 */
-	if (rcdu->info->gen < 3)
+	if (info->gen < 3 || info->model == R8A77970)
  		mode->clock = clamp(mode->clock, 30000, 150000);
According to the datasheet the frequency range for V3M is the same as for the H3 and M3 SoCs.
Indeed! I thought it's determined by the LVDPLLCR layout but it's not...

Quoted 8 lines

Quoted 2 lines
The range seems to have changed starting in datasheet version 0.52. I would fix the range in a separate patch first.
Yes.
Quoted 1 line
If you want I can send patches to fix this issue
Yes, please. You clearly know about DU more than me. :-)

I've prepared a patch, I'm testing it now and I'll then send it.

Quoted 7 lines

Quoted 5 lines
and the previous one, or you can write them and include them in v2. Let me know what you'd prefer.
Quoted 2 lines
  	else
  		mode->clock = clamp(mode->clock, 25175, 148500);
The lower bound documented on gen3 is 31 MHz indeed...

And I just found out that the latest versions of the Gen2 datasheets also document the 31 MHz - 148.5 MHz range. This will simplify the code.

Signature · 8 lines

Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Sergei Shtylyov

author

2018-01-12 14:38 UTC #4 lore raw

Quoted 20 lines · Laurent Pinchart

Quoted 20 lines
Quoted 13 lines
Quoted 6 lines
Add support for the R-Car V3M (R8A77970) SoC to the DU driver (this SoC has only 1 display port). Note that there are some differences with the other R-Car gen3 SoCs in the LVDS encoder part, e.g. LVDPLLCR has the same layout as on the R-Car gen2 SoCs...
Signed-off-by Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/[email protected]>
Could you please rebase this series on top of the LVDS rework posted as "[PATCH 00/10] R-Car DU: Convert LVDS code to bridge driver" (https://
www.spinics.net/lists/dri-devel/msg161931.html) ? It should make it easier to implement support for V3M. Please then split the DU and LVDS drivers changes in two patches.
Quoted 5 lines
Quoted 3 lines
---
   Documentation/devicetree/bindings/display/renesas,du.txt |    1
Please split the DT bindings changes to a separate patch.
I don't like putting a one-line chnage into a separate bindings patch...

[...]

Quoted 25 lines

Quoted 25 lines
Quoted 24 lines
Quoted 10 lines
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
===================================================================
drivers/gpu/drm/rcar-du/rcar_du_group.c +0 -0
@@ -133,10 +133,12 @@ static void rcar_du_group_setup(struct r
 
   	rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS);
 
   	/* Apply planes to CRTCs association. */
 
drivers/gpu/drm/rcar-du/rcar_du_group.c static void rcar_du_group_setup(struct r · line 143
Quoted 10 lines
drivers/gpu/drm/rcar-du/rcar_du_group.c +6 -4
-	mutex_lock(&rgrp->lock);
-	rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
-			    rgrp->dptsr_planes);
-	mutex_unlock(&rgrp->lock);
+	if (rcdu->info->num_crtcs > 1) {
+		mutex_lock(&rgrp->lock);
+		rcar_du_group_write(rgrp, DPTSR, (rgrp->dptsr_planes << 16) |
+				    rgrp->dptsr_planes);
+		mutex_unlock(&rgrp->lock);
+	}
Shouldn't you skip writing to the DPTSR register if there's a single DPTSR in the group ? That would then apply to M3-W as well, which doesn't have the DPTSR2 register. I'd split this change to a separate patch.

Quoted 4 lines

Quoted 2 lines
OK, I guess you know this stuff better -- I didn't know DPTSR2 is used at all... :-)
Should I send a patch for this as well ?

I thought that was a "previous" issue you meant? Please fix this too, if
it's not too much trouble. :-)

Quoted 37 lines

Quoted 37 lines
[...]
Quoted 4 lines
Quoted 4 lines
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===================================================================
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +0 -0
[...]
Quoted 7 lines
Quoted 7 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +1 -1
@@ -177,14 +185,14 @@ int rcar_du_lvdsenc_enable(struct rcar_d
   void rcar_du_lvdsenc_atomic_check(struct rcar_du_lvdsenc *lvds,
   				  struct drm_display_mode *mode)
   {
-	struct rcar_du_device *rcdu = lvds->dev;
+	const struct rcar_du_device_info *info = lvds->dev->info;
 
Quoted 15 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c int rcar_du_lvdsenc_enable(struct rcar_d · line 196
Quoted 12 lines
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +0 -1
   /*
    * The internal LVDS encoder has a restricted clock frequency
   	 operating
-	 * range (30MHz to 150MHz on Gen2, 25.175MHz to 148.5MHz on Gen3).
Clamp
drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c +3 -2
-	 * the clock accordingly.
+	 * range (30MHz to 150MHz on Gen2 and R-Car V3M, 25.175MHz to 148.5MHz
+	 * on Gen3). Clamp the clock accordingly.
  	 */
-	if (rcdu->info->gen < 3)
+	if (info->gen < 3 || info->model == R8A77970)
   		mode->clock = clamp(mode->clock, 30000, 150000);
According to the datasheet the frequency range for V3M is the same as for the H3 and M3 SoCs.
Indeed! I thought it's determined by the LVDPLLCR layout but it's not...
Quoted 2 lines
The range seems to have changed starting in datasheet version 0.52. I would fix the range in a separate patch first.

Quoted 9 lines

Quoted 5 lines
Yes.
Quoted 1 line
If you want I can send patches to fix this issue
Yes, please. You clearly know about DU more than me. :-)
I've prepared a patch, I'm testing it now and I'll then send it.
Quoted 1 line
Quoted 1 line
and the previous one, or you can write them and include them in v2. Let me

Here you're talking about the previous issue -- I figured it was about
DPTSR...

Quoted 9 lines

Quoted 6 lines
Quoted 4 lines
know what you'd prefer.
Quoted 2 lines
   	else
   		mode->clock = clamp(mode->clock, 25175, 148500);
The lower bound documented on gen3 is 31 MHz indeed...
And I just found out that the latest versions of the Gen2 datasheets also document the 31 MHz - 148.5 MHz range. This will simplify the code.

Indeed! Thanks for looking. :-)

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html