[PATCH 0/4] R-Car Salvator-X M3-W: Enable DU support

[PATCH 0/4] R-Car Salvator-X M3-W: Enable DU support

[Laurent Pinchart]

Hello,

This patch series enables DU support for the M3-W-based Salvator-X board. It
depends on the previously posted "[PATCH 0/3] R-Car M3-W: Add FCP, VSP and DU
clocks", "[PATCH] drm: rcar-du: Add R8A7796 device support" and "[PATCH 0/2]
R-Car FDP: Remove SoC-specific compatible strings" patch series.

For convenience I've pushed all patches to

	git://linuxtv.org/pinchartl/media.git drm/r8a7796/next

Patch 4/4 enables the DU and should not be merged at this time as it would
block support of memory above the 32bit 4GB boundary. All other patches are
candidates for v4.10.

Laurent Pinchart (4):
  arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances
  arm64: dts: renesas: r8a7796: Add VSP instances
  arm64: dts: renesas: r8a7796: Add DU device to DT
  arm64: dts: renesas: r8a7796-salvator-x: Enable DU

 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |  48 ++++++++
 arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 131 +++++++++++++++++++++
 2 files changed, 179 insertions(+)

-- 
Regards,

Laurent Pinchart

[PATCH 1/4] arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances

[Laurent Pinchart]

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

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 9217da983525..39b6ec7a2068 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -251,5 +251,47 @@
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
 			status = "disabled";
 		};
+
+		fcpf0: fcp@fe950000 {
+			compatible = "renesas,fcpf";
+			reg = <0 0xfe950000 0 0x200>;
+			clocks = <&cpg CPG_MOD 615>;
+			power-domains = <&sysc R8A7796_PD_A3VC>;
+		};
+
+		fcpvb0: fcp@fe96f000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfe96f000 0 0x200>;
+			clocks = <&cpg CPG_MOD 607>;
+			power-domains = <&sysc R8A7796_PD_A3VC>;
+		};
+
+		fcpvi0: fcp@fe9af000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfe9af000 0 0x200>;
+			clocks = <&cpg CPG_MOD 611>;
+			power-domains = <&sysc R8A7796_PD_A3VC>;
+		};
+
+		fcpvd0: fcp@fea27000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea27000 0 0x200>;
+			clocks = <&cpg CPG_MOD 603>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+		};
+
+		fcpvd1: fcp@fea2f000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea2f000 0 0x200>;
+			clocks = <&cpg CPG_MOD 602>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+		};
+
+		fcpvd2: fcp@fea37000 {
+			compatible = "renesas,fcpv";
+			reg = <0 0xfea37000 0 0x200>;
+			clocks = <&cpg CPG_MOD 601>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+		};
 	};
 };
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances

[Geert Uytterhoeven]

On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The FCPs handle the interface between various IP cores and memory. Add
> the instances related to the FDPs and VSP2s.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

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

[PATCH 2/4] arm64: dts: renesas: r8a7796: Add VSP instances

[Laurent Pinchart]

The r8a7796 has 5 VSP instances.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 50 ++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 39b6ec7a2068..15ab8f22b397 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -259,6 +259,16 @@
 			power-domains = <&sysc R8A7796_PD_A3VC>;
 		};
 
+		vspb: vsp@fe960000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfe960000 0 0x8000>;
+			interrupts = <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 626>;
+			power-domains = <&sysc R8A7796_PD_A3VC>;
+
+			renesas,fcp = <&fcpvb0>;
+		};
+
 		fcpvb0: fcp@fe96f000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfe96f000 0 0x200>;
@@ -266,6 +276,16 @@
 			power-domains = <&sysc R8A7796_PD_A3VC>;
 		};
 
+		vspi0: vsp@fe9a0000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfe9a0000 0 0x8000>;
+			interrupts = <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 631>;
+			power-domains = <&sysc R8A7796_PD_A3VC>;
+
+			renesas,fcp = <&fcpvi0>;
+		};
+
 		fcpvi0: fcp@fe9af000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfe9af000 0 0x200>;
@@ -273,6 +293,16 @@
 			power-domains = <&sysc R8A7796_PD_A3VC>;
 		};
 
+		vspd0: vsp@fea20000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea20000 0 0x4000>;
+			interrupts = <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 623>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+
+			renesas,fcp = <&fcpvd0>;
+		};
+
 		fcpvd0: fcp@fea27000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfea27000 0 0x200>;
@@ -280,6 +310,16 @@
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
 		};
 
+		vspd1: vsp@fea28000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea28000 0 0x4000>;
+			interrupts = <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 622>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+
+			renesas,fcp = <&fcpvd1>;
+		};
+
 		fcpvd1: fcp@fea2f000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfea2f000 0 0x200>;
@@ -287,6 +327,16 @@
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
 		};
 
+		vspd2: vsp@fea30000 {
+			compatible = "renesas,vsp2";
+			reg = <0 0xfea30000 0 0x4000>;
+			interrupts = <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 621>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+
+			renesas,fcp = <&fcpvd2>;
+		};
+
 		fcpvd2: fcp@fea37000 {
 			compatible = "renesas,fcpv";
 			reg = <0 0xfea37000 0 0x200>;
-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] arm64: dts: renesas: r8a7796: Add VSP instances

[Geert Uytterhoeven]

On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The r8a7796 has 5 VSP instances.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

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

[PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Laurent Pinchart]

Add the DU device to r8a7796.dtsi in a disabled state.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7796.dtsi | 39 ++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 15ab8f22b397..0a869f491dbc 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -343,5 +343,44 @@
 			clocks = <&cpg CPG_MOD 601>;
 			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
 		};
