[RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Jacopo Mondi]

Hello renesas list,
   this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
Ebisu board.

It's an RFT, as I don't have an Ebisu to test with :(

The series adds supports for the following items:

- PFC: add VIN groups and functions
- R-Car VIN and R-Car CSI-2: add support for R8A77990
- R8A77990: Add I2C, VIN and CSI-2 nodes
- Ebisu: describe HDMI and CVBS inputs

Each patch, when relevant reports difference between the upported BSP patch
and the proposed one.

I know Laurent should receive an Ebisu sooner or later, maybe we can sync for
testing :)

Thanks
   j

PS: the list of upported patches will be sent separately.

Jacopo Mondi (5):
  media: dt-bindings: media: rcar-vin: Add R8A77990 support
  media: rcar-vin: Add support for R-Car R8A77990
  dt-bindings: media: rcar-csi2: Add R8A77990
  media: rcar-csi2: Add R8A77990 support
  arm64: dts: renesas: ebisu: Add HDMI and CVBS input

Koji Matsuoka (1):
  arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

Takeshi Kihara (2):
  pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
  arm64: dts: r8a77990: Add I2C device nodes

 .../devicetree/bindings/media/rcar_vin.txt         |   1 +
 .../bindings/media/renesas,rcar-csi2.txt           |   1 +
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 ++++
 arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++
 drivers/media/platform/rcar-vin/rcar-core.c        |  20 +
 drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 504 +++++++++++++++++++++
 7 files changed, 823 insertions(+)

--
2.7.4

[RFT 1/8] media: dt-bindings: media: rcar-vin: Add R8A77990 support

[Jacopo Mondi]

Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
rcar-vin driver.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index 2f42005..dfd6058 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -23,6 +23,7 @@ on Gen3 platforms to a CSI-2 receiver.
    - "renesas,vin-r8a7796" for the R8A7796 device
    - "renesas,vin-r8a77965" for the R8A77965 device
    - "renesas,vin-r8a77970" for the R8A77970 device
+   - "renesas,vin-r8a77990" for the R8A77990 device
    - "renesas,vin-r8a77995" for the R8A77995 device
    - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
      device.
--
2.7.4

Re: [RFT 1/8] media: dt-bindings: media: rcar-vin: Add R8A77990 support

[Rob Herring]

On Mon, Aug 20, 2018 at 12:16:35PM +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
>  1 file changed, 1 insertion(+)

Why the inconsistent subjects for patches 1 and 3?

Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [RFT 1/8] media: dt-bindings: media: rcar-vin: Add R8A77990 support

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

Hi Rob,

On Mon, Aug 20, 2018 at 05:39:47PM -0500, Rob Herring wrote:
> On Mon, Aug 20, 2018 at 12:16:35PM +0200, Jacopo Mondi wrote:
> > Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
> > rcar-vin driver.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
> >  1 file changed, 1 insertion(+)
>
> Why the inconsistent subjects for patches 1 and 3?
>

Beacause I initially used "dt-bindings: media" for both then later realized all
other patches were "media: dt-bindings" and just fixed the first one :/

Sorry for being sloppy, I'll fix in v2.

Thanks
   j

> Otherwise,
>
> Reviewed-by: Rob Herring <robh@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[RFT 2/8] media: rcar-vin: Add support for R-Car R8A77990

[Jacopo Mondi]

Add R-Car E3 R8A77990 SoC to the rcar-vin supported ones.
Based on the experimental patch from Magnus Damm.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 8843367..907985d 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1089,6 +1089,22 @@ static const struct rvin_info rcar_info_r8a77970 = {
 	.routes = rcar_info_r8a77970_routes,
 };

+static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
+	{ .csi = RVIN_CSI40, .channel = 0, .vin = 4, .mask = BIT(0) | BIT(3) },
+	{ .csi = RVIN_CSI40, .channel = 0, .vin = 5, .mask = BIT(2) },
+	{ .csi = RVIN_CSI40, .channel = 1, .vin = 4, .mask = BIT(2) },
+	{ .csi = RVIN_CSI40, .channel = 1, .vin = 5, .mask = BIT(1) | BIT(3) },
+	{ /* Sentinel */ }
+};
+
+static const struct rvin_info rcar_info_r8a77990 = {
+	.model = RCAR_GEN3,
+	.use_mc = true,
+	.max_width = 4096,
+	.max_height = 4096,
+	.routes = rcar_info_r8a77990_routes,
+};
+
 static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
 	{ /* Sentinel */ }
 };
@@ -1147,6 +1163,10 @@ static const struct of_device_id rvin_of_id_table[] = {
 		.data = &rcar_info_r8a77970,
 	},
 	{
+		.compatible = "renesas,vin-r8a77990",
+		.data = &rcar_info_r8a77990,
+	},
+	{
 		.compatible = "renesas,vin-r8a77995",
 		.data = &rcar_info_r8a77995,
 	},
--
2.7.4

[RFT 3/8] dt-bindings: media: rcar-csi2: Add R8A77990

[Jacopo Mondi]

Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
index 2d385b6..2824489 100644
--- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
+++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
@@ -12,6 +12,7 @@ Mandatory properties
    - "renesas,r8a7796-csi2" for the R8A7796 device.
    - "renesas,r8a77965-csi2" for the R8A77965 device.
    - "renesas,r8a77970-csi2" for the R8A77970 device.
+   - "renesas,r8a77990-csi2" for the R8A77990 device.

  - reg: the register base and size for the device registers
  - interrupts: the interrupt for the device
--
2.7.4

Re: [RFT 3/8] dt-bindings: media: rcar-csi2: Add R8A77990

[Rob Herring]

On Mon, 20 Aug 2018 12:16:37 +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [RFT 3/8] dt-bindings: media: rcar-csi2: Add R8A77990

[Rob Herring]

On Mon, 20 Aug 2018 12:16:37 +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[RFT 4/8] media: rcar-csi2: Add R8A77990 support

[Jacopo Mondi]

Add support for R-Car E3 R8A77965 to R-Car CSI-2 driver.
Based on the experimental patch from Magnus Damm.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
The upported BSP patch
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=59d3ad972acdba8b75a14113efbbca7f9c709345
includes a few more things to handle E3:

- E3 supports a single VC: do not write to VCDT2 register
  I feel there is not harm in doing that, but in case I can add that check
- phtw_testin handling:
  it seems to me phtw_testing is handled for all SoCs in the mainline csi-2
  driver, and it is not necessary to add it specifically for E3. Niklas, could
  you maybe check?

Thanks
   j

---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index dc5ae80..f82b668 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -959,6 +959,11 @@ static const struct rcar_csi2_info rcar_csi2_info_r8a77970 = {
 	.confirm_start = rcsi2_confirm_start_v3m_e3,
 };

