* [PATCH 3/3] drm: rcar-du: add R8A77970 support
@ 2018-01-11 16:54 Sergei Shtylyov
2018-01-12 1:13 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2018-01-11 16:54 UTC (permalink / raw)
To: Laurent Pinchart, David Airlie, Rob Herring, Mark Rutland,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Sergei Shtylyov
[-- Attachment #1: drm-rcar-du-add-R8A77970-support.patch --]
[-- Type: text/plain, Size: 6211 bytes --]
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/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
---
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(-)
Index: linux/Documentation/devicetree/bindings/display/renesas,du.txt
===================================================================
--- linux.orig/Documentation/devicetree/bindings/display/renesas,du.txt
+++ 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.
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ 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 },
{ }
};
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -59,6 +59,7 @@ enum rcar_du_model {
R8A7794,
R8A7795,
R8A7796,
+ R8A77970,
};
/*
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ 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);
+ }
}
/*
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
+++ 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] drm: rcar-du: add R8A77970 support
2018-01-11 16:54 [PATCH 3/3] drm: rcar-du: add R8A77970 support Sergei Shtylyov
@ 2018-01-12 1:13 ` Laurent Pinchart
2018-01-12 9:23 ` Sergei Shtylyov
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2018-01-12 1:13 UTC (permalink / raw)
To: Sergei Shtylyov vi
Cc: David Airlie, Rob Herring, Mark Rutland, dri-devel,
linux-renesas-soc, devicetree
Hi Sergei,
Thank you for the patch.
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 <sergei.shtylyov@cogentembedded.com>
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.
> ---
> Documentation/devicetree/bindings/display/renesas,du.txt | 1
Please split the DT bindings changes to a separate patch.
> 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
> ===================================================================
> --- linux.orig/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ 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.
You also need to update the ports table further down in this file.
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ 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,
> + },
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.
> + },
> + .num_lvds = 1,
> + .dpll_ch = BIT(1),
This doesn't seem to be correct, there's no DPLL in V3M.
> +};
> +
> 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
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -59,6 +59,7 @@ enum rcar_du_model {
> R8A7794,
> R8A7795,
> R8A7796,
> + R8A77970,
> };
>
> /*
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ 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);
> + }
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.
> }
>
> /*
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> +++ 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);
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.
> else
> mode->clock = clamp(mode->clock, 25175, 148500);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] drm: rcar-du: add R8A77970 support
2018-01-12 1:13 ` Laurent Pinchart
@ 2018-01-12 9:23 ` Sergei Shtylyov
2018-01-12 14:30 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2018-01-12 9:23 UTC (permalink / raw)
To: Laurent Pinchart
Cc: David Airlie, Rob Herring, Mark Rutland,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
On 1/12/2018 4:13 AM, Laurent Pinchart 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 <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
>
> 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.
>
>> ---
>> 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...
>> 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
>> ===================================================================
>> --- linux.orig/Documentation/devicetree/bindings/display/renesas,du.txt
>> +++ 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.
>
> You also need to update the ports table further down in this file.
... but this one seems to justify dpoing it that way. :-)
>> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> ===================================================================
>> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> +++ 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,
>> + },
>
> 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..
>> + },
>> + .num_lvds = 1,
>> + .dpll_ch = BIT(1),
>
> This doesn't seem to be correct, there's no DPLL in V3M.
Indeed, thanks!
[...]
>> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> ===================================================================
>> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> +++ 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);
>> + }
>
> 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... :-)
[...]
>> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
>> ===================================================================
>> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
>> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
[...]
>> @@ -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);
>
> 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...
> The range seems to have changed starting in datasheet version
> 0.52. I would fix the range in a separate patch first.
Yes.
> If you want I can send patches to fix this issue
Yes, please. You clearly know about DU more than me. :-)
> and the previous one, or you
> can write them and include them in v2. Let me know what you'd prefer.
>
>> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] drm: rcar-du: add R8A77970 support
2018-01-12 9:23 ` Sergei Shtylyov
@ 2018-01-12 14:30 ` Laurent Pinchart
2018-01-12 14:38 ` Sergei Shtylyov
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2018-01-12 14:30 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Mark Rutland, devicetree, David Airlie, dri-devel,
linux-renesas-soc, Rob Herring
Hi Sergei,
On Friday, 12 January 2018 11:23:00 EET Sergei Shtylyov wrote:
> On 1/12/2018 4:13 AM, Laurent Pinchart 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 <sergei.shtylyov@cogentembedded.com>
> >
> > 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.
> >
> >> ---
> >>
> >> 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...
> >> 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]
> >> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
> >> ===================================================================
> >> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c
> >> +++ 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);
> >> + }
> >
> > 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... :-)
Should I send a patch for this as well ?
> [...]
>
> >> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> >> ===================================================================
> >> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
> >> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
>
> [...]
>
> >> @@ -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);
> >
> > 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...
>
> > The range seems to have changed starting in datasheet version
> > 0.52. I would fix the range in a separate patch first.
>
> Yes.
>
> > 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.
> > and the previous one, or you can write them and include them in v2. Let me
> > know what you'd prefer.
> >
> >> 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.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] drm: rcar-du: add R8A77970 support
2018-01-12 14:30 ` Laurent Pinchart
@ 2018-01-12 14:38 ` Sergei Shtylyov
0 siblings, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2018-01-12 14:38 UTC (permalink / raw)
To: Laurent Pinchart
Cc: David Airlie, Rob Herring, Mark Rutland,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
On 01/12/2018 05:30 PM, Laurent Pinchart 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 <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
>>>
>>> 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.
>>>
>>>> ---
>>>>
>>>> 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...
[...]
>>>> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_group.c
>>>> ===================================================================
>>>> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_group.c
>>>> +++ 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);
>>>> + }
>>>
>>> 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... :-)
>
> 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. :-)
>> [...]
>>
>>>> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
>>>> ===================================================================
>>>> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
>>>> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c
>>
>> [...]
>>
>>>> @@ -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);
>>>
>>> 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...
>>
>>> The range seems to have changed starting in datasheet version
>>> 0.52. I would fix the range in a separate patch first.
>>
>> Yes.
>>
>>> 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.
>
>>> 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...
>>> know what you'd prefer.
>>>
>>>> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-12 14:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-11 16:54 [PATCH 3/3] drm: rcar-du: add R8A77970 support Sergei Shtylyov
2018-01-12 1:13 ` Laurent Pinchart
2018-01-12 9:23 ` Sergei Shtylyov
2018-01-12 14:30 ` Laurent Pinchart
2018-01-12 14:38 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).