+
+		du: display@feb00000 {
+			compatible = "renesas,du-r8a7796";
+			reg = <0 0xfeb00000 0 0x70000>,
+			      <0 0xfeb90000 0 0x14>;
+			reg-names = "du", "lvds.0";
+			interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 724>,
+				 <&cpg CPG_MOD 723>,
+				 <&cpg CPG_MOD 722>,
+				 <&cpg CPG_MOD 727>;
+			clock-names = "du.0", "du.1", "du.2", "lvds.0";
+			status = "disabled";
+
+			vsps = <&vspd0 &vspd1 &vspd2>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					du_out_rgb: endpoint {
+					};
+				};
+				port@1 {
+					reg = <1>;
+					du_out_hdmi0: endpoint {
+					};
+				};
+				port@2 {
+					reg = <2>;
+					du_out_lvds0: endpoint {
+					};
+				};
+			};
+		};
 	};
 };
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Geert Uytterhoeven]

On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Add the DU device to r8a7796.dtsi in a disabled state.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

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

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Laurent Pinchart]

Hi Simon,

On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> > Add the DU device to r8a7796.dtsi in a disabled state.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Could you please pick patches 1/4 to 3/4 from this series and apply them to 
your tree ? For convenience I've pushed them to

	git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt

along with patch "arm64: dts: renesas: r8a7795: Remove FCP SoC-specific 
compatible strings" that has been acked too. If you pull from that branch 
please make sure you skip the top-most patch "arm64: dts: renesas: r8a7796-
salvator-x: Enable DU" for now.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Simon Horman]

On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> > On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> > > Add the DU device to r8a7796.dtsi in a disabled state.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Could you please pick patches 1/4 to 3/4 from this series and apply them to 
> your tree ? For convenience I've pushed them to
> 
> 	git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
> 
> along with patch "arm64: dts: renesas: r8a7795: Remove FCP SoC-specific 
> compatible strings" that has been acked too. If you pull from that branch 
> please make sure you skip the top-most patch "arm64: dts: renesas: r8a7796-
> salvator-x: Enable DU" for now.

Sure, done.

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Magnus Damm]

Hi Simon, everyone,

On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman <horms@verge.net.au> wrote:
> On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
>> Hi Simon,
>>
>> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
>> > On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
>> > > Add the DU device to r8a7796.dtsi in a disabled state.
>> > >
>> > > Signed-off-by: Laurent Pinchart
>> > > <laurent.pinchart+renesas@ideasonboard.com>
>> >
>> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> Could you please pick patches 1/4 to 3/4 from this series and apply them to
>> your tree ? For convenience I've pushed them to
>>
>>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
>>
>> along with patch "arm64: dts: renesas: r8a7795: Remove FCP SoC-specific
>> compatible strings" that has been acked too. If you pull from that branch
>> please make sure you skip the top-most patch "arm64: dts: renesas: r8a7796-
>> salvator-x: Enable DU" for now.
>
> Sure, done.

I think we should hold off with the upstreaming of the DU and VSP
integration code for now. Sorry for noticing this late, but I thought
we had already discussed the integration order and that merge of
non-64-bit capable devices need to be put on hold.

In particular, not so much the DU device (this patch) but more the VSP
instances. The reason for that is that VSP devices can only perform
32-bit bus mastering without IOMMU. I believe next step for all this
would be to enable all on-board memory on r8a7796 Salvator-x, but I
think Geert is working on that.

Regarding IOMMU support, more effort is needed to get the IPMMU driver
with r8a7796 (and r8a7795) support upstream. Some stuff for r8a7795
exists in renesas-drivers and experimental r8a7796 support as well.

One issue is that the DU driver itself is not ready to be used with
IOMMU yet (below patches not upstream yet), and more work is needed as
well - please see my reply (and prototype patch that fixes the issue
locally):
[PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP
http://www.spinics.net/lists/dri-devel/msg117494.html

Also, the r8a7796 DU driver code is not yet merged upstream and the
binding is also not yet documented. So either way you look at it it
seems a tad early for upstream merge.

Thanks,

/ magnus

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Geert Uytterhoeven]

Hi Magnus,

On Thu, Oct 27, 2016 at 8:52 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> In particular, not so much the DU device (this patch) but more the VSP
> instances. The reason for that is that VSP devices can only perform
> 32-bit bus mastering without IOMMU. I believe next step for all this
> would be to enable all on-board memory on r8a7796 Salvator-x, but I
> think Geert is working on that.

Actually 64-bit memory is already enabled on r8a7796/salvator-x, as U-Boot
(all firmware versions I've seen) silently adds the missing memory chunks
to the DTB.

The same is true on r8a7795/salvator-x with recent firmware (e.g. v2.12.0).

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 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Magnus Damm]

Hi Geert,

On Thu, Oct 27, 2016 at 4:04 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Thu, Oct 27, 2016 at 8:52 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> In particular, not so much the DU device (this patch) but more the VSP
>> instances. The reason for that is that VSP devices can only perform
>> 32-bit bus mastering without IOMMU. I believe next step for all this
>> would be to enable all on-board memory on r8a7796 Salvator-x, but I
>> think Geert is working on that.
>
> Actually 64-bit memory is already enabled on r8a7796/salvator-x, as U-Boot
> (all firmware versions I've seen) silently adds the missing memory chunks
> to the DTB.
>
> The same is true on r8a7795/salvator-x with recent firmware (e.g. v2.12.0).

Ok, so we should figure out a reasonable policy how to handle this in
a good way.

Shall we describe all the memory in the DTS file for consistency? Or
no memory at all? Describing some memory seems a bit like a strange
middle ground in my opinion.

Cheers,

/ magnus

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Simon Horman]

