Skip to contentSkip to conversation

pinctrl: sh-pfc: r8a7794: DU support

PATCH In review Magnus Damm 6 participants 12 messages Nov 17, 2015 – Dec 21, 2015

Subsystems (3)
GPIO SuperH Pin control
lore
Share & tools
lore

Magnus Damm

author
#0 lore raw

cover pinctrl: sh-pfc: r8a7794: DU support

pinctrl: sh-pfc: r8a7794: DU support

[PATCH 01/04] pinctrl: sh-pfc: r8a7794: Add DU pin groups [PATCH 02/04] pinctrl: sh-pfc: r8a7794: Separate DU CDE and DISP [PATCH 03/04] pinctrl: sh-pfc: r8a7794: Add missing dot clock signals [PATCH 04/04] pinctrl: sh-pfc: r8a7794: Break out ODDF from sync

These patches take the r8a7794 PFC DU support code from the BSP and reworks it to fit the r8a7794 ALT board. Tested with the ALT VGA port - by default PFC is not used however enabling PFC using an incremental (yet to be posted) patch works well.

It is worth noting that patch 2-4 modifies the pin groups. This means that the upstream DT ABI for PFC DU will differ compared to the unreviewed BSP code.

In general it is not considered good practice to change the pin groups and break compatibility since they are part of the DT ABI.

For this particular case upstream never have had PFC DU support for r8a7794, so treating the BSP bindings as experimental and migrate away seems reasonable.

Signed-off-by Magnus Damm <[email protected]>

---

Written against v4.4-rc1

 drivers/pinctrl/sh-pfc/pfc-r8a7794.c |  282 ++++++++++++++++++++++++++++++----
 1 file changed, 252 insertions(+), 30 deletions(-)
author
#1 lore raw

PATCH 1/4 pinctrl: sh-pfc: r8a7794: Add DU pin groups

From: Koji Matsuoka <[email protected]>

r8a7794 PFC DU support from the R-Car Gen2 v1.9.4 BSP

Signed-off-by Koji Matsuoka <[email protected]>
Signed-off-by Magnus Damm <[email protected]>

---

 drivers/pinctrl/sh-pfc/pfc-r8a7794.c | 170 +++++++++++++++++++++++++++++++++++
 1 file changed, 170 insertions(+)
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +170 -0
index b25851c..0af7c12 100644
@@ -1306,6 +1306,140 @@ static const struct sh_pfc_pin pinmux_pins[] = {
 	PINMUX_GPIO_GP_ALL(),
 };
 
