[PATCH 01/17] dt-bindings: display: renesas: du: Increase indent in output table

[PATCH 01/17] dt-bindings: display: renesas: du: Increase indent in output table

[Kieran Bingham]

The DU output table lists the port combinations for each supported DU
type.  Newer models of R-Car Gen3 platforms have an increased string
length.

Increase the table indentation in preparation for supporting new target
types.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 .../bindings/display/renesas,du.txt           | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
index c9cd17f99702..a36a6e7ee54f 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -47,20 +47,20 @@ bindings specified in Documentation/devicetree/bindings/graph.txt.
 The following table lists for each supported model the port number
 corresponding to each DU output.
 
-                      Port0          Port1          Port2          Port3
+                        Port0          Port1          Port2          Port3
 -----------------------------------------------------------------------------
- R8A7743 (RZ/G1M)     DPAD 0         LVDS 0         -              -
- R8A7745 (RZ/G1E)     DPAD 0         DPAD 1         -              -
- R8A7779 (R-Car H1)   DPAD 0         DPAD 1         -              -
- R8A7790 (R-Car H2)   DPAD 0         LVDS 0         LVDS 1         -
- R8A7791 (R-Car M2-W) DPAD 0         LVDS 0         -              -
- R8A7792 (R-Car V2H)  DPAD 0         DPAD 1         -              -
- R8A7793 (R-Car M2-N) DPAD 0         LVDS 0         -              -
- R8A7794 (R-Car E2)   DPAD 0         DPAD 1         -              -
- R8A7795 (R-Car H3)   DPAD 0         HDMI 0         HDMI 1         LVDS 0
- R8A7796 (R-Car M3-W) DPAD 0         HDMI 0         LVDS 0         -
- R8A77970 (R-Car V3M) DPAD 0         LVDS 0         -              -
- R8A77995 (R-Car D3)  DPAD 0         LVDS 0         LVDS 1         -
+ R8A7743 (RZ/G1M)       DPAD 0         LVDS 0         -              -
+ R8A7745 (RZ/G1E)       DPAD 0         DPAD 1         -              -
+ R8A7779 (R-Car H1)     DPAD 0         DPAD 1         -              -
+ R8A7790 (R-Car H2)     DPAD 0         LVDS 0         LVDS 1         -
+ R8A7791 (R-Car M2-W)   DPAD 0         LVDS 0         -              -
+ R8A7792 (R-Car V2H)    DPAD 0         DPAD 1         -              -
+ R8A7793 (R-Car M2-N)   DPAD 0         LVDS 0         -              -
+ R8A7794 (R-Car E2)     DPAD 0         DPAD 1         -              -
+ R8A7795 (R-Car H3)     DPAD 0         HDMI 0         HDMI 1         LVDS 0
+ R8A7796 (R-Car M3-W)   DPAD 0         HDMI 0         LVDS 0         -
+ R8A77970 (R-Car V3M)   DPAD 0         LVDS 0         -              -
+ R8A77995 (R-Car D3)    DPAD 0         LVDS 0         LVDS 1         -
 
 
 Example: R8A7795 (R-Car H3) ES2.0 DU
-- 
2.17.0

Re: [PATCH 01/17] dt-bindings: display: renesas: du: Increase indent in output table

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:30 EEST Kieran Bingham wrote:
> The DU output table lists the port combinations for each supported DU
> type.  Newer models of R-Car Gen3 platforms have an increased string
> length.
> 
> Increase the table indentation in preparation for supporting new target
> types.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

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

and applied to my tree.

> ---
>  .../bindings/display/renesas,du.txt           | 26 +++++++++----------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt
> b/Documentation/devicetree/bindings/display/renesas,du.txt index
> c9cd17f99702..a36a6e7ee54f 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> @@ -47,20 +47,20 @@ bindings specified in
> Documentation/devicetree/bindings/graph.txt. The following table lists for
> each supported model the port number corresponding to each DU output.
> 
> -                      Port0          Port1          Port2          Port3
> +                        Port0          Port1          Port2          Port3
>  ---------------------------------------------------------------------------
> -- - R8A7743 (RZ/G1M)     DPAD 0         LVDS 0         -              - -
> R8A7745 (RZ/G1E)     DPAD 0         DPAD 1         -              - -
> R8A7779 (R-Car H1)   DPAD 0         DPAD 1         -              - -
> R8A7790 (R-Car H2)   DPAD 0         LVDS 0         LVDS 1         - -
> R8A7791 (R-Car M2-W) DPAD 0         LVDS 0         -              - -
> R8A7792 (R-Car V2H)  DPAD 0         DPAD 1         -              - -
> R8A7793 (R-Car M2-N) DPAD 0         LVDS 0         -              - -
> R8A7794 (R-Car E2)   DPAD 0         DPAD 1         -              - -
> R8A7795 (R-Car H3)   DPAD 0         HDMI 0         HDMI 1         LVDS 0 -
> R8A7796 (R-Car M3-W) DPAD 0         HDMI 0         LVDS 0         - -
> R8A77970 (R-Car V3M) DPAD 0         LVDS 0         -              - -
> R8A77995 (R-Car D3)  DPAD 0         LVDS 0         LVDS 1         - +
> R8A7743 (RZ/G1M)       DPAD 0         LVDS 0         -              - +
> R8A7745 (RZ/G1E)       DPAD 0         DPAD 1         -              - +
> R8A7779 (R-Car H1)     DPAD 0         DPAD 1         -              - +
> R8A7790 (R-Car H2)     DPAD 0         LVDS 0         LVDS 1         - +
> R8A7791 (R-Car M2-W)   DPAD 0         LVDS 0         -              - +
> R8A7792 (R-Car V2H)    DPAD 0         DPAD 1         -              - +
> R8A7793 (R-Car M2-N)   DPAD 0         LVDS 0         -              - +
> R8A7794 (R-Car E2)     DPAD 0         DPAD 1         -              - +
> R8A7795 (R-Car H3)     DPAD 0         HDMI 0         HDMI 1         LVDS 0
> + R8A7796 (R-Car M3-W)   DPAD 0         HDMI 0         LVDS 0         - +
> R8A77970 (R-Car V3M)   DPAD 0         LVDS 0         -              - +
> R8A77995 (R-Car D3)    DPAD 0         LVDS 0         LVDS 1         -
> 
> 
>  Example: R8A7795 (R-Car H3) ES2.0 DU


-- 
Regards,

Laurent Pinchart

[PATCH 02/17] dt-bindings: display: renesas: du: Document the R8A77965 bindings

[Kieran Bingham]

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
index a36a6e7ee54f..7c6854bd0a04 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/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-r8a77965" for R8A77965 (R-Car M3-N) compatible DU
     - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU
     - "renesas,du-r8a77995" for R8A77995 (R-Car D3) compatible DU
 
@@ -59,6 +60,7 @@ corresponding to each DU output.
  R8A7794 (R-Car E2)     DPAD 0         DPAD 1         -              -
  R8A7795 (R-Car H3)     DPAD 0         HDMI 0         HDMI 1         LVDS 0
  R8A7796 (R-Car M3-W)   DPAD 0         HDMI 0         LVDS 0         -
+ R8A77965 (R-Car M3-N)  DPAD 0         HDMI 0         LVDS 0         -
  R8A77970 (R-Car V3M)   DPAD 0         LVDS 0         -              -
  R8A77995 (R-Car D3)    DPAD 0         LVDS 0         LVDS 1         -
 
-- 
2.17.0

Re: [PATCH 02/17] dt-bindings: display: renesas: du: Document the R8A77965 bindings

[Kieran Bingham]

Ahem - this one seems to have lost it's commit message.

Apologies :)

--
Kieran


On 26/04/18 17:53, Kieran Bingham wrote:
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> index a36a6e7ee54f..7c6854bd0a04 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ b/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-r8a77965" for R8A77965 (R-Car M3-N) compatible DU
>      - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU
>      - "renesas,du-r8a77995" for R8A77995 (R-Car D3) compatible DU
>  
> @@ -59,6 +60,7 @@ corresponding to each DU output.
>   R8A7794 (R-Car E2)     DPAD 0         DPAD 1         -              -
>   R8A7795 (R-Car H3)     DPAD 0         HDMI 0         HDMI 1         LVDS 0
>   R8A7796 (R-Car M3-W)   DPAD 0         HDMI 0         LVDS 0         -
> + R8A77965 (R-Car M3-N)  DPAD 0         HDMI 0         LVDS 0         -
>   R8A77970 (R-Car V3M)   DPAD 0         LVDS 0         -              -
>   R8A77995 (R-Car D3)    DPAD 0         LVDS 0         LVDS 1         -
>  
> 

Re: [PATCH 02/17] dt-bindings: display: renesas: du: Document the R8A77965 bindings

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:57:32 EEST Kieran Bingham wrote:
> Ahem - this one seems to have lost it's commit message.
> 
> Apologies :)

Apart from that, this looks good to me.

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

and applied to my tree with the commit message

Document the M3-N (r8a77965) SoC in the R-Car DU bindings

Let me know if you would like a different message.

> On 26/04/18 17:53, Kieran Bingham wrote:
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> > 
> >  Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt
> > b/Documentation/devicetree/bindings/display/renesas,du.txt index
> > a36a6e7ee54f..7c6854bd0a04 100644
> > --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> > +++ b/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-r8a77965" for R8A77965 (R-Car M3-N) compatible DU
> >      - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU
> >      - "renesas,du-r8a77995" for R8A77995 (R-Car D3) compatible DU
> > 
> > @@ -59,6 +60,7 @@ corresponding to each DU output.
> > 
> >   R8A7794 (R-Car E2)     DPAD 0         DPAD 1         -              -
> >   R8A7795 (R-Car H3)     DPAD 0         HDMI 0         HDMI 1         LVDS
> >   0
> >   R8A7796 (R-Car M3-W)   DPAD 0         HDMI 0         LVDS 0         -
> > + R8A77965 (R-Car M3-N)  DPAD 0         HDMI 0         LVDS 0         -
> >   R8A77970 (R-Car V3M)   DPAD 0         LVDS 0         -              -
> >   R8A77995 (R-Car D3)    DPAD 0         LVDS 0         LVDS 1         -

-- 
Regards,

Laurent Pinchart

Re: [PATCH 02/17] dt-bindings: display: renesas: du: Document the R8A77965 bindings

[Kieran Bingham]

[-- Attachment #1.1: Type: text/plain, Size: 2231 bytes --]

Hi Laurent,

On 26/04/18 21:10, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 19:57:32 EEST Kieran Bingham wrote:
>> Ahem - this one seems to have lost it's commit message.
>>
>> Apologies :)
> 
> Apart from that, this looks good to me.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> and applied to my tree with the commit message
> 
> Document the M3-N (r8a77965) SoC in the R-Car DU bindings
> 

That's perfect, thanks - and saves me sending a v1.1

Regards

Kieran

> Let me know if you would like a different message.
> 
>> On 26/04/18 17:53, Kieran Bingham wrote:
>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>> ---
>>>
>>>  Documentation/devicetree/bindings/display/renesas,du.txt | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt
>>> b/Documentation/devicetree/bindings/display/renesas,du.txt index
>>> a36a6e7ee54f..7c6854bd0a04 100644
>>> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
>>> +++ b/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-r8a77965" for R8A77965 (R-Car M3-N) compatible DU
>>>      - "renesas,du-r8a77970" for R8A77970 (R-Car V3M) compatible DU
>>>      - "renesas,du-r8a77995" for R8A77995 (R-Car D3) compatible DU
>>>
>>> @@ -59,6 +60,7 @@ corresponding to each DU output.
>>>
>>>   R8A7794 (R-Car E2)     DPAD 0         DPAD 1         -              -
>>>   R8A7795 (R-Car H3)     DPAD 0         HDMI 0         HDMI 1         LVDS
>>>   0
>>>   R8A7796 (R-Car M3-W)   DPAD 0         HDMI 0         LVDS 0         -
>>> + R8A77965 (R-Car M3-N)  DPAD 0         HDMI 0         LVDS 0         -
>>>   R8A77970 (R-Car V3M)   DPAD 0         LVDS 0         -              -
>>>   R8A77995 (R-Car D3)    DPAD 0         LVDS 0         LVDS 1         -
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 03/17] pinctrl: sh-pfc: r8a77965: Add DU RGB output pins, groups and functions

[Kieran Bingham]

This patch adds pins, groups and functions for parallel RGB output
signals from DU. The HDMI and TCON pins are added to separate groups.

Based on a similar patch of the R8A7796 PFC driver by Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se>.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[Kieran: Rebase on top of tree]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77965.c | 116 ++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
index 3771b2d10f39..f5a37d3ea753 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
@@ -1662,6 +1662,102 @@ static const unsigned int avb_avtp_capture_b_mux[] = {
 	AVB_AVTP_CAPTURE_B_MARK,
 };
 