On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
> Hi Simon, everyone,
> 
> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
> >> Hi Simon,
> >>
> >> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> >> > On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> >> > > Add the DU device to r8a7796.dtsi in a disabled state.
> >> > >
> >> > > Signed-off-by: Laurent Pinchart
> >> > > <laurent.pinchart+renesas@ideasonboard.com>
> >> >
> >> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>
> >> Could you please pick patches 1/4 to 3/4 from this series and apply them to
> >> your tree ? For convenience I've pushed them to
> >>
> >>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
> >>
> >> along with patch "arm64: dts: renesas: r8a7795: Remove FCP SoC-specific
> >> compatible strings" that has been acked too. If you pull from that branch
> >> please make sure you skip the top-most patch "arm64: dts: renesas: r8a7796-
> >> salvator-x: Enable DU" for now.
> >
> > Sure, done.
> 
> I think we should hold off with the upstreaming of the DU and VSP
> integration code for now. Sorry for noticing this late, but I thought
> we had already discussed the integration order and that merge of
> non-64-bit capable devices need to be put on hold.
> 
> In particular, not so much the DU device (this patch) but more the VSP
> instances. The reason for that is that VSP devices can only perform
> 32-bit bus mastering without IOMMU. I believe next step for all this
> would be to enable all on-board memory on r8a7796 Salvator-x, but I
> think Geert is working on that.

So you would like to see the following patches dropped?

ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances

> Regarding IOMMU support, more effort is needed to get the IPMMU driver
> with r8a7796 (and r8a7795) support upstream. Some stuff for r8a7795
> exists in renesas-drivers and experimental r8a7796 support as well.
> 
> One issue is that the DU driver itself is not ready to be used with
> IOMMU yet (below patches not upstream yet), and more work is needed as
> well - please see my reply (and prototype patch that fixes the issue
> locally):
> [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP
> http://www.spinics.net/lists/dri-devel/msg117494.html
> 
> Also, the r8a7796 DU driver code is not yet merged upstream and the
> binding is also not yet documented. So either way you look at it it
> seems a tad early for upstream merge.
> 
> Thanks,
> 
> / magnus
> 

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Magnus Damm]

Hi Simon,

On Thu, Oct 27, 2016 at 4:13 PM, Simon Horman <horms@verge.net.au> wrote:
> On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
>> Hi Simon, everyone,
>>
>> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
>> >> Hi Simon,
>> >>
>> >> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
>> >> > On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
>> >> > > Add the DU device to r8a7796.dtsi in a disabled state.
>> >> > >
>> >> > > Signed-off-by: Laurent Pinchart
>> >> > > <laurent.pinchart+renesas@ideasonboard.com>
>> >> >
>> >> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >>
>> >> Could you please pick patches 1/4 to 3/4 from this series and apply them to
>> >> your tree ? For convenience I've pushed them to
>> >>
>> >>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
>> >>
>> >> along with patch "arm64: dts: renesas: r8a7795: Remove FCP SoC-specific
>> >> compatible strings" that has been acked too. If you pull from that branch
>> >> please make sure you skip the top-most patch "arm64: dts: renesas: r8a7796-
>> >> salvator-x: Enable DU" for now.
>> >
>> > Sure, done.
>>
>> I think we should hold off with the upstreaming of the DU and VSP
>> integration code for now. Sorry for noticing this late, but I thought
>> we had already discussed the integration order and that merge of
>> non-64-bit capable devices need to be put on hold.
>>
>> In particular, not so much the DU device (this patch) but more the VSP
>> instances. The reason for that is that VSP devices can only perform
>> 32-bit bus mastering without IOMMU. I believe next step for all this
>> would be to enable all on-board memory on r8a7796 Salvator-x, but I
>> think Geert is working on that.
>
> So you would like to see the following patches dropped?
>
> ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
> 0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances

Yes, that would be great. Sorry for not noticing earlier.

Thanks,

/ magnus

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Simon Horman]

On Thu, Oct 27, 2016 at 04:25:35PM +0900, Magnus Damm wrote:
> Hi Simon,
> 
> On Thu, Oct 27, 2016 at 4:13 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
> >> Hi Simon, everyone,
> >>
> >> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
> >> >> Hi Simon,
> >> >>
> >> >> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> >> >> > On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> >> >> > > Add the DU device to r8a7796.dtsi in a disabled state.
> >> >> > >
> >> >> > > Signed-off-by: Laurent Pinchart
> >> >> > > <laurent.pinchart+renesas@ideasonboard.com>
> >> >> >
> >> >> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> >>
> >> >> Could you please pick patches 1/4 to 3/4 from this series and apply them to
> >> >> your tree ? For convenience I've pushed them to
> >> >>
> >> >>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
> >> >>
> >> >> along with patch "arm64: dts: renesas: r8a7795: Remove FCP SoC-specific
> >> >> compatible strings" that has been acked too. If you pull from that branch
> >> >> please make sure you skip the top-most patch "arm64: dts: renesas: r8a7796-
> >> >> salvator-x: Enable DU" for now.
> >> >
> >> > Sure, done.
> >>
> >> I think we should hold off with the upstreaming of the DU and VSP
> >> integration code for now. Sorry for noticing this late, but I thought
> >> we had already discussed the integration order and that merge of
> >> non-64-bit capable devices need to be put on hold.
> >>
> >> In particular, not so much the DU device (this patch) but more the VSP
> >> instances. The reason for that is that VSP devices can only perform
> >> 32-bit bus mastering without IOMMU. I believe next step for all this
> >> would be to enable all on-board memory on r8a7796 Salvator-x, but I
> >> think Geert is working on that.
> >
> > So you would like to see the following patches dropped?
> >
> > ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
> > 0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances
> 
> Yes, that would be great. Sorry for not noticing earlier.