+static const struct rcar_csi2_info rcar_csi2_info_r8a77990 = {
+	.init_phtw = rcsi2_init_phtw_v3m_e3,
+	.confirm_start = rcsi2_confirm_start_v3m_e3,
+};
+
 static const struct of_device_id rcar_csi2_of_table[] = {
 	{
 		.compatible = "renesas,r8a7795-csi2",
@@ -976,6 +981,10 @@ static const struct of_device_id rcar_csi2_of_table[] = {
 		.compatible = "renesas,r8a77970-csi2",
 		.data = &rcar_csi2_info_r8a77970,
 	},
+	{
+		.compatible = "renesas,r8a77990-csi2",
+		.data = &rcar_csi2_info_r8a77990,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
--
2.7.4

[RFT 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[Jacopo Mondi]

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

This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 504 ++++++++++++++++++++++++++++++++++
 1 file changed, 504 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
index 5ea63e5..46c0b06 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
@@ -1982,6 +1982,446 @@ static const unsigned int usb30_id_mux[] = {
 	USB3HS0_ID_MARK,
 };
 
+/* - VIN4 ------------------------------------------------------------------- */
+static const unsigned int vin4_data8_a_pins[] = {
+	RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
+	RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
+	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
+};
+
+static const unsigned int vin4_data8_a_mux[] = {
+	VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
+	VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
+	VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
+	VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
+};
+
+static const unsigned int vin4_data10_a_pins[] = {
+	RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
+	RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
+	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+};
+
+static const unsigned int vin4_data10_a_mux[] = {
+	VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
+	VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
+	VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
+	VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+};
+
+static const unsigned int vin4_data12_a_pins[] = {
+	RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
+	RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
+	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+};
+
+static const unsigned int vin4_data12_a_mux[] = {
+	VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
+	VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
+	VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
+	VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+};
+
+static const unsigned int vin4_data16_a_pins[] = {
+	RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
+	RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
+	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+	RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+	RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+};
+
+static const unsigned int vin4_data16_a_mux[] = {
+	VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
+	VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
+	VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
+	VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+	VI4_DATA12_MARK,  VI4_DATA13_MARK,
+	VI4_DATA14_MARK,  VI4_DATA15_MARK,
+};
+
+static const unsigned int vin4_data20_a_pins[] = {
+	RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
+	RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
+	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+	RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+	RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+	RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+};
+
+static const unsigned int vin4_data20_a_mux[] = {
+	VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
+	VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
+	VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
+	VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+	VI4_DATA12_MARK,  VI4_DATA13_MARK,
+	VI4_DATA14_MARK,  VI4_DATA15_MARK,
+	VI4_DATA16_MARK,  VI4_DATA17_MARK,
+	VI4_DATA18_MARK,  VI4_DATA19_MARK,
+};
+
+static const unsigned int vin4_data24_a_pins[] = {
+	RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
+	RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
+	RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
+	RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+	RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+	RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+	RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+	RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
+};
+
+static const unsigned int vin4_data24_a_mux[] = {
+	VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
+	VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
+	VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
+	VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+	VI4_DATA12_MARK,  VI4_DATA13_MARK,
+	VI4_DATA14_MARK,  VI4_DATA15_MARK,
+	VI4_DATA16_MARK,  VI4_DATA17_MARK,
+	VI4_DATA18_MARK,  VI4_DATA19_MARK,
+	VI4_DATA20_MARK,  VI4_DATA21_MARK,
+	VI4_DATA22_MARK,  VI4_DATA23_MARK,
+};
+
+static const unsigned int vin4_data8_b_pins[] = {
+	RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
+	RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+	RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
+	RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+};
+
+static const unsigned int vin4_data8_b_mux[] = {
+	VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
+	VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
+	VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
+	VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
+};
+
+static const unsigned int vin4_data10_b_pins[] = {
+	RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
+	RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+	RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
+	RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+};
+
+static const unsigned int vin4_data10_b_mux[] = {
+	VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
+	VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
+	VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
+	VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+};
+
+static const unsigned int vin4_data12_b_pins[] = {
+	RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
+	RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+	RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
+	RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+};
+
+static const unsigned int vin4_data12_b_mux[] = {
+	VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
+	VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
+	VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
+	VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+};
+
+static const unsigned int vin4_data16_b_pins[] = {
+	RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
+	RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+	RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
+	RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+	RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+	RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+};
+
+static const unsigned int vin4_data16_b_mux[] = {
+	VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
+	VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
+	VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
+	VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+	VI4_DATA12_MARK,  VI4_DATA13_MARK,
+	VI4_DATA14_MARK,  VI4_DATA15_MARK,
+};
+
+static const unsigned int vin4_data20_b_pins[] = {
+	RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
+	RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+	RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
+	RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+	RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+	RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+	RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+};
+
+static const unsigned int vin4_data20_b_mux[] = {
+	VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
+	VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
+	VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
+	VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+	VI4_DATA12_MARK,  VI4_DATA13_MARK,
+	VI4_DATA14_MARK,  VI4_DATA15_MARK,
+	VI4_DATA16_MARK,  VI4_DATA17_MARK,
+	VI4_DATA18_MARK,  VI4_DATA19_MARK,
+};
+
+static const unsigned int vin4_data24_b_pins[] = {
+	RCAR_GP_PIN(1, 8),  RCAR_GP_PIN(1, 11),
+	RCAR_GP_PIN(1, 21), RCAR_GP_PIN(1, 22),
+	RCAR_GP_PIN(0, 5),  RCAR_GP_PIN(0, 6),
+	RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+	RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+	RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+	RCAR_GP_PIN(1, 9),  RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 15),
+	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+	RCAR_GP_PIN(1, 19), RCAR_GP_PIN(0, 1),
+};
+
+static const unsigned int vin4_data24_b_mux[] = {
+	VI4_DATA0_B_MARK, VI4_DATA1_B_MARK,
+	VI4_DATA2_B_MARK, VI4_DATA3_B_MARK,
+	VI4_DATA4_B_MARK, VI4_DATA5_B_MARK,
+	VI4_DATA6_B_MARK, VI4_DATA7_B_MARK,
+	VI4_DATA8_MARK,   VI4_DATA9_MARK,
+	VI4_DATA10_MARK,  VI4_DATA11_MARK,
+	VI4_DATA12_MARK,  VI4_DATA13_MARK,
+	VI4_DATA14_MARK,  VI4_DATA15_MARK,
+	VI4_DATA16_MARK,  VI4_DATA17_MARK,
+	VI4_DATA18_MARK,  VI4_DATA19_MARK,
+	VI4_DATA20_MARK,  VI4_DATA21_MARK,
+	VI4_DATA22_MARK,  VI4_DATA23_MARK,
+};
+
+static const unsigned int vin4_data8_sft8_pins[] = {
+	RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
+	RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
+	RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
+	RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
+};
+
+static const unsigned int vin4_data8_sft8_mux[] = {
+	VI4_DATA8_MARK,  VI4_DATA9_MARK,
+	VI4_DATA10_MARK, VI4_DATA11_MARK,
+	VI4_DATA12_MARK, VI4_DATA13_MARK,
+	VI4_DATA14_MARK, VI4_DATA15_MARK,
+};
+
+static const unsigned int vin4_sync_pins[] = {
+	/* HSYNC, VSYNC */
+	RCAR_GP_PIN(2, 25), RCAR_GP_PIN(2, 24),
+};
+
+static const unsigned int vin4_sync_mux[] = {
+	VI4_HSYNC_N_MARK, VI4_VSYNC_N_MARK,
+};
+
+static const unsigned int vin4_field_pins[] = {
+	RCAR_GP_PIN(2, 23),
+};
+
+static const unsigned int vin4_field_mux[] = {
+	VI4_FIELD_MARK,
+};
+
+static const unsigned int vin4_clkenb_pins[] = {
+	RCAR_GP_PIN(1, 2),
+};
+
+static const unsigned int vin4_clkenb_mux[] = {
+	VI4_CLKENB_MARK,
+};
+
+static const unsigned int vin4_clk_pins[] = {
+	RCAR_GP_PIN(2, 22),
+};
+
+static const unsigned int vin4_clk_mux[] = {
+	VI4_CLK_MARK,
+};
+
+/* - VIN5 ------------------------------------------------------------------- */
+static const unsigned int vin5_data8_a_pins[] = {
+	RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
+	RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+};
+
+static const unsigned int vin5_data8_a_mux[] = {
+	VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
+	VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
+	VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
+	VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
+};
+
+static const unsigned int vin5_data8_sft8_a_pins[] = {
+	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
+	RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
+	RCAR_GP_PIN(0, 8),  RCAR_GP_PIN(0, 10),
+	RCAR_GP_PIN(0, 2),  RCAR_GP_PIN(0, 3),
+};
+
+static const unsigned int vin5_data8_sft8_a_mux[] = {
+	VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
+	VI5_DATA10_A_MARK, VI5_DATA11_A_MARK,
+	VI5_DATA12_A_MARK, VI5_DATA13_A_MARK,
+	VI5_DATA14_A_MARK, VI5_DATA15_A_MARK,
+};
+
+static const unsigned int vin5_data10_a_pins[] = {
+	RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
+	RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
+};
+
+static const unsigned int vin5_data10_a_mux[] = {
+	VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
+	VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
+	VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
+	VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
+	VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
+};
+
+static const unsigned int vin5_data12_a_pins[] = {
+	RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
+	RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
+	RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
+};
+
+static const unsigned int vin5_data12_a_mux[] = {
+	VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
+	VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
+	VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
+	VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
+	VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
+	VI5_DATA10_A_MARK, VI5_DATA11_A_MARK,
+};
+
+static const unsigned int vin5_data16_a_pins[] = {
+	RCAR_GP_PIN(1, 1),  RCAR_GP_PIN(1, 2),
+	RCAR_GP_PIN(1, 19), RCAR_GP_PIN(1, 12),
+	RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 16),
+	RCAR_GP_PIN(1, 17), RCAR_GP_PIN(1, 18),
+	RCAR_GP_PIN(0, 12), RCAR_GP_PIN(0, 13),
+	RCAR_GP_PIN(0, 9),  RCAR_GP_PIN(0, 11),
+	RCAR_GP_PIN(0, 8),  RCAR_GP_PIN(0, 10),
+	RCAR_GP_PIN(0, 2),  RCAR_GP_PIN(0, 3),
+};
+
+static const unsigned int vin5_data16_a_mux[] = {
+	VI5_DATA0_A_MARK,  VI5_DATA1_A_MARK,
+	VI5_DATA2_A_MARK,  VI5_DATA3_A_MARK,
+	VI5_DATA4_A_MARK,  VI5_DATA5_A_MARK,
+	VI5_DATA6_A_MARK,  VI5_DATA7_A_MARK,
+	VI5_DATA8_A_MARK,  VI5_DATA9_A_MARK,
+	VI5_DATA10_A_MARK, VI5_DATA11_A_MARK,
+	VI5_DATA12_A_MARK, VI5_DATA13_A_MARK,
+	VI5_DATA14_A_MARK, VI5_DATA15_A_MARK,
+};
+
+static const unsigned int vin5_data8_b_pins[] = {
+	RCAR_GP_PIN(2, 23), RCAR_GP_PIN(0, 4),
+	RCAR_GP_PIN(0, 7),  RCAR_GP_PIN(0, 12),
+	RCAR_GP_PIN(0, 13), RCAR_GP_PIN(0, 14),
+	RCAR_GP_PIN(0, 16), RCAR_GP_PIN(0, 17),
+};
+
+static const unsigned int vin5_data8_b_mux[] = {
+	VI5_DATA0_B_MARK,  VI5_DATA1_B_MARK,
+	VI5_DATA2_B_MARK,  VI5_DATA3_B_MARK,
+	VI5_DATA4_B_MARK,  VI5_DATA5_B_MARK,
+	VI5_DATA6_B_MARK,  VI5_DATA7_B_MARK,
+};
+
+static const unsigned int vin5_sync_a_pins[] = {
+	/* HSYNC_N, VSYNC_N */
+	RCAR_GP_PIN(1, 8), RCAR_GP_PIN(1, 9),
+};
+
+static const unsigned int vin5_sync_a_mux[] = {
+	VI5_HSYNC_N_A_MARK, VI5_VSYNC_N_A_MARK,
+};
+
+static const unsigned int vin5_field_a_pins[] = {
+	RCAR_GP_PIN(1, 10),
+};
+
+static const unsigned int vin5_field_a_mux[] = {
+	VI5_FIELD_A_MARK,
+};
+
+static const unsigned int vin5_clkenb_a_pins[] = {
+	RCAR_GP_PIN(0, 1),
+};
+
+static const unsigned int vin5_clkenb_a_mux[] = {
+	VI5_CLKENB_A_MARK,
+};
+
+static const unsigned int vin5_clk_a_pins[] = {
+	RCAR_GP_PIN(1, 0),
+};
+
+static const unsigned int vin5_clk_a_mux[] = {
+	VI5_CLK_A_MARK,
+};
+
+static const unsigned int vin5_clk_b_pins[] = {
+	RCAR_GP_PIN(2, 22),
+};
+
+static const unsigned int vin5_clk_b_mux[] = {
+	VI5_CLK_B_MARK,
+};
+
 static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(avb_link),
 	SH_PFC_PIN_GROUP(avb_magic),
@@ -2056,6 +2496,34 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
 	SH_PFC_PIN_GROUP(usb0_id),
 	SH_PFC_PIN_GROUP(usb30),
 	SH_PFC_PIN_GROUP(usb30_id),
+	SH_PFC_PIN_GROUP(vin4_data8_a),
+	SH_PFC_PIN_GROUP(vin4_data10_a),
+	SH_PFC_PIN_GROUP(vin4_data12_a),
+	SH_PFC_PIN_GROUP(vin4_data16_a),
+	SH_PFC_PIN_GROUP(vin4_data20_a),
+	SH_PFC_PIN_GROUP(vin4_data24_a),
+	SH_PFC_PIN_GROUP(vin4_data8_b),
+	SH_PFC_PIN_GROUP(vin4_data10_b),
+	SH_PFC_PIN_GROUP(vin4_data12_b),
+	SH_PFC_PIN_GROUP(vin4_data16_b),
+	SH_PFC_PIN_GROUP(vin4_data20_b),
+	SH_PFC_PIN_GROUP(vin4_data24_b),
+	SH_PFC_PIN_GROUP(vin4_data8_sft8),
+	SH_PFC_PIN_GROUP(vin4_sync),
+	SH_PFC_PIN_GROUP(vin4_field),
+	SH_PFC_PIN_GROUP(vin4_clkenb),
+	SH_PFC_PIN_GROUP(vin4_clk),
+	SH_PFC_PIN_GROUP(vin5_data8_a),
+	SH_PFC_PIN_GROUP(vin5_data8_sft8_a),
+	SH_PFC_PIN_GROUP(vin5_data10_a),
+	SH_PFC_PIN_GROUP(vin5_data12_a),
+	SH_PFC_PIN_GROUP(vin5_data16_a),
+	SH_PFC_PIN_GROUP(vin5_data8_b),
+	SH_PFC_PIN_GROUP(vin5_sync_a),
+	SH_PFC_PIN_GROUP(vin5_field_a),
+	SH_PFC_PIN_GROUP(vin5_clkenb_a),
+	SH_PFC_PIN_GROUP(vin5_clk_a),
+	SH_PFC_PIN_GROUP(vin5_clk_b),
 };
 
 static const char * const avb_groups[] = {
@@ -2200,6 +2668,40 @@ static const char * const usb30_groups[] = {
 	"usb30_id",
 };
 
+static const char * const vin4_groups[] = {
+	"vin4_data8_a",
+	"vin4_data10_a",
+	"vin4_data12_a",
+	"vin4_data16_a",
+	"vin4_data20_a",
+	"vin4_data24_a",
+	"vin4_data8_b",
+	"vin4_data10_b",
+	"vin4_data12_b",
+	"vin4_data16_b",
+	"vin4_data20_b",
+	"vin4_data24_b",
+	"vin4_data8_sft8",
+	"vin4_sync",
+	"vin4_field",
+	"vin4_clkenb",
+	"vin4_clk",
+};
+
+static const char * const vin5_groups[] = {
+	"vin5_data8_a",
+	"vin5_data8_sft8_a",
+	"vin5_data10_a",
+	"vin5_data12_a",
+	"vin5_data16_a",
+	"vin5_data8_b",
+	"vin5_sync_a",
+	"vin5_field_a",
+	"vin5_clkenb_a",
+	"vin5_clk_a",
+	"vin5_clk_b",
+};
+
 static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(avb),
 	SH_PFC_FUNCTION(i2c1),
@@ -2224,6 +2726,8 @@ static const struct sh_pfc_function pinmux_functions[] = {
 	SH_PFC_FUNCTION(scif_clk),
 	SH_PFC_FUNCTION(usb0),
 	SH_PFC_FUNCTION(usb30),
+	SH_PFC_FUNCTION(vin4),
+	SH_PFC_FUNCTION(vin5),
 };
 
 static const struct pinmux_cfg_reg pinmux_config_regs[] = {
-- 
2.7.4

Re: [RFT 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[Geert Uytterhoeven]

Hi Jacopo,

On Mon, Aug 20, 2018 at 12:17 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks for your patch!

> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> @@ -1982,6 +1982,446 @@ static const unsigned int usb30_id_mux[] = {
>         USB3HS0_ID_MARK,
>  };
>
> +/* - VIN4 ------------------------------------------------------------------- */
> +static const unsigned int vin4_data8_a_pins[] = {
> +       RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> +       RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> +       RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> +       RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> +};
> +
> +static const unsigned int vin4_data8_a_mux[] = {
> +       VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> +       VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> +       VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> +       VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> +};
> +
> +static const unsigned int vin4_data10_a_pins[] = {
> +       RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> +       RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> +       RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> +       RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> +       RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +};
> +
> +static const unsigned int vin4_data10_a_mux[] = {
> +       VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> +       VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> +       VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> +       VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> +       VI4_DATA8_MARK,   VI4_DATA9_MARK,
> +};

Can you please use union vin_data and VIN_DATA_PIN_GROUP(), to reduce
redundancies, cfr. commit 9942a5b52990b8d5 ("pinctrl: sh-pfc: r8a7795:
Deduplicate VIN4 pin definitions")?
Or do you want to do that in a follow-up patch (which means more
review work)?

> +static const unsigned int vin4_data8_sft8_pins[] = {
> +       RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> +       RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> +       RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> +       RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> +};

R-Car H3 and M3-W don't have the data8_sft8 groups yet (the BSP has).
IIRC, that's due to rcar-vin not yet supporting that mode. Niklas?

> +static const unsigned int vin5_sync_a_pins[] = {
> +       /* HSYNC_N, VSYNC_N */
> +       RCAR_GP_PIN(1, 8), RCAR_GP_PIN(1, 9),
> +};
> +
> +static const unsigned int vin5_sync_a_mux[] = {
> +       VI5_HSYNC_N_A_MARK, VI5_VSYNC_N_A_MARK,
> +};
> +
> +static const unsigned int vin5_field_a_pins[] = {
> +       RCAR_GP_PIN(1, 10),
> +};
> +
> +static const unsigned int vin5_field_a_mux[] = {
> +       VI5_FIELD_A_MARK,
> +};
> +
> +static const unsigned int vin5_clkenb_a_pins[] = {
> +       RCAR_GP_PIN(0, 1),
> +};
> +
> +static const unsigned int vin5_clkenb_a_mux[] = {
> +       VI5_CLKENB_A_MARK,
> +};

"A" groups without "B" groups? Usually we drop the "_A" suffix in that case.

They're actually named like that in hardware user manual rev. 1.00 (and no
update in the errata)?

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: [RFT 5/8] pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 4673 bytes --]

Hi Geert,

On Tue, Aug 28, 2018 at 09:46:57AM +0200, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Mon, Aug 20, 2018 at 12:17 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> >
> > This patch adds VIN{4,5} pins, groups and functions to the R8A77990 SoC.
> >
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Thanks for your patch!

Thanks for review.

>
> > --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> > @@ -1982,6 +1982,446 @@ static const unsigned int usb30_id_mux[] = {
> >         USB3HS0_ID_MARK,
> >  };
> >
> > +/* - VIN4 ------------------------------------------------------------------- */
> > +static const unsigned int vin4_data8_a_pins[] = {
> > +       RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> > +       RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> > +       RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> > +       RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> > +};
> > +
> > +static const unsigned int vin4_data8_a_mux[] = {
> > +       VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> > +       VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> > +       VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> > +       VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> > +};
> > +
> > +static const unsigned int vin4_data10_a_pins[] = {
> > +       RCAR_GP_PIN(2, 6),  RCAR_GP_PIN(2, 7),
> > +       RCAR_GP_PIN(2, 8),  RCAR_GP_PIN(2, 9),
> > +       RCAR_GP_PIN(2, 10), RCAR_GP_PIN(2, 11),
> > +       RCAR_GP_PIN(2, 12), RCAR_GP_PIN(2, 13),
> > +       RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> > +};
> > +
> > +static const unsigned int vin4_data10_a_mux[] = {
> > +       VI4_DATA0_A_MARK, VI4_DATA1_A_MARK,
> > +       VI4_DATA2_A_MARK, VI4_DATA3_A_MARK,
> > +       VI4_DATA4_A_MARK, VI4_DATA5_A_MARK,
> > +       VI4_DATA6_A_MARK, VI4_DATA7_A_MARK,
> > +       VI4_DATA8_MARK,   VI4_DATA9_MARK,
> > +};
>
> Can you please use union vin_data and VIN_DATA_PIN_GROUP(), to reduce
> redundancies, cfr. commit 9942a5b52990b8d5 ("pinctrl: sh-pfc: r8a7795:
> Deduplicate VIN4 pin definitions")?
> Or do you want to do that in a follow-up patch (which means more
> review work)?

I will have to resend anyhow, so I can make this change in v2.

>
> > +static const unsigned int vin4_data8_sft8_pins[] = {
> > +       RCAR_GP_PIN(1, 4),  RCAR_GP_PIN(1, 5),
> > +       RCAR_GP_PIN(1, 6),  RCAR_GP_PIN(1, 7),
> > +       RCAR_GP_PIN(1, 3),  RCAR_GP_PIN(1, 10),
> > +       RCAR_GP_PIN(1, 13), RCAR_GP_PIN(1, 14),
> > +};
>
> R-Car H3 and M3-W don't have the data8_sft8 groups yet (the BSP has).
> IIRC, that's due to rcar-vin not yet supporting that mode. Niklas?

Not sure what SFT mode is. The only registers naming this are related
to "Shift Down Volume" and RGB->YCrCb conversion. I'll wait for
Niklas, and eventually remove the pin group.
>
> > +static const unsigned int vin5_sync_a_pins[] = {
> > +       /* HSYNC_N, VSYNC_N */
> > +       RCAR_GP_PIN(1, 8), RCAR_GP_PIN(1, 9),
> > +};
> > +
> > +static const unsigned int vin5_sync_a_mux[] = {
> > +       VI5_HSYNC_N_A_MARK, VI5_VSYNC_N_A_MARK,
> > +};
> > +
> > +static const unsigned int vin5_field_a_pins[] = {
> > +       RCAR_GP_PIN(1, 10),
> > +};
> > +
> > +static const unsigned int vin5_field_a_mux[] = {
> > +       VI5_FIELD_A_MARK,
> > +};
> > +
> > +static const unsigned int vin5_clkenb_a_pins[] = {
> > +       RCAR_GP_PIN(0, 1),
> > +};
> > +
> > +static const unsigned int vin5_clkenb_a_mux[] = {
> > +       VI5_CLKENB_A_MARK,
> > +};
>
> "A" groups without "B" groups? Usually we drop the "_A" suffix in that case.
>
> They're actually named like that in hardware user manual rev. 1.00 (and no
> update in the errata)?

It's true, the sync signal pins of VI4 do not have any "_a" or "_b"
mark, while the VI5 ones do. Eg. we have "VI5_VSYNC#_A" and
"VI4_VSYNC#". This might be an error in the chip manual or a
suggestion that VIN5_DATA.*_B pins do not support synchronism signals
and can only work with embedded synchronization. This seems unlikely
to me, and checking Table 26.20.3 that lists the supported input format
for each channel, I don't see anything like that mentioned.

Morimoto-san, Shimoda-san, could you please verify why those pins,
have a different naming scheme?

Thanks
   j

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[RFT 6/8] arm64: dts: r8a77990: Add VIN and CSI-2 device nodes

[Jacopo Mondi]

From: Koji Matsuoka <koji.matsuoka.xm@renesas.com>

Add device nodes for VIN4, VIN5 and CSI40 to R-Car E3 R8A77990 device tree.

Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
Compared to other SoCs, VIN4 and VIN5 on E3 have a single endpoint.
If I describe with a unit address, I get a DTC warning

arch/arm64/boot/dts/renesas/r8a77990-ebisu.dtb: Warning (graph_child_address): /soc/video@e6ef4000/ports/port@1: graph node has single child node 'endpoint@0', #address-cells/#size-cells are not necessary

So I removed the unit address, hoping this won't break the VIN DT parsing
routine

 				#size-cells = <0>;

 				port@1 {
-					#address-cells = <1>;
-					#size-cells = <0>;
-
 					reg = <1>;

-					vin4csi40: endpoint@0 {
-						reg = <0>;
+					vin4csi40: endpoint {
 						remote-endpoint= <&csi40vin4>;
 					};
 				};

Just pointing it out in case somethine goes bad when testing.


---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 79 +++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index 2c8f119..b7d498b 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -337,6 +337,85 @@
 			status = "disabled";
 		};

+		csi40: csi2@feaa0000 {
+			compatible = "renesas,r8a77990-csi2", "renesas,rcar-gen3-csi2";
+			reg = <0 0xfeaa0000 0 0x10000>;
+			interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 716>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 716>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <1>;
+
+					csi40vin4: endpoint@0 {
+						reg = <0>;
+						remote-endpoint = <&vin4csi40>;
+					};
+					csi40vin5: endpoint@1 {
+						reg = <1>;
+						remote-endpoint = <&vin5csi40>;
+					};
+				};
+			};
+		};
+
+		vin4: video@e6ef4000 {
+			compatible = "renesas,vin-r8a77990";
+			reg = <0 0xe6ef4000 0 0x1000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 807>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 807>;
+			renesas,id = <4>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+
+					vin4csi40: endpoint {
+						remote-endpoint= <&csi40vin4>;
+					};
+				};
+			};
+		};
+
+		vin5: video@e6ef5000 {
+			compatible = "renesas,vin-r8a77990";
+			reg = <0 0xe6ef5000 0 0x1000>;
+			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 806>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 806>;
+			renesas,id = <5>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+
+					vin5csi40: endpoint {
+						remote-endpoint= <&csi40vin5>;
+					};
+				};
+			};
+		};
+
 		scif2: serial@e6e88000 {
 			compatible = "renesas,scif-r8a77990",
 				     "renesas,rcar-gen3-scif", "renesas,scif";
--
2.7.4

[RFT 7/8] arm64: dts: r8a77990: Add I2C device nodes

[Jacopo Mondi]

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

Add device nodes for I2C ch{0,1,2,3,4,5,6,7} to R-Car E3 R8A77990 device tree.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 123 ++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index b7d498b..2f6d507 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -14,6 +14,17 @@
 	#address-cells = <2>;
 	#size-cells = <2>;

+	aliases {
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
+		i2c2 = &i2c2;
+		i2c3 = &i2c3;
+		i2c4 = &i2c4;
+		i2c5 = &i2c5;
+		i2c6 = &i2c6;
+		i2c7 = &i2c7;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -185,6 +196,118 @@
 			resets = <&cpg 906>;
 		};

+		i2c0: i2c@e6500000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6500000 0 0x40>;
+			interrupts = <GIC_SPI 287 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 931>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 931>;
+			i2c-scl-internal-delay-ns = <110>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@e6508000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6508000 0 0x40>;
+			interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 930>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 930>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c2: i2c@e6510000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6510000 0 0x40>;
+			interrupts = <GIC_SPI 286 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 929>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 929>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c3: i2c@e66d0000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66d0000 0 0x40>;
+			interrupts = <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 928>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 928>;
+			i2c-scl-internal-delay-ns = <110>;
+			status = "disabled";
+		};
+
+		i2c4: i2c@e66d8000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66d8000 0 0x40>;
+			interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 927>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 927>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c5: i2c@e66e0000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66e0000 0 0x40>;
+			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 919>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 919>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c6: i2c@e66e8000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe66e8000 0 0x40>;
+			interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 918>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 918>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
+		i2c7: i2c@e6690000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "renesas,i2c-r8a77990",
+				     "renesas,rcar-gen3-i2c";
+			reg = <0 0xe6690000 0 0x40>;
+			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 1003>;
+			power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
+			resets = <&cpg 1003>;
+			i2c-scl-internal-delay-ns = <6>;
+			status = "disabled";
+		};
+
 		pfc: pin-controller@e6060000 {
 			compatible = "renesas,pfc-r8a77990";
 			reg = <0 0xe6060000 0 0x508>;
--
2.7.4

Re: [RFT 7/8] arm64: dts: r8a77990: Add I2C device nodes

[Geert Uytterhoeven]

Hi Jacopo,

Thanks for your patch!

On Mon, Aug 20, 2018 at 12:17 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> Add device nodes for I2C ch{0,1,2,3,4,5,6,7} to R-Car E3 R8A77990 device tree.

ch[0-7]?

>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Buses 0 and 3 are the only buses hosting devices on Ebisu, and i2cdetect
sees them, so:

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[RFT 8/8] arm64: dts: renesas: ebisu: Add HDMI and CVBS input

[Jacopo Mondi]

Add HDMI and CVBS inputs device nodes to R-Car E3 Ebisu board.

Both HDMI and CVBS inputs are connected to an ADV7482 video decoder hooked to
the SoC CSI-2 receiver port.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
The upported BSP patch:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=97287065b27d987f96a33bfbbd52586ac091ee6d
described the ADV7482 output as 'adv7482_txa_cp' and 'adv7482_txa_sd'.

This seems wrong to me, as per Ebisu's schematics, the TXA output channel is a
single one and it is routed to CSI40 input. The BSP patch connects two
endpoints, with differente 'data-lanes' values to the same CSI input, and this
seems odd to me.

There is an issue here that the mainline ADV748x drivers does not support
dynamic routing, and thus HDMI is always routed to TXA and analogue inputs to
TXB, but that's a separate issue and DTS should only describe which connections
are in place in the board, so I didn't include the adv748x's 'port@11' defined
in the BSP patch (which is also wrong as it should be port@b, and the 11th port
described TXB, not a different input routing to port@a).

Please have a look and confirm my understanding.
Thanks
  j
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 86 ++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index 2bc3a48..d2faf3e 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -28,6 +28,29 @@
 		/* first 128MB is reserved for secure area. */
 		reg = <0x0 0x48000000 0x0 0x38000000>;
 	};