+/* - DU --------------------------------------------------------------------- */
+static const unsigned int du_rgb666_pins[] = {
+	/* R[7:2], G[7:2], B[7:2] */
+	RCAR_GP_PIN(0, 15), RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 13),
+	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 11), RCAR_GP_PIN(0, 10),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 13),
+	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 18),
+	RCAR_GP_PIN(1, 7),  RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 2),
+};
+
+static const unsigned int du_rgb666_mux[] = {
+	DU_DR7_MARK, DU_DR6_MARK, DU_DR5_MARK, DU_DR4_MARK,
+	DU_DR3_MARK, DU_DR2_MARK,
+	DU_DG7_MARK, DU_DG6_MARK, DU_DG5_MARK, DU_DG4_MARK,
+	DU_DG3_MARK, DU_DG2_MARK,
+	DU_DB7_MARK, DU_DB6_MARK, DU_DB5_MARK, DU_DB4_MARK,
+	DU_DB3_MARK, DU_DB2_MARK,
+};
+
+static const unsigned int du_rgb888_pins[] = {
+	/* R[7:0], G[7:0], B[7:0] */
+	RCAR_GP_PIN(0, 15), RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 13),
+	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 11), RCAR_GP_PIN(0, 10),
+	RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 8),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 13),
+	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 18),
+	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 16),
+	RCAR_GP_PIN(1, 7),  RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 2),
+	RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 0),
+};
+
+static const unsigned int du_rgb888_mux[] = {
+	DU_DR7_MARK, DU_DR6_MARK, DU_DR5_MARK, DU_DR4_MARK,
+	DU_DR3_MARK, DU_DR2_MARK, DU_DR1_MARK, DU_DR0_MARK,
+	DU_DG7_MARK, DU_DG6_MARK, DU_DG5_MARK, DU_DG4_MARK,
+	DU_DG3_MARK, DU_DG2_MARK, DU_DG1_MARK, DU_DG0_MARK,
+	DU_DB7_MARK, DU_DB6_MARK, DU_DB5_MARK, DU_DB4_MARK,
+	DU_DB3_MARK, DU_DB2_MARK, DU_DB1_MARK, DU_DB0_MARK,
+};
+
+static const unsigned int du_clk_out_0_pins[] = {
+	/* CLKOUT */
+	RCAR_GP_PIN(1, 27),
+};
+
+static const unsigned int du_clk_out_0_mux[] = {
+	DU_DOTCLKOUT0_MARK
+};
+
+static const unsigned int du_clk_out_1_pins[] = {
+	/* CLKOUT */
+	RCAR_GP_PIN(2, 3),
+};
+
+static const unsigned int du_clk_out_1_mux[] = {
+	DU_DOTCLKOUT1_MARK
+};
+
+static const unsigned int du_sync_pins[] = {
+	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC */
+	RCAR_GP_PIN(2, 5), RCAR_GP_PIN(2, 4),
+};
+
+static const unsigned int du_sync_mux[] = {
+	DU_EXVSYNC_DU_VSYNC_MARK, DU_EXHSYNC_DU_HSYNC_MARK
+};
+
+static const unsigned int du_oddf_pins[] = {
+	/* EXDISP/EXODDF/EXCDE */
+	RCAR_GP_PIN(2, 2),
+};
+
+static const unsigned int du_oddf_mux[] = {
+	DU_EXODDF_DU_ODDF_DISP_CDE_MARK,
+};
+
+static const unsigned int du_cde_pins[] = {
+	/* CDE */
+	RCAR_GP_PIN(2, 0),
+};
+
+static const unsigned int du_cde_mux[] = {
+	DU_CDE_MARK,
+};
+
+static const unsigned int du_disp_pins[] = {
+	/* DISP */
+	RCAR_GP_PIN(2, 1),
+};
+
+static const unsigned int du_disp_mux[] = {
+	DU_DISP_MARK,
+};
+
 /* - INTC-EX ---------------------------------------------------------------- */
 static const unsigned int intc_ex_irq0_pins[] = {
 	/* IRQ0 */
@@ -2756,6 +2852,14 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(avb_avtp_capture_a),
 	SH_PFC_PIN_GROUP(avb_avtp_match_b),
 	SH_PFC_PIN_GROUP(avb_avtp_capture_b),
+	SH_PFC_PIN_GROUP(du_rgb666),
+	SH_PFC_PIN_GROUP(du_rgb888),
+	SH_PFC_PIN_GROUP(du_clk_out_0),
+	SH_PFC_PIN_GROUP(du_clk_out_1),
+	SH_PFC_PIN_GROUP(du_sync),
+	SH_PFC_PIN_GROUP(du_oddf),
+	SH_PFC_PIN_GROUP(du_cde),
+	SH_PFC_PIN_GROUP(du_disp),
 	SH_PFC_PIN_GROUP(intc_ex_irq0),
 	SH_PFC_PIN_GROUP(intc_ex_irq1),
 	SH_PFC_PIN_GROUP(intc_ex_irq2),
@@ -2922,6 +3026,17 @@ static const char * const avb_groups[] = {
 	"avb_avtp_capture_b",
 };
 
+static const char * const du_groups[] = {
+	"du_rgb666",
+	"du_rgb888",
+	"du_clk_out_0",
+	"du_clk_out_1",
+	"du_sync",
+	"du_oddf",
+	"du_cde",
+	"du_disp",
+};
+
 static const char * const intc_ex_groups[] = {
 	"intc_ex_irq0",
 	"intc_ex_irq1",
@@ -3139,6 +3254,7 @@ static const char * const usb30_groups[] = {
 
 static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(avb),
+	SH_PFC_FUNCTION(du),
 	SH_PFC_FUNCTION(intc_ex),
 	SH_PFC_FUNCTION(msiof0),
 	SH_PFC_FUNCTION(msiof1),
-- 
2.17.0

Re: [PATCH 03/17] pinctrl: sh-pfc: r8a77965: Add DU RGB output pins, groups and functions

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:32 EEST Kieran Bingham wrote:
> This patch adds pins, groups and functions for parallel RGB output
> signals from DU. The HDMI and TCON pins are added to separate groups.
> 
> Based on a similar patch of the R8A7796 PFC driver by Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se>.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [Kieran: Rebase on top of tree]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

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

I expect Geert to take this patch in his tree.

> ---
>  drivers/pinctrl/sh-pfc/pfc-r8a77965.c | 116 ++++++++++++++++++++++++++
>  1 file changed, 116 insertions(+)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c index 3771b2d10f39..f5a37d3ea753
> 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
> @@ -1662,6 +1662,102 @@ static const unsigned int avb_avtp_capture_b_mux[] =
> { AVB_AVTP_CAPTURE_B_MARK,
>  };
> 
> +/* - DU ---------------------------------------------------------------- */
> +static const unsigned int du_rgb666_pins[] = {
> +	/* R[7:2], G[7:2], B[7:2] */
> +	RCAR_GP_PIN(0, 15), RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 13),
> +	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 11), RCAR_GP_PIN(0, 10),
> +	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 13),
> +	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 18),
> +	RCAR_GP_PIN(1, 7),  RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 5),
> +	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 2),
> +};
> +
> +static const unsigned int du_rgb666_mux[] = {
> +	DU_DR7_MARK, DU_DR6_MARK, DU_DR5_MARK, DU_DR4_MARK,
> +	DU_DR3_MARK, DU_DR2_MARK,
> +	DU_DG7_MARK, DU_DG6_MARK, DU_DG5_MARK, DU_DG4_MARK,
> +	DU_DG3_MARK, DU_DG2_MARK,
> +	DU_DB7_MARK, DU_DB6_MARK, DU_DB5_MARK, DU_DB4_MARK,
> +	DU_DB3_MARK, DU_DB2_MARK,
> +};
> +
> +static const unsigned int du_rgb888_pins[] = {
> +	/* R[7:0], G[7:0], B[7:0] */
> +	RCAR_GP_PIN(0, 15), RCAR_GP_PIN(0, 14), RCAR_GP_PIN(0, 13),
> +	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 11), RCAR_GP_PIN(0, 10),
> +	RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 8),
> +	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 14), RCAR_GP_PIN(1, 13),
> +	RCAR_GP_PIN(1, 12), RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 18),
> +	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 16),
> +	RCAR_GP_PIN(1, 7),  RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 5),
> +	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 2),
> +	RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 0),
> +};
> +
> +static const unsigned int du_rgb888_mux[] = {
> +	DU_DR7_MARK, DU_DR6_MARK, DU_DR5_MARK, DU_DR4_MARK,
> +	DU_DR3_MARK, DU_DR2_MARK, DU_DR1_MARK, DU_DR0_MARK,
> +	DU_DG7_MARK, DU_DG6_MARK, DU_DG5_MARK, DU_DG4_MARK,
> +	DU_DG3_MARK, DU_DG2_MARK, DU_DG1_MARK, DU_DG0_MARK,
> +	DU_DB7_MARK, DU_DB6_MARK, DU_DB5_MARK, DU_DB4_MARK,
> +	DU_DB3_MARK, DU_DB2_MARK, DU_DB1_MARK, DU_DB0_MARK,
> +};
> +
> +static const unsigned int du_clk_out_0_pins[] = {
> +	/* CLKOUT */
> +	RCAR_GP_PIN(1, 27),
> +};
> +
> +static const unsigned int du_clk_out_0_mux[] = {
> +	DU_DOTCLKOUT0_MARK
> +};
> +
> +static const unsigned int du_clk_out_1_pins[] = {
> +	/* CLKOUT */
> +	RCAR_GP_PIN(2, 3),
> +};
> +
> +static const unsigned int du_clk_out_1_mux[] = {
> +	DU_DOTCLKOUT1_MARK
> +};
> +
> +static const unsigned int du_sync_pins[] = {
> +	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC */
> +	RCAR_GP_PIN(2, 5), RCAR_GP_PIN(2, 4),
> +};
> +
> +static const unsigned int du_sync_mux[] = {
> +	DU_EXVSYNC_DU_VSYNC_MARK, DU_EXHSYNC_DU_HSYNC_MARK
> +};
> +
> +static const unsigned int du_oddf_pins[] = {
> +	/* EXDISP/EXODDF/EXCDE */
> +	RCAR_GP_PIN(2, 2),
> +};
> +
> +static const unsigned int du_oddf_mux[] = {
> +	DU_EXODDF_DU_ODDF_DISP_CDE_MARK,
> +};
> +
> +static const unsigned int du_cde_pins[] = {
> +	/* CDE */
> +	RCAR_GP_PIN(2, 0),
> +};
> +
> +static const unsigned int du_cde_mux[] = {
> +	DU_CDE_MARK,
> +};
> +
> +static const unsigned int du_disp_pins[] = {
> +	/* DISP */
> +	RCAR_GP_PIN(2, 1),
> +};
> +
> +static const unsigned int du_disp_mux[] = {
> +	DU_DISP_MARK,
> +};
> +
>  /* - INTC-EX ----------------------------------------------------------- */
>  static const unsigned int intc_ex_irq0_pins[] = {
>  	/* IRQ0 */
> @@ -2756,6 +2852,14 @@ static const struct sh_pfc_pin_group pinmux_groups[]
> = { SH_PFC_PIN_GROUP(avb_avtp_capture_a),
>  	SH_PFC_PIN_GROUP(avb_avtp_match_b),
>  	SH_PFC_PIN_GROUP(avb_avtp_capture_b),
> +	SH_PFC_PIN_GROUP(du_rgb666),
> +	SH_PFC_PIN_GROUP(du_rgb888),
> +	SH_PFC_PIN_GROUP(du_clk_out_0),
> +	SH_PFC_PIN_GROUP(du_clk_out_1),
> +	SH_PFC_PIN_GROUP(du_sync),
> +	SH_PFC_PIN_GROUP(du_oddf),
> +	SH_PFC_PIN_GROUP(du_cde),
> +	SH_PFC_PIN_GROUP(du_disp),
>  	SH_PFC_PIN_GROUP(intc_ex_irq0),
>  	SH_PFC_PIN_GROUP(intc_ex_irq1),
>  	SH_PFC_PIN_GROUP(intc_ex_irq2),
> @@ -2922,6 +3026,17 @@ static const char * const avb_groups[] = {
>  	"avb_avtp_capture_b",
>  };
> 
> +static const char * const du_groups[] = {
> +	"du_rgb666",
> +	"du_rgb888",
> +	"du_clk_out_0",
> +	"du_clk_out_1",
> +	"du_sync",
> +	"du_oddf",
> +	"du_cde",
> +	"du_disp",
> +};
> +
>  static const char * const intc_ex_groups[] = {
>  	"intc_ex_irq0",
>  	"intc_ex_irq1",
> @@ -3139,6 +3254,7 @@ static const char * const usb30_groups[] = {
> 
>  static const struct sh_pfc_function pinmux_functions[] = {
>  	SH_PFC_FUNCTION(avb),
> +	SH_PFC_FUNCTION(du),
>  	SH_PFC_FUNCTION(intc_ex),
>  	SH_PFC_FUNCTION(msiof0),
>  	SH_PFC_FUNCTION(msiof1),

-- 
Regards,

Laurent Pinchart

[PATCH 04/17] drm: rcar-du: Use the correct naming for ODPM fields in DEFR6

[Kieran Bingham]

The naming of the fields for the ODPM signals in the DU extensional
function control register 6 (DEFR6) is incorrect against the data sheets
for both R-Car Gen2 and R-Car Gen3.

Rename the fields to match the datasheet.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_group.c |  4 ++--
 drivers/gpu/drm/rcar-du/rcar_du_regs.h  | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 2f37ea901873..eead202c95c7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -46,10 +46,10 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data)
 
 static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp)
 {
-	u32 defr6 = DEFR6_CODE | DEFR6_ODPM12_DISP;
+	u32 defr6 = DEFR6_CODE | DEFR6_ODPM02_DISP;
 
 	if (rgrp->num_crtcs > 1)
-		defr6 |= DEFR6_ODPM22_DISP;
+		defr6 |= DEFR6_ODPM12_DISP;
 
 	rcar_du_group_write(rgrp, DEFR6, defr6);
 }
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
index d5bae99d3cfe..9dfd220ceda1 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
@@ -187,14 +187,14 @@
 
 #define DEFR6			0x000e8
 #define DEFR6_CODE		(0x7778 << 16)
-#define DEFR6_ODPM22_DSMR	(0 << 10)
-#define DEFR6_ODPM22_DISP	(2 << 10)
-#define DEFR6_ODPM22_CDE	(3 << 10)
-#define DEFR6_ODPM22_MASK	(3 << 10)
-#define DEFR6_ODPM12_DSMR	(0 << 8)
-#define DEFR6_ODPM12_DISP	(2 << 8)
-#define DEFR6_ODPM12_CDE	(3 << 8)
-#define DEFR6_ODPM12_MASK	(3 << 8)
+#define DEFR6_ODPM12_DSMR	(0 << 10)
+#define DEFR6_ODPM12_DISP	(2 << 10)
+#define DEFR6_ODPM12_CDE	(3 << 10)
+#define DEFR6_ODPM12_MASK	(3 << 10)
+#define DEFR6_ODPM02_DSMR	(0 << 8)
+#define DEFR6_ODPM02_DISP	(2 << 8)
+#define DEFR6_ODPM02_CDE	(3 << 8)
+#define DEFR6_ODPM02_MASK	(3 << 8)
 #define DEFR6_TCNE1		(1 << 6)
 #define DEFR6_TCNE0		(1 << 4)
 #define DEFR6_MLOS1		(1 << 2)
-- 
2.17.0

Re: [PATCH 04/17] drm: rcar-du: Use the correct naming for ODPM fields in DEFR6

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:33 EEST Kieran Bingham wrote:
> The naming of the fields for the ODPM signals in the DU extensional
> function control register 6 (DEFR6) is incorrect against the data sheets
> for both R-Car Gen2 and R-Car Gen3.
> 
> Rename the fields to match the datasheet.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

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

and taken in my tree.

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_group.c |  4 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  | 16 ++++++++--------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> b/drivers/gpu/drm/rcar-du/rcar_du_group.c index 2f37ea901873..eead202c95c7
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -46,10 +46,10 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32
> reg, u32 data)
> 
>  static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp)
>  {
> -	u32 defr6 = DEFR6_CODE | DEFR6_ODPM12_DISP;
> +	u32 defr6 = DEFR6_CODE | DEFR6_ODPM02_DISP;
> 
>  	if (rgrp->num_crtcs > 1)
> -		defr6 |= DEFR6_ODPM22_DISP;
> +		defr6 |= DEFR6_ODPM12_DISP;
> 
>  	rcar_du_group_write(rgrp, DEFR6, defr6);
>  }
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> b/drivers/gpu/drm/rcar-du/rcar_du_regs.h index d5bae99d3cfe..9dfd220ceda1
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -187,14 +187,14 @@
> 
>  #define DEFR6			0x000e8
>  #define DEFR6_CODE		(0x7778 << 16)
> -#define DEFR6_ODPM22_DSMR	(0 << 10)
> -#define DEFR6_ODPM22_DISP	(2 << 10)
> -#define DEFR6_ODPM22_CDE	(3 << 10)
> -#define DEFR6_ODPM22_MASK	(3 << 10)
> -#define DEFR6_ODPM12_DSMR	(0 << 8)
> -#define DEFR6_ODPM12_DISP	(2 << 8)
> -#define DEFR6_ODPM12_CDE	(3 << 8)
> -#define DEFR6_ODPM12_MASK	(3 << 8)
> +#define DEFR6_ODPM12_DSMR	(0 << 10)
> +#define DEFR6_ODPM12_DISP	(2 << 10)
> +#define DEFR6_ODPM12_CDE	(3 << 10)
> +#define DEFR6_ODPM12_MASK	(3 << 10)
> +#define DEFR6_ODPM02_DSMR	(0 << 8)
> +#define DEFR6_ODPM02_DISP	(2 << 8)
> +#define DEFR6_ODPM02_CDE	(3 << 8)
> +#define DEFR6_ODPM02_MASK	(3 << 8)
>  #define DEFR6_TCNE1		(1 << 6)
>  #define DEFR6_TCNE0		(1 << 4)
>  #define DEFR6_MLOS1		(1 << 2)

-- 
Regards,

Laurent Pinchart

[PATCH 05/17] drm: rcar-du: Split CRTC handling to support hardware indexing

[Kieran Bingham]

The DU CRTC driver does not support distinguishing between a hardware
index, and a software (CRTC) index in the event that a DU channel might
not be populated by the hardware.

Support this by adapting the rcar_du_device_info structure to store a
bitmask of available channels rather than a count of CRTCs. The count
can then be obtained by determining the hamming weight of the bitmask.

This allows the rcar_du_crtc_create() function to distinguish between
both index types, and non-populated DU channels will be skipped without
leaving a gap in the software CRTC indexes.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 26 ++++++++++++++------------
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  3 ++-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c  | 20 ++++++++++----------
 drivers/gpu/drm/rcar-du/rcar_du_drv.h  |  4 ++--
 drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 17 ++++++++++++-----
 5 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 5a15dfd66343..36ce194c13b5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -902,7 +902,8 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
  * Initialization
  */
 
-int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
+int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
+			unsigned int hwindex)
 {
 	static const unsigned int mmio_offsets[] = {
 		DU0_REG_OFFSET, DU1_REG_OFFSET, DU2_REG_OFFSET, DU3_REG_OFFSET
@@ -910,7 +911,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
 
 	struct rcar_du_device *rcdu = rgrp->dev;
 	struct platform_device *pdev = to_platform_device(rcdu->dev);
-	struct rcar_du_crtc *rcrtc = &rcdu->crtcs[index];
+	struct rcar_du_crtc *rcrtc = &rcdu->crtcs[swindex];
 	struct drm_crtc *crtc = &rcrtc->crtc;
 	struct drm_plane *primary;
 	unsigned int irqflags;
@@ -922,7 +923,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
 
 	/* Get the CRTC clock and the optional external clock. */
 	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
-		sprintf(clk_name, "du.%u", index);
+		sprintf(clk_name, "du.%u", hwindex);
 		name = clk_name;
 	} else {
 		name = NULL;
@@ -930,16 +931,16 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
 
 	rcrtc->clock = devm_clk_get(rcdu->dev, name);
 	if (IS_ERR(rcrtc->clock)) {
-		dev_err(rcdu->dev, "no clock for CRTC %u\n", index);
+		dev_err(rcdu->dev, "no clock for CRTC %u\n", swindex);
 		return PTR_ERR(rcrtc->clock);
 	}
 
-	sprintf(clk_name, "dclkin.%u", index);
+	sprintf(clk_name, "dclkin.%u", hwindex);
 	clk = devm_clk_get(rcdu->dev, clk_name);
 	if (!IS_ERR(clk)) {
 		rcrtc->extclock = clk;
 	} else if (PTR_ERR(rcrtc->clock) == -EPROBE_DEFER) {
-		dev_info(rcdu->dev, "can't get external clock %u\n", index);
+		dev_info(rcdu->dev, "can't get external clock %u\n", hwindex);
 		return -EPROBE_DEFER;
 	}
 
@@ -948,13 +949,13 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
 	spin_lock_init(&rcrtc->vblank_lock);
 
 	rcrtc->group = rgrp;
-	rcrtc->mmio_offset = mmio_offsets[index];
-	rcrtc->index = index;
+	rcrtc->mmio_offset = mmio_offsets[hwindex];
+	rcrtc->index = hwindex;
 
 	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
 		primary = &rcrtc->vsp->planes[rcrtc->vsp_pipe].plane;
 	else
-		primary = &rgrp->planes[index % 2].plane;
+		primary = &rgrp->planes[hwindex % 2].plane;
 
 	ret = drm_crtc_init_with_planes(rcdu->ddev, crtc, primary, NULL,
 					rcdu->info->gen <= 2 ?
@@ -970,7 +971,8 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
 
 	/* Register the interrupt handler. */
 	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
-		irq = platform_get_irq(pdev, index);
+		/* The IRQ's are associated with the CRTC (sw)index */
+		irq = platform_get_irq(pdev, swindex);
 		irqflags = 0;
 	} else {
 		irq = platform_get_irq(pdev, 0);
@@ -978,7 +980,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
 	}
 
 	if (irq < 0) {
-		dev_err(rcdu->dev, "no IRQ for CRTC %u\n", index);
+		dev_err(rcdu->dev, "no IRQ for CRTC %u\n", swindex);
 		return irq;
 	}
 
@@ -986,7 +988,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
 			       dev_name(rcdu->dev), rcrtc);
 	if (ret < 0) {
 		dev_err(rcdu->dev,
-			"failed to register IRQ for CRTC %u\n", index);
+			"failed to register IRQ for CRTC %u\n", swindex);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index 518ee2c60eb8..5f003a16abc5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -99,7 +99,8 @@ enum rcar_du_output {
 	RCAR_DU_OUTPUT_MAX,
 };
 
-int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index);
+int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
+			unsigned int hwindex);
 void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
 void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 05745e86d73e..d6ebc628fc22 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -40,7 +40,7 @@ static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-	.num_crtcs = 2,
