[PATCH 0/3] arm64: dts: Draak: Enable HDMI input and VIN4

[PATCH 0/3] arm64: dts: Draak: Enable HDMI input and VIN4

[Jacopo Mondi]

Hello,
   this series enables HDMI input and VIN4 on R-Car D3 Draak board.

The Draak board has an HDMI input connected to an HDMI decoder that feeds
the VIN capture interface through its parallel video interface.

The series requires the just sent:
[PATCH 0/5] rcar-vin: Add support for digital input on Gen3

and enables image capture operations on D3 Draak board.

The series has been developed on top of media-master tree but applies cleanly
on top of latest renesas-driver.

Geert: would you like a topic branch for this series to be included in
renesas-drivers?

Patches for testing are available at:
git://jmondi.org/linux d3/media-master/driver
git://jmondi.org/linux d3/media-master/dts
git://jmondi.org/linux d3/media-master/test
git://jmondi.org/vin-tests d3

Thanks
    j

Jacopo Mondi (3):
  dt-bindings: media: rcar-vin: Add R8A77995 support
  arm64: dts: renesas: r8a77995: Add VIN4
  arm64: dts: renesas: draak: Describe HDMI input

 .../devicetree/bindings/media/rcar_vin.txt         |  1 +
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts     | 68 ++++++++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a77995.dtsi          | 11 ++++
 3 files changed, 80 insertions(+)

--
2.7.4

[PATCH 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support

[Jacopo Mondi]

Add compatible string for R-Car D3 R8A7795 to 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 a19517e1..5c6f2a7 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -22,6 +22,7 @@ on Gen3 platforms to a CSI-2 receiver.
    - "renesas,vin-r8a7795" for the R8A7795 device
    - "renesas,vin-r8a7796" for the R8A7796 device
    - "renesas,vin-r8a77970" for the R8A77970 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: [PATCH 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-11 12:00:00 +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  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 a19517e1..5c6f2a7 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -22,6 +22,7 @@ on Gen3 platforms to a CSI-2 receiver.
>     - "renesas,vin-r8a7795" for the R8A7795 device
>     - "renesas,vin-r8a7796" for the R8A7796 device
>     - "renesas,vin-r8a77970" for the R8A77970 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
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support

[Simon Horman]

On Fri, May 11, 2018 at 12:00:00PM +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support

[jacopo mondi]

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

Hi Simon,

On Fri, May 11, 2018 at 03:35:14PM +0200, Simon Horman wrote:
> On Fri, May 11, 2018 at 12:00:00PM +0200, Jacopo Mondi wrote:
> > Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
> > rcar-vin driver.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
>

Does this goes in through your tree? I will send a v2, should I
include this one or you have collected it already?

Thanks
   j

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

Re: [PATCH 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support

[Simon Horman]

On Tue, May 15, 2018 at 10:36:27AM +0200, jacopo mondi wrote:
> Hi Simon,
> 
> On Fri, May 11, 2018 at 03:35:14PM +0200, Simon Horman wrote:
> > On Fri, May 11, 2018 at 12:00:00PM +0200, Jacopo Mondi wrote:
> > > Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
> > > rcar-vin driver.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> >
> 
> Does this goes in through your tree? I will send a v2, should I
> include this one or you have collected it already?

No, it means I am reviewing it with the expectation that it will
go through someone else's tree. So if you need to post a v2 please include
this patch with my Reviewed-by tag.

Re: [PATCH 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Friday, 11 May 2018 13:00:00 EEST Jacopo Mondi wrote:
> Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

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

> ---
>  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
> a19517e1..5c6f2a7 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -22,6 +22,7 @@ on Gen3 platforms to a CSI-2 receiver.
>     - "renesas,vin-r8a7795" for the R8A7795 device
>     - "renesas,vin-r8a7796" for the R8A7796 device
>     - "renesas,vin-r8a77970" for the R8A77970 device
> +   - "renesas,vin-r8a77995" for the R8A77995 device
>     - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
>       device.

-- 
Regards,

Laurent Pinchart

[PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4

[Jacopo Mondi]

Describe VIN4 interface for R-Car D3 R8A77995 SoC.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 82aed7e..bdf7017 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -783,6 +783,17 @@
 				};
 			};
 		};
+
+		vin4: video@e6ef4000 {
+			compatible = "renesas,vin-r8a77995";
+			reg = <0 0xe6ef4000 0 0x1000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 807>;
+			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
+			resets = <&cpg 807>;
+			renesas,id = <4>;
+			status = "disabled";
+		};
 	};
 
 	timer {
-- 
2.7.4

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-11 12:00:01 +0200, Jacopo Mondi wrote:
> Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> index 82aed7e..bdf7017 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -783,6 +783,17 @@
>  				};
>  			};
>  		};
> +
> +		vin4: video@e6ef4000 {
> +			compatible = "renesas,vin-r8a77995";
> +			reg = <0 0xe6ef4000 0 0x1000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 807>;
> +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> +			resets = <&cpg 807>;
> +			renesas,id = <4>;
> +			status = "disabled";
> +		};
>  	};
>  
>  	timer {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4

[Simon Horman]

On Fri, May 11, 2018 at 01:25:23PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-11 12:00:01 +0200, Jacopo Mondi wrote:
> > Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > index 82aed7e..bdf7017 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > @@ -783,6 +783,17 @@
> >  				};
> >  			};
> >  		};
> > +
> > +		vin4: video@e6ef4000 {
> > +			compatible = "renesas,vin-r8a77995";
> > +			reg = <0 0xe6ef4000 0 0x1000>;
> > +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 807>;
> > +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> > +			resets = <&cpg 807>;
> > +			renesas,id = <4>;
> > +			status = "disabled";
> > +		};
> >  	};