No problem, I will make it so.

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Simon Horman]

On Thu, Oct 27, 2016 at 04:25:35PM +0900, Magnus Damm wrote:
> Hi Simon,
> 
> On Thu, Oct 27, 2016 at 4:13 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
> >> Hi Simon, everyone,
> >>
> >> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
> >> >> Hi Simon,
> >> >>
> >> >> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> >> >> > On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> >> >> > > Add the DU device to r8a7796.dtsi in a disabled state.
> >> >> > >
> >> >> > > Signed-off-by: Laurent Pinchart
> >> >> > > <laurent.pinchart+renesas@ideasonboard.com>
> >> >> >
> >> >> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> >>
> >> >> Could you please pick patches 1/4 to 3/4 from this series and apply them to
> >> >> your tree ? For convenience I've pushed them to
> >> >>
> >> >>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
> >> >>
> >> >> along with patch "arm64: dts: renesas: r8a7795: Remove FCP SoC-specific
> >> >> compatible strings" that has been acked too. If you pull from that branch
> >> >> please make sure you skip the top-most patch "arm64: dts: renesas: r8a7796-
> >> >> salvator-x: Enable DU" for now.
> >> >
> >> > Sure, done.
> >>
> >> I think we should hold off with the upstreaming of the DU and VSP
> >> integration code for now. Sorry for noticing this late, but I thought
> >> we had already discussed the integration order and that merge of
> >> non-64-bit capable devices need to be put on hold.
> >>
> >> In particular, not so much the DU device (this patch) but more the VSP
> >> instances. The reason for that is that VSP devices can only perform
> >> 32-bit bus mastering without IOMMU. I believe next step for all this
> >> would be to enable all on-board memory on r8a7796 Salvator-x, but I
> >> think Geert is working on that.
> >
> > So you would like to see the following patches dropped?
> >
> > ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
> > 0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances
> 
> Yes, that would be great. Sorry for not noticing earlier.

I have also dropped the following patch as the vsp nodes it uses
no longer exist.

1a349bbbbb1c arm64: dts: renesas: r8a7796: Add DU device to DT

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Laurent Pinchart]

Hi Magnus,

On Thursday 27 Oct 2016 16:25:35 Magnus Damm wrote:
> On Thu, Oct 27, 2016 at 4:13 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
> >> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman <horms@verge.net.au> wrote:
> >>> On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
> >>>> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> >>>>> On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> >>>>>> Add the DU device to r8a7796.dtsi in a disabled state.
> >>>>>> 
> >>>>>> Signed-off-by: Laurent Pinchart
> >>>>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>>> 
> >>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>> 
> >>>> Could you please pick patches 1/4 to 3/4 from this series and apply
> >>>> them to your tree ? For convenience I've pushed them to
> >>>> 
> >>>>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
> >>>> 
> >>>> along with patch "arm64: dts: renesas: r8a7795: Remove FCP
> >>>> SoC-specific compatible strings" that has been acked too. If you pull
> >>>> from that branch please make sure you skip the top-most patch "arm64:
> >>>> dts: renesas: r8a7796-salvator-x: Enable DU" for now.
> >>> 
> >>> Sure, done.
> >> 
> >> I think we should hold off with the upstreaming of the DU and VSP
> >> integration code for now. Sorry for noticing this late, but I thought
> >> we had already discussed the integration order and that merge of
> >> non-64-bit capable devices need to be put on hold.
> >> 
> >> In particular, not so much the DU device (this patch) but more the VSP
> >> instances. The reason for that is that VSP devices can only perform
> >> 32-bit bus mastering without IOMMU. I believe next step for all this
> >> would be to enable all on-board memory on r8a7796 Salvator-x, but I
> >> think Geert is working on that.
> > 
> > So you would like to see the following patches dropped?
> > 
> > ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
> > 0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances
> 
> Yes, that would be great. Sorry for not noticing earlier.

I'm sorry, but I don't agree with that. First of all the FCP has no issue with 
>4GB memory as it doesn't perform DMA itself. Then, while the IPMMU is 
required to operate the VSP with >4GB memory, enabling the VSP instances won't 
break anything as unlike the DU the VSPs won't be used by standard kernel or 
userspace components.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Laurent Pinchart]

Hello Magnus,

On Thursday 27 Oct 2016 18:40:31 Laurent Pinchart wrote:
> On Thursday 27 Oct 2016 16:25:35 Magnus Damm wrote:
> > On Thu, Oct 27, 2016 at 4:13 PM, Simon Horman wrote:
> >> On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
> >>> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman wrote:
> >>>> On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
> >>>>> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> >>>>>> On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> >>>>>>> Add the DU device to r8a7796.dtsi in a disabled state.
> >>>>>>> 
> >>>>>>> Signed-off-by: Laurent Pinchart
> >>>>>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>>>> 
> >>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>>> 
> >>>>> Could you please pick patches 1/4 to 3/4 from this series and apply
> >>>>> them to your tree ? For convenience I've pushed them to
> >>>>> 
> >>>>>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
> >>>>> 
> >>>>> along with patch "arm64: dts: renesas: r8a7795: Remove FCP
> >>>>> SoC-specific compatible strings" that has been acked too. If you pull
> >>>>> from that branch please make sure you skip the top-most patch "arm64:
> >>>>> dts: renesas: r8a7796-salvator-x: Enable DU" for now.
> >>>> 
> >>>> Sure, done.
> >>> 
> >>> I think we should hold off with the upstreaming of the DU and VSP
> >>> integration code for now. Sorry for noticing this late, but I thought
> >>> we had already discussed the integration order and that merge of
> >>> non-64-bit capable devices need to be put on hold.
> >>> 
> >>> In particular, not so much the DU device (this patch) but more the VSP
> >>> instances. The reason for that is that VSP devices can only perform
> >>> 32-bit bus mastering without IOMMU. I believe next step for all this
> >>> would be to enable all on-board memory on r8a7796 Salvator-x, but I
> >>> think Geert is working on that.
> >> 
> >> So you would like to see the following patches dropped?
> >> 
> >> ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
> >> 0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances
> > 
> > Yes, that would be great. Sorry for not noticing earlier.
> 
> I'm sorry, but I don't agree with that. First of all the FCP has no issue
> with >4GB memory as it doesn't perform DMA itself. Then, while the IPMMU is
> 
> required to operate the VSP with >4GB memory, enabling the VSP instances
> won't break anything as unlike the DU the VSPs won't be used by standard
> kernel or userspace components.