+	.channel_mask = BIT(0) | BIT(1),
 	.routes = {
 		/*
 		 * R8A7743 has one RGB output and one LVDS output
@@ -61,7 +61,7 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-	.num_crtcs = 2,
+	.channel_mask = BIT(0) | BIT(1),
 	.routes = {
 		/*
 		 * R8A7745 has two RGB outputs
@@ -80,7 +80,7 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
 static const struct rcar_du_device_info rcar_du_r8a7779_info = {
 	.gen = 2,
 	.features = 0,
-	.num_crtcs = 2,
+	.channel_mask = BIT(0) | BIT(1),
 	.routes = {
 		/*
 		 * R8A7779 has two RGB outputs and one (currently unsupported)
@@ -102,7 +102,7 @@ static const struct rcar_du_device_info rcar_du_r8a7790_info = {
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
 	.quirks = RCAR_DU_QUIRK_ALIGN_128B,
-	.num_crtcs = 3,
+	.channel_mask = BIT(0) | BIT(1) | BIT(2),
 	.routes = {
 		/*
 		 * R8A7790 has one RGB output, two LVDS outputs and one
@@ -129,7 +129,7 @@ static const struct rcar_du_device_info rcar_du_r8a7791_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-	.num_crtcs = 2,
+	.channel_mask = BIT(0) | BIT(1),
 	.routes = {
 		/*
 		 * R8A779[13] has one RGB output, one LVDS output and one
@@ -151,7 +151,7 @@ static const struct rcar_du_device_info rcar_du_r8a7792_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-	.num_crtcs = 2,
+	.channel_mask = BIT(0) | BIT(1),
 	.routes = {
 		/* R8A7792 has two RGB outputs. */
 		[RCAR_DU_OUTPUT_DPAD0] = {
@@ -169,7 +169,7 @@ static const struct rcar_du_device_info rcar_du_r8a7794_info = {
 	.gen = 2,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
-	.num_crtcs = 2,
+	.channel_mask = BIT(0) | BIT(1),
 	.routes = {
 		/*
 		 * R8A7794 has two RGB outputs and one (currently unsupported)
@@ -191,7 +191,7 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = {
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE,
-	.num_crtcs = 4,
+	.channel_mask = BIT(0) | BIT(1) | BIT(2) | BIT(3),
 	.routes = {
 		/*
 		 * R8A7795 has one RGB output, two HDMI outputs and one
@@ -223,7 +223,7 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = {
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE,
-	.num_crtcs = 3,
+	.channel_mask = BIT(0) | BIT(1) | BIT(2),
 	.routes = {
 		/*
 		 * R8A7796 has one RGB output, one LVDS output and one HDMI
@@ -251,7 +251,7 @@ static const struct rcar_du_device_info rcar_du_r8a77970_info = {
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
 		  | RCAR_DU_FEATURE_VSP1_SOURCE,
-	.num_crtcs = 1,
+	.channel_mask = BIT(0),
 	.routes = {
 		/* R8A77970 has one RGB output and one LVDS output. */
 		[RCAR_DU_OUTPUT_DPAD0] = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index 5c7ec15818c7..7a5de66deec2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -52,7 +52,7 @@ struct rcar_du_output_routing {
  * @gen: device generation (2 or 3)
  * @features: device features (RCAR_DU_FEATURE_*)
  * @quirks: device quirks (RCAR_DU_QUIRK_*)
- * @num_crtcs: total number of CRTCs
+ * @channel_mask: bit mask of supported DU channels
  * @routes: array of CRTC to output routes, indexed by output (RCAR_DU_OUTPUT_*)
  * @num_lvds: number of internal LVDS encoders
  */
@@ -60,7 +60,7 @@ struct rcar_du_device_info {
 	unsigned int gen;
 	unsigned int features;
 	unsigned int quirks;
-	unsigned int num_crtcs;
+	unsigned int channel_mask;
 	struct rcar_du_output_routing routes[RCAR_DU_OUTPUT_MAX];
 	unsigned int num_lvds;
 	unsigned int dpll_ch;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index cf5b422fc753..19a445fbc879 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -559,6 +559,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 	struct drm_fbdev_cma *fbdev;
 	unsigned int num_encoders;
 	unsigned int num_groups;
+	unsigned int swi, hwi;
 	unsigned int i;
 	int ret;
 
@@ -571,7 +572,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 	dev->mode_config.funcs = &rcar_du_mode_config_funcs;
 	dev->mode_config.helper_private = &rcar_du_mode_config_helper;
 
-	rcdu->num_crtcs = rcdu->info->num_crtcs;
+	rcdu->num_crtcs = hweight8(rcdu->info->channel_mask);
 
 	ret = rcar_du_properties_init(rcdu);
 	if (ret < 0)
@@ -581,7 +582,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 	 * Initialize vertical blanking interrupts handling. Start with vblank
 	 * disabled for all CRTCs.
 	 */
-	ret = drm_vblank_init(dev, (1 << rcdu->info->num_crtcs) - 1);
+	ret = drm_vblank_init(dev, (1 << rcdu->num_crtcs) - 1);
 	if (ret < 0)
 		return ret;
 
@@ -623,10 +624,16 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 	}
 
 	/* Create the CRTCs. */
-	for (i = 0; i < rcdu->num_crtcs; ++i) {
-		struct rcar_du_group *rgrp = &rcdu->groups[i / 2];
+	for (swi = 0, hwi = 0; swi < rcdu->num_crtcs; ++hwi) {
+		struct rcar_du_group *rgrp;
+
+		/* Skip unpopulated DU channels */
+		if (!(rcdu->info->channel_mask & BIT(hwi)))
+			continue;
+
+		rgrp = &rcdu->groups[hwi / 2];
 
-		ret = rcar_du_crtc_create(rgrp, i);
+		ret = rcar_du_crtc_create(rgrp, swi++, hwi);
 		if (ret < 0)
 			return ret;
 	}
-- 
2.17.0

Re: [PATCH 05/17] drm: rcar-du: Split CRTC handling to support hardware indexing

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:34 EEST Kieran Bingham wrote:
> The DU CRTC driver does not support distinguishing between a hardware
> index, and a software (CRTC) index in the event that a DU channel might
> not be populated by the hardware.
> 
> Support this by adapting the rcar_du_device_info structure to store a
> bitmask of available channels rather than a count of CRTCs. The count
> can then be obtained by determining the hamming weight of the bitmask.
> 
> This allows the rcar_du_crtc_create() function to distinguish between
> both index types, and non-populated DU channels will be skipped without
> leaving a gap in the software CRTC indexes.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 26 ++++++++++++++------------
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  3 ++-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c  | 20 ++++++++++----------
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h  |  4 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 17 ++++++++++++-----
>  5 files changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 5a15dfd66343..36ce194c13b5
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -902,7 +902,8 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
>   * Initialization
>   */
> 
> -int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
> +int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
> +			unsigned int hwindex)
>  {
>  	static const unsigned int mmio_offsets[] = {
>  		DU0_REG_OFFSET, DU1_REG_OFFSET, DU2_REG_OFFSET, DU3_REG_OFFSET
> @@ -910,7 +911,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int index)
> 
>  	struct rcar_du_device *rcdu = rgrp->dev;
>  	struct platform_device *pdev = to_platform_device(rcdu->dev);
> -	struct rcar_du_crtc *rcrtc = &rcdu->crtcs[index];
> +	struct rcar_du_crtc *rcrtc = &rcdu->crtcs[swindex];
>  	struct drm_crtc *crtc = &rcrtc->crtc;
>  	struct drm_plane *primary;
>  	unsigned int irqflags;
> @@ -922,7 +923,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int index)
> 
>  	/* Get the CRTC clock and the optional external clock. */
>  	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
> -		sprintf(clk_name, "du.%u", index);
> +		sprintf(clk_name, "du.%u", hwindex);
>  		name = clk_name;
>  	} else {
>  		name = NULL;
> @@ -930,16 +931,16 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int index)
> 
>  	rcrtc->clock = devm_clk_get(rcdu->dev, name);
>  	if (IS_ERR(rcrtc->clock)) {
> -		dev_err(rcdu->dev, "no clock for CRTC %u\n", index);
> +		dev_err(rcdu->dev, "no clock for CRTC %u\n", swindex);

How about

		dev_err(rcdu->dev, "no clock for DU channel %u\n", hwindex);

I think that would be clearer, because at this stage we're dealing with 
hardware resources, so matching the datasheet numbers seems better to me.

>  		return PTR_ERR(rcrtc->clock);
>  	}
> 
> -	sprintf(clk_name, "dclkin.%u", index);
> +	sprintf(clk_name, "dclkin.%u", hwindex);
>  	clk = devm_clk_get(rcdu->dev, clk_name);
>  	if (!IS_ERR(clk)) {
>  		rcrtc->extclock = clk;
>  	} else if (PTR_ERR(rcrtc->clock) == -EPROBE_DEFER) {
> -		dev_info(rcdu->dev, "can't get external clock %u\n", index);
> +		dev_info(rcdu->dev, "can't get external clock %u\n", hwindex);
>  		return -EPROBE_DEFER;
>  	}
> 
> @@ -948,13 +949,13 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int index) spin_lock_init(&rcrtc->vblank_lock);
> 
>  	rcrtc->group = rgrp;
> -	rcrtc->mmio_offset = mmio_offsets[index];
> -	rcrtc->index = index;
> +	rcrtc->mmio_offset = mmio_offsets[hwindex];
> +	rcrtc->index = hwindex;
> 
>  	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
>  		primary = &rcrtc->vsp->planes[rcrtc->vsp_pipe].plane;
>  	else
> -		primary = &rgrp->planes[index % 2].plane;
> +		primary = &rgrp->planes[hwindex % 2].plane;

This shouldn't make a difference because when RCAR_DU_FEATURE_VSP1_SOURCE 
isn't set we're running on Gen2, and don't need to deal with indices, but from 
a conceptual point of view, wouldn't the software index be better here ? 
Missing hardware channels won't be visible from userspace, so taking the first 
plane of the group as the primary plane would seem better to me.

>  	ret = drm_crtc_init_with_planes(rcdu->ddev, crtc, primary, NULL,
>  					rcdu->info->gen <= 2 ?
> @@ -970,7 +971,8 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int index)
> 
>  	/* Register the interrupt handler. */
>  	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
> -		irq = platform_get_irq(pdev, index);
> +		/* The IRQ's are associated with the CRTC (sw)index */

s/index/index./

> +		irq = platform_get_irq(pdev, swindex);
>  		irqflags = 0;
>  	} else {
>  		irq = platform_get_irq(pdev, 0);
> @@ -978,7 +980,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int index) }
> 
>  	if (irq < 0) {
> -		dev_err(rcdu->dev, "no IRQ for CRTC %u\n", index);
> +		dev_err(rcdu->dev, "no IRQ for CRTC %u\n", swindex);
>  		return irq;
>  	}
> 
> @@ -986,7 +988,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int index) dev_name(rcdu->dev), rcrtc);
>  	if (ret < 0) {
>  		dev_err(rcdu->dev,
> -			"failed to register IRQ for CRTC %u\n", index);
> +			"failed to register IRQ for CRTC %u\n", swindex);
>  		return ret;
>  	}
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 518ee2c60eb8..5f003a16abc5
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -99,7 +99,8 @@ enum rcar_du_output {
>  	RCAR_DU_OUTPUT_MAX,
>  };
> 
> -int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index);
> +int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
> +			unsigned int hwindex);
>  void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
>  void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 05745e86d73e..d6ebc628fc22
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -40,7 +40,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7743_info = { .gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> -	.num_crtcs = 2,
> +	.channel_mask = BIT(0) | BIT(1),

I'd write it BIT(1) | BIT(0) to match the usual little-endian order. Same 
comment for the other info structure instances.

>  	.routes = {
>  		/*
>  		 * R8A7743 has one RGB output and one LVDS output
> @@ -61,7 +61,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7745_info = { .gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> -	.num_crtcs = 2,
> +	.channel_mask = BIT(0) | BIT(1),
>  	.routes = {
>  		/*
>  		 * R8A7745 has two RGB outputs
> @@ -80,7 +80,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7745_info = { static const struct rcar_du_device_info
> rcar_du_r8a7779_info = {
>  	.gen = 2,
>  	.features = 0,
> -	.num_crtcs = 2,
> +	.channel_mask = BIT(0) | BIT(1),
>  	.routes = {
>  		/*
>  		 * R8A7779 has two RGB outputs and one (currently unsupported)
> @@ -102,7 +102,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7790_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>  	.quirks = RCAR_DU_QUIRK_ALIGN_128B,
> -	.num_crtcs = 3,
> +	.channel_mask = BIT(0) | BIT(1) | BIT(2),
>  	.routes = {
>  		/*
>  		 * R8A7790 has one RGB output, two LVDS outputs and one
> @@ -129,7 +129,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7791_info = { .gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> -	.num_crtcs = 2,
> +	.channel_mask = BIT(0) | BIT(1),
>  	.routes = {
>  		/*
>  		 * R8A779[13] has one RGB output, one LVDS output and one
> @@ -151,7 +151,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7792_info = { .gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> -	.num_crtcs = 2,
> +	.channel_mask = BIT(0) | BIT(1),
>  	.routes = {
>  		/* R8A7792 has two RGB outputs. */
>  		[RCAR_DU_OUTPUT_DPAD0] = {
> @@ -169,7 +169,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7794_info = { .gen = 2,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> -	.num_crtcs = 2,
> +	.channel_mask = BIT(0) | BIT(1),
>  	.routes = {
>  		/*
>  		 * R8A7794 has two RGB outputs and one (currently unsupported)
> @@ -191,7 +191,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7795_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
> -	.num_crtcs = 4,
> +	.channel_mask = BIT(0) | BIT(1) | BIT(2) | BIT(3),
>  	.routes = {
>  		/*
>  		 * R8A7795 has one RGB output, two HDMI outputs and one
> @@ -223,7 +223,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7796_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
> -	.num_crtcs = 3,
> +	.channel_mask = BIT(0) | BIT(1) | BIT(2),
>  	.routes = {
>  		/*
>  		 * R8A7796 has one RGB output, one LVDS output and one HDMI
> @@ -251,7 +251,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a77970_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
> -	.num_crtcs = 1,
> +	.channel_mask = BIT(0),
>  	.routes = {
>  		/* R8A77970 has one RGB output and one LVDS output. */
>  		[RCAR_DU_OUTPUT_DPAD0] = {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 5c7ec15818c7..7a5de66deec2
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -52,7 +52,7 @@ struct rcar_du_output_routing {
>   * @gen: device generation (2 or 3)
>   * @features: device features (RCAR_DU_FEATURE_*)
>   * @quirks: device quirks (RCAR_DU_QUIRK_*)
> - * @num_crtcs: total number of CRTCs
> + * @channel_mask: bit mask of supported DU channels

Nitpicking, how about channels_mask ?

>   * @routes: array of CRTC to output routes, indexed by output
> (RCAR_DU_OUTPUT_*) * @num_lvds: number of internal LVDS encoders
>   */
> @@ -60,7 +60,7 @@ struct rcar_du_device_info {
>  	unsigned int gen;
>  	unsigned int features;
>  	unsigned int quirks;
> -	unsigned int num_crtcs;
> +	unsigned int channel_mask;
>  	struct rcar_du_output_routing routes[RCAR_DU_OUTPUT_MAX];
>  	unsigned int num_lvds;
>  	unsigned int dpll_ch;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index cf5b422fc753..19a445fbc879
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -559,6 +559,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  	struct drm_fbdev_cma *fbdev;
>  	unsigned int num_encoders;
>  	unsigned int num_groups;
> +	unsigned int swi, hwi;

One variable per line please. I would also call them swindex and hwindex, that 
would be clearer in my opinion.

>  	unsigned int i;
>  	int ret;
> 
> @@ -571,7 +572,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  	dev->mode_config.funcs = &rcar_du_mode_config_funcs;
>  	dev->mode_config.helper_private = &rcar_du_mode_config_helper;
> 
> -	rcdu->num_crtcs = rcdu->info->num_crtcs;
> +	rcdu->num_crtcs = hweight8(rcdu->info->channel_mask);
> 
>  	ret = rcar_du_properties_init(rcdu);
>  	if (ret < 0)
> @@ -581,7 +582,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  	 * Initialize vertical blanking interrupts handling. Start with vblank
>  	 * disabled for all CRTCs.
>  	 */
> -	ret = drm_vblank_init(dev, (1 << rcdu->info->num_crtcs) - 1);
> +	ret = drm_vblank_init(dev, (1 << rcdu->num_crtcs) - 1);
>  	if (ret < 0)
>  		return ret;
> 
> @@ -623,10 +624,16 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  	}
> 
>  	/* Create the CRTCs. */
> -	for (i = 0; i < rcdu->num_crtcs; ++i) {
> -		struct rcar_du_group *rgrp = &rcdu->groups[i / 2];
> +	for (swi = 0, hwi = 0; swi < rcdu->num_crtcs; ++hwi) {
> +		struct rcar_du_group *rgrp;
> +
> +		/* Skip unpopulated DU channels */

s/channels/channels./

> +		if (!(rcdu->info->channel_mask & BIT(hwi)))
> +			continue;
> +
> +		rgrp = &rcdu->groups[hwi / 2];
> 
> -		ret = rcar_du_crtc_create(rgrp, i);
> +		ret = rcar_du_crtc_create(rgrp, swi++, hwi);
>  		if (ret < 0)
>  			return ret;
>  	}

This is going to turn into an infinite loop if we ever get the num_crtcs 
calculation wrong, but I don't see why that should be the case, so I'm OK with 
the implementation.

With all those small issues fixed,

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

-- 
Regards,

Laurent Pinchart

Re: [PATCH 05/17] drm: rcar-du: Split CRTC handling to support hardware indexing

[Kieran Bingham]

Hi Laurent,

On 26/04/18 21:30, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 19:53:34 EEST Kieran Bingham wrote:
>> The DU CRTC driver does not support distinguishing between a hardware
>> index, and a software (CRTC) index in the event that a DU channel might
>> not be populated by the hardware.
>>
>> Support this by adapting the rcar_du_device_info structure to store a
>> bitmask of available channels rather than a count of CRTCs. The count
>> can then be obtained by determining the hamming weight of the bitmask.
>>
>> This allows the rcar_du_crtc_create() function to distinguish between
>> both index types, and non-populated DU channels will be skipped without
>> leaving a gap in the software CRTC indexes.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 26 ++++++++++++++------------
>>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h |  3 ++-
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.c  | 20 ++++++++++----------
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.h  |  4 ++--
>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c  | 17 ++++++++++++-----
>>  5 files changed, 40 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 5a15dfd66343..36ce194c13b5
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
>> @@ -902,7 +902,8 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
>>   * Initialization
>>   */
>>
>> -int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index)
>> +int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
>> +			unsigned int hwindex)
>>  {
>>  	static const unsigned int mmio_offsets[] = {
>>  		DU0_REG_OFFSET, DU1_REG_OFFSET, DU2_REG_OFFSET, DU3_REG_OFFSET
>> @@ -910,7 +911,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
>> unsigned int index)
>>
>>  	struct rcar_du_device *rcdu = rgrp->dev;
>>  	struct platform_device *pdev = to_platform_device(rcdu->dev);
>> -	struct rcar_du_crtc *rcrtc = &rcdu->crtcs[index];
>> +	struct rcar_du_crtc *rcrtc = &rcdu->crtcs[swindex];
>>  	struct drm_crtc *crtc = &rcrtc->crtc;
>>  	struct drm_plane *primary;
>>  	unsigned int irqflags;
>> @@ -922,7 +923,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
>> unsigned int index)
>>
>>  	/* Get the CRTC clock and the optional external clock. */
>>  	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
>> -		sprintf(clk_name, "du.%u", index);
>> +		sprintf(clk_name, "du.%u", hwindex);
>>  		name = clk_name;
>>  	} else {
>>  		name = NULL;
>> @@ -930,16 +931,16 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
>> unsigned int index)
>>
>>  	rcrtc->clock = devm_clk_get(rcdu->dev, name);
>>  	if (IS_ERR(rcrtc->clock)) {
>> -		dev_err(rcdu->dev, "no clock for CRTC %u\n", index);
>> +		dev_err(rcdu->dev, "no clock for CRTC %u\n", swindex);
> 
> How about
> 
> 		dev_err(rcdu->dev, "no clock for DU channel %u\n", hwindex);
> 
> I think that would be clearer, because at this stage we're dealing with 
> hardware resources, so matching the datasheet numbers seems better to me.

Yes, I agree.
Changed.


>>  		return PTR_ERR(rcrtc->clock);
>>  	}
>>
>> -	sprintf(clk_name, "dclkin.%u", index);
>> +	sprintf(clk_name, "dclkin.%u", hwindex);
>>  	clk = devm_clk_get(rcdu->dev, clk_name);
>>  	if (!IS_ERR(clk)) {
>>  		rcrtc->extclock = clk;
>>  	} else if (PTR_ERR(rcrtc->clock) == -EPROBE_DEFER) {
>> -		dev_info(rcdu->dev, "can't get external clock %u\n", index);
>> +		dev_info(rcdu->dev, "can't get external clock %u\n", hwindex);
>>  		return -EPROBE_DEFER;
>>  	}
>>
>> @@ -948,13 +949,13 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
>> unsigned int index) spin_lock_init(&rcrtc->vblank_lock);
>>
>>  	rcrtc->group = rgrp;
>> -	rcrtc->mmio_offset = mmio_offsets[index];
>> -	rcrtc->index = index;
>> +	rcrtc->mmio_offset = mmio_offsets[hwindex];
>> +	rcrtc->index = hwindex;
>>
>>  	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
>>  		primary = &rcrtc->vsp->planes[rcrtc->vsp_pipe].plane;
>>  	else
>> -		primary = &rgrp->planes[index % 2].plane;
>> +		primary = &rgrp->planes[hwindex % 2].plane;
> 
> This shouldn't make a difference because when RCAR_DU_FEATURE_VSP1_SOURCE 
> isn't set we're running on Gen2, and don't need to deal with indices, but from 
> a conceptual point of view, wouldn't the software index be better here ? 
> Missing hardware channels won't be visible from userspace, so taking the first 
> plane of the group as the primary plane would seem better to me.

That's fine by me - updated.


> 
>>  	ret = drm_crtc_init_with_planes(rcdu->ddev, crtc, primary, NULL,
>>  					rcdu->info->gen <= 2 ?
>> @@ -970,7 +971,8 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
>> unsigned int index)
>>
>>  	/* Register the interrupt handler. */
>>  	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
>> -		irq = platform_get_irq(pdev, index);
>> +		/* The IRQ's are associated with the CRTC (sw)index */
> 
> s/index/index./
> 
>> +		irq = platform_get_irq(pdev, swindex);
>>  		irqflags = 0;
>>  	} else {
>>  		irq = platform_get_irq(pdev, 0);
>> @@ -978,7 +980,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
>> unsigned int index) }
>>
>>  	if (irq < 0) {
>> -		dev_err(rcdu->dev, "no IRQ for CRTC %u\n", index);
>> +		dev_err(rcdu->dev, "no IRQ for CRTC %u\n", swindex);
>>  		return irq;
>>  	}
>>
>> @@ -986,7 +988,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
>> unsigned int index) dev_name(rcdu->dev), rcrtc);
>>  	if (ret < 0) {
>>  		dev_err(rcdu->dev,
>> -			"failed to register IRQ for CRTC %u\n", index);
>> +			"failed to register IRQ for CRTC %u\n", swindex);
>>  		return ret;
>>  	}
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 518ee2c60eb8..5f003a16abc5
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
>> @@ -99,7 +99,8 @@ enum rcar_du_output {
>>  	RCAR_DU_OUTPUT_MAX,
>>  };
>>
>> -int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int index);
>> +int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
>> +			unsigned int hwindex);
>>  void rcar_du_crtc_suspend(struct rcar_du_crtc *rcrtc);
>>  void rcar_du_crtc_resume(struct rcar_du_crtc *rcrtc);
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 05745e86d73e..d6ebc628fc22
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> @@ -40,7 +40,7 @@ static const struct rcar_du_device_info
>> rzg1_du_r8a7743_info = { .gen = 2,
>>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>> -	.num_crtcs = 2,
>> +	.channel_mask = BIT(0) | BIT(1),
> 
> I'd write it BIT(1) | BIT(0) to match the usual little-endian order. Same 
> comment for the other info structure instances.
> 

Not a fan - but it's ok with me. :) Changed.

This is different to the usage on the .dpll_ch though ...

>>  	.routes = {
>>  		/*
>>  		 * R8A7743 has one RGB output and one LVDS output
>> @@ -61,7 +61,7 @@ static const struct rcar_du_device_info
>> rzg1_du_r8a7745_info = { .gen = 2,
>>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>> -	.num_crtcs = 2,
>> +	.channel_mask = BIT(0) | BIT(1),
>>  	.routes = {
>>  		/*
>>  		 * R8A7745 has two RGB outputs
>> @@ -80,7 +80,7 @@ static const struct rcar_du_device_info
>> rzg1_du_r8a7745_info = { static const struct rcar_du_device_info
>> rcar_du_r8a7779_info = {
>>  	.gen = 2,
>>  	.features = 0,
>> -	.num_crtcs = 2,
>> +	.channel_mask = BIT(0) | BIT(1),
>>  	.routes = {
>>  		/*
>>  		 * R8A7779 has two RGB outputs and one (currently unsupported)
>> @@ -102,7 +102,7 @@ static const struct rcar_du_device_info
>> rcar_du_r8a7790_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>>  	.quirks = RCAR_DU_QUIRK_ALIGN_128B,
>> -	.num_crtcs = 3,
>> +	.channel_mask = BIT(0) | BIT(1) | BIT(2),
>>  	.routes = {
>>  		/*
>>  		 * R8A7790 has one RGB output, two LVDS outputs and one
>> @@ -129,7 +129,7 @@ static const struct rcar_du_device_info
>> rcar_du_r8a7791_info = { .gen = 2,
>>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>> -	.num_crtcs = 2,
>> +	.channel_mask = BIT(0) | BIT(1),
>>  	.routes = {
>>  		/*
>>  		 * R8A779[13] has one RGB output, one LVDS output and one
>> @@ -151,7 +151,7 @@ static const struct rcar_du_device_info
>> rcar_du_r8a7792_info = { .gen = 2,
>>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>> -	.num_crtcs = 2,
>> +	.channel_mask = BIT(0) | BIT(1),
>>  	.routes = {
>>  		/* R8A7792 has two RGB outputs. */
>>  		[RCAR_DU_OUTPUT_DPAD0] = {
>> @@ -169,7 +169,7 @@ static const struct rcar_du_device_info
>> rcar_du_r8a7794_info = { .gen = 2,
>>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
>> -	.num_crtcs = 2,
>> +	.channel_mask = BIT(0) | BIT(1),
>>  	.routes = {
>>  		/*
>>  		 * R8A7794 has two RGB outputs and one (currently unsupported)
>> @@ -191,7 +191,7 @@ static const struct rcar_du_device_info
>> rcar_du_r8a7795_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
>> -	.num_crtcs = 4,
>> +	.channel_mask = BIT(0) | BIT(1) | BIT(2) | BIT(3),
>>  	.routes = {
>>  		/*
>>  		 * R8A7795 has one RGB output, two HDMI outputs and one
>> @@ -223,7 +223,7 @@ static const struct rcar_du_device_info
>> rcar_du_r8a7796_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
>> -	.num_crtcs = 3,
>> +	.channel_mask = BIT(0) | BIT(1) | BIT(2),
>>  	.routes = {
>>  		/*
>>  		 * R8A7796 has one RGB output, one LVDS output and one HDMI
>> @@ -251,7 +251,7 @@ static const struct rcar_du_device_info
>> rcar_du_r8a77970_info = { .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>>  		  | RCAR_DU_FEATURE_VSP1_SOURCE,
>> -	.num_crtcs = 1,
>> +	.channel_mask = BIT(0),
>>  	.routes = {
>>  		/* R8A77970 has one RGB output and one LVDS output. */
>>  		[RCAR_DU_OUTPUT_DPAD0] = {
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index 5c7ec15818c7..7a5de66deec2
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> @@ -52,7 +52,7 @@ struct rcar_du_output_routing {
>>   * @gen: device generation (2 or 3)
>>   * @features: device features (RCAR_DU_FEATURE_*)
>>   * @quirks: device quirks (RCAR_DU_QUIRK_*)
>> - * @num_crtcs: total number of CRTCs
>> + * @channel_mask: bit mask of supported DU channels
> 
> Nitpicking, how about channels_mask ?
> 
>>   * @routes: array of CRTC to output routes, indexed by output
>> (RCAR_DU_OUTPUT_*) * @num_lvds: number of internal LVDS encoders
>>   */
>> @@ -60,7 +60,7 @@ struct rcar_du_device_info {
>>  	unsigned int gen;
>>  	unsigned int features;
>>  	unsigned int quirks;
>> -	unsigned int num_crtcs;
>> +	unsigned int channel_mask;
>>  	struct rcar_du_output_routing routes[RCAR_DU_OUTPUT_MAX];
>>  	unsigned int num_lvds;
>>  	unsigned int dpll_ch;
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index cf5b422fc753..19a445fbc879
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -559,6 +559,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>>  	struct drm_fbdev_cma *fbdev;
>>  	unsigned int num_encoders;
>>  	unsigned int num_groups;
>> +	unsigned int swi, hwi;
> 
> One variable per line please. I would also call them swindex and hwindex, that 
> would be clearer in my opinion.
> 

Fixed (on both accounts)


>>  	unsigned int i;
>>  	int ret;
>>
>> @@ -571,7 +572,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>>  	dev->mode_config.funcs = &rcar_du_mode_config_funcs;
>>  	dev->mode_config.helper_private = &rcar_du_mode_config_helper;
>>
>> -	rcdu->num_crtcs = rcdu->info->num_crtcs;
>> +	rcdu->num_crtcs = hweight8(rcdu->info->channel_mask);
>>
>>  	ret = rcar_du_properties_init(rcdu);
>>  	if (ret < 0)
>> @@ -581,7 +582,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>>  	 * Initialize vertical blanking interrupts handling. Start with vblank
>>  	 * disabled for all CRTCs.
>>  	 */
>> -	ret = drm_vblank_init(dev, (1 << rcdu->info->num_crtcs) - 1);
>> +	ret = drm_vblank_init(dev, (1 << rcdu->num_crtcs) - 1);
>>  	if (ret < 0)
>>  		return ret;
>>
>> @@ -623,10 +624,16 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>>  	}
>>
>>  	/* Create the CRTCs. */
>> -	for (i = 0; i < rcdu->num_crtcs; ++i) {
>> -		struct rcar_du_group *rgrp = &rcdu->groups[i / 2];
>> +	for (swi = 0, hwi = 0; swi < rcdu->num_crtcs; ++hwi) {
>> +		struct rcar_du_group *rgrp;
>> +
>> +		/* Skip unpopulated DU channels */
> 
> s/channels/channels./

Done.


> 
>> +		if (!(rcdu->info->channel_mask & BIT(hwi)))
>> +			continue;
>> +
>> +		rgrp = &rcdu->groups[hwi / 2];
>>
>> -		ret = rcar_du_crtc_create(rgrp, i);
>> +		ret = rcar_du_crtc_create(rgrp, swi++, hwi);
>>  		if (ret < 0)
>>  			return ret;
>>  	}
> 
> This is going to turn into an infinite loop if we ever get the num_crtcs 
> calculation wrong, but I don't see why that should be the case, so I'm OK with 
> the implementation.

Yes, I have considered this. I actually started out by making the loop condition
based on "swi < hweight8(rcdu->info->channel_mask)" because of this. Thus that
would ensure that if the channel_mask was ever unset or 0 then the loop would exit.

However then I figured there's no point duplicating the hweight8 call and that
the num_crtc's should represent the same value.

As long as no one tries to increment the num_crtc's arbitrarily - this should be
safe even if the rcdu->info->channel_mask is blank.


> With all those small issues fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, Tag collected.

--
Kieran

[PATCH 06/17] drm: rcar-du: Allow DU groups to work with hardware indexing

[Kieran Bingham]

The group objects assume linear indexing, and more so always assume that
channel 0 of any active group is used.

Now that the CRTC objects support non-linear indexing, adapt the groups
to remove assumptions that channel 0 is utilised in each group by using
the channel mask provided in the device structures.

Finally ensure that the RGB routing is determined from the index of the
CRTC object (which represents the hardware DU channel index).

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_group.c | 14 +++++++++-----
 drivers/gpu/drm/rcar-du/rcar_du_group.h |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  5 ++++-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index eead202c95c7..c52091fe02ba 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -46,9 +46,12 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32 reg, u32 data)
 
 static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp)
 {
-	u32 defr6 = DEFR6_CODE | DEFR6_ODPM02_DISP;
+	u32 defr6 = DEFR6_CODE;
 
-	if (rgrp->num_crtcs > 1)
+	if (rgrp->channel_mask & BIT(0))
+		defr6 |= DEFR6_ODPM02_DISP;
+
+	if (rgrp->channel_mask & BIT(1))
 		defr6 |= DEFR6_ODPM12_DISP;
 
 	rcar_du_group_write(rgrp, DEFR6, defr6);
@@ -80,10 +83,11 @@ static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp)
 		 * On Gen3 VSPD routing can't be configured, but DPAD routing
 		 * needs to be set despite having a single option available.
 		 */
-		u32 crtc = ffs(possible_crtcs) - 1;
+		unsigned int rgb_crtc = ffs(possible_crtcs) - 1;
+		struct rcar_du_crtc *crtc = &rcdu->crtcs[rgb_crtc];
 
-		if (crtc / 2 == rgrp->index)
-			defr8 |= DEFR8_DRGBS_DU(crtc);
+		if (crtc->index / 2 == rgrp->index)
+			defr8 |= DEFR8_DRGBS_DU(crtc->index);
 	}
 
 	rcar_du_group_write(rgrp, DEFR8, defr8);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h
index 5e3adc6b31b5..d29a68e006a7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
@@ -25,6 +25,7 @@ struct rcar_du_device;
  * @dev: the DU device
  * @mmio_offset: registers offset in the device memory map
  * @index: group index
+ * @channel_mask: bitmask of populated DU channels in this group
  * @num_crtcs: number of CRTCs in this group (1 or 2)
  * @use_count: number of users of the group (rcar_du_group_(get|put))
  * @used_crtcs: number of CRTCs currently in use
@@ -39,6 +40,7 @@ struct rcar_du_group {
 	unsigned int mmio_offset;
 	unsigned int index;
 
+	unsigned int channel_mask;
 	unsigned int num_crtcs;
 	unsigned int use_count;
 	unsigned int used_crtcs;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 19a445fbc879..45fb554fd3c7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -597,7 +597,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 		rgrp->dev = rcdu;
 		rgrp->mmio_offset = mmio_offsets[i];
 		rgrp->index = i;
-		rgrp->num_crtcs = min(rcdu->num_crtcs - 2 * i, 2U);
+		/* Extract the channel mask for this group only */
+		rgrp->channel_mask = (rcdu->info->channel_mask >> (2 * i))
+				   & GENMASK(1, 0);
+		rgrp->num_crtcs = hweight8(rgrp->channel_mask);
 
 		/*
 		 * If we have more than one CRTCs in this group pre-associate
-- 
2.17.0

Re: [PATCH 06/17] drm: rcar-du: Allow DU groups to work with hardware indexing

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:35 EEST Kieran Bingham wrote:
> The group objects assume linear indexing, and more so always assume that
> channel 0 of any active group is used.
> 
> Now that the CRTC objects support non-linear indexing, adapt the groups
> to remove assumptions that channel 0 is utilised in each group by using
> the channel mask provided in the device structures.
> 
> Finally ensure that the RGB routing is determined from the index of the
> CRTC object (which represents the hardware DU channel index).
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 14 +++++++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_group.h |  2 ++
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  5 ++++-
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> b/drivers/gpu/drm/rcar-du/rcar_du_group.c index eead202c95c7..c52091fe02ba
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -46,9 +46,12 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32
> reg, u32 data)
> 
>  static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp)
>  {
> -	u32 defr6 = DEFR6_CODE | DEFR6_ODPM02_DISP;
> +	u32 defr6 = DEFR6_CODE;
> 
> -	if (rgrp->num_crtcs > 1)
> +	if (rgrp->channel_mask & BIT(0))
> +		defr6 |= DEFR6_ODPM02_DISP;
> +
> +	if (rgrp->channel_mask & BIT(1))
>  		defr6 |= DEFR6_ODPM12_DISP;

So much cleaner with the channels mask, I like this.

>  	rcar_du_group_write(rgrp, DEFR6, defr6);
> @@ -80,10 +83,11 @@ static void rcar_du_group_setup_defr8(struct
> rcar_du_group *rgrp) * On Gen3 VSPD routing can't be configured, but DPAD
> routing
>  		 * needs to be set despite having a single option available.
>  		 */
> -		u32 crtc = ffs(possible_crtcs) - 1;
> +		unsigned int rgb_crtc = ffs(possible_crtcs) - 1;
> +		struct rcar_du_crtc *crtc = &rcdu->crtcs[rgb_crtc];
> 
> -		if (crtc / 2 == rgrp->index)
> -			defr8 |= DEFR8_DRGBS_DU(crtc);
> +		if (crtc->index / 2 == rgrp->index)
> +			defr8 |= DEFR8_DRGBS_DU(crtc->index);
>  	}
> 
>  	rcar_du_group_write(rgrp, DEFR8, defr8);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h
> b/drivers/gpu/drm/rcar-du/rcar_du_group.h index 5e3adc6b31b5..d29a68e006a7
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
> @@ -25,6 +25,7 @@ struct rcar_du_device;
>   * @dev: the DU device
>   * @mmio_offset: registers offset in the device memory map
>   * @index: group index
> + * @channel_mask: bitmask of populated DU channels in this group
>   * @num_crtcs: number of CRTCs in this group (1 or 2)
>   * @use_count: number of users of the group (rcar_du_group_(get|put))
>   * @used_crtcs: number of CRTCs currently in use
> @@ -39,6 +40,7 @@ struct rcar_du_group {
>  	unsigned int mmio_offset;
>  	unsigned int index;
> 
> +	unsigned int channel_mask;

Depending on how you like my suggestion in patch 05/17, this might be better 
called channels_mask.

>  	unsigned int num_crtcs;
>  	unsigned int use_count;
>  	unsigned int used_crtcs;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 19a445fbc879..45fb554fd3c7
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -597,7 +597,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  		rgrp->dev = rcdu;
>  		rgrp->mmio_offset = mmio_offsets[i];
>  		rgrp->index = i;
> -		rgrp->num_crtcs = min(rcdu->num_crtcs - 2 * i, 2U);
> +		/* Extract the channel mask for this group only */

s/only/only./

> +		rgrp->channel_mask = (rcdu->info->channel_mask >> (2 * i))
> +				   & GENMASK(1, 0);
> +		rgrp->num_crtcs = hweight8(rgrp->channel_mask);

You could optimize this by computing it as

	rgrp->num_crtcs = (rgrp->channel_mask >> 1)
			| (rgrp->channel_mask & 1);

as you know that only two bits at most can be set. Up to you.

>  		/*
>  		 * If we have more than one CRTCs in this group pre-associate

With those small issues fixed,

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

-- 
Regards,

Laurent Pinchart

Re: [PATCH 06/17] drm: rcar-du: Allow DU groups to work with hardware indexing

[Kieran Bingham]

Hi Laurent,

On 26/04/18 21:36, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 19:53:35 EEST Kieran Bingham wrote:
>> The group objects assume linear indexing, and more so always assume that
>> channel 0 of any active group is used.
>>
>> Now that the CRTC objects support non-linear indexing, adapt the groups
>> to remove assumptions that channel 0 is utilised in each group by using
>> the channel mask provided in the device structures.
>>
>> Finally ensure that the RGB routing is determined from the index of the
>> CRTC object (which represents the hardware DU channel index).
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 14 +++++++++-----
>>  drivers/gpu/drm/rcar-du/rcar_du_group.h |  2 ++
>>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  5 ++++-
>>  3 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_group.c index eead202c95c7..c52091fe02ba
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
>> @@ -46,9 +46,12 @@ void rcar_du_group_write(struct rcar_du_group *rgrp, u32
>> reg, u32 data)
>>
>>  static void rcar_du_group_setup_pins(struct rcar_du_group *rgrp)
>>  {
>> -	u32 defr6 = DEFR6_CODE | DEFR6_ODPM02_DISP;
>> +	u32 defr6 = DEFR6_CODE;
>>
>> -	if (rgrp->num_crtcs > 1)
>> +	if (rgrp->channel_mask & BIT(0))
>> +		defr6 |= DEFR6_ODPM02_DISP;
>> +
>> +	if (rgrp->channel_mask & BIT(1))
>>  		defr6 |= DEFR6_ODPM12_DISP;
> 
> So much cleaner with the channels mask, I like this.

:-D

> 
>>  	rcar_du_group_write(rgrp, DEFR6, defr6);
>> @@ -80,10 +83,11 @@ static void rcar_du_group_setup_defr8(struct
>> rcar_du_group *rgrp) * On Gen3 VSPD routing can't be configured, but DPAD
>> routing
>>  		 * needs to be set despite having a single option available.
>>  		 */
>> -		u32 crtc = ffs(possible_crtcs) - 1;
>> +		unsigned int rgb_crtc = ffs(possible_crtcs) - 1;
>> +		struct rcar_du_crtc *crtc = &rcdu->crtcs[rgb_crtc];
>>
>> -		if (crtc / 2 == rgrp->index)
>> -			defr8 |= DEFR8_DRGBS_DU(crtc);
>> +		if (crtc->index / 2 == rgrp->index)
>> +			defr8 |= DEFR8_DRGBS_DU(crtc->index);
>>  	}
>>
>>  	rcar_du_group_write(rgrp, DEFR8, defr8);
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_group.h index 5e3adc6b31b5..d29a68e006a7
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
>> @@ -25,6 +25,7 @@ struct rcar_du_device;
>>   * @dev: the DU device
>>   * @mmio_offset: registers offset in the device memory map
>>   * @index: group index
>> + * @channel_mask: bitmask of populated DU channels in this group
>>   * @num_crtcs: number of CRTCs in this group (1 or 2)
>>   * @use_count: number of users of the group (rcar_du_group_(get|put))
>>   * @used_crtcs: number of CRTCs currently in use
>> @@ -39,6 +40,7 @@ struct rcar_du_group {
>>  	unsigned int mmio_offset;
>>  	unsigned int index;
>>
>> +	unsigned int channel_mask;
> 
> Depending on how you like my suggestion in patch 05/17, this might be better 
> called channels_mask.

Done.

> 
>>  	unsigned int num_crtcs;
>>  	unsigned int use_count;
>>  	unsigned int used_crtcs;
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 19a445fbc879..45fb554fd3c7
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -597,7 +597,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>>  		rgrp->dev = rcdu;
>>  		rgrp->mmio_offset = mmio_offsets[i];
>>  		rgrp->index = i;
>> -		rgrp->num_crtcs = min(rcdu->num_crtcs - 2 * i, 2U);
>> +		/* Extract the channel mask for this group only */
> 
> s/only/only./
> 
>> +		rgrp->channel_mask = (rcdu->info->channel_mask >> (2 * i))
>> +				   & GENMASK(1, 0);
>> +		rgrp->num_crtcs = hweight8(rgrp->channel_mask);
> 
> You could optimize this by computing it as
> 
> 	rgrp->num_crtcs = (rgrp->channel_mask >> 1)
> 			| (rgrp->channel_mask & 1);
> 
> as you know that only two bits at most can be set. Up to you.

Hrm... that looks like a neat trick - but I might leave this as hweight if you
don't object.
We're not on a hot-path here, and hweight is purposefully designed to count
bits, and thus self documenting ... whereas bit-magic is ... magic :D

 (Don't get me wrong though I am a fan of magic :D, and I love good bit-tricks)

> 
>>  		/*
>>  		 * If we have more than one CRTCs in this group pre-associate
> 
> With those small issues fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Thanks, collected.

Kieran

[PATCH 07/17] drm: rcar-du: Add R8A77965 support

[Kieran Bingham]

The R8A77965 (M3-N) SoC provides VGA, HDMI and LVDS output.

This platform is unusual in that the VGA is connected to DU3 leaving DU2
unpopulated. This is reflected by the channel_mask accordingly.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index d6ebc628fc22..4d195ff8c569 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -246,6 +246,34 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = {
 	.dpll_ch =  BIT(1),
 };
 
+static const struct rcar_du_device_info rcar_du_r8a77965_info = {
+	.gen = 3,
+	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
+	.channel_mask = BIT(0) | BIT(1) | BIT(3),
+	.routes = {
+		/*
+		 * R8A77965 has one RGB output, one LVDS output and one HDMI
+		 * output.
+		 */
+		[RCAR_DU_OUTPUT_DPAD0] = {
+			.possible_crtcs = BIT(2),
+			.port = 0,
+		},
+		[RCAR_DU_OUTPUT_HDMI0] = {
+			.possible_crtcs = BIT(1),
+			.port = 1,
+		},
+		[RCAR_DU_OUTPUT_LVDS0] = {
+			.possible_crtcs = BIT(0),
+			.port = 2,
+		},
+	},
+	.num_lvds = 1,
+	.dpll_ch =  BIT(1),
+};
+
 static const struct rcar_du_device_info rcar_du_r8a77970_info = {
 	.gen = 3,
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
@@ -277,6 +305,7 @@ static const struct of_device_id rcar_du_of_table[] = {
 	{ .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-r8a77965", .data = &rcar_du_r8a77965_info },
 	{ .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info },
 	{ }
 };
-- 
2.17.0

Re: [PATCH 07/17] drm: rcar-du: Add R8A77965 support

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:36 EEST Kieran Bingham wrote:
> The R8A77965 (M3-N) SoC provides VGA, HDMI and LVDS output.
> 
> This platform is unusual in that the VGA is connected to DU3 leaving DU2
> unpopulated. This is reflected by the channel_mask accordingly.

I'd write s/VGA/DPAD/g (or s/VGA/RGB/g) as the DPAD output can be used for 
other purposes than VGA.

> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 29 +++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index d6ebc628fc22..4d195ff8c569
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -246,6 +246,34 @@ static const struct rcar_du_device_info
> rcar_du_r8a7796_info = { .dpll_ch =  BIT(1),
>  };
> 
> +static const struct rcar_du_device_info rcar_du_r8a77965_info = {
> +	.gen = 3,
> +	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> +		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
> +		  | RCAR_DU_FEATURE_VSP1_SOURCE,
> +	.channel_mask = BIT(0) | BIT(1) | BIT(3),

Depending on what you think of my suggestions for patch 05/17, you might want 
to reverse the bit order here.

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

I wonder whether it wouldn't be easier to read if we replaced possible_crtcs 
with possible_channels, as this structure describes the hardware and had its 
num_crtcs field replaced with a channel_mask. This would require converting 
the possible_channels field to a possible_crtcs field in 
rcar_du_modeset_init(), and I think that no change would be needed in 
rcar_du_group_setup_defr8() (but please double check). On the other hand, no 
code would be simplified, and rcar_du_modeset_init() would gain some 
additional complexity, so it might not be worth it.

Either way this patch looks good to me.

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

> +			.port = 2,
> +		},
> +	},
> +	.num_lvds = 1,
> +	.dpll_ch =  BIT(1),
> +};
> +
>  static const struct rcar_du_device_info rcar_du_r8a77970_info = {
>  	.gen = 3,
>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> @@ -277,6 +305,7 @@ static const struct of_device_id rcar_du_of_table[] = {
>  	{ .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-r8a77965", .data = &rcar_du_r8a77965_info },
>  	{ .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info },
>  	{ }
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 07/17] drm: rcar-du: Add R8A77965 support

[Kieran Bingham]

On 26/04/18 21:43, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 19:53:36 EEST Kieran Bingham wrote:
>> The R8A77965 (M3-N) SoC provides VGA, HDMI and LVDS output.
>>
>> This platform is unusual in that the VGA is connected to DU3 leaving DU2
>> unpopulated. This is reflected by the channel_mask accordingly.
> 
> I'd write s/VGA/DPAD/g (or s/VGA/RGB/g) as the DPAD output can be used for 
> other purposes than VGA.
> 
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 29 +++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index d6ebc628fc22..4d195ff8c569
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> @@ -246,6 +246,34 @@ static const struct rcar_du_device_info
>> rcar_du_r8a7796_info = { .dpll_ch =  BIT(1),
>>  };
>>
>> +static const struct rcar_du_device_info rcar_du_r8a77965_info = {
>> +	.gen = 3,
>> +	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>> +		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
>> +		  | RCAR_DU_FEATURE_VSP1_SOURCE,
>> +	.channel_mask = BIT(0) | BIT(1) | BIT(3),
> 
> Depending on what you think of my suggestions for patch 05/17, you might want 
> to reverse the bit order here.

Done.

> 
>> +	.routes = {
>> +		/*
>> +		 * R8A77965 has one RGB output, one LVDS output and one HDMI
>> +		 * output.
>> +		 */
>> +		[RCAR_DU_OUTPUT_DPAD0] = {
>> +			.possible_crtcs = BIT(2),
>> +			.port = 0,
>> +		},
>> +		[RCAR_DU_OUTPUT_HDMI0] = {
>> +			.possible_crtcs = BIT(1),
>> +			.port = 1,
>> +		},
>> +		[RCAR_DU_OUTPUT_LVDS0] = {
>> +			.possible_crtcs = BIT(0),
> 
> I wonder whether it wouldn't be easier to read if we replaced possible_crtcs 
> with possible_channels, as this structure describes the hardware and had its 
> num_crtcs field replaced with a channel_mask. This would require converting 
> the possible_channels field to a possible_crtcs field in 
> rcar_du_modeset_init(), and I think that no change would be needed in 
> rcar_du_group_setup_defr8() (but please double check). On the other hand, no 
> code would be simplified, and rcar_du_modeset_init() would gain some 
> additional complexity, so it might not be worth it.

I think we can leave this for now and consider it later if worth while.

> 
> Either way this patch looks good to me.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, collected.

--
Kieran


> 
>> +			.port = 2,
>> +		},
>> +	},
>> +	.num_lvds = 1,
>> +	.dpll_ch =  BIT(1),
>> +};
>> +
>>  static const struct rcar_du_device_info rcar_du_r8a77970_info = {
>>  	.gen = 3,
>>  	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
>> @@ -277,6 +305,7 @@ static const struct of_device_id rcar_du_of_table[] = {
>>  	{ .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-r8a77965", .data = &rcar_du_r8a77965_info },
>>  	{ .compatible = "renesas,du-r8a77970", .data = &rcar_du_r8a77970_info },
>>  	{ }
>>  };
> 

[PATCH 08/17] arm64: dts: r8a77965: Provide sysc header definitions

[Kieran Bingham]

Include the r8a77965-sysc header to provide power domain enumerations
and definitions.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index b12f41755aea..02de36b9e581 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -10,6 +10,7 @@
 
 #include <dt-bindings/clock/renesas-cpg-mssr.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/power/r8a77965-sysc.h>
 
 #define CPG_AUDIO_CLK_I		10
 
-- 
2.17.0

Re: [PATCH 08/17] arm64: dts: r8a77965: Provide sysc header definitions

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:37 EEST Kieran Bingham wrote:
> Include the r8a77965-sysc header to provide power domain enumerations
> and definitions.

I think you can squash this with the first patch that will use the header.

> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
> b12f41755aea..02de36b9e581 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -10,6 +10,7 @@
> 
>  #include <dt-bindings/clock/renesas-cpg-mssr.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/power/r8a77965-sysc.h>
> 
>  #define CPG_AUDIO_CLK_I		10

-- 
Regards,

Laurent Pinchart

[PATCH 09/17] arm64: dts: r8a77965: Use the correct CPG header

[Kieran Bingham]

The SoC dtsi includes the generic renesas-cpg-mssr header, which does
not contain all of the relevant SoC specific definitions.

Adapt this to be the r8a77965 specific header.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 02de36b9e581..894903a59bdc 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -8,7 +8,7 @@
  * Copyright (C) 2016 Renesas Electronics Corp.
  */
 
-#include <dt-bindings/clock/renesas-cpg-mssr.h>
+#include <dt-bindings/clock/r8a77965-cpg-mssr.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/power/r8a77965-sysc.h>
 
-- 
2.17.0

Re: [PATCH 09/17] arm64: dts: r8a77965: Use the correct CPG header

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:38 EEST Kieran Bingham wrote:
> The SoC dtsi includes the generic renesas-cpg-mssr header, which does
> not contain all of the relevant SoC specific definitions.
> 
> Adapt this to be the r8a77965 specific header.

I would squash this with patch 13/17.

> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
> 02de36b9e581..894903a59bdc 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -8,7 +8,7 @@
>   * Copyright (C) 2016 Renesas Electronics Corp.
>   */
> 
> -#include <dt-bindings/clock/renesas-cpg-mssr.h>
> +#include <dt-bindings/clock/r8a77965-cpg-mssr.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/power/r8a77965-sysc.h>


-- 
Regards,

Laurent Pinchart

[PATCH 10/17] arm64: dts: r8a77965: Add FCPF and FCPV instances

[Kieran Bingham]

The FCPs handle the interface between various IP cores and memory. Add
the instances related to the FDPs and VSP2s.

Based on a similar patch of the R8A7796 device tree
by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[Kieran: Rebase to top of tree]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 40 +++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 894903a59bdc..1f44ed7c1b1c 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1001,6 +1001,46 @@
 			/* placeholder */
 		};
 
+		fcpf0: fcp@fe950000 {
+			compatible = "renesas,fcpf";
+			reg = <0 0xfe950000 0 0x200>;
+			clocks = <&cpg CPG_MOD 615>;
+			power-domains = <&sysc R8A77965_PD_A3VP>;
+			resets = <&cpg 615>;
+		};
+
+		fcpvb0: fcp@fe96f000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfe96f000 0 0x200>;
+			clocks = <&cpg CPG_MOD 607>;
+			power-domains = <&sysc R8A77965_PD_A3VP>;
+			resets = <&cpg 607>;
+		};
+
+		fcpvi0: fcp@fe9af000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfe9af000 0 0x200>;
+			clocks = <&cpg CPG_MOD 611>;
+			power-domains = <&sysc R8A77965_PD_A3VP>;
+			resets = <&cpg 611>;
+		};
+
+		fcpvd0: fcp@fea27000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea27000 0 0x200>;
+			clocks = <&cpg CPG_MOD 603>;
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 603>;
+		};
+
+		fcpvd1: fcp@fea2f000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea2f000 0 0x200>;
+			clocks = <&cpg CPG_MOD 602>;
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 602>;
+		};
+
 		csi20: csi2@fea80000 {
 			reg = <0 0xfea80000 0 0x10000>;
 			/* placeholder */
-- 
2.17.0

Re: [PATCH 10/17] arm64: dts: r8a77965: Add FCPF and FCPV instances

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:39 EEST Kieran Bingham wrote:
> The FCPs handle the interface between various IP cores and memory. Add
> the instances related to the FDPs and VSP2s.
> 
> Based on a similar patch of the R8A7796 device tree
> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [Kieran: Rebase to top of tree]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

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

> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 40 +++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
> 894903a59bdc..1f44ed7c1b1c 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -1001,6 +1001,46 @@
>  			/* placeholder */
>  		};
> 
> +		fcpf0: fcp@fe950000 {
> +			compatible = "renesas,fcpf";
> +			reg = <0 0xfe950000 0 0x200>;
> +			clocks = <&cpg CPG_MOD 615>;
> +			power-domains = <&sysc R8A77965_PD_A3VP>;
> +			resets = <&cpg 615>;
> +		};
> +
> +		fcpvb0: fcp@fe96f000 {
> +			compatible = "renesas,fcpv";
> +			reg = <0 0xfe96f000 0 0x200>;
> +			clocks = <&cpg CPG_MOD 607>;
> +			power-domains = <&sysc R8A77965_PD_A3VP>;
> +			resets = <&cpg 607>;
> +		};
> +
> +		fcpvi0: fcp@fe9af000 {
> +			compatible = "renesas,fcpv";
> +			reg = <0 0xfe9af000 0 0x200>;
> +			clocks = <&cpg CPG_MOD 611>;
> +			power-domains = <&sysc R8A77965_PD_A3VP>;
> +			resets = <&cpg 611>;
> +		};
> +
> +		fcpvd0: fcp@fea27000 {
> +			compatible = "renesas,fcpv";
> +			reg = <0 0xfea27000 0 0x200>;
> +			clocks = <&cpg CPG_MOD 603>;
> +			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
> +			resets = <&cpg 603>;
> +		};
> +
> +		fcpvd1: fcp@fea2f000 {
> +			compatible = "renesas,fcpv";
> +			reg = <0 0xfea2f000 0 0x200>;
> +			clocks = <&cpg CPG_MOD 602>;
> +			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
> +			resets = <&cpg 602>;
> +		};
> +
>  		csi20: csi2@fea80000 {
>  			reg = <0 0xfea80000 0 0x10000>;
>  			/* placeholder */

-- 
Regards,

Laurent Pinchart

[PATCH 11/17] arm64: dts: r8a77965: Add VSP instances

[Kieran Bingham]

The r8a77965 has 4 VSP instances.

Based on a similar patch of the R8A7796 device tree
by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[Kieran: Rebased to top of tree, fixed sort orders]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 44 +++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 1f44ed7c1b1c..e92e6b03333a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1009,6 +1009,17 @@
 			resets = <&cpg 615>;
 		};
 
+		vspb: vsp@fe960000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfe960000 0 0x8000>;
+			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 626>;
+			power-domains = <&sysc R8A77965_PD_A3VP>;
+			resets = <&cpg 626>;
+
+			renesas,fcp = <&fcpvb0>;
+		};
+
 		fcpvb0: fcp@fe96f000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfe96f000 0 0x200>;
@@ -1017,6 +1028,17 @@
 			resets = <&cpg 607>;
 		};
 
+		vspi0: vsp@fe9a0000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfe9a0000 0 0x8000>;
+			interrupts = <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 631>;
+			power-domains = <&sysc R8A77965_PD_A3VP>;
+			resets = <&cpg 631>;
+
+			renesas,fcp = <&fcpvi0>;
+		};
+
 		fcpvi0: fcp@fe9af000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfe9af000 0 0x200>;
@@ -1025,6 +1047,17 @@
 			resets = <&cpg 611>;
 		};
 