+
+	cvbs-in {
+		compatible = "composite-video-connector";
+		label = "CVBS IN";
+
+		port {
+			cvbs_con: endpoint {
+				remote-endpoint = <&adv7482_ain7>;
+			};
+		};
+	};
+
+	hdmi-in {
+		compatible = "hdmi-connector";
+		label = "HDMI IN";
+		type = "a";
+
+		port {
+			hdmi_in_con: endpoint {
+				remote-endpoint = <&adv7482_hdmi>;
+			};
+		};
+	};
 };

 &avb {
@@ -47,6 +70,22 @@
 	};
 };

+&csi40 {
+	status = "okay";
+
+	ports {
+		port@0 {
+			reg = <0>;
+
+			csi40_in: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&adv7482_txa>;
+			};
+		};
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -55,6 +94,49 @@
 	clock-frequency = <48000000>;
 };

+&i2c0 {
+	status = "okay";
+
+	video-receiver@70 {
+		compatible = "adi,adv7482";
+		reg = <0x70>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		interrupt-parent = <&gpio0>;
+		interrupt-names = "intrq1", "intrq2";
+		interrupts = <7 IRQ_TYPE_LEVEL_LOW>,
+			     <17 IRQ_TYPE_LEVEL_LOW>;
+
+		port@7 {
+			reg = <7>;
+
+			adv7482_ain7: endpoint {
+				remote-endpoint = <&cvbs_con>;
+			};
+		};
+
+		port@8 {
+			reg = <8>;
+
+			adv7482_hdmi: endpoint {
+				remote-endpoint = <&hdmi_in_con>;
+			};
+		};
+
+		port@a {
+			reg = <0xa>;
+
+			adv7482_txa: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&csi40_in>;
+			};
+		};
+	};
+};
+
 &ohci0 {
 	status = "okay";
 };