Thanks, I have moved the new node to preserve sorting of nodes by bus
address and applied the result. It is as follows:

From: Jacopo Mondi <jacopo+renesas@jmondi.org>
Subject: [PATCH] arm64: dts: renesas: r8a77995: Add VIN4

Describe VIN4 interface for R-Car D3 R8A77995 SoC.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
[simon: sorted node by bus address]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index ba98865b0c9b..2506f46293e8 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -610,6 +610,17 @@
 			status = "disabled";
 		};
 
+		vin4: video@e6ef4000 {
+			compatible = "renesas,vin-r8a77995";
+			reg = <0 0xe6ef4000 0 0x1000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 807>;
+			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
+			resets = <&cpg 807>;
+			renesas,id = <4>;
+			status = "disabled";
+		};
+
 		ohci0: usb@ee080000 {
 			compatible = "generic-ohci";
 			reg = <0 0xee080000 0 0x100>;
-- 
2.11.0

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4

[jacopo mondi]

Hi Simon,

On Fri, May 11, 2018 at 03:45:16PM +0200, Simon Horman wrote:
> On Fri, May 11, 2018 at 01:25:23PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> > 
> > Thanks for your work.
> > 
> > On 2018-05-11 12:00:01 +0200, Jacopo Mondi wrote:
> > > Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> > > 
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > 
> > Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > 
> > > ---
> > >  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > index 82aed7e..bdf7017 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > > @@ -783,6 +783,17 @@
> > >  				};
> > >  			};
> > >  		};
> > > +
> > > +		vin4: video@e6ef4000 {
> > > +			compatible = "renesas,vin-r8a77995";
> > > +			reg = <0 0xe6ef4000 0 0x1000>;
> > > +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&cpg CPG_MOD 807>;
> > > +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> > > +			resets = <&cpg 807>;
> > > +			renesas,id = <4>;
> > > +			status = "disabled";
> > > +		};
> > >  	};
> 
> Thanks, I have moved the new node to preserve sorting of nodes by bus
> address and applied the result. It is as follows:

Great, thanks for doing this, I should have take care of sorting nodes
opprtunely.

Thanks
   j

> 
> From: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Subject: [PATCH] arm64: dts: renesas: r8a77995: Add VIN4
> 
> Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> [simon: sorted node by bus address]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> index ba98865b0c9b..2506f46293e8 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -610,6 +610,17 @@
>  			status = "disabled";
>  		};
s  
> +		vin4: video@e6ef4000 {
> +			compatible = "renesas,vin-r8a77995";
> +			reg = <0 0xe6ef4000 0 0x1000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 807>;
> +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> +			resets = <&cpg 807>;
> +			renesas,id = <4>;
> +			status = "disabled";
> +		};
> +
>  		ohci0: usb@ee080000 {
>  			compatible = "generic-ohci";
>  			reg = <0 0xee080000 0 0x100>;
> -- 
> 2.11.0
> 

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4

[Laurent Pinchart]

Hello,

On Friday, 11 May 2018 16:45:16 EEST Simon Horman wrote:
> On Fri, May 11, 2018 at 01:25:23PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> > 
> > Thanks for your work.
> > 
> > On 2018-05-11 12:00:01 +0200, Jacopo Mondi wrote:
> > > Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> > > 
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > 
> > Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > 
> >> ---
> >> 
> >>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >> 
> >> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> >> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index 82aed7e..bdf7017
> >> 100644
> >> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> >> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> >> @@ -783,6 +783,17 @@
> >>  				};
> >>  			};
> >>  		};
> >> +
> >> +		vin4: video@e6ef4000 {
> >> +			compatible = "renesas,vin-r8a77995";
> >> +			reg = <0 0xe6ef4000 0 0x1000>;
> >> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> >> +			clocks = <&cpg CPG_MOD 807>;
> >> +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> >> +			resets = <&cpg 807>;
> >> +			renesas,id = <4>;
> >> +			status = "disabled";
> >> +		};
> >>  	};
> 
> Thanks, I have moved the new node to preserve sorting of nodes by bus
> address and applied the result. It is as follows:
> 
> From: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Subject: [PATCH] arm64: dts: renesas: r8a77995: Add VIN4
> 
> Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> [simon: sorted node by bus address]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

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