+		vspd0: vsp@fea20000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea20000 0 0x4000>;
+			interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 623>;
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 623>;
+
+			renesas,fcp = <&fcpvd0>;
+		};
+
 		fcpvd0: fcp@fea27000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfea27000 0 0x200>;
@@ -1033,6 +1066,17 @@
 			resets = <&cpg 603>;
 		};
 
+		vspd1: vsp@fea28000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea28000 0 0x4000>;
+			interrupts = <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 622>;
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 622>;
+
+			renesas,fcp = <&fcpvd1>;
+		};
+
 		fcpvd1: fcp@fea2f000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfea2f000 0 0x200>;
-- 
2.17.0

Re: [PATCH 11/17] arm64: dts: r8a77965: Add VSP instances

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:40 EEST Kieran Bingham wrote:
> The r8a77965 has 4 VSP instances.
> 
> Based on a similar patch of the R8A7796 device tree
> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [Kieran: Rebased to top of tree, fixed sort orders]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 44 +++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
> 1f44ed7c1b1c..e92e6b03333a 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -1009,6 +1009,17 @@
>  			resets = <&cpg 615>;
>  		};
> 
> +		vspb: vsp@fe960000 {
> +			compatible = "renesas,vsp2";
> +			reg = <0 0xfe960000 0 0x8000>;
> +			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 626>;
> +			power-domains = <&sysc R8A77965_PD_A3VP>;
> +			resets = <&cpg 626>;
> +
> +			renesas,fcp = <&fcpvb0>;
> +		};
> +
>  		fcpvb0: fcp@fe96f000 {
>  			compatible = "renesas,fcpv";
>  			reg = <0 0xfe96f000 0 0x200>;
> @@ -1017,6 +1028,17 @@
>  			resets = <&cpg 607>;
>  		};
> 
> +		vspi0: vsp@fe9a0000 {
> +			compatible = "renesas,vsp2";
> +			reg = <0 0xfe9a0000 0 0x8000>;
> +			interrupts = <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 631>;
> +			power-domains = <&sysc R8A77965_PD_A3VP>;
> +			resets = <&cpg 631>;
> +
> +			renesas,fcp = <&fcpvi0>;
> +		};
> +
>  		fcpvi0: fcp@fe9af000 {
>  			compatible = "renesas,fcpv";
>  			reg = <0 0xfe9af000 0 0x200>;
> @@ -1025,6 +1047,17 @@
>  			resets = <&cpg 611>;
>  		};
> 
> +		vspd0: vsp@fea20000 {
> +			compatible = "renesas,vsp2";
> +			reg = <0 0xfea20000 0 0x4000>;

RFP2 has a CLUT so the register range needs to be extended. I'd recommend 
covering the entire space (0x8000) even if no LUT or CLU module is present.

> +			interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 623>;
> +			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
> +			resets = <&cpg 623>;
> +
> +			renesas,fcp = <&fcpvd0>;
> +		};
> +
>  		fcpvd0: fcp@fea27000 {
>  			compatible = "renesas,fcpv";
>  			reg = <0 0xfea27000 0 0x200>;
> @@ -1033,6 +1066,17 @@
>  			resets = <&cpg 603>;
>  		};
> 
> +		vspd1: vsp@fea28000 {
> +			compatible = "renesas,vsp2";
> +			reg = <0 0xfea28000 0 0x4000>;

Same here.

With this fixed,

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

> +			interrupts = <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 622>;
> +			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
> +			resets = <&cpg 622>;
> +
> +			renesas,fcp = <&fcpvd1>;
> +		};
> +
>  		fcpvd1: fcp@fea2f000 {
>  			compatible = "renesas,fcpv";
>  			reg = <0 0xfea2f000 0 0x200>;


-- 
Regards,

Laurent Pinchart

Re: [PATCH 11/17] arm64: dts: r8a77965: Add VSP instances

[Kieran Bingham]

Hi Laurent,

On 26/04/18 22:11, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 19:53:40 EEST Kieran Bingham wrote:
>> The r8a77965 has 4 VSP instances.
>>
>> Based on a similar patch of the R8A7796 device tree
>> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.
>>
>> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>> [Kieran: Rebased to top of tree, fixed sort orders]
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 44 +++++++++++++++++++++++
>>  1 file changed, 44 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
>> 1f44ed7c1b1c..e92e6b03333a 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> @@ -1009,6 +1009,17 @@
>>  			resets = <&cpg 615>;
>>  		};
>>
>> +		vspb: vsp@fe960000 {
>> +			compatible = "renesas,vsp2";
>> +			reg = <0 0xfe960000 0 0x8000>;
>> +			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&cpg CPG_MOD 626>;
>> +			power-domains = <&sysc R8A77965_PD_A3VP>;
>> +			resets = <&cpg 626>;
>> +
>> +			renesas,fcp = <&fcpvb0>;
>> +		};
>> +
>>  		fcpvb0: fcp@fe96f000 {
>>  			compatible = "renesas,fcpv";
>>  			reg = <0 0xfe96f000 0 0x200>;
>> @@ -1017,6 +1028,17 @@
>>  			resets = <&cpg 607>;
>>  		};
>>
>> +		vspi0: vsp@fe9a0000 {
>> +			compatible = "renesas,vsp2";
>> +			reg = <0 0xfe9a0000 0 0x8000>;
>> +			interrupts = <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&cpg CPG_MOD 631>;
>> +			power-domains = <&sysc R8A77965_PD_A3VP>;
>> +			resets = <&cpg 631>;
>> +
>> +			renesas,fcp = <&fcpvi0>;
>> +		};
>> +
>>  		fcpvi0: fcp@fe9af000 {
>>  			compatible = "renesas,fcpv";
>>  			reg = <0 0xfe9af000 0 0x200>;
>> @@ -1025,6 +1047,17 @@
>>  			resets = <&cpg 611>;
>>  		};
>>
>> +		vspd0: vsp@fea20000 {
>> +			compatible = "renesas,vsp2";
>> +			reg = <0 0xfea20000 0 0x4000>;
> 
> RFP2 has a CLUT so the register range needs to be extended. I'd recommend 
> covering the entire space (0x8000) even if no LUT or CLU module is present.

Ah yes, of course.
Updated.

> 
>> +			interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&cpg CPG_MOD 623>;
>> +			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
>> +			resets = <&cpg 623>;
>> +
>> +			renesas,fcp = <&fcpvd0>;
>> +		};
>> +
>>  		fcpvd0: fcp@fea27000 {
>>  			compatible = "renesas,fcpv";
>>  			reg = <0 0xfea27000 0 0x200>;
>> @@ -1033,6 +1066,17 @@
>>  			resets = <&cpg 603>;
>>  		};
>>
>> +		vspd1: vsp@fea28000 {
>> +			compatible = "renesas,vsp2";
>> +			reg = <0 0xfea28000 0 0x4000>;
> 
> Same here.
> 

And here...


> With this fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Collected.

--
Kieran


> 
>> +			interrupts = <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&cpg CPG_MOD 622>;
>> +			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
>> +			resets = <&cpg 622>;
>> +
>> +			renesas,fcp = <&fcpvd1>;
>> +		};
>> +
>>  		fcpvd1: fcp@fea2f000 {
>>  			compatible = "renesas,fcpv";
>>  			reg = <0 0xfea2f000 0 0x200>;
> 
> 

Re: [PATCH 11/17] arm64: dts: r8a77965: Add VSP instances

[Geert Uytterhoeven]

Hi Laurent,

CC Sergei (for V3H/V3M)

On Thu, Apr 26, 2018 at 11:11 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thursday, 26 April 2018 19:53:40 EEST Kieran Bingham wrote:
>> The r8a77965 has 4 VSP instances.
>>
>> Based on a similar patch of the R8A7796 device tree
>> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.
>>
>> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>> [Kieran: Rebased to top of tree, fixed sort orders]
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 44 +++++++++++++++++++++++
>>  1 file changed, 44 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
>> 1f44ed7c1b1c..e92e6b03333a 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi

>> @@ -1025,6 +1047,17 @@
>>                       resets = <&cpg 611>;
>>               };
>>
>> +             vspd0: vsp@fea20000 {
>> +                     compatible = "renesas,vsp2";
>> +                     reg = <0 0xfea20000 0 0x4000>;
>
> RFP2 has a CLUT so the register range needs to be extended. I'd recommend
> covering the entire space (0x8000) even if no LUT or CLU module is present.

Even on V3H/V3M, which have some part of the CLUT, and could do
with 0x5000?

Note that this makes it overlap with fcpvd0 on all R-Car Gen3 SoCs,
as mentioned by Simon on IRC.

>> +                     interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>;
>> +                     clocks = <&cpg CPG_MOD 623>;
>> +                     power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
>> +                     resets = <&cpg 623>;
>> +
>> +                     renesas,fcp = <&fcpvd0>;
>> +             };
>> +
>>               fcpvd0: fcp@fea27000 {
>>                       compatible = "renesas,fcpv";
>>                       reg = <0 0xfea27000 0 0x200>;

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 11/17] arm64: dts: r8a77965: Add VSP instances