Ping ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Magnus Damm]

Hi Laurent,

On Wed, Nov 16, 2016 at 4:12 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hello Magnus,
>
> On Thursday 27 Oct 2016 18:40:31 Laurent Pinchart wrote:
>> On Thursday 27 Oct 2016 16:25:35 Magnus Damm wrote:
>> > On Thu, Oct 27, 2016 at 4:13 PM, Simon Horman wrote:
>> >> On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
>> >>> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman wrote:
>> >>>> On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
>> >>>>> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
>> >>>>>> On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
>> >>>>>>> Add the DU device to r8a7796.dtsi in a disabled state.
>> >>>>>>>
>> >>>>>>> Signed-off-by: Laurent Pinchart
>> >>>>>>> <laurent.pinchart+renesas@ideasonboard.com>
>> >>>>>>
>> >>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >>>>>
>> >>>>> Could you please pick patches 1/4 to 3/4 from this series and apply
>> >>>>> them to your tree ? For convenience I've pushed them to
>> >>>>>
>> >>>>>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
>> >>>>>
>> >>>>> along with patch "arm64: dts: renesas: r8a7795: Remove FCP
>> >>>>> SoC-specific compatible strings" that has been acked too. If you pull
>> >>>>> from that branch please make sure you skip the top-most patch "arm64:
>> >>>>> dts: renesas: r8a7796-salvator-x: Enable DU" for now.
>> >>>>
>> >>>> Sure, done.
>> >>>
>> >>> I think we should hold off with the upstreaming of the DU and VSP
>> >>> integration code for now. Sorry for noticing this late, but I thought
>> >>> we had already discussed the integration order and that merge of
>> >>> non-64-bit capable devices need to be put on hold.
>> >>>
>> >>> In particular, not so much the DU device (this patch) but more the VSP
>> >>> instances. The reason for that is that VSP devices can only perform
>> >>> 32-bit bus mastering without IOMMU. I believe next step for all this
>> >>> would be to enable all on-board memory on r8a7796 Salvator-x, but I
>> >>> think Geert is working on that.
>> >>
>> >> So you would like to see the following patches dropped?
>> >>
>> >> ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
>> >> 0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances
>> >
>> > Yes, that would be great. Sorry for not noticing earlier.
>>
>> I'm sorry, but I don't agree with that. First of all the FCP has no issue
>> with >4GB memory as it doesn't perform DMA itself. Then, while the IPMMU is
>>
>> required to operate the VSP with >4GB memory, enabling the VSP instances
>> won't break anything as unlike the DU the VSPs won't be used by standard
>> kernel or userspace components.
>
> Ping ?

Thanks for the ping.

First of all, we might have slightly different view of the hardware,
so this might need some further discussions. Also, this topic in my
mind is mainly about DT integration code merge ordering for r8a7796 to
enable >4GB memory access early on without introducing any potential
issues.

Regarding the hardware and FCP, VSP and DU, I think we for R-Car Gen3
can agree on that the DU does not perform any bus mastering itself,
but relies of VSP for this to happen. VSP however in my opinion also
relies on FCP for bus mastering on R-Car Gen3. So this is conflicting
with your statement that FCP does not perform DMA itself - maybe you
are referring to the kernel driver instead of the hardware? Our
potentially different view makes me confused! =)

About the integration patches for FCP, VSP and DU, what do you propose
merging? I think it is easy to draw a line in the sand and say that
since the DU is lacking support for IPMMU on R-Car Gen3 in upstream it
is too early to try to integrate any related components including VSP
and FCP (that are used with DU). I don't see the merit of
half-integrated support.

Once the DU driver code for r8a7796 (including support for using
IPMMU) is ready upstream merge it will be simple enough to enable in
DT, don't you think? If the code happens to be ready and available in
-next then I think DT changes can be merged in parallel as well.

Thanks,

/ magnus

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Geert Uytterhoeven]

Hi Magnus,

On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> First of all, we might have slightly different view of the hardware,
> so this might need some further discussions. Also, this topic in my
> mind is mainly about DT integration code merge ordering for r8a7796 to
> enable >4GB memory access early on without introducing any potential
> issues.

Note that >4GB memory is already enabled on r8a7796 by U-Boot.

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 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Magnus Damm]

Hi Geert,

On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> First of all, we might have slightly different view of the hardware,
>> so this might need some further discussions. Also, this topic in my
>> mind is mainly about DT integration code merge ordering for r8a7796 to
>> enable >4GB memory access early on without introducing any potential
>> issues.
>
> Note that >4GB memory is already enabled on r8a7796 by U-Boot.

Right, can you remind me - did we get any conclusion about how to
handle the memory ranges in DTS and the ones from u-boot?

It would be good with a consistent plan how to handle such.

Thanks,

/ magnus

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Geert Uytterhoeven]

Hi Magnus,