> ---
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index
> ba98865b0c9b..2506f46293e8 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -610,6 +610,17 @@
>  			status = "disabled";
>  		};
> 
> +		vin4: video@e6ef4000 {
> +			compatible = "renesas,vin-r8a77995";
> +			reg = <0 0xe6ef4000 0 0x1000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 807>;
> +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> +			resets = <&cpg 807>;
> +			renesas,id = <4>;
> +			status = "disabled";
> +		};
> +
>  		ohci0: usb@ee080000 {
>  			compatible = "generic-ohci";
>  			reg = <0 0xee080000 0 0x100>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: Add VIN4

[Simon Horman]

On Mon, May 14, 2018 at 05:36:41AM +0300, Laurent Pinchart wrote:
> Hello,
> 
> On Friday, 11 May 2018 16:45:16 EEST Simon Horman wrote:
> > On Fri, May 11, 2018 at 01:25:23PM +0200, Niklas Söderlund wrote:
> > > Hi Jacopo,
> > > 
> > > Thanks for your work.
> > > 
> > > On 2018-05-11 12:00:01 +0200, Jacopo Mondi wrote:
> > > > Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> > > > 
> > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > 
> > > Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > 
> > >> ---
> > >> 
> > >>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 11 +++++++++++
> > >>  1 file changed, 11 insertions(+)
> > >> 
> > >> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > >> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index 82aed7e..bdf7017
> > >> 100644
> > >> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > >> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> > >> @@ -783,6 +783,17 @@
> > >>  				};
> > >>  			};
> > >>  		};
> > >> +
> > >> +		vin4: video@e6ef4000 {
> > >> +			compatible = "renesas,vin-r8a77995";
> > >> +			reg = <0 0xe6ef4000 0 0x1000>;
> > >> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > >> +			clocks = <&cpg CPG_MOD 807>;
> > >> +			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> > >> +			resets = <&cpg 807>;
> > >> +			renesas,id = <4>;
> > >> +			status = "disabled";
> > >> +		};
> > >>  	};
> > 
> > Thanks, I have moved the new node to preserve sorting of nodes by bus
> > address and applied the result. It is as follows:
> > 
> > From: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > Subject: [PATCH] arm64: dts: renesas: r8a77995: Add VIN4
> > 
> > Describe VIN4 interface for R-Car D3 R8A77995 SoC.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > [simon: sorted node by bus address]
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, tag added.

[PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Jacopo Mondi]

Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
development board.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index d03f194..e0ce462 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -59,6 +59,17 @@
 		};
 	};
 
+	hdmi-in {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi_con_in: endpoint {
+				remote-endpoint = <&adv7612_in>;
+			};
+		};
+	};
+
 	memory@48000000 {
 		device_type = "memory";
 		/* first 128MB is reserved for secure area. */
@@ -142,6 +153,11 @@
 		groups = "usb0";
 		function = "usb0";
 	};
+
+	vin4_pins: vin4 {
+		groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
+		function = "vin4";
+	};
 };
 
 &i2c0 {
@@ -154,6 +170,35 @@
 		reg = <0x50>;
 		pagesize = <8>;
 	};
+
+	hdmi-decoder@4c {
+		compatible = "adi,adv7612";
+		reg = <0x4c>;
+		default-input = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				adv7612_in: endpoint {
+					remote-endpoint = <&hdmi_con_in>;
+				};
+			};
+
+			port@2 {
+				reg = <2>;
+				adv7612_out: endpoint {
+					pclk-sample = <0>;
+					hsync-active = <0>;
+					vsync-active = <0>;
+
+					remote-endpoint = <&vin4_in>;
+				};
+			};
+		};
+	};
 };
 
 &i2c1 {
@@ -246,3 +291,26 @@
 	timeout-sec = <60>;
 	status = "okay";
 };
+
+&vin4 {
+	pinctrl-0 = <&vin4_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			vin4_in: endpoint {
+				hsync-active = <0>;
+				vsync-active = <0>;
+
+				remote-endpoint = <&adv7612_out>;
+			};
+		};
+	};
+};
-- 
2.7.4

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Simon Horman]

On Fri, May 11, 2018 at 12:00:02PM +0200, Jacopo Mondi wrote:
> Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> development board.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Hi Niklas,

As you reviewed the rest of the series I'm wondering if you're planning
to review this patch too.

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi Simon,

On 2018-05-13 10:17:50 +0200, Simon Horman wrote:
> On Fri, May 11, 2018 at 12:00:02PM +0200, Jacopo Mondi wrote:
> > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > development board.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> Hi Niklas,
> 
> As you reviewed the rest of the series I'm wondering if you're planning
> to review this patch too.

Yes, I did not have schematics for D3 on hand when reviewing the rest of 
the series. Will review it now that I do, thanks for the ping :-)

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> development board.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> index d03f194..e0ce462 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -59,6 +59,17 @@
>  		};
>  	};
>  
> +	hdmi-in {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_con_in: endpoint {
> +				remote-endpoint = <&adv7612_in>;
> +			};
> +		};
> +	};
> +
>  	memory@48000000 {
>  		device_type = "memory";
>  		/* first 128MB is reserved for secure area. */
> @@ -142,6 +153,11 @@
>  		groups = "usb0";
>  		function = "usb0";
>  	};
> +
> +	vin4_pins: vin4 {
> +		groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> +		function = "vin4";
> +	};
>  };
>  
>  &i2c0 {
> @@ -154,6 +170,35 @@
>  		reg = <0x50>;
>  		pagesize = <8>;
>  	};
> +
> +	hdmi-decoder@4c {
> +		compatible = "adi,adv7612";
> +		reg = <0x4c>;
> +		default-input = <0>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				adv7612_in: endpoint {
> +					remote-endpoint = <&hdmi_con_in>;
> +				};
> +			};
> +
> +			port@2 {
> +				reg = <2>;
> +				adv7612_out: endpoint {
> +					pclk-sample = <0>;
> +					hsync-active = <0>;
> +					vsync-active = <0>;

This differs from the Gen2 DT bindings which is a very similar hardware 
setup using the same components. Defining these properties will make the 
bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.

This will change how the hardware is configured for capture if the media 
bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe 
this it not an issue here but still I'm curious to why this differ 
between Gen2 and Gen3 :-)

> +
> +					remote-endpoint = <&vin4_in>;
> +				};
> +			};
> +		};
> +	};
>  };
>  
>  &i2c1 {
> @@ -246,3 +291,26 @@
>  	timeout-sec = <60>;
>  	status = "okay";
>  };
> +
> +&vin4 {
> +	pinctrl-0 = <&vin4_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			vin4_in: endpoint {
> +				hsync-active = <0>;
> +				vsync-active = <0>;

Comparing this to the Gen2 bindings some properties are missing,

bus-width = <24>;
pclk-sample = <1>;
data-active = <1>;

This is not a big deal as the VIN driver don't use these properties so 
no functional change should come of this but still a difference.  

Over all I'm happy with this change but before I add my tag I would like 
to understand why it differs from the Gen2 configuration for the adv7612 
properties.

Also on a side not it is possible with hardware switches on the board 
switch the VIN4 source to a completely different pipeline CVBS connector 
-> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as 
this seems to be how the boards are shipped. But maybe mentioning this 
in the commit message would not hurt if you end-up resending the patch.

> +
> +				remote-endpoint = <&adv7612_out>;
> +			};
> +		};
> +	};
> +};
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Laurent Pinchart]