[Laurent Pinchart]

Hi Geert,

On Friday, 8 June 2018 12:29:29 EEST Geert Uytterhoeven wrote:
> On Thu, Apr 26, 2018 at 11:11 PM, Laurent Pinchart wrote:
> > On Thursday, 26 April 2018 19:53:40 EEST Kieran Bingham wrote:
> >> The r8a77965 has 4 VSP instances.
> >> 
> >> Based on a similar patch of the R8A7796 device tree
> >> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.
> >> 
> >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> >> [Kieran: Rebased to top of tree, fixed sort orders]
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> ---
> >> 
> >>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 44 +++++++++++++++++++++++
> >>  1 file changed, 44 insertions(+)
> >> 
> >> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> >> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
> >> 1f44ed7c1b1c..e92e6b03333a 100644
> >> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> >> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> >> @@ -1025,6 +1047,17 @@
> >>                       resets = <&cpg 611>;
> >>               };
> >> 
> >> +             vspd0: vsp@fea20000 {
> >> +                     compatible = "renesas,vsp2";
> >> +                     reg = <0 0xfea20000 0 0x4000>;
> > 
> > RFP2 has a CLUT so the register range needs to be extended. I'd recommend
> > covering the entire space (0x8000) even if no LUT or CLU module is
> > present.
> 
> Even on V3H/V3M, which have some part of the CLUT, and could do
> with 0x5000?
> 
> Note that this makes it overlap with fcpvd0 on all R-Car Gen3 SoCs,
> as mentioned by Simon on IRC.

My bad :-/ I'll submit fixes shortly.

> >> +                     interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>;
> >> +                     clocks = <&cpg CPG_MOD 623>;
> >> +                     power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
> >> +                     resets = <&cpg 623>;
> >> +
> >> +                     renesas,fcp = <&fcpvd0>;
> >> +             };
> >> +
> >>               fcpvd0: fcp@fea27000 {
> >>                       compatible = "renesas,fcpv";
> >>                       reg = <0 0xfea27000 0 0x200>;

-- 
Regards,

Laurent Pinchart

[PATCH 12/17] arm64: dts: r8a77965: Populate the DU instance placeholder

[Kieran Bingham]

The DU entity node has been previously added but only as a placeholder.
Populate the node with the properties to use the device.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index e92e6b03333a..8a40bba53027 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1106,9 +1106,21 @@
 		};
 
 		du: display@feb00000 {
+			compatible = "renesas,du-r8a77965";
 			reg = <0 0xfeb00000 0 0x80000>,
 			      <0 0xfeb90000 0 0x14>;
-			/* placeholder */
+			reg-names = "du", "lvds.0";
+			interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 724>,
+				 <&cpg CPG_MOD 723>,
+				 <&cpg CPG_MOD 721>,
+				 <&cpg CPG_MOD 727>;
+			clock-names = "du.0", "du.1", "du.3", "lvds.0";
+			status = "disabled";
+
+			vsps = <&vspd0 0 &vspd1 0 &vspd0 1>;
 
 			ports {
 				#address-cells = <1>;
-- 
2.17.0

Re: [PATCH 12/17] arm64: dts: r8a77965: Populate the DU instance placeholder

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:41 EEST Kieran Bingham wrote:
> The DU entity node has been previously added but only as a placeholder.
> Populate the node with the properties to use the device.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
> e92e6b03333a..8a40bba53027 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -1106,9 +1106,21 @@
>  		};
> 
>  		du: display@feb00000 {
> +			compatible = "renesas,du-r8a77965";
>  			reg = <0 0xfeb00000 0 0x80000>,
>  			      <0 0xfeb90000 0 0x14>;
> -			/* placeholder */
> +			reg-names = "du", "lvds.0";

The LVDS encoder has been split to a separate node in current bindings. You 
can remove the reg-names property and the second reg entry.

> +			interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 724>,
> +				 <&cpg CPG_MOD 723>,
> +				 <&cpg CPG_MOD 721>,
> +				 <&cpg CPG_MOD 727>;
> +			clock-names = "du.0", "du.1", "du.3", "lvds.0";

And you can remove the LVDS clock as well (from both clocks and clock-names).

With this fixed,

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

> +			status = "disabled";
> +
> +			vsps = <&vspd0 0 &vspd1 0 &vspd0 1>;
> 
>  			ports {
>  				#address-cells = <1>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 12/17] arm64: dts: r8a77965: Populate the DU instance placeholder

[Kieran Bingham]

On 26/04/18 22:15, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 19:53:41 EEST Kieran Bingham wrote:
>> The DU entity node has been previously added but only as a placeholder.
>> Populate the node with the properties to use the device.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
>> e92e6b03333a..8a40bba53027 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> @@ -1106,9 +1106,21 @@
>>  		};
>>
>>  		du: display@feb00000 {
>> +			compatible = "renesas,du-r8a77965";
>>  			reg = <0 0xfeb00000 0 0x80000>,
>>  			      <0 0xfeb90000 0 0x14>;
>> -			/* placeholder */
>> +			reg-names = "du", "lvds.0";
> 
> The LVDS encoder has been split to a separate node in current bindings. You 
> can remove the reg-names property and the second reg entry.

Done.

> 
>> +			interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 270 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&cpg CPG_MOD 724>,
>> +				 <&cpg CPG_MOD 723>,
>> +				 <&cpg CPG_MOD 721>,
>> +				 <&cpg CPG_MOD 727>;
>> +			clock-names = "du.0", "du.1", "du.3", "lvds.0";
> 
> And you can remove the LVDS clock as well (from both clocks and clock-names).
> 
> With this fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Collected, thanks.

--
Kieran


> 
>> +			status = "disabled";
>> +
>> +			vsps = <&vspd0 0 &vspd1 0 &vspd0 1>;
>>
>>  			ports {
>>  				#address-cells = <1>;
> 

[PATCH 13/17] arm64: dts: r8a77965: Add HDMI encoder instance

[Kieran Bingham]

Add the HDMI encoder to the R8A77965 DT in disabled state.

Based on a similar patch of the R8A7796 device tree
by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[Kieran: Rebase to top of tree]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 28 +++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 8a40bba53027..972be1460a32 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1105,6 +1105,33 @@
 			};
 		};
 
+		hdmi0: hdmi@fead0000 {
+			compatible = "renesas,r8a77965-hdmi",
+				     "renesas,rcar-gen3-hdmi";
+			reg = <0 0xfead0000 0 0x10000>;
+			interrupts = <GIC_SPI 389 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 729>,
+				 <&cpg CPG_CORE R8A77965_CLK_HDMI>;
+			clock-names = "iahb", "isfr";
+			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+			resets = <&cpg 729>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					dw_hdmi0_in: endpoint {
+						remote-endpoint = <&du_out_hdmi0>;
+					};
+				};
+				port@1 {
+					reg = <1>;
+				};
+			};
+		};
+
 		du: display@feb00000 {
 			compatible = "renesas,du-r8a77965";
 			reg = <0 0xfeb00000 0 0x80000>,
@@ -1134,6 +1161,7 @@
 				port@1 {
 					reg = <1>;
 					du_out_hdmi0: endpoint {
+						remote-endpoint = <&dw_hdmi0_in>;
 					};
 				};
 				port@2 {
-- 
2.17.0

Re: [PATCH 13/17] arm64: dts: r8a77965: Add HDMI encoder instance

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:42 EEST Kieran Bingham wrote:
> Add the HDMI encoder to the R8A77965 DT in disabled state.
> 
> Based on a similar patch of the R8A7796 device tree
> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [Kieran: Rebase to top of tree]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

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

> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 28 +++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index
> 8a40bba53027..972be1460a32 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -1105,6 +1105,33 @@
>  			};
>  		};
> 
> +		hdmi0: hdmi@fead0000 {
> +			compatible = "renesas,r8a77965-hdmi",
> +				     "renesas,rcar-gen3-hdmi";
> +			reg = <0 0xfead0000 0 0x10000>;
> +			interrupts = <GIC_SPI 389 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 729>,
> +				 <&cpg CPG_CORE R8A77965_CLK_HDMI>;
> +			clock-names = "iahb", "isfr";
> +			power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
> +			resets = <&cpg 729>;
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				port@0 {
> +					reg = <0>;
> +					dw_hdmi0_in: endpoint {
> +						remote-endpoint = <&du_out_hdmi0>;
> +					};
> +				};
> +				port@1 {
> +					reg = <1>;
> +				};
> +			};
> +		};
> +
>  		du: display@feb00000 {
>  			compatible = "renesas,du-r8a77965";
>  			reg = <0 0xfeb00000 0 0x80000>,
> @@ -1134,6 +1161,7 @@
>  				port@1 {
>  					reg = <1>;
>  					du_out_hdmi0: endpoint {
> +						remote-endpoint = <&dw_hdmi0_in>;
>  					};
>  				};
>  				port@2 {

-- 
Regards,

Laurent Pinchart

[PATCH 14/17] arm64: dts: r8a77965-salvator-x: Add DU external dot clocks

[Kieran Bingham]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

The DU1 external dot clock is provided by the fixed frequency clock
generator X21, while the DU0 and DU3 clocks are provided by the
programmable Versaclock5 clock generator.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
index 75d890d91df9..a2b8fb20fef8 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
@@ -19,3 +19,15 @@
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
 };
+
+&du {
+	clocks = <&cpg CPG_MOD 724>,
+		 <&cpg CPG_MOD 723>,
+		 <&cpg CPG_MOD 721>,
+		 <&cpg CPG_MOD 727>,
+		 <&versaclock5 1>,
+		 <&x21_clk>,
+		 <&versaclock5 2>;
+	clock-names = "du.0", "du.1", "du.3", "lvds.0",
+		      "dclkin.0", "dclkin.1", "dclkin.3";
+};
-- 
2.17.0

Re: [PATCH 14/17] arm64: dts: r8a77965-salvator-x: Add DU external dot clocks

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:43 EEST Kieran Bingham wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> The DU1 external dot clock is provided by the fixed frequency clock
> generator X21, while the DU0 and DU3 clocks are provided by the
> programmable Versaclock5 clock generator.

I think you can squash this with patch 15/17.

> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts index
> 75d890d91df9..a2b8fb20fef8 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> @@ -19,3 +19,15 @@
>  		reg = <0x0 0x48000000 0x0 0x78000000>;
>  	};
>  };
> +
> +&du {
> +	clocks = <&cpg CPG_MOD 724>,
> +		 <&cpg CPG_MOD 723>,
> +		 <&cpg CPG_MOD 721>,
> +		 <&cpg CPG_MOD 727>,
> +		 <&versaclock5 1>,
> +		 <&x21_clk>,
> +		 <&versaclock5 2>;
> +	clock-names = "du.0", "du.1", "du.3", "lvds.0",
> +		      "dclkin.0", "dclkin.1", "dclkin.3";
> +};