On Thu, Nov 17, 2016 at 9:51 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> First of all, we might have slightly different view of the hardware,
>>> so this might need some further discussions. Also, this topic in my
>>> mind is mainly about DT integration code merge ordering for r8a7796 to
>>> enable >4GB memory access early on without introducing any potential
>>> issues.
>>
>> Note that >4GB memory is already enabled on r8a7796 by U-Boot.
>
> Right, can you remind me - did we get any conclusion about how to
> handle the memory ranges in DTS and the ones from u-boot?
>
> It would be good with a consistent plan how to handle such.

I think we should just apply "arm64: dts: r8a7796: salvator-x: Update memory
node to 4 GiB map".

IMHO it doesn't make much sense to pretend the memory is not present nor
enabled.

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 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Magnus Damm]

Hi Geert,

On Mon, Nov 21, 2016 at 8:09 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Thu, Nov 17, 2016 at 9:51 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>> First of all, we might have slightly different view of the hardware,
>>>> so this might need some further discussions. Also, this topic in my
>>>> mind is mainly about DT integration code merge ordering for r8a7796 to
>>>> enable >4GB memory access early on without introducing any potential
>>>> issues.
>>>
>>> Note that >4GB memory is already enabled on r8a7796 by U-Boot.
>>
>> Right, can you remind me - did we get any conclusion about how to
>> handle the memory ranges in DTS and the ones from u-boot?
>>
>> It would be good with a consistent plan how to handle such.
>
> I think we should just apply "arm64: dts: r8a7796: salvator-x: Update memory
> node to 4 GiB map".

I guess that we cannot control what u-boot will pass to us, but with
the patch above at least we have some chance of having a consistent
memory map.

> IMHO it doesn't make much sense to pretend the memory is not present nor
> enabled.

Enabling all the memory makes sense at this point, but I'd like us to
keep considering performance of bounce buffers and/or IPMMU when
merging different on-chip devices that may not support addressing of
the full physical memory space.

We earlier had issues with "enable-and-forget" development approach in
case of USB host over on-chip PCI for the R-Car Gen2 family of
devices. In that case the hardware was unable to do bus mastering to
all the memory but this was not considered as part of the upstreaming
plan and instead came as a nasty surprise later on. So for each device
that we develop code for and integrate i would like to make sure that
we understand how memory handling is supposed to work and what
potential workarounds we may have to use.

Thanks!

/ magnus

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Geert Uytterhoeven]

Hi Magnus,

(this time with CC kept)

On Fri, Nov 25, 2016 at 9:16 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Mon, Nov 21, 2016 at 8:09 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Thu, Nov 17, 2016 at 9:51 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>>> First of all, we might have slightly different view of the hardware,
>>>>> so this might need some further discussions. Also, this topic in my
>>>>> mind is mainly about DT integration code merge ordering for r8a7796 to
>>>>> enable >4GB memory access early on without introducing any potential
>>>>> issues.
>>>>
>>>> Note that >4GB memory is already enabled on r8a7796 by U-Boot.
>>>
>>> Right, can you remind me - did we get any conclusion about how to
>>> handle the memory ranges in DTS and the ones from u-boot?
>>>
>>> It would be good with a consistent plan how to handle such.
>>
>> I think we should just apply "arm64: dts: r8a7796: salvator-x: Update memory
>> node to 4 GiB map".
>
> I guess that we cannot control what u-boot will pass to us, but with
> the patch above at least we have some chance of having a consistent
> memory map.

Indeed.

>> IMHO it doesn't make much sense to pretend the memory is not present nor
>> enabled.
>
> Enabling all the memory makes sense at this point, but I'd like us to
> keep considering performance of bounce buffers and/or IPMMU when
> merging different on-chip devices that may not support addressing of
> the full physical memory space.
>
> We earlier had issues with "enable-and-forget" development approach in
> case of USB host over on-chip PCI for the R-Car Gen2 family of
> devices. In that case the hardware was unable to do bus mastering to
> all the memory but this was not considered as part of the upstreaming
> plan and instead came as a nasty surprise later on. So for each device
> that we develop code for and integrate i would like to make sure that
> we understand how memory handling is supposed to work and what
> potential workarounds we may have to use.

Sure.

All existing devices in r8a7796.dtsi in Simon's tree either use PIO, or DMA
through SYS-DMAC. The latter supports 64-bit addressing hardware-wise,
but the software side needs a patch to enable that, cfr. "[PATCH/RFC 5/5]
dmaengine: rcar-dmac: Widen DMA mask to 40 bits".
Without that, it still works, but using swiotlb bounce buffers.

Once that's fixed, there are no performance deteriorations due to bounce
buffers, with the current set of enabled devices.

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 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Simon Horman]