@@ -94,6 +176,10 @@
 	status = "okay";
 };

+&vin4 {
+	status = "okay";
+};
+
 &xhci0 {
 	pinctrl-0 = <&usb30_pins>;
 	pinctrl-names = "default";
--
2.7.4

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Laurent Pinchart]

Hi Jacopo,

On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> Hello renesas list,
>    this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> Ebisu board.
> 
> It's an RFT, as I don't have an Ebisu to test with :(
> 
> The series adds supports for the following items:
> 
> - PFC: add VIN groups and functions
> - R-Car VIN and R-Car CSI-2: add support for R8A77990
> - R8A77990: Add I2C, VIN and CSI-2 nodes
> - Ebisu: describe HDMI and CVBS inputs
> 
> Each patch, when relevant reports difference between the upported BSP patch
> and the proposed one.
> 
> I know Laurent should receive an Ebisu sooner or later, maybe we can sync
> for testing :)

I've given the series a first test, and I think a bit more work is needed :-)

[    1.455533] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
port@7/endpoint on port 7
[    1.464683] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
port@8/endpoint on port 8
[    1.473728] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
port@a/endpoint on port 10
[    1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
[    1.639470] adv748x 0-0070: No endpoint found for txb
[    1.644653] adv748x 0-0070: Failed to probe TXB

> PS: the list of upported patches will be sent separately.
> 
> Jacopo Mondi (5):
>   media: dt-bindings: media: rcar-vin: Add R8A77990 support
>   media: rcar-vin: Add support for R-Car R8A77990
>   dt-bindings: media: rcar-csi2: Add R8A77990
>   media: rcar-csi2: Add R8A77990 support
>   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> 
> Koji Matsuoka (1):
>   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> 
> Takeshi Kihara (2):
>   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
>   arm64: dts: r8a77990: Add I2C device nodes
> 
>  .../devicetree/bindings/media/rcar_vin.txt         |   1 +
>  .../bindings/media/renesas,rcar-csi2.txt           |   1 +
>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 ++++
>  arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++
>  drivers/media/platform/rcar-vin/rcar-core.c        |  20 +
>  drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
>  drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 504 ++++++++++++++++++
>  7 files changed, 823 insertions(+)

-- 
Regards,

Laurent Pinchart

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Niklas Söderlund]