-- 
Regards,

Laurent Pinchart

[PATCH 15/17] arm64: dts: r8a77965-salvator-x: Enable HDMI output

[Kieran Bingham]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

Enable the HDMI encoder for the M3N Salvator-X board and hook it up to
the HDMI connector.

Based on a similar patches of the the Salvator-X board
on the R8A7796 SoC device tree
by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 .../boot/dts/renesas/r8a77965-salvator-x.dts    | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
index a2b8fb20fef8..11e8d43e9e11 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
@@ -31,3 +31,20 @@
 	clock-names = "du.0", "du.1", "du.3", "lvds.0",
 		      "dclkin.0", "dclkin.1", "dclkin.3";
 };
+
+&hdmi0 {
+	status = "okay";
+
+	ports {
+		port@1 {
+			reg = <1>;
+			rcar_dw_hdmi0_out: endpoint {
+				remote-endpoint = <&hdmi0_con>;
+			};
+		};
+	};
+};
+
+&hdmi0_con {
+	remote-endpoint = <&rcar_dw_hdmi0_out>;
+};
-- 
2.17.0

Re: [PATCH 15/17] arm64: dts: r8a77965-salvator-x: Enable HDMI output

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:44 EEST Kieran Bingham wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> Enable the HDMI encoder for the M3N Salvator-X board and hook it up to
> the HDMI connector.
> 
> Based on a similar patches of the the Salvator-X board
> on the R8A7796 SoC device tree
> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

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

