[3/3] drm: rcar-du: add R8A77970 support

Message ID 20180111170103.676775968@cogentembedded.com
State New
Headers show
Series
  • Add R-Car V3M (R8A77970) support to the DU driver
Related show

Commit Message

Sergei Shtylyov Jan. 11, 2018, 4:54 p.m.
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>

---
 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(-)


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

Comments

Laurent Pinchart Jan. 12, 2018, 1:13 a.m. | #1
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);
Sergei Shtylyov Jan. 12, 2018, 9:23 a.m. | #2
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(-)
>>
>> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laurent Pinchart Jan. 12, 2018, 2:30 p.m. | #3
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.
Sergei Shtylyov Jan. 12, 2018, 2:38 p.m. | #4
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@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...

[...]
>>>> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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);