On Fri, Nov 25, 2016 at 10:22:59AM +0100, Geert Uytterhoeven wrote:
> Hi Magnus,
> 
> (this time with CC kept)
> 
> On Fri, Nov 25, 2016 at 9:16 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> > On Mon, Nov 21, 2016 at 8:09 PM, Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> >> On Thu, Nov 17, 2016 at 9:51 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >>> On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven
> >>> <geert@linux-m68k.org> wrote:
> >>>> On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> >>>>> First of all, we might have slightly different view of the hardware,
> >>>>> so this might need some further discussions. Also, this topic in my
> >>>>> mind is mainly about DT integration code merge ordering for r8a7796 to
> >>>>> enable >4GB memory access early on without introducing any potential
> >>>>> issues.
> >>>>
> >>>> Note that >4GB memory is already enabled on r8a7796 by U-Boot.
> >>>
> >>> Right, can you remind me - did we get any conclusion about how to
> >>> handle the memory ranges in DTS and the ones from u-boot?
> >>>
> >>> It would be good with a consistent plan how to handle such.
> >>
> >> I think we should just apply "arm64: dts: r8a7796: salvator-x: Update memory
> >> node to 4 GiB map".
> >
> > I guess that we cannot control what u-boot will pass to us, but with
> > the patch above at least we have some chance of having a consistent
> > memory map.
> 
> Indeed.
> 
> >> IMHO it doesn't make much sense to pretend the memory is not present nor
> >> enabled.
> >
> > Enabling all the memory makes sense at this point, but I'd like us to
> > keep considering performance of bounce buffers and/or IPMMU when
> > merging different on-chip devices that may not support addressing of
> > the full physical memory space.
> >
> > We earlier had issues with "enable-and-forget" development approach in
> > case of USB host over on-chip PCI for the R-Car Gen2 family of
> > devices. In that case the hardware was unable to do bus mastering to
> > all the memory but this was not considered as part of the upstreaming
> > plan and instead came as a nasty surprise later on. So for each device
> > that we develop code for and integrate i would like to make sure that
> > we understand how memory handling is supposed to work and what
> > potential workarounds we may have to use.
> 
> Sure.
> 
> All existing devices in r8a7796.dtsi in Simon's tree either use PIO, or DMA
> through SYS-DMAC. The latter supports 64-bit addressing hardware-wise,
> but the software side needs a patch to enable that, cfr. "[PATCH/RFC 5/5]
> dmaengine: rcar-dmac: Widen DMA mask to 40 bits".
> Without that, it still works, but using swiotlb bounce buffers.
> 
> Once that's fixed, there are no performance deteriorations due to bounce
> buffers, with the current set of enabled devices.

So we can enable all the memory with a low risk of regression?

Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Geert Uytterhoeven]

Hi Simon,

On Thu, Dec 8, 2016 at 2:28 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Nov 25, 2016 at 10:22:59AM +0100, Geert Uytterhoeven wrote:
>> On Fri, Nov 25, 2016 at 9:16 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> > On Mon, Nov 21, 2016 at 8:09 PM, Geert Uytterhoeven
>> > <geert@linux-m68k.org> wrote:
>> >> On Thu, Nov 17, 2016 at 9:51 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> >>> On Thu, Nov 17, 2016 at 5:31 PM, Geert Uytterhoeven
>> >>> <geert@linux-m68k.org> wrote:
>> >>>> On Thu, Nov 17, 2016 at 3:28 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> >>>>> First of all, we might have slightly different view of the hardware,
>> >>>>> so this might need some further discussions. Also, this topic in my
>> >>>>> mind is mainly about DT integration code merge ordering for r8a7796 to
>> >>>>> enable >4GB memory access early on without introducing any potential
>> >>>>> issues.
>> >>>>
>> >>>> Note that >4GB memory is already enabled on r8a7796 by U-Boot.
>> >>>
>> >>> Right, can you remind me - did we get any conclusion about how to
>> >>> handle the memory ranges in DTS and the ones from u-boot?
>> >>>
>> >>> It would be good with a consistent plan how to handle such.
>> >>
>> >> I think we should just apply "arm64: dts: r8a7796: salvator-x: Update memory
>> >> node to 4 GiB map".
>> >
>> > I guess that we cannot control what u-boot will pass to us, but with
>> > the patch above at least we have some chance of having a consistent
>> > memory map.
>>
>> Indeed.
>>
>> >> IMHO it doesn't make much sense to pretend the memory is not present nor
>> >> enabled.
>> >
>> > Enabling all the memory makes sense at this point, but I'd like us to
>> > keep considering performance of bounce buffers and/or IPMMU when
>> > merging different on-chip devices that may not support addressing of
>> > the full physical memory space.
>> >
>> > We earlier had issues with "enable-and-forget" development approach in
>> > case of USB host over on-chip PCI for the R-Car Gen2 family of
>> > devices. In that case the hardware was unable to do bus mastering to
>> > all the memory but this was not considered as part of the upstreaming
>> > plan and instead came as a nasty surprise later on. So for each device
>> > that we develop code for and integrate i would like to make sure that
>> > we understand how memory handling is supposed to work and what
>> > potential workarounds we may have to use.
>>
>> Sure.
>>
>> All existing devices in r8a7796.dtsi in Simon's tree either use PIO, or DMA
>> through SYS-DMAC. The latter supports 64-bit addressing hardware-wise,
>> but the software side needs a patch to enable that, cfr. "[PATCH/RFC 5/5]
>> dmaengine: rcar-dmac: Widen DMA mask to 40 bits".
>> Without that, it still works, but using swiotlb bounce buffers.
>>
>> Once that's fixed, there are no performance deteriorations due to bounce
>> buffers, with the current set of enabled devices.
>
> So we can enable all the memory with a low risk of regression?

Yes.

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 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT

[Laurent Pinchart]

Hi Magnus,