How about adding the VGA output too ? I think you can add both VGA and HDMI in 
a single patch that enables display in one go for the board. Same comment for 
the Salvator-XS.

> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  .../boot/dts/renesas/r8a77965-salvator-x.dts    | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts index
> a2b8fb20fef8..11e8d43e9e11 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> @@ -31,3 +31,20 @@
>  	clock-names = "du.0", "du.1", "du.3", "lvds.0",
>  		      "dclkin.0", "dclkin.1", "dclkin.3";
>  };
> +
> +&hdmi0 {
> +	status = "okay";
> +
> +	ports {
> +		port@1 {
> +			reg = <1>;
> +			rcar_dw_hdmi0_out: endpoint {
> +				remote-endpoint = <&hdmi0_con>;
> +			};
> +		};
> +	};
> +};
> +
> +&hdmi0_con {
> +	remote-endpoint = <&rcar_dw_hdmi0_out>;
> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 15/17] arm64: dts: r8a77965-salvator-x: Enable HDMI output

[Kieran Bingham]

Hi Laurent,

On 26/04/18 22:21, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 26 April 2018 19:53:44 EEST Kieran Bingham wrote:
>> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>>
>> Enable the HDMI encoder for the M3N Salvator-X board and hook it up to
>> the HDMI connector.
>>
>> Based on a similar patches of the the Salvator-X board
>> on the R8A7796 SoC device tree
>> by Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, collected.