Hello,

On Sunday, 13 May 2018 15:57:55 EEST Niklas Söderlund wrote:
> On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > development board.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > 
> >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 +++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index d03f194..e0ce462
> > 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > @@ -59,6 +59,17 @@
> > 
> >  		};
> >  	
> >  	};
> > 
> > +	hdmi-in {
> > +		compatible = "hdmi-connector";
> > +		type = "a";
> > +
> > +		port {
> > +			hdmi_con_in: endpoint {
> > +				remote-endpoint = <&adv7612_in>;
> > +			};
> > +		};
> > +	};
> > +
> > 
> >  	memory@48000000 {
> >  	
> >  		device_type = "memory";
> >  		/* first 128MB is reserved for secure area. */
> > 
> > @@ -142,6 +153,11 @@
> > 
> >  		groups = "usb0";
> >  		function = "usb0";
> >  	
> >  	};
> > 
> > +
> > +	vin4_pins: vin4 {
> > +		groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> > +		function = "vin4";
> > +	};
> > 
> >  };
> >  
> >  &i2c0 {
> > 
> > @@ -154,6 +170,35 @@
> > 
> >  		reg = <0x50>;
> >  		pagesize = <8>;
> >  	
> >  	};
> > 
> > +
> > +	hdmi-decoder@4c {
> > +		compatible = "adi,adv7612";
> > +		reg = <0x4c>;
> > +		default-input = <0>;
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +				adv7612_in: endpoint {
> > +					remote-endpoint = <&hdmi_con_in>;
> > +				};
> > +			};
> > +
> > +			port@2 {
> > +				reg = <2>;
> > +				adv7612_out: endpoint {
> > +					pclk-sample = <0>;
> > +					hsync-active = <0>;
> > +					vsync-active = <0>;
> 
> This differs from the Gen2 DT bindings which is a very similar hardware
> setup using the same components. Defining these properties will make the
> bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.
> 
> This will change how the hardware is configured for capture if the media
> bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe
> this it not an issue here but still I'm curious to why this differ
> between Gen2 and Gen3 :-)
> 
> > +
> > +					remote-endpoint = <&vin4_in>;
> > +				};
> > +			};
> > +		};
> > +	};
> > 
> >  };
> >  
> >  &i2c1 {
> > 
> > @@ -246,3 +291,26 @@
> > 
> >  	timeout-sec = <60>;
> >  	status = "okay";
> >  
> >  };
> > 
> > +
> > +&vin4 {
> > +	pinctrl-0 = <&vin4_pins>;
> > +	pinctrl-names = "default";
> > +
> > +	status = "okay";
> > +
> > +	ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		port@0 {
> > +			reg = <0>;
> > +
> > +			vin4_in: endpoint {
> > +				hsync-active = <0>;
> > +				vsync-active = <0>;
> 
> Comparing this to the Gen2 bindings some properties are missing,
> 
> bus-width = <24>;
> pclk-sample = <1>;
> data-active = <1>;
> 
> This is not a big deal as the VIN driver don't use these properties so
> no functional change should come of this but still a difference.

I think the VIN DT bindings should be updated to explicitly list the endpoint 
properties that are mandatory, optional, or not allowed.

> Over all I'm happy with this change but before I add my tag I would like
> to understand why it differs from the Gen2 configuration for the adv7612
> properties.
> 
> Also on a side not it is possible with hardware switches on the board
> switch the VIN4 source to a completely different pipeline CVBS connector
> -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> this seems to be how the boards are shipped. But maybe mentioning this
> in the commit message would not hurt if you end-up resending the patch.
> 
> > +
> > +				remote-endpoint = <&adv7612_out>;
> > +			};
> > +		};
> > +	};
> > +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi Laurent,

On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:

[snip]