+/* - DU --------------------------------------------------------------------- */
+static const unsigned int du0_rgb666_pins[] = {
+	/* R[7:2], G[7:2], B[7:2] */
+	RCAR_GP_PIN(2, 7),  RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 5),
+	RCAR_GP_PIN(2, 4),  RCAR_GP_PIN(2, 3),  RCAR_GP_PIN(2, 2),
+	RCAR_GP_PIN(2, 15), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 13),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 10),
+	RCAR_GP_PIN(2, 23), RCAR_GP_PIN(2, 22), RCAR_GP_PIN(2, 21),
+	RCAR_GP_PIN(2, 20), RCAR_GP_PIN(2, 19), RCAR_GP_PIN(2, 18),
+};
+static const unsigned int du0_rgb666_mux[] = {
+	DU0_DR7_MARK, DU0_DR6_MARK, DU0_DR5_MARK, DU0_DR4_MARK,
+	DU0_DR3_MARK, DU0_DR2_MARK,
+	DU0_DG7_MARK, DU0_DG6_MARK, DU0_DG5_MARK, DU0_DG4_MARK,
+	DU0_DG3_MARK, DU0_DG2_MARK,
+	DU0_DB7_MARK, DU0_DB6_MARK, DU0_DB5_MARK, DU0_DB4_MARK,
+	DU0_DB3_MARK, DU0_DB2_MARK,
+};
+static const unsigned int du0_rgb888_pins[] = {
+	/* R[7:0], G[7:0], B[7:0] */
+	RCAR_GP_PIN(2, 7),  RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 5),
+	RCAR_GP_PIN(2, 4),  RCAR_GP_PIN(2, 3),  RCAR_GP_PIN(2, 2),
+	RCAR_GP_PIN(2, 1),  RCAR_GP_PIN(2, 0),
+	RCAR_GP_PIN(2, 15), RCAR_GP_PIN(2, 14), RCAR_GP_PIN(2, 13),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 11), RCAR_GP_PIN(2, 10),
+	RCAR_GP_PIN(2, 9),  RCAR_GP_PIN(2, 8),
+	RCAR_GP_PIN(2, 23), RCAR_GP_PIN(2, 22), RCAR_GP_PIN(2, 21),
+	RCAR_GP_PIN(2, 20), RCAR_GP_PIN(2, 19), RCAR_GP_PIN(2, 18),
+	RCAR_GP_PIN(2, 17), RCAR_GP_PIN(2, 16),
+};
+static const unsigned int du0_rgb888_mux[] = {
+	DU0_DR7_MARK, DU0_DR6_MARK, DU0_DR5_MARK, DU0_DR4_MARK,
+	DU0_DR3_MARK, DU0_DR2_MARK, DU0_DR1_MARK, DU0_DR0_MARK,
+	DU0_DG7_MARK, DU0_DG6_MARK, DU0_DG5_MARK, DU0_DG4_MARK,
+	DU0_DG3_MARK, DU0_DG2_MARK, DU0_DG1_MARK, DU0_DG0_MARK,
+	DU0_DB7_MARK, DU0_DB6_MARK, DU0_DB5_MARK, DU0_DB4_MARK,
+	DU0_DB3_MARK, DU0_DB2_MARK, DU0_DB1_MARK, DU0_DB0_MARK,
+};
+static const unsigned int du1_rgb666_pins[] = {
+	/* R[7:2], G[7:2], B[7:2] */
+	RCAR_GP_PIN(4, 7),  RCAR_GP_PIN(4, 6),  RCAR_GP_PIN(4, 5),
+	RCAR_GP_PIN(4, 4),  RCAR_GP_PIN(4, 3),  RCAR_GP_PIN(4, 2),
+	RCAR_GP_PIN(4, 15), RCAR_GP_PIN(4, 14), RCAR_GP_PIN(4, 13),
+	RCAR_GP_PIN(4, 12), RCAR_GP_PIN(4, 11), RCAR_GP_PIN(4, 10),
+	RCAR_GP_PIN(4, 23), RCAR_GP_PIN(4, 22), RCAR_GP_PIN(4, 21),
+	RCAR_GP_PIN(4, 20), RCAR_GP_PIN(4, 19), RCAR_GP_PIN(4, 18),
+};
+static const unsigned int du1_rgb666_mux[] = {
+	DU1_DR7_MARK, DU1_DR6_MARK, DU1_DR5_MARK, DU1_DR4_MARK,
+	DU1_DR3_MARK, DU1_DR2_MARK,
+	DU1_DG7_MARK, DU1_DG6_MARK, DU1_DG5_MARK, DU1_DG4_MARK,
+	DU1_DG3_MARK, DU1_DG2_MARK,
+	DU1_DB7_MARK, DU1_DB6_MARK, DU1_DB5_MARK, DU1_DB4_MARK,
+	DU1_DB3_MARK, DU1_DB2_MARK,
+};
+static const unsigned int du1_rgb888_pins[] = {
+	/* R[7:0], G[7:0], B[7:0] */
+	RCAR_GP_PIN(4, 7),  RCAR_GP_PIN(4, 6),  RCAR_GP_PIN(4, 5),
+	RCAR_GP_PIN(4, 4),  RCAR_GP_PIN(4, 3),  RCAR_GP_PIN(4, 2),
+	RCAR_GP_PIN(4, 1),  RCAR_GP_PIN(4, 0),
+	RCAR_GP_PIN(4, 15), RCAR_GP_PIN(4, 14), RCAR_GP_PIN(4, 13),
+	RCAR_GP_PIN(4, 12), RCAR_GP_PIN(4, 11), RCAR_GP_PIN(4, 10),
+	RCAR_GP_PIN(4, 9),  RCAR_GP_PIN(4, 8),
+	RCAR_GP_PIN(4, 23), RCAR_GP_PIN(4, 22), RCAR_GP_PIN(4, 21),
+	RCAR_GP_PIN(4, 20), RCAR_GP_PIN(4, 19), RCAR_GP_PIN(4, 18),
+	RCAR_GP_PIN(4, 17), RCAR_GP_PIN(4, 16),
+};
+static const unsigned int du1_rgb888_mux[] = {
+	DU1_DR7_MARK, DU1_DR6_MARK, DU1_DR5_MARK, DU1_DR4_MARK,
+	DU1_DR3_MARK, DU1_DR2_MARK, DU1_DR1_MARK, DU1_DR0_MARK,
+	DU1_DG7_MARK, DU1_DG6_MARK, DU1_DG5_MARK, DU1_DG4_MARK,
+	DU1_DG3_MARK, DU1_DG2_MARK, DU1_DG1_MARK, DU1_DG0_MARK,
+	DU1_DB7_MARK, DU1_DB6_MARK, DU1_DB5_MARK, DU1_DB4_MARK,
+	DU1_DB3_MARK, DU1_DB2_MARK, DU1_DB1_MARK, DU1_DB0_MARK,
+};
+static const unsigned int du0_clk_out_pins[] = {
+	/* CLKOUT */
+	RCAR_GP_PIN(2, 25),
+};
+static const unsigned int du0_clk_out_mux[] = {
+	DU0_DOTCLKOUT0_MARK
+};
+static const unsigned int du1_clk_out_pins[] = {
+	/* CLKOUT */
+	RCAR_GP_PIN(4, 26),
+};
+static const unsigned int du1_clk_out_mux[] = {
+	DU1_DOTCLKOUT1_MARK
+};
+static const unsigned int du0_sync_pins[] = {
+	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC, EXDISP/EXODDF/EXCDE */
+	RCAR_GP_PIN(2, 29), RCAR_GP_PIN(2, 28), RCAR_GP_PIN(2, 27),
+};
+static const unsigned int du0_sync_mux[] = {
+	DU0_EXODDF_DU0_ODDF_DISP_CDE_MARK,
+	DU0_EXVSYNC_DU0_VSYNC_MARK, DU0_EXHSYNC_DU0_HSYNC_MARK
+};
+static const unsigned int du1_sync_pins[] = {
+	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC, EXDISP/EXODDF/EXCDE */
+	RCAR_GP_PIN(4, 29), RCAR_GP_PIN(4, 28), RCAR_GP_PIN(4, 27),
+};
+static const unsigned int du1_sync_mux[] = {
+	DU1_EXODDF_DU1_ODDF_DISP_CDE_MARK,
+	DU1_EXVSYNC_DU1_VSYNC_MARK, DU1_EXHSYNC_DU1_HSYNC_MARK
+};
+static const unsigned int du0_cde_disp_pins[] = {
+	/* CDE DISP */
+	RCAR_GP_PIN(2, 31), RCAR_GP_PIN(2, 30),
+};
+static const unsigned int du0_cde_disp_mux[] = {
+	DU0_CDE_MARK, DU0_DISP_MARK
+};
+static const unsigned int du1_cde_disp_pins[] = {
+	/* CDE DISP */
+	RCAR_GP_PIN(4, 31), RCAR_GP_PIN(4, 30),
+};
+static const unsigned int du1_cde_disp_mux[] = {
+	DU1_CDE_MARK, DU1_DISP_MARK
+};
+static const unsigned int du0_clk_in_pins[] = {
+	/* CLKIN */
+	RCAR_GP_PIN(2, 24),
+};
+static const unsigned int du0_clk_in_mux[] = {
+	DU0_DOTCLKIN_MARK
+};
+static const unsigned int du1_clk_in_pins[] = {
+	/* CLKIN */
+	RCAR_GP_PIN(4, 24),
+};
+static const unsigned int du1_clk_in_mux[] = {
+	DU1_DOTCLKIN_MARK
+};
+
 /* - ETH -------------------------------------------------------------------- */
 static const unsigned int eth_link_pins[] = {
 	/* LINK */

@@ -2543,6 +2677,18 @@ static const unsigned int usb1_mux[] = {
 };
 
 static const struct sh_pfc_pin_group pinmux_groups[] = {
+	SH_PFC_PIN_GROUP(du0_rgb666),
+	SH_PFC_PIN_GROUP(du0_rgb888),
+	SH_PFC_PIN_GROUP(du1_rgb666),
+	SH_PFC_PIN_GROUP(du1_rgb888),
+	SH_PFC_PIN_GROUP(du0_clk_out),
+	SH_PFC_PIN_GROUP(du1_clk_out),
+	SH_PFC_PIN_GROUP(du0_sync),
+	SH_PFC_PIN_GROUP(du1_sync),
+	SH_PFC_PIN_GROUP(du0_cde_disp),
+	SH_PFC_PIN_GROUP(du1_cde_disp),
+	SH_PFC_PIN_GROUP(du0_clk_in),
+	SH_PFC_PIN_GROUP(du1_clk_in),
 	SH_PFC_PIN_GROUP(eth_link),
 	SH_PFC_PIN_GROUP(eth_magic),
 	SH_PFC_PIN_GROUP(eth_mdio),

@@ -2714,6 +2860,27 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(usb1),
 };
 
+static const char * const du_groups[] = {
+	"du0_rgb666",
+	"du0_rgb888",
+	"du1_rgb666",
+	"du1_rgb888",
+	"du0_clk_out",
+	"du1_clk_out",
+	"du0_sync",
+	"du1_sync",
+	"du0_cde_disp",
+	"du1_cde_disp",
+};
+
+static const char * const du0_groups[] = {
+	"du0_clk_in",
+};
+
+static const char * const du1_groups[] = {
+	"du1_clk_in",
+};
+
 static const char * const eth_groups[] = {
 	"eth_link",
 	"eth_magic",

@@ -2985,6 +3152,9 @@ static const char * const usb1_groups[] = {
 };
 
 static const struct sh_pfc_function pinmux_functions[] = {
+	SH_PFC_FUNCTION(du),
+	SH_PFC_FUNCTION(du0),
+	SH_PFC_FUNCTION(du1),
 	SH_PFC_FUNCTION(eth),
 	SH_PFC_FUNCTION(hscif0),
 	SH_PFC_FUNCTION(hscif1),
#3 lore raw

Re: [PATCH 01/04] pinctrl: sh-pfc: r8a7794: Add DU pin groups

Hi Linus,

Quoted 9 lines · Linus Walleij
Quoted 6 lines

From: Koji Matsuoka <[email protected]>

r8a7794 PFC DU support from the R-Car Gen2 v1.9.4 BSP

Signed-off-by Koji Matsuoka <[email protected]>
Signed-off-by Magnus Damm <[email protected]>

Acked-by for all from my side, Geert, I expect that you queue these with the rest of the SH PFC stuff.

Thank you.

I will postpone until HDMI has been tested on ALT, cfr. the discussion in response to the cover letter.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

author
#4 lore raw

PATCH 2/4 pinctrl: sh-pfc: r8a7794: Separate DU CDE and DISP

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Separate DU CDE and DISP signals to let the r8a7794 ALT board that lacks CDE handle the DISP signal by itself.

The groups "du0_cde_disp" and "du1_cde_disp" are replaced by "du0_cde", "du1_cde", "du0_disp" and "du1_disp".

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/pinctrl/sh-pfc/pfc-r8a7794.c |   50 +++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 16 deletions(-)
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +34 -16
@@ -1616,19 +1616,33 @@ static const unsigned int du1_sync_mux[]
 	DU1_EXODDF_DU1_ODDF_DISP_CDE_MARK,
 	DU1_EXVSYNC_DU1_VSYNC_MARK, DU1_EXHSYNC_DU1_HSYNC_MARK
 };
-static const unsigned int du0_cde_disp_pins[] = {
-	/* CDE DISP */
-	RCAR_GP_PIN(2, 31), RCAR_GP_PIN(2, 30),
-};
-static const unsigned int du0_cde_disp_mux[] = {
-	DU0_CDE_MARK, DU0_DISP_MARK
-};
-static const unsigned int du1_cde_disp_pins[] = {
-	/* CDE DISP */
-	RCAR_GP_PIN(4, 31), RCAR_GP_PIN(4, 30),
+static const unsigned int du0_cde_pins[] = {
+	/* CDE */
+	RCAR_GP_PIN(2, 31),
 };
-static const unsigned int du1_cde_disp_mux[] = {
-	DU1_CDE_MARK, DU1_DISP_MARK
+static const unsigned int du0_cde_mux[] = {
+	DU0_CDE_MARK,
+};
+static const unsigned int du1_cde_pins[] = {
+	/* CDE */
+	RCAR_GP_PIN(4, 31),
+};
+static const unsigned int du1_cde_mux[] = {
+	DU1_CDE_MARK
+};
+static const unsigned int du0_disp_pins[] = {
+	/* DISP */
+	RCAR_GP_PIN(2, 30),
+};
+static const unsigned int du0_disp_mux[] = {
+	DU0_DISP_MARK
+};
+static const unsigned int du1_disp_pins[] = {
+	/* DISP */
+	RCAR_GP_PIN(4, 30),
+};
+static const unsigned int du1_disp_mux[] = {
+	DU1_DISP_MARK
 };
 static const unsigned int du0_clk_in_pins[] = {
 	/* CLKIN */

@@ -2932,8 +2946,10 @@ static const struct sh_pfc_pin_group pin
 	SH_PFC_PIN_GROUP(du1_clk_out),
 	SH_PFC_PIN_GROUP(du0_sync),
 	SH_PFC_PIN_GROUP(du1_sync),
-	SH_PFC_PIN_GROUP(du0_cde_disp),
-	SH_PFC_PIN_GROUP(du1_cde_disp),
+	SH_PFC_PIN_GROUP(du0_cde),
+	SH_PFC_PIN_GROUP(du1_cde),
+	SH_PFC_PIN_GROUP(du0_disp),
+	SH_PFC_PIN_GROUP(du1_disp),
 	SH_PFC_PIN_GROUP(du0_clk_in),
 	SH_PFC_PIN_GROUP(du1_clk_in),
 	SH_PFC_PIN_GROUP(eth_link),

@@ -3122,8 +3138,10 @@ static const char * const du_groups[] =
 	"du1_clk_out",
 	"du0_sync",
 	"du1_sync",
-	"du0_cde_disp",
-	"du1_cde_disp",
+	"du0_cde",
+	"du1_cde",
+	"du0_disp",
+	"du1_disp",
 };
 
 static const char * const du0_groups[] = {
author
#5 lore raw

PATCH 3/4 pinctrl: sh-pfc: r8a7794: Add missing dot clock signals

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Add missing r8a7794 DU dot clock output signals and in particular the DU1_DOTCLKOUT0 signal on GP4_25 which is needed by DU1 on the r8a7794 ALT board.

The groups "du0_clk_out" and "du1_clk_out" are replaced by "du0_dotclkout0", "du0_dotclkout1", "du1_dotclkout0" and "du1_dotclkout1".

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/pinctrl/sh-pfc/pfc-r8a7794.c |   34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +26 -8
@@ -1586,18 +1586,32 @@ static const unsigned int du1_rgb888_mux
 	DU1_DB7_MARK, DU1_DB6_MARK, DU1_DB5_MARK, DU1_DB4_MARK,
 	DU1_DB3_MARK, DU1_DB2_MARK, DU1_DB1_MARK, DU1_DB0_MARK,
 };
-static const unsigned int du0_clk_out_pins[] = {
+static const unsigned int du0_clk0_out_pins[] = {
 	/* CLKOUT */
 	RCAR_GP_PIN(2, 25),
 };
-static const unsigned int du0_clk_out_mux[] = {
+static const unsigned int du0_clk0_out_mux[] = {
 	DU0_DOTCLKOUT0_MARK
 };
-static const unsigned int du1_clk_out_pins[] = {
+static const unsigned int du0_clk1_out_pins[] = {
+	/* CLKOUT */
+	RCAR_GP_PIN(2, 26),
+};
+static const unsigned int du0_clk1_out_mux[] = {
+	DU0_DOTCLKOUT1_MARK
+};
+static const unsigned int du1_clk0_out_pins[] = {
+	/* CLKOUT */
+	RCAR_GP_PIN(4, 25),
+};
+static const unsigned int du1_clk0_out_mux[] = {
+	DU1_DOTCLKOUT0_MARK
+};
+static const unsigned int du1_clk1_out_pins[] = {
 	/* CLKOUT */
 	RCAR_GP_PIN(4, 26),
 };
-static const unsigned int du1_clk_out_mux[] = {
+static const unsigned int du1_clk1_out_mux[] = {
 	DU1_DOTCLKOUT1_MARK
 };
 static const unsigned int du0_sync_pins[] = {

@@ -2942,8 +2956,10 @@ static const struct sh_pfc_pin_group pin
 	SH_PFC_PIN_GROUP(du0_rgb888),
 	SH_PFC_PIN_GROUP(du1_rgb666),
 	SH_PFC_PIN_GROUP(du1_rgb888),
-	SH_PFC_PIN_GROUP(du0_clk_out),
-	SH_PFC_PIN_GROUP(du1_clk_out),
+	SH_PFC_PIN_GROUP(du0_clk0_out),
+	SH_PFC_PIN_GROUP(du0_clk1_out),
+	SH_PFC_PIN_GROUP(du1_clk0_out),
+	SH_PFC_PIN_GROUP(du1_clk1_out),
 	SH_PFC_PIN_GROUP(du0_sync),
 	SH_PFC_PIN_GROUP(du1_sync),
 	SH_PFC_PIN_GROUP(du0_cde),

@@ -3134,8 +3150,10 @@ static const char * const du_groups[] =
 	"du0_rgb888",
 	"du1_rgb666",
 	"du1_rgb888",
-	"du0_clk_out",
-	"du1_clk_out",
+	"du0_dotclkout0",
+	"du0_dotclkout1",
+	"du1_dotclkout0",
+	"du1_dotclkout1",
 	"du0_sync",
 	"du1_sync",
 	"du0_cde",
#6 lore raw

Re: [PATCH 03/04] pinctrl: sh-pfc: r8a7794: Add missing dot clock signals

Hello.

Quoted 18 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Add missing r8a7794 DU dot clock output signals and in particular the DU1_DOTCLKOUT0 signal on GP4_25 which is needed by DU1 on the r8a7794 ALT board.

The groups "du0_clk_out" and "du1_clk_out" are replaced by "du0_dotclkout0", "du0_dotclkout1", "du1_dotclkout0" and "du1_dotclkout1".

Signed-off-by Magnus Damm <[email protected]>

---

  drivers/pinctrl/sh-pfc/pfc-r8a7794.c |   34 ++++++++++++++++++++++++++--------
  1 file changed, 26 insertions(+), 8 deletions(-)
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +0 -0

[...]

drivers/pinctrl/sh-pfc/pfc-r8a7794.c static const struct sh_pfc_pin_group pin · line 2962

Quoted 10 lines
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +4 -2
@@ -2942,8 +2956,10 @@ static const struct sh_pfc_pin_group pin
  	SH_PFC_PIN_GROUP(du0_rgb888),
  	SH_PFC_PIN_GROUP(du1_rgb666),
  	SH_PFC_PIN_GROUP(du1_rgb888),
-	SH_PFC_PIN_GROUP(du0_clk_out),
-	SH_PFC_PIN_GROUP(du1_clk_out),
+	SH_PFC_PIN_GROUP(du0_clk0_out),
+	SH_PFC_PIN_GROUP(du0_clk1_out),
+	SH_PFC_PIN_GROUP(du1_clk0_out),
+	SH_PFC_PIN_GROUP(du1_clk1_out),

Hm, this kinda contradicts your chngnelog, no?

Quoted 6 lines
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +0 -0
  	SH_PFC_PIN_GROUP(du0_sync),
  	SH_PFC_PIN_GROUP(du1_sync),
  	SH_PFC_PIN_GROUP(du0_cde),

@@ -3134,8 +3150,10 @@ static const char * const du_groups[] =
  	"du0_rgb888",
  	"du1_rgb666",

drivers/pinctrl/sh-pfc/pfc-r8a7794.c static const char * const du_groups[] = · line 3159

Quoted 10 lines
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +4 -2
  	"du1_rgb888",
-	"du0_clk_out",
-	"du1_clk_out",
+	"du0_dotclkout0",
+	"du0_dotclkout1",
+	"du1_dotclkout0",
+	"du1_dotclkout1",
  	"du0_sync",
  	"du1_sync",
  	"du0_cde",

I'm getting the following on the SILK board with this patch:

[drm] Initialized drm 1.1.0 20060810
sh-pfc e6060000.pin-controller: does not have pin group du1_dotclkout0 sh-pfc e6060000.pin-controller: invalid group du1_dotclkout0 in map table [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [drm] No driver support for vblank timestamp query.
rcar-du feb00000.display: fb0: frame buffer device
[drm] Initialized rcar-du 1.0.0 20130110 on minor 0

MBR, Sergei

author
#7 lore raw

PATCH 4/4 pinctrl: sh-pfc: r8a7794: Break out ODDF from sync

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Break out the r8a7794 DU ODDF (EXDISP/EXODDF/EXDE) signal from the sync group into a separate unit. This makes the sync group fit with the r8a7794 ALT board that only uses HSYNC and VSYNC signals.

This makes the r8a7794 PFC similar to the existing r8a7791 PFC DU implementation that also groups together HSYNC and VSYNC without ODDF.

The groups "du0_oddf" and "du1_oddf" are added and the "du0_sync" and "du1_sync" groups are modified.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/pinctrl/sh-pfc/pfc-r8a7794.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)
drivers/pinctrl/sh-pfc/pfc-r8a7794.c +22 -6
@@ -1615,21 +1615,33 @@ static const unsigned int du1_clk1_out_m
 	DU1_DOTCLKOUT1_MARK
 };
 static const unsigned int du0_sync_pins[] = {
-	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC, EXDISP/EXODDF/EXCDE */
-	RCAR_GP_PIN(2, 29), RCAR_GP_PIN(2, 28), RCAR_GP_PIN(2, 27),
+	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC */
+	RCAR_GP_PIN(2, 28), RCAR_GP_PIN(2, 27),
 };
 static const unsigned int du0_sync_mux[] = {
-	DU0_EXODDF_DU0_ODDF_DISP_CDE_MARK,
 	DU0_EXVSYNC_DU0_VSYNC_MARK, DU0_EXHSYNC_DU0_HSYNC_MARK
 };
 static const unsigned int du1_sync_pins[] = {
-	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC, EXDISP/EXODDF/EXCDE */
-	RCAR_GP_PIN(4, 29), RCAR_GP_PIN(4, 28), RCAR_GP_PIN(4, 27),
+	/* EXVSYNC/VSYNC, EXHSYNC/HSYNC */
+	RCAR_GP_PIN(4, 28), RCAR_GP_PIN(4, 27),
 };
 static const unsigned int du1_sync_mux[] = {
-	DU1_EXODDF_DU1_ODDF_DISP_CDE_MARK,
 	DU1_EXVSYNC_DU1_VSYNC_MARK, DU1_EXHSYNC_DU1_HSYNC_MARK
 };
+static const unsigned int du0_oddf_pins[] = {
+	/* EXDISP/EXODDF/EXCDE */
+	RCAR_GP_PIN(2, 29),
+};
+static const unsigned int du0_oddf_mux[] = {
+	DU0_EXODDF_DU0_ODDF_DISP_CDE_MARK,
+};
+static const unsigned int du1_oddf_pins[] = {
+	/* EXDISP/EXODDF/EXCDE */
+	RCAR_GP_PIN(4, 29),
+};
+static const unsigned int du1_oddf_mux[] = {
+	DU1_EXODDF_DU1_ODDF_DISP_CDE_MARK,
+};
 static const unsigned int du0_cde_pins[] = {
 	/* CDE */
 	RCAR_GP_PIN(2, 31),

@@ -2962,6 +2974,8 @@ static const struct sh_pfc_pin_group pin
 	SH_PFC_PIN_GROUP(du1_clk1_out),
 	SH_PFC_PIN_GROUP(du0_sync),
 	SH_PFC_PIN_GROUP(du1_sync),
+	SH_PFC_PIN_GROUP(du0_oddf),
+	SH_PFC_PIN_GROUP(du1_oddf),
 	SH_PFC_PIN_GROUP(du0_cde),
 	SH_PFC_PIN_GROUP(du1_cde),
 	SH_PFC_PIN_GROUP(du0_disp),

@@ -3156,6 +3170,8 @@ static const char * const du_groups[] =
 	"du1_dotclkout1",
 	"du0_sync",
 	"du1_sync",
+	"du0_oddf",
+	"du1_oddf",
 	"du0_cde",
 	"du1_cde",
 	"du0_disp",
#8 lore raw
In reply to Magnus Damm

Hi Magnus,

Thank you for the patch.

Quoted 16 lines · On Tuesday 17 November 2015 12:18:32 Magnus Damm wrote:

pinctrl: sh-pfc: r8a7794: DU support

[PATCH 01/04] pinctrl: sh-pfc: r8a7794: Add DU pin groups [PATCH 02/04] pinctrl: sh-pfc: r8a7794: Separate DU CDE and DISP [PATCH 03/04] pinctrl: sh-pfc: r8a7794: Add missing dot clock signals [PATCH 04/04] pinctrl: sh-pfc: r8a7794: Break out ODDF from sync

These patches take the r8a7794 PFC DU support code from the BSP and reworks it to fit the r8a7794 ALT board. Tested with the ALT VGA port - by default PFC is not used however enabling PFC using an incremental (yet to be posted) patch works well.

It is worth noting that patch 2-4 modifies the pin groups. This means that the upstream DT ABI for PFC DU will differ compared to the unreviewed BSP code.

Quoted 6 lines

In general it is not considered good practice to change the pin groups and break compatibility since they are part of the DT ABI.

For this particular case upstream never have had PFC DU support for r8a7794, so treating the BSP bindings as experimental and migrate away seems reasonable.

If we start considering DT bindings that never went upstream as stable we'll have a big problem. I mean even bigger than the upstream DT bindings stability problem :-)

Quoted 1 line
Signed-off-by Magnus Damm <[email protected]>

Wouldn't it make sense to merge the 4 patches together ?

Quoted 6 lines

---

Written against v4.4-rc1

 drivers/pinctrl/sh-pfc/pfc-r8a7794.c |  282 +++++++++++++++++++++++++++----
 1 file changed, 252 insertions(+), 30 deletions(-)
Signature · 3 lines
Regards,

Laurent Pinchart
author
#9 lore raw

Hi Laurent,

Quoted 24 lines · Laurent Pinchart

Hi Magnus,

Thank you for the patch.

Quoted 19 lines · On Tuesday 17 November 2015 12:18:32 Magnus Damm wrote:

pinctrl: sh-pfc: r8a7794: DU support

[PATCH 01/04] pinctrl: sh-pfc: r8a7794: Add DU pin groups [PATCH 02/04] pinctrl: sh-pfc: r8a7794: Separate DU CDE and DISP [PATCH 03/04] pinctrl: sh-pfc: r8a7794: Add missing dot clock signals [PATCH 04/04] pinctrl: sh-pfc: r8a7794: Break out ODDF from sync

These patches take the r8a7794 PFC DU support code from the BSP and reworks it to fit the r8a7794 ALT board. Tested with the ALT VGA port - by default PFC is not used however enabling PFC using an incremental (yet to be posted) patch works well.

It is worth noting that patch 2-4 modifies the pin groups. This means that the upstream DT ABI for PFC DU will differ compared to the unreviewed BSP code.

In general it is not considered good practice to change the pin groups and break compatibility since they are part of the DT ABI.

Quoted 7 lines
Quoted 3 lines

For this particular case upstream never have had PFC DU support for r8a7794, so treating the BSP bindings as experimental and migrate away seems reasonable.

If we start considering DT bindings that never went upstream as stable we'll have a big problem. I mean even bigger than the upstream DT bindings stability problem :-)

I'm not saying that local DT hacks should be considered stable, more that it as usual makes sense to follow upstream first with proper DT review process early on.

Quoted 3 lines
Quoted 1 line
Signed-off-by Magnus Damm <[email protected]>

Wouldn't it make sense to merge the 4 patches together ?

Yeah, I guess so. My feeling is also that it would be good to verify HDMI on ALT before commiting to DT bindings. Right now only one DU channel is tested.

Cheers,

/ magnus

#10 lore raw

Hi Magnus,

Quoted 33 lines · On Friday 20 November 2015 16:16:15 Magnus Damm wrote:
Quoted 20 lines · Laurent Pinchart
Quoted 19 lines · On Tuesday 17 November 2015 12:18:32 Magnus Damm wrote:

pinctrl: sh-pfc: r8a7794: DU support

[PATCH 01/04] pinctrl: sh-pfc: r8a7794: Add DU pin groups [PATCH 02/04] pinctrl: sh-pfc: r8a7794: Separate DU CDE and DISP [PATCH 03/04] pinctrl: sh-pfc: r8a7794: Add missing dot clock signals [PATCH 04/04] pinctrl: sh-pfc: r8a7794: Break out ODDF from sync

These patches take the r8a7794 PFC DU support code from the BSP and reworks it to fit the r8a7794 ALT board. Tested with the ALT VGA port - by default PFC is not used however enabling PFC using an incremental (yet to be posted) patch works well.

It is worth noting that patch 2-4 modifies the pin groups. This means that the upstream DT ABI for PFC DU will differ compared to the unreviewed BSP code.

In general it is not considered good practice to change the pin groups and break compatibility since they are part of the DT ABI.

Quoted 7 lines
Quoted 3 lines

For this particular case upstream never have had PFC DU support for r8a7794, so treating the BSP bindings as experimental and migrate away seems reasonable.

If we start considering DT bindings that never went upstream as stable we'll have a big problem. I mean even bigger than the upstream DT bindings stability problem :-)

I'm not saying that local DT hacks should be considered stable, more that it as usual makes sense to follow upstream first with proper DT review process early on.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Magnus Damm <[email protected]>

Wouldn't it make sense to merge the 4 patches together ?

Yeah, I guess so. My feeling is also that it would be good to verify HDMI on ALT before commiting to DT bindings. Right now only one DU channel is tested.

I agree with that.

Signature · 3 lines
Regards,

Laurent Pinchart
#11 lore raw
In reply to Laurent Pinchart
Quoted 24 lines · Laurent Pinchart

Hi Magnus,

Thank you for the patch.

Quoted 19 lines · On Tuesday 17 November 2015 12:18:32 Magnus Damm wrote:

pinctrl: sh-pfc: r8a7794: DU support

[PATCH 01/04] pinctrl: sh-pfc: r8a7794: Add DU pin groups [PATCH 02/04] pinctrl: sh-pfc: r8a7794: Separate DU CDE and DISP [PATCH 03/04] pinctrl: sh-pfc: r8a7794: Add missing dot clock signals [PATCH 04/04] pinctrl: sh-pfc: r8a7794: Break out ODDF from sync

These patches take the r8a7794 PFC DU support code from the BSP and reworks it to fit the r8a7794 ALT board. Tested with the ALT VGA port - by default PFC is not used however enabling PFC using an incremental (yet to be posted) patch works well.

It is worth noting that patch 2-4 modifies the pin groups. This means that the upstream DT ABI for PFC DU will differ compared to the unreviewed BSP code.

In general it is not considered good practice to change the pin groups and break compatibility since they are part of the DT ABI.

Quoted 7 lines
Quoted 3 lines

For this particular case upstream never have had PFC DU support for r8a7794, so treating the BSP bindings as experimental and migrate away seems reasonable.

If we start considering DT bindings that never went upstream as stable we'll have a big problem. I mean even bigger than the upstream DT bindings stability problem :-)

FWIW, I agree.

Quoted 16 lines
Quoted 1 line
Signed-off-by Magnus Damm <[email protected]>

Wouldn't it make sense to merge the 4 patches together ?

Quoted 6 lines

---

Written against v4.4-rc1

 drivers/pinctrl/sh-pfc/pfc-r8a7794.c |  282 +++++++++++++++++++++++++++----
 1 file changed, 252 insertions(+), 30 deletions(-)
Signature · 3 lines
Regards,

Laurent Pinchart