> 
> How about adding the VGA output too ? I think you can add both VGA and HDMI in 
> a single patch that enables display in one go for the board. Same comment for 
> the Salvator-XS.

I believe the VGA output is handled by the salvator-common.dtsi, and thus
doesn't need to be included in this patch.

> 
>> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>>  .../boot/dts/renesas/r8a77965-salvator-x.dts    | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
>> b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts index
>> a2b8fb20fef8..11e8d43e9e11 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
>> +++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
>> @@ -31,3 +31,20 @@
>>  	clock-names = "du.0", "du.1", "du.3", "lvds.0",
>>  		      "dclkin.0", "dclkin.1", "dclkin.3";
>>  };
>> +
>> +&hdmi0 {
>> +	status = "okay";
>> +
>> +	ports {
>> +		port@1 {
>> +			reg = <1>;
>> +			rcar_dw_hdmi0_out: endpoint {
>> +				remote-endpoint = <&hdmi0_con>;
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&hdmi0_con {
>> +	remote-endpoint = <&rcar_dw_hdmi0_out>;
>> +};
> 

[PATCH 16/17] arm64: dts: r8a77965-salvator-xs: Add DU external dot clocks

[Kieran Bingham]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

The DU1 external dot clock is provided by the fixed frequency clock
generator X21, while the DU0 and DU3 clocks are provided by the
programmable Versaclock6 clock generator.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts b/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
index a83a00deed9e..2223cc2bd6bc 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
@@ -19,3 +19,15 @@
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
 };
+
+&du {
+	clocks = <&cpg CPG_MOD 724>,
+		 <&cpg CPG_MOD 723>,
+		 <&cpg CPG_MOD 721>,
+		 <&cpg CPG_MOD 727>,
+		 <&versaclock6 1>,
+		 <&x21_clk>,
+		 <&versaclock6 2>;
+	clock-names = "du.0", "du.1", "du.3", "lvds.0",
+		      "dclkin.0", "dclkin.1", "dclkin.3";
+};
-- 
2.17.0

Re: [PATCH 16/17] arm64: dts: r8a77965-salvator-xs: Add DU external dot clocks

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thursday, 26 April 2018 19:53:45 EEST Kieran Bingham wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> The DU1 external dot clock is provided by the fixed frequency clock
> generator X21, while the DU0 and DU3 clocks are provided by the
> programmable Versaclock6 clock generator.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
> b/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts index
> a83a00deed9e..2223cc2bd6bc 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
> @@ -19,3 +19,15 @@
>  		reg = <0x0 0x48000000 0x0 0x78000000>;
>  	};
>  };
> +
> +&du {
> +	clocks = <&cpg CPG_MOD 724>,
> +		 <&cpg CPG_MOD 723>,
> +		 <&cpg CPG_MOD 721>,
> +		 <&cpg CPG_MOD 727>,
> +		 <&versaclock6 1>,
> +		 <&x21_clk>,
> +		 <&versaclock6 2>;
> +	clock-names = "du.0", "du.1", "du.3", "lvds.0",
> +		      "dclkin.0", "dclkin.1", "dclkin.3";

There's no LVDS clock in the DU anymore, you can drop it.

As for patch 14/17, I think you can squash this one with 17/17.

> +};

-- 
Regards,

Laurent Pinchart

[PATCH 17/17] arm64: dts: r8a77965-salvator-xs: Enable HDMI output

[Kieran Bingham]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch enables the HDMI encoder for the Salvator-X 2nd version board
on the R8A77965 SoC and hook it up to the HDMI connector.

Based on code of the Salvator-X 2nd version board
on the R8A7796 SoC device tree
by Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 .../boot/dts/renesas/r8a77965-salvator-xs.dts   | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts b/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
index 2223cc2bd6bc..dcf1849f1a67 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
@@ -31,3 +31,20 @@
 	clock-names = "du.0", "du.1", "du.3", "lvds.0",
 		      "dclkin.0", "dclkin.1", "dclkin.3";
 };
+
+&hdmi0 {
+	status = "okay";
+
+	ports {
+		port@1 {
+			reg = <1>;
+			rcar_dw_hdmi0_out: endpoint {
+				remote-endpoint = <&hdmi0_con>;
+			};
+		};
+	};
+};
+
+&hdmi0_con {
+	remote-endpoint = <&rcar_dw_hdmi0_out>;
+};
-- 
2.17.0