> > > +&vin4 {
> > > +	pinctrl-0 = <&vin4_pins>;
> > > +	pinctrl-names = "default";
> > > +
> > > +	status = "okay";
> > > +
> > > +	ports {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		port@0 {
> > > +			reg = <0>;
> > > +
> > > +			vin4_in: endpoint {
> > > +				hsync-active = <0>;
> > > +				vsync-active = <0>;
> > 
> > Comparing this to the Gen2 bindings some properties are missing,
> > 
> > bus-width = <24>;
> > pclk-sample = <1>;
> > data-active = <1>;
> > 
> > This is not a big deal as the VIN driver don't use these properties so
> > no functional change should come of this but still a difference.
> 
> I think the VIN DT bindings should be updated to explicitly list the endpoint 
> properties that are mandatory, optional, or not allowed.

I think it's documented as it reference video-interfaces.txt which lists 
all these properties as optional. And in deed they are all optional.  If 
the VIN driver makes use of all the optional ones is another matter. How 
do we know that the remote subdevice is not looking at its remote 
endpoint for bus parameters not considered by the rcar-vin driver?

The thing is that the rcar-vin driver only looks at the remote endpoint 
for these properties and ignores the on its local endpoint. Maybe some 
v4l2 framework change is needed here to make sure the bus properties are 
the same on both endpoints of a link. But I fear such a change would 
break a lot of stuff.

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi again,

On 2018-05-14 11:49:00 +0200, Niklas Söderlund wrote:
> Hi Laurent,
> 
> On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:
> 
> [snip]
> 
> > > > +&vin4 {
> > > > +	pinctrl-0 = <&vin4_pins>;
> > > > +	pinctrl-names = "default";
> > > > +
> > > > +	status = "okay";
> > > > +
> > > > +	ports {
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		port@0 {
> > > > +			reg = <0>;
> > > > +
> > > > +			vin4_in: endpoint {
> > > > +				hsync-active = <0>;
> > > > +				vsync-active = <0>;
> > > 
> > > Comparing this to the Gen2 bindings some properties are missing,
> > > 
> > > bus-width = <24>;
> > > pclk-sample = <1>;
> > > data-active = <1>;
> > > 
> > > This is not a big deal as the VIN driver don't use these properties so
> > > no functional change should come of this but still a difference.
> > 
> > I think the VIN DT bindings should be updated to explicitly list the endpoint 
> > properties that are mandatory, optional, or not allowed.
> 
> I think it's documented as it reference video-interfaces.txt which lists 
> all these properties as optional. And in deed they are all optional.  If 
> the VIN driver makes use of all the optional ones is another matter. How 
> do we know that the remote subdevice is not looking at its remote 
> endpoint for bus parameters not considered by the rcar-vin driver?
> 
> The thing is that the rcar-vin driver only looks at the remote endpoint 
> for these properties and ignores the on its local endpoint. Maybe some 
> v4l2 framework change is needed here to make sure the bus properties are 
> the same on both endpoints of a link. But I fear such a change would 
> break a lot of stuff.

Jacopo pointed out this statement is untrue. The rcar-vin only looks at 
it's local endpoint not the remote endpoint for it's bus parameters. The 
callback provided to v4l2_async_notifier_parse_fwnode_endpoints() 
confused me as the subdevice passed to it is the one describe the remote 
endpoint while the v4l2_fwnode_endpoint argument is that of the local 
endpoint. Sorry for the confusion and thanks Jacopo for correcting me.

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Laurent Pinchart]

Hi Niklas,

On Monday, 14 May 2018 12:49:00 EEST Niklas Söderlund wrote:
> On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:
> 
> [snip]
> 
> >>> +&vin4 {
> >>> +	pinctrl-0 = <&vin4_pins>;
> >>> +	pinctrl-names = "default";
> >>> +
> >>> +	status = "okay";
> >>> +
> >>> +	ports {
> >>> +		#address-cells = <1>;
> >>> +		#size-cells = <0>;
> >>> +
> >>> +		port@0 {
> >>> +			reg = <0>;
> >>> +			vin4_in: endpoint {
> >>> +				hsync-active = <0>;
> >>> +				vsync-active = <0>;
> >> 
> >> Comparing this to the Gen2 bindings some properties are missing,
> >> 
> >> bus-width = <24>;
> >> pclk-sample = <1>;
> >> data-active = <1>;
> >> 
> >> This is not a big deal as the VIN driver don't use these properties so
> >> no functional change should come of this but still a difference.
> > 
> > I think the VIN DT bindings should be updated to explicitly list the
> > endpoint properties that are mandatory, optional, or not allowed.
> 
> I think it's documented as it reference video-interfaces.txt which lists
> all these properties as optional. And in deed they are all optional.

I don't think that's good enough. They're all listed as optional in video-
interfaces.txt as the generic documentation can't know whether a particular 
device will require a particular property or not. It's the responsibility of 
device DT bindings to refine the bindings description. The VIN DT bindings 
should explicitly list the properties that apply to the VIN and tell whether 
they're optional or mandatory for the VIN. For optional properties, the 
default behaviour when the property is not specified should be documented too.

For instance, does VIN support selecting which pixel clock edge to sample data 
on ? If so the pclk-sample property should listed as either mandatory or 
optional with a documented default, even if not used by the driver today.

> If the VIN driver makes use of all the optional ones is another matter. How
> do we know that the remote subdevice is not looking at its remote
> endpoint for bus parameters not considered by the rcar-vin driver?

No driver should parse properties of remote nodes, as those properties are to 
be interpreted in the context of the remote node's DT bindings, which the 
driver doesn't know about. Parsing OF graph properties (ports and endpoints) 
is an exception, as by connecting a remote node to the local node with OF 
graph properties you imply that the remote node uses OF graph DT bindings, so 
those properties (and only those properties) can be parsed.

> The thing is that the rcar-vin driver only looks at the remote endpoint
> for these properties and ignores the on its local endpoint. Maybe some
> v4l2 framework change is needed here to make sure the bus properties are
> the same on both endpoints of a link. But I fear such a change would
> break a lot of stuff.

Properties are specified on both endpoints to account for components such as 
inverter gates between the two devices. They can thus be different on the two 
sides, that's perfectly valid. The VIN driver should parse its local 
properties, not the remote properties.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[jacopo mondi]

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

Hi Niklas,

On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > development board.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > index d03f194..e0ce462 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > @@ -59,6 +59,17 @@
> >  		};
> >  	};
> >
> > +	hdmi-in {
> > +		compatible = "hdmi-connector";
> > +		type = "a";
> > +
> > +		port {
> > +			hdmi_con_in: endpoint {
> > +				remote-endpoint = <&adv7612_in>;
> > +			};
> > +		};
> > +	};
> > +
> >  	memory@48000000 {
> >  		device_type = "memory";
> >  		/* first 128MB is reserved for secure area. */
> > @@ -142,6 +153,11 @@
> >  		groups = "usb0";
> >  		function = "usb0";
> >  	};
> > +
> > +	vin4_pins: vin4 {
> > +		groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> > +		function = "vin4";
> > +	};
> >  };
> >
> >  &i2c0 {
> > @@ -154,6 +170,35 @@
> >  		reg = <0x50>;
> >  		pagesize = <8>;
> >  	};
> > +
> > +	hdmi-decoder@4c {
> > +		compatible = "adi,adv7612";
> > +		reg = <0x4c>;
> > +		default-input = <0>;
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +				adv7612_in: endpoint {
> > +					remote-endpoint = <&hdmi_con_in>;
> > +				};
> > +			};
> > +
> > +			port@2 {
> > +				reg = <2>;
> > +				adv7612_out: endpoint {
> > +					pclk-sample = <0>;
> > +					hsync-active = <0>;
> > +					vsync-active = <0>;
>
> This differs from the Gen2 DT bindings which is a very similar hardware
> setup using the same components. Defining these properties will make the
> bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.

And that's what we want....

>
> This will change how the hardware is configured for capture if the media
> bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe
> this it not an issue here but still I'm curious to why this differ
> between Gen2 and Gen3 :-)

Actually this won't impact the VIN configuration as this is the
'remote endpoint' from VIN perspective and the properties used to
configure the interface are the ones in the 'local endpoint'.

>
> > +
> > +					remote-endpoint = <&vin4_in>;
> > +				};
> > +			};
> > +		};
> > +	};
> >  };
> >
> >  &i2c1 {
> > @@ -246,3 +291,26 @@
> >  	timeout-sec = <60>;
> >  	status = "okay";
> >  };
> > +
> > +&vin4 {
> > +	pinctrl-0 = <&vin4_pins>;
> > +	pinctrl-names = "default";
> > +
> > +	status = "okay";
> > +
> > +	ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		port@0 {
> > +			reg = <0>;
> > +
> > +			vin4_in: endpoint {
> > +				hsync-active = <0>;
> > +				vsync-active = <0>;
>
> Comparing this to the Gen2 bindings some properties are missing,
>
> bus-width = <24>;
> pclk-sample = <1>;
> data-active = <1>;

The VIN driver does not parse them, so there is no value in having
them there, if not confusing people as it happened to me reading the
Gen2 DT.

>
> This is not a big deal as the VIN driver don't use these properties so
> no functional change should come of this but still a difference.

Exactly.

On a side note. I have not seen a way to configure the pixel clock
sampling level in the interface datasheet. The register used to
configure synchronism signals polarities is VnDMR2, and there I read
we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
pixel clock) polarities. Is it configured through some other
register?
>
> Over all I'm happy with this change but before I add my tag I would like
> to understand why it differs from the Gen2 configuration for the adv7612
> properties.
>
> Also on a side not it is possible with hardware switches on the board
> switch the VIN4 source to a completely different pipeline CVBS connector
> -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> this seems to be how the boards are shipped. But maybe mentioning this
> in the commit message would not hurt if you end-up resending the patch.

Oh I see. SW-49 to SW-52 enables the HDMI input, SW53-SW54 CVBS one.
And actually, reading the 'initial setting of slide switches' in the
Draak board manual, it turns out that the board default configuration
is with CVBS input selected... What should we do here? reflect
defaults in the DT, or prioritize HDMI?

Thanks
   j

>
> > +
> > +				remote-endpoint = <&adv7612_out>;
> > +			};
> > +		};
> > +	};
> > +};
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Niklas Söderlund]

Hi Jacopo,

On 2018-05-14 09:39:34 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your patch.
> >
> > On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> > > Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> > > development board.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 ++++++++++++++++++++++++++
> > >  1 file changed, 68 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > index d03f194..e0ce462 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > > @@ -59,6 +59,17 @@
> > >  		};
> > >  	};
> > >
> > > +	hdmi-in {
> > > +		compatible = "hdmi-connector";
> > > +		type = "a";
> > > +
> > > +		port {
> > > +			hdmi_con_in: endpoint {
> > > +				remote-endpoint = <&adv7612_in>;
> > > +			};
> > > +		};
> > > +	};
> > > +
> > >  	memory@48000000 {
> > >  		device_type = "memory";
> > >  		/* first 128MB is reserved for secure area. */
> > > @@ -142,6 +153,11 @@
> > >  		groups = "usb0";
> > >  		function = "usb0";
> > >  	};
> > > +
> > > +	vin4_pins: vin4 {
> > > +		groups = "vin4_data24", "vin4_sync", "vin4_clk", "vin4_clkenb";
> > > +		function = "vin4";
> > > +	};
> > >  };
> > >
> > >  &i2c0 {
> > > @@ -154,6 +170,35 @@
> > >  		reg = <0x50>;
> > >  		pagesize = <8>;
> > >  	};
> > > +
> > > +	hdmi-decoder@4c {
> > > +		compatible = "adi,adv7612";
> > > +		reg = <0x4c>;
> > > +		default-input = <0>;
> > > +
> > > +		ports {
> > > +			#address-cells = <1>;
> > > +			#size-cells = <0>;
> > > +
> > > +			port@0 {
> > > +				reg = <0>;
> > > +				adv7612_in: endpoint {
> > > +					remote-endpoint = <&hdmi_con_in>;
> > > +				};
> > > +			};
> > > +
> > > +			port@2 {
> > > +				reg = <2>;
> > > +				adv7612_out: endpoint {
> > > +					pclk-sample = <0>;
> > > +					hsync-active = <0>;
> > > +					vsync-active = <0>;
> >
> > This differs from the Gen2 DT bindings which is a very similar hardware
> > setup using the same components. Defining these properties will make the
> > bus marked as V4L2_MBUS_PARALLEL instead of V4L2_MBUS_BT656.
> 
> And that's what we want....
> 
> >
> > This will change how the hardware is configured for capture if the media
> > bus is in a UYVY format, see VNMC_INF register in rvin_setup(). Maybe
> > this it not an issue here but still I'm curious to why this differ
> > between Gen2 and Gen3 :-)
> 
> Actually this won't impact the VIN configuration as this is the
> 'remote endpoint' from VIN perspective and the properties used to
> configure the interface are the ones in the 'local endpoint'.

You are right, sorry for the confusion and thanks for educating me :-)

> 
> >
> > > +
> > > +					remote-endpoint = <&vin4_in>;
> > > +				};
> > > +			};
> > > +		};
> > > +	};
> > >  };
> > >
> > >  &i2c1 {
> > > @@ -246,3 +291,26 @@
> > >  	timeout-sec = <60>;
> > >  	status = "okay";
> > >  };
> > > +
> > > +&vin4 {
> > > +	pinctrl-0 = <&vin4_pins>;
> > > +	pinctrl-names = "default";
> > > +
> > > +	status = "okay";
> > > +
> > > +	ports {
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>;
> > > +
> > > +		port@0 {
> > > +			reg = <0>;
> > > +
> > > +			vin4_in: endpoint {
> > > +				hsync-active = <0>;
> > > +				vsync-active = <0>;
> >
> > Comparing this to the Gen2 bindings some properties are missing,
> >
> > bus-width = <24>;
> > pclk-sample = <1>;
> > data-active = <1>;
> 
> The VIN driver does not parse them, so there is no value in having
> them there, if not confusing people as it happened to me reading the
> Gen2 DT.

I have no objection removing them. Trying to understand why the 
description differed from Gen2.

> 
> >
> > This is not a big deal as the VIN driver don't use these properties so
> > no functional change should come of this but still a difference.
> 
> Exactly.
> 
> On a side note. I have not seen a way to configure the pixel clock
> sampling level in the interface datasheet. The register used to
> configure synchronism signals polarities is VnDMR2, and there I read
> we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
> pixel clock) polarities. Is it configured through some other
> register?

I have not seen such a register no.

> >
> > Over all I'm happy with this change but before I add my tag I would like
> > to understand why it differs from the Gen2 configuration for the adv7612
> > properties.
> >
> > Also on a side not it is possible with hardware switches on the board
> > switch the VIN4 source to a completely different pipeline CVBS connector
> > -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> > this seems to be how the boards are shipped. But maybe mentioning this
> > in the commit message would not hurt if you end-up resending the patch.
> 
> Oh I see. SW-49 to SW-52 enables the HDMI input, SW53-SW54 CVBS one.
> And actually, reading the 'initial setting of slide switches' in the
> Draak board manual, it turns out that the board default configuration
> is with CVBS input selected... What should we do here? reflect
> defaults in the DT, or prioritize HDMI?

I feel this is a question for Laurent. My feeling for how we handled 
this in other cases is to go with the board default settings. I'm 
however sure there are exceptions to the rule. So maybe we should go 
with the most useful (what ever that is) configuration?

> 
> Thanks
>    j
> 
> >
> > > +
> > > +				remote-endpoint = <&adv7612_out>;
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Regards,
> > Niklas Söderlund



-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Laurent Pinchart]

Hi Niklas,

On Monday, 14 May 2018 13:23:26 EEST Niklas Söderlund wrote:
> On 2018-05-14 09:39:34 +0200, Jacopo Mondi wrote:
> > On Sun, May 13, 2018 at 02:57:55PM +0200, Niklas Söderlund wrote:
> >> On 2018-05-11 12:00:02 +0200, Jacopo Mondi wrote:
> >>> Describe HDMI input connected to VIN4 interface for R-Car D3 Draak
> >>> development board.
> >>> 
> >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >>> ---
> >>> 
> >>>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 68 +++++++++++++++++++
> >>>  1 file changed, 68 insertions(+)
> >>> 
> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> >>> d03f194..e0ce462 100644
> >>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts

[snip]

> >>> +&vin4 {
> >>> +	pinctrl-0 = <&vin4_pins>;
> >>> +	pinctrl-names = "default";
> >>> +
> >>> +	status = "okay";
> >>> +
> >>> +	ports {
> >>> +		#address-cells = <1>;
> >>> +		#size-cells = <0>;
> >>> +
> >>> +		port@0 {
> >>> +			reg = <0>;
> >>> +
> >>> +			vin4_in: endpoint {
> >>> +				hsync-active = <0>;
> >>> +				vsync-active = <0>;
> >> 
> >> Comparing this to the Gen2 bindings some properties are missing,
> >> 
> >> bus-width = <24>;
> >> pclk-sample = <1>;
> >> data-active = <1>;
> > 
> > The VIN driver does not parse them, so there is no value in having
> > them there, if not confusing people as it happened to me reading the
> > Gen2 DT.
> 
> I have no objection removing them. Trying to understand why the
> description differed from Gen2.
> 
> >> This is not a big deal as the VIN driver don't use these properties so
> >> no functional change should come of this but still a difference.
> > 
> > Exactly.
> > 
> > On a side note. I have not seen a way to configure the pixel clock
> > sampling level in the interface datasheet. The register used to
> > configure synchronism signals polarities is VnDMR2, and there I read
> > we can configure HSYNC/VSYNC and CLOCKENB (which is data enable, not
> > pixel clock) polarities. Is it configured through some other
> > register?
> 
> I have not seen such a register no.
> 
> >> Over all I'm happy with this change but before I add my tag I would like
> >> to understand why it differs from the Gen2 configuration for the adv7612
> >> properties.
> >> 
> >> Also on a side not it is possible with hardware switches on the board
> >> switch the VIN4 source to a completely different pipeline CVBS connector
> >> -> adv7180 -> VIN4. But I think it's best we keep the HDMI as default as
> >> this seems to be how the boards are shipped. But maybe mentioning this
> >> in the commit message would not hurt if you end-up resending the patch.
> > 
> > Oh I see. SW-49 to SW-52 enables the HDMI input, SW53-SW54 CVBS one.
> > And actually, reading the 'initial setting of slide switches' in the
> > Draak board manual, it turns out that the board default configuration
> > is with CVBS input selected... What should we do here? reflect
> > defaults in the DT, or prioritize HDMI?
> 
> I feel this is a question for Laurent. My feeling for how we handled
> this in other cases is to go with the board default settings. I'm
> however sure there are exceptions to the rule. So maybe we should go
> with the most useful (what ever that is) configuration?

I think I'd go with CVBS as I don't think HDMI would be considered as the most 
useful configuration here. The Draak board is unlikely to be used by us as a 
reference platform to test HDMI capture, is it ?

This being said, you can instantiate the adv7612 and HDMI connector in DT, 
without connecting them to the VIN. That would make it easy to quickly change 
the configuration.

> >>> +
> >>> +				remote-endpoint = <&adv7612_out>;
> >>> +			};
> >>> +		};
> >>> +	};
> >>> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/3] arm64: dts: Draak: Enable HDMI input and VIN4

[Geert Uytterhoeven]

Hi Jacopo,

On Fri, May 11, 2018 at 11:59 AM, Jacopo Mondi
<jacopo+renesas@jmondi.org> wrote:
>    this series enables HDMI input and VIN4 on R-Car D3 Draak board.
>
> The Draak board has an HDMI input connected to an HDMI decoder that feeds
> the VIN capture interface through its parallel video interface.
>
> The series requires the just sent:
> [PATCH 0/5] rcar-vin: Add support for digital input on Gen3
>
> and enables image capture operations on D3 Draak board.
>
> The series has been developed on top of media-master tree but applies cleanly
> on top of latest renesas-driver.
>
> Geert: would you like a topic branch for this series to be included in
> renesas-drivers?

It seems patch 2 has been applied by Simon already, but there is some
discussion pending on patch 3?

> Patches for testing are available at:
> git://jmondi.org/linux d3/media-master/driver
> git://jmondi.org/linux d3/media-master/dts
> git://jmondi.org/linux d3/media-master/test
> git://jmondi.org/vin-tests d3
>
> Thanks
>     j
>
> Jacopo Mondi (3):
>   dt-bindings: media: rcar-vin: Add R8A77995 support
>   arm64: dts: renesas: r8a77995: Add VIN4
>   arm64: dts: renesas: draak: Describe HDMI input
>
>  .../devicetree/bindings/media/rcar_vin.txt         |  1 +
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts     | 68 ++++++++++++++++++++++
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi          | 11 ++++
>  3 files changed, 80 insertions(+)

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH 0/3] arm64: dts: Draak: Enable HDMI input and VIN4

[Simon Horman]

On Mon, May 14, 2018 at 10:33:44PM +0200, Geert Uytterhoeven wrote:
> Hi Jacopo,
> 
> On Fri, May 11, 2018 at 11:59 AM, Jacopo Mondi
> <jacopo+renesas@jmondi.org> wrote:
> >    this series enables HDMI input and VIN4 on R-Car D3 Draak board.
> >
> > The Draak board has an HDMI input connected to an HDMI decoder that feeds
> > the VIN capture interface through its parallel video interface.
> >
> > The series requires the just sent:
> > [PATCH 0/5] rcar-vin: Add support for digital input on Gen3
> >
> > and enables image capture operations on D3 Draak board.
> >
> > The series has been developed on top of media-master tree but applies cleanly
> > on top of latest renesas-driver.
> >
> > Geert: would you like a topic branch for this series to be included in
> > renesas-drivers?
> 
> It seems patch 2 has been applied by Simon already, but there is some
> discussion pending on patch 3?

Yes, that is correct.

Also, for extra fun, I moved the nodes when applying patch 2.

> > Patches for testing are available at:
> > git://jmondi.org/linux d3/media-master/driver
> > git://jmondi.org/linux d3/media-master/dts
> > git://jmondi.org/linux d3/media-master/test
> > git://jmondi.org/vin-tests d3
> >
> > Thanks
> >     j
> >
> > Jacopo Mondi (3):
> >   dt-bindings: media: rcar-vin: Add R8A77995 support
> >   arm64: dts: renesas: r8a77995: Add VIN4
> >   arm64: dts: renesas: draak: Describe HDMI input
> >
> >  .../devicetree/bindings/media/rcar_vin.txt         |  1 +
> >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts     | 68 ++++++++++++++++++++++
> >  arch/arm64/boot/dts/renesas/r8a77995.dtsi          | 11 ++++
> >  3 files changed, 80 insertions(+)
> 
> 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
>