Hi Laurent and Jacopo,

On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> > Hello renesas list,
> >    this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> > Ebisu board.
> > 
> > It's an RFT, as I don't have an Ebisu to test with :(
> > 
> > The series adds supports for the following items:
> > 
> > - PFC: add VIN groups and functions
> > - R-Car VIN and R-Car CSI-2: add support for R8A77990
> > - R8A77990: Add I2C, VIN and CSI-2 nodes
> > - Ebisu: describe HDMI and CVBS inputs
> > 
> > Each patch, when relevant reports difference between the upported BSP patch
> > and the proposed one.
> > 
> > I know Laurent should receive an Ebisu sooner or later, maybe we can sync
> > for testing :)
> 
> I've given the series a first test, and I think a bit more work is needed :-)
> 
> [    1.455533] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> port@7/endpoint on port 7
> [    1.464683] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> port@8/endpoint on port 8
> [    1.473728] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> port@a/endpoint on port 10
> [    1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> [    1.639470] adv748x 0-0070: No endpoint found for txb
> [    1.644653] adv748x 0-0070: Failed to probe TXB

I fear this is a design choice in the adv748x driver. Currently the 
driver requires both of its two CSI-2 transmitters to be connected/used 
else probe fails. Furthermore the HDMI capture is always routed to TXA 
while the analog capture is always routed to TXB.

Now that we have a board where only TXA is connected but both HDMI and 
analog captures are used maybe it's time to do some more work on v4l2 
and the adv748x driver ;-P What's missing:

- Probe should be OK with either TXA or TXB connected and not bail if 
  not both are used.
- The media_device_ops or at least the .link_notify() callback of that 
  struct must be changed so not one driver in the media graph is 
  responsible for all links. In this case rcar-vin provides the callback 
  and rcar-vin should not judge which links between the adv748x 
  subdevices are OK to enable/disable. Currently the links between the 
  adv748x subdevices are immutably enabled to avoid this particular 
  problem.

> 
> > PS: the list of upported patches will be sent separately.
> > 
> > Jacopo Mondi (5):
> >   media: dt-bindings: media: rcar-vin: Add R8A77990 support
> >   media: rcar-vin: Add support for R-Car R8A77990
> >   dt-bindings: media: rcar-csi2: Add R8A77990
> >   media: rcar-csi2: Add R8A77990 support
> >   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> > 
> > Koji Matsuoka (1):
> >   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> > 
> > Takeshi Kihara (2):
> >   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
> >   arm64: dts: r8a77990: Add I2C device nodes
> > 
> >  .../devicetree/bindings/media/rcar_vin.txt         |   1 +
> >  .../bindings/media/renesas,rcar-csi2.txt           |   1 +
> >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 ++++
> >  arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++
> >  drivers/media/platform/rcar-vin/rcar-core.c        |  20 +
> >  drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
> >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 504 ++++++++++++++++++
> >  7 files changed, 823 insertions(+)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

-- 
Regards,
Niklas S�derlund

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 4472 bytes --]

Hi Laurent, Niklas,

On Sat, Aug 25, 2018 at 08:37:15AM +0200, Niklas Söderlund wrote:
> Hi Laurent and Jacopo,
>
> On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> > > Hello renesas list,
> > >    this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> > > Ebisu board.
> > >
> > > It's an RFT, as I don't have an Ebisu to test with :(
> > >
> > > The series adds supports for the following items:
> > >
> > > - PFC: add VIN groups and functions
> > > - R-Car VIN and R-Car CSI-2: add support for R8A77990
> > > - R8A77990: Add I2C, VIN and CSI-2 nodes
> > > - Ebisu: describe HDMI and CVBS inputs
> > >
> > > Each patch, when relevant reports difference between the upported BSP patch
> > > and the proposed one.
> > >
> > > I know Laurent should receive an Ebisu sooner or later, maybe we can sync
> > > for testing :)
> >
> > I've given the series a first test, and I think a bit more work is needed :-)
> >
> > [    1.455533] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > port@7/endpoint on port 7
> > [    1.464683] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > port@8/endpoint on port 8
> > [    1.473728] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > port@a/endpoint on port 10
> > [    1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> > [    1.639470] adv748x 0-0070: No endpoint found for txb
> > [    1.644653] adv748x 0-0070: Failed to probe TXB
>
> I fear this is a design choice in the adv748x driver. Currently the
> driver requires both of its two CSI-2 transmitters to be connected/used
> else probe fails. Furthermore the HDMI capture is always routed to TXA
> while the analog capture is always routed to TXB.
>
> Now that we have a board where only TXA is connected but both HDMI and
> analog captures are used maybe it's time to do some more work on v4l2
> and the adv748x driver ;-P What's missing:
>
> - Probe should be OK with either TXA or TXB connected and not bail if
>   not both are used.

I have three patches for this I hope to share as soon as I'll be able
to do some more testing

> - The media_device_ops or at least the .link_notify() callback of that
>   struct must be changed so not one driver in the media graph is
>   responsible for all links. In this case rcar-vin provides the callback
>   and rcar-vin should not judge which links between the adv748x
>   subdevices are OK to enable/disable. Currently the links between the
>   adv748x subdevices are immutably enabled to avoid this particular
>   problem.

Uh, I didn't get this :/ Care to elaborate more?

What I was about to do, instead, given the fixed HDMI->TXA and AFE->TXB
routing in the adv748x driver was to insert a .link_validate() callback for
both the HDMI and AFE backends, that checks for the availability of
the corresponding output endpoint. This seems to me that this makes
easy when dynamic routing will be added to do the same on the
dynamically configured sink pad.
What do you think?

Thanks
  j

>
> >
> > > PS: the list of upported patches will be sent separately.
> > >
> > > Jacopo Mondi (5):
> > >   media: dt-bindings: media: rcar-vin: Add R8A77990 support
> > >   media: rcar-vin: Add support for R-Car R8A77990
> > >   dt-bindings: media: rcar-csi2: Add R8A77990
> > >   media: rcar-csi2: Add R8A77990 support
> > >   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> > >
> > > Koji Matsuoka (1):
> > >   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> > >
> > > Takeshi Kihara (2):
> > >   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
> > >   arm64: dts: r8a77990: Add I2C device nodes
> > >
> > >  .../devicetree/bindings/media/rcar_vin.txt         |   1 +
> > >  .../bindings/media/renesas,rcar-csi2.txt           |   1 +
> > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 ++++
> > >  arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++
> > >  drivers/media/platform/rcar-vin/rcar-core.c        |  20 +
> > >  drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
> > >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 504 ++++++++++++++++++
> > >  7 files changed, 823 insertions(+)
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> >
> >
>
> --
> Regards,
> Niklas Söderlund

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 5556 bytes --]