On Thursday 17 Nov 2016 11:28:07 Magnus Damm wrote:
> On Wed, Nov 16, 2016 at 4:12 AM, Laurent Pinchart wrote:
> > On Thursday 27 Oct 2016 18:40:31 Laurent Pinchart wrote:
> >> On Thursday 27 Oct 2016 16:25:35 Magnus Damm wrote:
> >>> On Thu, Oct 27, 2016 at 4:13 PM, Simon Horman wrote:
> >>>> On Thu, Oct 27, 2016 at 03:52:44PM +0900, Magnus Damm wrote:
> >>>>> On Thu, Oct 20, 2016 at 5:56 PM, Simon Horman wrote:
> >>>>>> On Tue, Oct 18, 2016 at 12:19:26PM +0300, Laurent Pinchart wrote:
> >>>>>>> On Tuesday 18 Oct 2016 11:05:32 Geert Uytterhoeven wrote:
> >>>>>>>> On Mon, Oct 17, 2016 at 11:34 PM, Laurent Pinchart wrote:
> >>>>>>>>> Add the DU device to r8a7796.dtsi in a disabled state.
> >>>>>>>>> 
> >>>>>>>>> Signed-off-by: Laurent Pinchart
> >>>>>>>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>>>>>> 
> >>>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>>>>> 
> >>>>>>> Could you please pick patches 1/4 to 3/4 from this series and apply
> >>>>>>> them to your tree ? For convenience I've pushed them to
> >>>>>>> 
> >>>>>>>       git://linuxtv.org/pinchartl/media.git drm/r8a7796/dt
> >>>>>>> 
> >>>>>>> along with patch "arm64: dts: renesas: r8a7795: Remove FCP
> >>>>>>> SoC-specific compatible strings" that has been acked too. If you
> >>>>>>> pull from that branch please make sure you skip the top-most patch
> >>>>>>> "arm64: dts: renesas: r8a7796-salvator-x: Enable DU" for now.
> >>>>>> 
> >>>>>> Sure, done.
> >>>>> 
> >>>>> I think we should hold off with the upstreaming of the DU and VSP
> >>>>> integration code for now. Sorry for noticing this late, but I thought
> >>>>> we had already discussed the integration order and that merge of
> >>>>> non-64-bit capable devices need to be put on hold.
> >>>>> 
> >>>>> In particular, not so much the DU device (this patch) but more the
> >>>>> VSP instances. The reason for that is that VSP devices can only
> >>>>> perform 32-bit bus mastering without IOMMU. I believe next step for
> >>>>> all this would be to enable all on-board memory on r8a7796 Salvator-
> >>>>> x, but I think Geert is working on that.
> >>>> 
> >>>> So you would like to see the following patches dropped?
> >>>> 
> >>>> ee73acb13978 arm64: dts: renesas: r8a7796: Add VSP instances
> >>>> 0a6519d782fd arm64: dts: renesas: r8a7796: Add FCPF and FCPV instances
> >>> 
> >>> Yes, that would be great. Sorry for not noticing earlier.
> >> 
> >> I'm sorry, but I don't agree with that. First of all the FCP has no issue
> >> with >4GB memory as it doesn't perform DMA itself. Then, while the IPMMU
> >> is required to operate the VSP with >4GB memory, enabling the VSP
> >> instances won't break anything as unlike the DU the VSPs won't be used by
> >> standard kernel or userspace components.
> > 
> > Ping ?
> 
> Thanks for the ping.
> 
> First of all, we might have slightly different view of the hardware,
> so this might need some further discussions. Also, this topic in my
> mind is mainly about DT integration code merge ordering for r8a7796 to
> enable >4GB memory access early on without introducing any potential
> issues.
> 
> Regarding the hardware and FCP, VSP and DU, I think we for R-Car Gen3
> can agree on that the DU does not perform any bus mastering itself,
> but relies of VSP for this to happen. VSP however in my opinion also
> relies on FCP for bus mastering on R-Car Gen3. So this is conflicting
> with your statement that FCP does not perform DMA itself - maybe you
> are referring to the kernel driver instead of the hardware? Our
> potentially different view makes me confused! =)

The VSP is a resource, similar to an IOMMU, through which memory accesses go. 
As such it doesn't initiate DMA, but only handles the memory accesses on 
behalf of bus masters (VSP in this case). You can clearly see that through the 
fact that the FCP has no memory address register.

> About the integration patches for FCP, VSP and DU, what do you propose
> merging? I think it is easy to draw a line in the sand and say that
> since the DU is lacking support for IPMMU on R-Car Gen3 in upstream it
> is too early to try to integrate any related components including VSP
> and FCP (that are used with DU). I don't see the merit of
> half-integrated support.

It avoids having to carry out-of-tree patches around for testing. That only 
seems quite useful to me. I agree that we shouldn't enable features that would 
break operation of the board with >4GB memory, but adding the FCP, VSP and DU 
(the latter in a disabled state) to r8a7796.dtsi will not break anything as 
the devices will not be used.

> Once the DU driver code for r8a7796 (including support for using
> IPMMU) is ready upstream merge it will be simple enough to enable in
> DT, don't you think? If the code happens to be ready and available in
> -next then I think DT changes can be merged in parallel as well.

-- 
Regards,

Laurent Pinchart

[PATCH 4/4] arm64: dts: renesas: r8a7796-salvator-x: Enable DU

[Laurent Pinchart]

Only the VGA output is supported for now.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index 90e9a76c8b30..5bb25f2e3c51 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -29,6 +29,38 @@
 		/* first 128MB is reserved for secure area. */
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
+
+	vga-encoder {
+		compatible = "adi,adv7123";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				adv7123_in: endpoint {
+					remote-endpoint = <&du_out_rgb>;
+				};
+			};
+			port@1 {
+				reg = <1>;
+				adv7123_out: endpoint {
+					remote-endpoint = <&vga_in>;
+				};
+			};
+		};
+	};
+
+	vga {
+		compatible = "vga-connector";
+
+		port {
+			vga_in: endpoint {
+				remote-endpoint = <&adv7123_out>;
+			};
+		};
+	};
 };
 
 &pfc {
@@ -45,6 +77,22 @@
 	};
 };
 
+&du {
+	status = "okay";
+
+	ports {
+		port@0 {
+			endpoint {
+				remote-endpoint = <&adv7123_in>;
+			};
+		};
+		port@2 {
+			lvds_connector: endpoint {
+			};
+		};
+	};
+};
+
 &extal_clk {
 	clock-frequency = <16666666>;
 };
-- 
Regards,

Laurent Pinchart