Hi Niklas,
        A few more talk on the adv748x link handling, please bear with me...

On Sat, Aug 25, 2018 at 03:18:06PM +0200, jacopo mondi wrote:
> Hi Laurent, Niklas,
>
> On Sat, Aug 25, 2018 at 08:37:15AM +0200, Niklas Söderlund wrote:
> > Hi Laurent and Jacopo,
> >
> > On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> > > Hi Jacopo,
> > >
> > > On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> > > > Hello renesas list,
> > > >    this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> > > > Ebisu board.
> > > >
> > > > It's an RFT, as I don't have an Ebisu to test with :(
> > > >
> > > > The series adds supports for the following items:
> > > >
> > > > - PFC: add VIN groups and functions
> > > > - R-Car VIN and R-Car CSI-2: add support for R8A77990
> > > > - R8A77990: Add I2C, VIN and CSI-2 nodes
> > > > - Ebisu: describe HDMI and CVBS inputs
> > > >
> > > > Each patch, when relevant reports difference between the upported BSP patch
> > > > and the proposed one.
> > > >
> > > > I know Laurent should receive an Ebisu sooner or later, maybe we can sync
> > > > for testing :)
> > >
> > > I've given the series a first test, and I think a bit more work is needed :-)
> > >
> > > [    1.455533] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > port@7/endpoint on port 7
> > > [    1.464683] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > port@8/endpoint on port 8
> > > [    1.473728] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > port@a/endpoint on port 10
> > > [    1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> > > [    1.639470] adv748x 0-0070: No endpoint found for txb
> > > [    1.644653] adv748x 0-0070: Failed to probe TXB
> >
> > I fear this is a design choice in the adv748x driver. Currently the
> > driver requires both of its two CSI-2 transmitters to be connected/used
> > else probe fails. Furthermore the HDMI capture is always routed to TXA
> > while the analog capture is always routed to TXB.
> >
> > Now that we have a board where only TXA is connected but both HDMI and
> > analog captures are used maybe it's time to do some more work on v4l2
> > and the adv748x driver ;-P What's missing:
> >
> > - Probe should be OK with either TXA or TXB connected and not bail if
> >   not both are used.
>
> I have three patches for this I hope to share as soon as I'll be able
> to do some more testing
>
> > - The media_device_ops or at least the .link_notify() callback of that
> >   struct must be changed so not one driver in the media graph is
> >   responsible for all links. In this case rcar-vin provides the callback
> >   and rcar-vin should not judge which links between the adv748x
> >   subdevices are OK to enable/disable. Currently the links between the
> >   adv748x subdevices are immutably enabled to avoid this particular
> >   problem.
>
> Uh, I didn't get this :/ Care to elaborate more?
>

I'm thinking if it is not enough to just provide a .link_setup()
callback to the (enabled) csi-2 sink pads of the adv748x and handle
routing between the afe/hdmi sources and that sink, without the vin's
registered link_notify playing any role in that.

> What I was about to do, instead, given the fixed HDMI->TXA and AFE->TXB
> routing in the adv748x driver was to insert a .link_validate() callback for
> both the HDMI and AFE backends, that checks for the availability of
> the corresponding output endpoint. This seems to me that this makes
> easy when dynamic routing will be added to do the same on the
> dynamically configured sink pad.
> What do you think?

On a second thought if a CSI-2 sink it's not enabled it is not part of
the media graph neither, so it should not be possible to link it in any
pipeline, so no link validation is required. The link simply can't
exist.

It seems to me that to support Ebisu-like designs is then enough to
make the adv748x driver probe with a single csi-2 output enabled and
handle power management accordingly. I will share patches for this
briefly.

>
> Thanks
>   j
>
> >
> > >
> > > > PS: the list of upported patches will be sent separately.
> > > >
> > > > Jacopo Mondi (5):
> > > >   media: dt-bindings: media: rcar-vin: Add R8A77990 support
> > > >   media: rcar-vin: Add support for R-Car R8A77990
> > > >   dt-bindings: media: rcar-csi2: Add R8A77990
> > > >   media: rcar-csi2: Add R8A77990 support
> > > >   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> > > >
> > > > Koji Matsuoka (1):
> > > >   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> > > >
> > > > Takeshi Kihara (2):
> > > >   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
> > > >   arm64: dts: r8a77990: Add I2C device nodes
> > > >
> > > >  .../devicetree/bindings/media/rcar_vin.txt         |   1 +
> > > >  .../bindings/media/renesas,rcar-csi2.txt           |   1 +
> > > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 ++++
> > > >  arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++
> > > >  drivers/media/platform/rcar-vin/rcar-core.c        |  20 +
> > > >  drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
> > > >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 504 ++++++++++++++++++
> > > >  7 files changed, 823 insertions(+)
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> > >
> > >
> > >
> >
> > --
> > Regards,
> > Niklas Söderlund



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Niklas Söderlund]

Hi Jacopo,

On 2018-08-27 11:49:56 +0200, Jacopo Mondi wrote:
> Hi Niklas,
>         A few more talk on the adv748x link handling, please bear with me...
> 
> On Sat, Aug 25, 2018 at 03:18:06PM +0200, jacopo mondi wrote:
> > Hi Laurent, Niklas,
> >
> > On Sat, Aug 25, 2018 at 08:37:15AM +0200, Niklas S�derlund wrote:
> > > Hi Laurent and Jacopo,
> > >
> > > On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> > > > Hi Jacopo,
> > > >
> > > > On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> > > > > Hello renesas list,
> > > > >    this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> > > > > Ebisu board.
> > > > >
> > > > > It's an RFT, as I don't have an Ebisu to test with :(
> > > > >
> > > > > The series adds supports for the following items:
> > > > >
> > > > > - PFC: add VIN groups and functions
> > > > > - R-Car VIN and R-Car CSI-2: add support for R8A77990
> > > > > - R8A77990: Add I2C, VIN and CSI-2 nodes
> > > > > - Ebisu: describe HDMI and CVBS inputs
> > > > >
> > > > > Each patch, when relevant reports difference between the upported BSP patch
> > > > > and the proposed one.
> > > > >
> > > > > I know Laurent should receive an Ebisu sooner or later, maybe we can sync
> > > > > for testing :)
> > > >
> > > > I've given the series a first test, and I think a bit more work is needed :-)
> > > >
> > > > [    1.455533] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > > port@7/endpoint on port 7
> > > > [    1.464683] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > > port@8/endpoint on port 8
> > > > [    1.473728] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > > port@a/endpoint on port 10
> > > > [    1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> > > > [    1.639470] adv748x 0-0070: No endpoint found for txb
> > > > [    1.644653] adv748x 0-0070: Failed to probe TXB
> > >
> > > I fear this is a design choice in the adv748x driver. Currently the
> > > driver requires both of its two CSI-2 transmitters to be connected/used
> > > else probe fails. Furthermore the HDMI capture is always routed to TXA
> > > while the analog capture is always routed to TXB.
> > >
> > > Now that we have a board where only TXA is connected but both HDMI and
> > > analog captures are used maybe it's time to do some more work on v4l2
> > > and the adv748x driver ;-P What's missing:
> > >
> > > - Probe should be OK with either TXA or TXB connected and not bail if
> > >   not both are used.
> >
> > I have three patches for this I hope to share as soon as I'll be able
> > to do some more testing
> >
> > > - The media_device_ops or at least the .link_notify() callback of that
> > >   struct must be changed so not one driver in the media graph is
> > >   responsible for all links. In this case rcar-vin provides the callback
> > >   and rcar-vin should not judge which links between the adv748x
> > >   subdevices are OK to enable/disable. Currently the links between the
> > >   adv748x subdevices are immutably enabled to avoid this particular
> > >   problem.
> >
> > Uh, I didn't get this :/ Care to elaborate more?
> >
> 
> I'm thinking if it is not enough to just provide a .link_setup()
> callback to the (enabled) csi-2 sink pads of the adv748x and handle
> routing between the afe/hdmi sources and that sink, without the vin's
> registered link_notify playing any role in that.

That is my point, the v4l2 framework needs work to allow all drivers to 
provide a .link_setup() callback. And this as you describe will conflict 
with the current solution where there is only one possible such 
callback. So in addition to being able to have multiple such callbacks 
the current drivers implementing one would need to be updated to ignore 
links which it do not care about.

In our case the .link_setup() in rcar-vin should not care about the 
links between the adv748x internal subdevice. Of course the other way 
around is also true, the .link_setup() in adv748x should not care about 
the links handled by rcar-vin :-)

As you discovers this is not possible today and might require some work 
or maybe even a different design then the one outlined above.

> 
> > What I was about to do, instead, given the fixed HDMI->TXA and AFE->TXB
> > routing in the adv748x driver was to insert a .link_validate() callback for
> > both the HDMI and AFE backends, that checks for the availability of
> > the corresponding output endpoint. This seems to me that this makes
> > easy when dynamic routing will be added to do the same on the
> > dynamically configured sink pad.
> > What do you think?
> 
> On a second thought if a CSI-2 sink it's not enabled it is not part of
> the media graph neither, so it should not be possible to link it in any
> pipeline, so no link validation is required. The link simply can't
> exist.
> 
> It seems to me that to support Ebisu-like designs is then enough to
> make the adv748x driver probe with a single csi-2 output enabled and
> handle power management accordingly. I will share patches for this
> briefly.
> 
> >
> > Thanks
> >   j
> >
> > >
> > > >
> > > > > PS: the list of upported patches will be sent separately.
> > > > >
> > > > > Jacopo Mondi (5):
> > > > >   media: dt-bindings: media: rcar-vin: Add R8A77990 support
> > > > >   media: rcar-vin: Add support for R-Car R8A77990
> > > > >   dt-bindings: media: rcar-csi2: Add R8A77990
> > > > >   media: rcar-csi2: Add R8A77990 support
> > > > >   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> > > > >
> > > > > Koji Matsuoka (1):
> > > > >   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> > > > >
> > > > > Takeshi Kihara (2):
> > > > >   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
> > > > >   arm64: dts: r8a77990: Add I2C device nodes
> > > > >
> > > > >  .../devicetree/bindings/media/rcar_vin.txt         |   1 +
> > > > >  .../bindings/media/renesas,rcar-csi2.txt           |   1 +
> > > > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 ++++
> > > > >  arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++
> > > > >  drivers/media/platform/rcar-vin/rcar-core.c        |  20 +
> > > > >  drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
> > > > >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 504 ++++++++++++++++++
> > > > >  7 files changed, 823 insertions(+)
> > > >
> > > > --
> > > > Regards,
> > > >
> > > > Laurent Pinchart
> > > >
> > > >
> > > >
> > >
> > > --
> > > Regards,
> > > Niklas S�derlund
> 
> 



-- 
Regards,
Niklas S�derlund

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 10569 bytes --]

Hi Niklas,

On Mon, Aug 27, 2018 at 03:23:45PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> On 2018-08-27 11:49:56 +0200, Jacopo Mondi wrote:
> > Hi Niklas,
> >         A few more talk on the adv748x link handling, please bear with me...
> >
> > On Sat, Aug 25, 2018 at 03:18:06PM +0200, jacopo mondi wrote:
> > > Hi Laurent, Niklas,
> > >
> > > On Sat, Aug 25, 2018 at 08:37:15AM +0200, Niklas Söderlund wrote:
> > > > Hi Laurent and Jacopo,
> > > >
> > > > On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> > > > > Hi Jacopo,
> > > > >
> > > > > On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> > > > > > Hello renesas list,
> > > > > >    this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> > > > > > Ebisu board.
> > > > > >
> > > > > > It's an RFT, as I don't have an Ebisu to test with :(
> > > > > >
> > > > > > The series adds supports for the following items:
> > > > > >
> > > > > > - PFC: add VIN groups and functions
> > > > > > - R-Car VIN and R-Car CSI-2: add support for R8A77990
> > > > > > - R8A77990: Add I2C, VIN and CSI-2 nodes
> > > > > > - Ebisu: describe HDMI and CVBS inputs
> > > > > >
> > > > > > Each patch, when relevant reports difference between the upported BSP patch
> > > > > > and the proposed one.
> > > > > >
> > > > > > I know Laurent should receive an Ebisu sooner or later, maybe we can sync
> > > > > > for testing :)
> > > > >
> > > > > I've given the series a first test, and I think a bit more work is needed :-)
> > > > >
> > > > > [    1.455533] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > > > port@7/endpoint on port 7
> > > > > [    1.464683] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > > > port@8/endpoint on port 8
> > > > > [    1.473728] adv748x 0-0070: Endpoint /soc/i2c@e6500000/video-receiver@70/
> > > > > port@a/endpoint on port 10
> > > > > [    1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> > > > > [    1.639470] adv748x 0-0070: No endpoint found for txb
> > > > > [    1.644653] adv748x 0-0070: Failed to probe TXB
> > > >
> > > > I fear this is a design choice in the adv748x driver. Currently the
> > > > driver requires both of its two CSI-2 transmitters to be connected/used
> > > > else probe fails. Furthermore the HDMI capture is always routed to TXA
> > > > while the analog capture is always routed to TXB.
> > > >
> > > > Now that we have a board where only TXA is connected but both HDMI and
> > > > analog captures are used maybe it's time to do some more work on v4l2
> > > > and the adv748x driver ;-P What's missing:
> > > >
> > > > - Probe should be OK with either TXA or TXB connected and not bail if
> > > >   not both are used.
> > >
> > > I have three patches for this I hope to share as soon as I'll be able
> > > to do some more testing
> > >
> > > > - The media_device_ops or at least the .link_notify() callback of that
> > > >   struct must be changed so not one driver in the media graph is
> > > >   responsible for all links. In this case rcar-vin provides the callback
> > > >   and rcar-vin should not judge which links between the adv748x
> > > >   subdevices are OK to enable/disable. Currently the links between the
> > > >   adv748x subdevices are immutably enabled to avoid this particular
> > > >   problem.
> > >
> > > Uh, I didn't get this :/ Care to elaborate more?
> > >
> >
> > I'm thinking if it is not enough to just provide a .link_setup()
> > callback to the (enabled) csi-2 sink pads of the adv748x and handle
> > routing between the afe/hdmi sources and that sink, without the vin's
> > registered link_notify playing any role in that.
>
> That is my point, the v4l2 framework needs work to allow all drivers to
> provide a .link_setup() callback. And this as you describe will conflict
> with the current solution where there is only one possible such
> callback. So in addition to being able to have multiple such callbacks
> the current drivers implementing one would need to be updated to ignore
> links which it do not care about.
>
> In our case the .link_setup() in rcar-vin should not care about the
> links between the adv748x internal subdevice. Of course the other way
> around is also true, the .link_setup() in adv748x should not care about
> the links handled by rcar-vin :-)
>
> As you discovers this is not possible today and might require some work
> or maybe even a different design then the one outlined above.
>
Myabe I'm missing some piece for sure, but why do you think it is not
possible?

I've applied, for testing, the following patch:
https://paste.debian.net/1039515/

And that's the output I get

* At system boot, no link is enabled between the HDMI backend and the
  TXA output:

[root@alarm ~]# media-ctl -p -d /dev/media2
....
- entity 7: adv748x 4-0070 txa (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev21
        pad0: Sink
                [fmt:unknown/0x0]
                <- "adv748x 4-0070 hdmi":1 []
        pad1: Source
                [fmt:unknown/0x0]
                -> "rcar_csi2 feaa0000.csi2":0 [ENABLED,IMMUTABLE]

- entity 10: adv748x 4-0070 hdmi (2 pads, 1 link)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev20
        pad0: Sink
                [dv.caps:BT.656/1120 min:640x480@13000000 max:1920x1200@162000000 stds:CEA-861,DMT caps:progressive]
        pad1: Source
                [fmt:RGB888_1X24/1280x720 field:none colorspace:srgb]
                [dv.caps:BT.656/1120 min:640x480@13000000 max:1920x1200@162000000 stds:CEA-861,DMT caps:progressive]
                [dv.query:no-link]
                [dv.current:BT.656/1120 1280x720p30 (3300x750) stds:CEA-861 flags:can-reduce-fps,CE-video,has-cea861-vic]
                -> "adv748x 4-0070 txa":0 []
...

* Enable the link, and see that .link_setup() has been called for the
  adv748x
[root@alarm ~]# media-ctl -v -d /dev/media2 -l "'adv748x 4-0070 hdmi':1 -> 'adv748x 4-0070 txa':0 [1]"
[root@alarm ~]# dmesg | tail
...
[  183.294541] rvin_group_link_notify:126 SOURCE adv748x 4-0070 hdmi -> SINK adv748x 4-0070 txa
[  183.304663] adv748x_link_setup:332 LOCAL adv748x 4-0070 hdmi -> REMOTE adv748x 4-0070 txa
[  183.314207] adv748x_link_setup:332 LOCAL adv748x 4-0070 txa -> REMOTE adv748x 4-0070 hdmi
[  183.323720] rvin_group_link_notify:126 SOURCE adv748x 4-0070 hdmi -> SINK adv748x 4-0070 txa

[root@alarm ~]# media-ctl -p -d /dev/media2
....
- entity 7: adv748x 4-0070 txa (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev21
        pad0: Sink
                [fmt:unknown/0x0]
                <- "adv748x 4-0070 hdmi":1 [ENABLED]
        pad1: Source
                [fmt:unknown/0x0]
                -> "rcar_csi2 feaa0000.csi2":0 [ENABLED,IMMUTABLE]

- entity 10: adv748x 4-0070 hdmi (2 pads, 1 link)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev20
        pad0: Sink
                [dv.caps:BT.656/1120 min:640x480@13000000 max:1920x1200@162000000 stds:CEA-861,DMT caps:progressive]
        pad1: Source
                [fmt:RGB888_1X24/1280x720 field:none colorspace:srgb]
                [dv.caps:BT.656/1120 min:640x480@13000000 max:1920x1200@162000000 stds:CEA-861,DMT caps:progressive]
                [dv.query:no-link]
                [dv.current:BT.656/1120 1280x720p30 (3300x750) stds:CEA-861 flags:can-reduce-fps,CE-video,has-cea861-vic]
                -> "adv748x 4-0070 txa":0 [ENABLED]
...

So, at link creation time, the adv748x receives a .link_setup() callback (as
well as the vin driver does though...)

Isn't this enough to dynamically set-up links created between the HDMI/CVBS
inputs and one of the CSI-2 transmitters?

Thanks
  j

> >
> > > What I was about to do, instead, given the fixed HDMI->TXA and AFE->TXB
> > > routing in the adv748x driver was to insert a .link_validate() callback for
> > > both the HDMI and AFE backends, that checks for the availability of
> > > the corresponding output endpoint. This seems to me that this makes
> > > easy when dynamic routing will be added to do the same on the
> > > dynamically configured sink pad.
> > > What do you think?
> >
> > On a second thought if a CSI-2 sink it's not enabled it is not part of
> > the media graph neither, so it should not be possible to link it in any
> > pipeline, so no link validation is required. The link simply can't
> > exist.
> >
> > It seems to me that to support Ebisu-like designs is then enough to
> > make the adv748x driver probe with a single csi-2 output enabled and
> > handle power management accordingly. I will share patches for this
> > briefly.
> >
> > >
> > > Thanks
> > >   j
> > >
> > > >
> > > > >
> > > > > > PS: the list of upported patches will be sent separately.
> > > > > >
> > > > > > Jacopo Mondi (5):
> > > > > >   media: dt-bindings: media: rcar-vin: Add R8A77990 support
> > > > > >   media: rcar-vin: Add support for R-Car R8A77990
> > > > > >   dt-bindings: media: rcar-csi2: Add R8A77990
> > > > > >   media: rcar-csi2: Add R8A77990 support
> > > > > >   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> > > > > >
> > > > > > Koji Matsuoka (1):
> > > > > >   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> > > > > >
> > > > > > Takeshi Kihara (2):
> > > > > >   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
> > > > > >   arm64: dts: r8a77990: Add I2C device nodes
> > > > > >
> > > > > >  .../devicetree/bindings/media/rcar_vin.txt         |   1 +
> > > > > >  .../bindings/media/renesas,rcar-csi2.txt           |   1 +
> > > > > >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts     |  86 ++++
> > > > > >  arch/arm64/boot/dts/renesas/r8a77990.dtsi          | 202 +++++++++
> > > > > >  drivers/media/platform/rcar-vin/rcar-core.c        |  20 +
> > > > > >  drivers/media/platform/rcar-vin/rcar-csi2.c        |   9 +
> > > > > >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c              | 504 ++++++++++++++++++
> > > > > >  7 files changed, 823 insertions(+)
> > > > >
> > > > > --
> > > > > Regards,
> > > > >
> > > > > Laurent Pinchart
> > > > >
> > > > >
> > > > >
> > > >
> > > > --
> > > > Regards,
> > > > Niklas Söderlund
> >
> >
>
>
>
> --
> Regards,
> Niklas Söderlund

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Laurent Pinchart]

Hi Niklas,

On Monday, 27 August 2018 16:23:45 EEST Niklas Söderlund wrote:
> On 2018-08-27 11:49:56 +0200, Jacopo Mondi wrote:
> > On Sat, Aug 25, 2018 at 03:18:06PM +0200, jacopo mondi wrote:
> >> On Sat, Aug 25, 2018 at 08:37:15AM +0200, Niklas Söderlund wrote:
> >>> On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> >>>> On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> >>>>> Hello renesas list,
> >>>>> 
> >>>>> this series add supports for the HDMI and CVBS input to R-Car
> >>>>> E3 R8A77990 Ebisu board.
> >>>>> 
> >>>>> It's an RFT, as I don't have an Ebisu to test with :(
> >>>>> 
> >>>>> The series adds supports for the following items:
> >>>>> 
> >>>>> - PFC: add VIN groups and functions
> >>>>> - R-Car VIN and R-Car CSI-2: add support for R8A77990
> >>>>> - R8A77990: Add I2C, VIN and CSI-2 nodes
> >>>>> - Ebisu: describe HDMI and CVBS inputs
> >>>>> 
> >>>>> Each patch, when relevant reports difference between the upported
> >>>>> BSP patch and the proposed one.
> >>>>> 
> >>>>> I know Laurent should receive an Ebisu sooner or later, maybe we
> >>>>> can sync for testing :)
> >>>> 
> >>>> I've given the series a first test, and I think a bit more work is
> >>>> needed :-)
> >>>> 
> >>>> [    1.455533] adv748x 0-0070: Endpoint
> >>>> /soc/i2c@e6500000/video-receiver@70/ port@7/endpoint on port 7
> >>>> [    1.464683] adv748x 0-0070: Endpoint
> >>>> /soc/i2c@e6500000/video-receiver@70/ port@8/endpoint on port 8
> >>>> [    1.473728] adv748x 0-0070: Endpoint
> >>>> /soc/i2c@e6500000/video-receiver@70/ port@a/endpoint on port 10
> >>>> [    1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> >>>> [    1.639470] adv748x 0-0070: No endpoint found for txb
> >>>> [    1.644653] adv748x 0-0070: Failed to probe TXB
> >>> 
> >>> I fear this is a design choice in the adv748x driver. Currently the
> >>> driver requires both of its two CSI-2 transmitters to be
> >>> connected/used else probe fails. Furthermore the HDMI capture is always
> >>> routed to TXA while the analog capture is always routed to TXB.
> >>> 
> >>> Now that we have a board where only TXA is connected but both HDMI and
> >>> analog captures are used maybe it's time to do some more work on v4l2
> >>> and the adv748x driver ;-P What's missing:
> >>> 
> >>> - Probe should be OK with either TXA or TXB connected and not bail if
> >>>   not both are used.
> >> 
> >> I have three patches for this I hope to share as soon as I'll be able
> >> to do some more testing
> >> 
> >>> - The media_device_ops or at least the .link_notify() callback of that
> >>>   struct must be changed so not one driver in the media graph is
> >>>   responsible for all links. In this case rcar-vin provides the
> >>>   callback and rcar-vin should not judge which links between the
> >>>   adv748x subdevices are OK to enable/disable. Currently the links
> >>>   between the adv748x subdevices are immutably enabled to avoid this
> >>>   particular problem.
> >> 
> >> Uh, I didn't get this :/ Care to elaborate more?
> > 
> > I'm thinking if it is not enough to just provide a .link_setup()
> > callback to the (enabled) csi-2 sink pads of the adv748x and handle
> > routing between the afe/hdmi sources and that sink, without the vin's
> > registered link_notify playing any role in that.
> 
> That is my point, the v4l2 framework needs work to allow all drivers to
> provide a .link_setup() callback. And this as you describe will conflict
> with the current solution where there is only one possible such
> callback. So in addition to being able to have multiple such callbacks
> the current drivers implementing one would need to be updated to ignore
> links which it do not care about.

Isn't this already possible ? We have a single top-level .link_notify() 
operation at the media device level, but also .link_setup() operations for 
each entity.

> In our case the .link_setup() in rcar-vin should not care about the
> links between the adv748x internal subdevice. Of course the other way
> around is also true, the .link_setup() in adv748x should not care about
> the links handled by rcar-vin :-)
> 
> As you discovers this is not possible today and might require some work
> or maybe even a different design then the one outlined above.
> 
> >> What I was about to do, instead, given the fixed HDMI->TXA and AFE->TXB
> >> routing in the adv748x driver was to insert a .link_validate() callback
> >> for both the HDMI and AFE backends, that checks for the availability of
> >> the corresponding output endpoint. This seems to me that this makes
> >> easy when dynamic routing will be added to do the same on the
> >> dynamically configured sink pad.
> >> What do you think?
> > 
> > On a second thought if a CSI-2 sink it's not enabled it is not part of
> > the media graph neither, so it should not be possible to link it in any
> > pipeline, so no link validation is required. The link simply can't
> > exist.
> > 
> > It seems to me that to support Ebisu-like designs is then enough to
> > make the adv748x driver probe with a single csi-2 output enabled and
> > handle power management accordingly. I will share patches for this
> > briefly.

-- 
Regards,

Laurent Pinchart

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Niklas Söderlund]

Hi Jacopo and Laurent,

On 2018-08-28 13:40:34 +0300, Laurent Pinchart wrote:

[snip]

> > >>> - The media_device_ops or at least the .link_notify() callback 
> > >>> of that
> > >>>   struct must be changed so not one driver in the media graph is
> > >>>   responsible for all links. In this case rcar-vin provides the
> > >>>   callback and rcar-vin should not judge which links between the
> > >>>   adv748x subdevices are OK to enable/disable. Currently the links
> > >>>   between the adv748x subdevices are immutably enabled to avoid this
> > >>>   particular problem.
> > >> 
> > >> Uh, I didn't get this :/ Care to elaborate more?
> > > 
> > > I'm thinking if it is not enough to just provide a .link_setup()
> > > callback to the (enabled) csi-2 sink pads of the adv748x and handle
> > > routing between the afe/hdmi sources and that sink, without the vin's
> > > registered link_notify playing any role in that.
> > 
> > That is my point, the v4l2 framework needs work to allow all drivers to
> > provide a .link_setup() callback. And this as you describe will conflict
> > with the current solution where there is only one possible such
> > callback. So in addition to being able to have multiple such callbacks
> > the current drivers implementing one would need to be updated to ignore
> > links which it do not care about.
> 
> Isn't this already possible ? We have a single top-level .link_notify() 
> operation at the media device level, but also .link_setup() operations for 
> each entity.

You are both right this is possible today, my bad. I had confused 
link_notify() and link_setup() and thought they where the same and that 
there could be only one implementation in struct media_device_ops which 
handled all link changes.

I'm happy to be proven wrong here as it will allow the adv748x to change 
links between its subdevices :-) Jacopo thanks for being persistent 
here!

-- 
Regards,
Niklas S�derlund