* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-10-27 7:25 ` Magnus Damm
@ 2016-10-27 10:18 ` Simon Horman
2016-10-27 13:00 ` Simon Horman
2016-10-27 15:40 ` Laurent Pinchart
2 siblings, 0 replies; 28+ messages in thread
From: Simon Horman @ 2016-10-27 10:18 UTC (permalink / raw)
To: Magnus Damm; +Cc: Laurent Pinchart, Geert Uytterhoeven, Linux-Renesas
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-10-27 7:25 ` Magnus Damm
2016-10-27 10:18 ` Simon Horman
@ 2016-10-27 13:00 ` Simon Horman
2016-10-27 15:40 ` Laurent Pinchart
2 siblings, 0 replies; 28+ messages in thread
From: Simon Horman @ 2016-10-27 13:00 UTC (permalink / raw)
To: Magnus Damm; +Cc: Laurent Pinchart, Geert Uytterhoeven, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-10-27 7:25 ` Magnus Damm
2016-10-27 10:18 ` Simon Horman
2016-10-27 13:00 ` Simon Horman
@ 2016-10-27 15:40 ` Laurent Pinchart
2016-11-15 19:12 ` Laurent Pinchart
2 siblings, 1 reply; 28+ messages in thread
From: Laurent Pinchart @ 2016-10-27 15:40 UTC (permalink / raw)
To: Magnus Damm; +Cc: Simon Horman, Geert Uytterhoeven, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-10-27 15:40 ` Laurent Pinchart
@ 2016-11-15 19:12 ` Laurent Pinchart
2016-11-17 2:28 ` Magnus Damm
0 siblings, 1 reply; 28+ messages in thread
From: Laurent Pinchart @ 2016-11-15 19:12 UTC (permalink / raw)
To: Magnus Damm; +Cc: Simon Horman, Geert Uytterhoeven, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-15 19:12 ` Laurent Pinchart
@ 2016-11-17 2:28 ` Magnus Damm
2016-11-17 8:31 ` Geert Uytterhoeven
2016-11-17 9:12 ` Laurent Pinchart
0 siblings, 2 replies; 28+ messages in thread
From: Magnus Damm @ 2016-11-17 2:28 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Simon Horman, Geert Uytterhoeven, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-17 2:28 ` Magnus Damm
@ 2016-11-17 8:31 ` Geert Uytterhoeven
2016-11-17 8:51 ` Magnus Damm
2016-11-17 9:12 ` Laurent Pinchart
1 sibling, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2016-11-17 8:31 UTC (permalink / raw)
To: Magnus Damm; +Cc: Laurent Pinchart, Simon Horman, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-17 8:31 ` Geert Uytterhoeven
@ 2016-11-17 8:51 ` Magnus Damm
2016-11-21 11:09 ` Geert Uytterhoeven
0 siblings, 1 reply; 28+ messages in thread
From: Magnus Damm @ 2016-11-17 8:51 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Laurent Pinchart, Simon Horman, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-17 8:51 ` Magnus Damm
@ 2016-11-21 11:09 ` Geert Uytterhoeven
2016-11-25 8:16 ` Magnus Damm
0 siblings, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2016-11-21 11:09 UTC (permalink / raw)
To: Magnus Damm; +Cc: Laurent Pinchart, Simon Horman, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-21 11:09 ` Geert Uytterhoeven
@ 2016-11-25 8:16 ` Magnus Damm
2016-11-25 9:22 ` Geert Uytterhoeven
0 siblings, 1 reply; 28+ messages in thread
From: Magnus Damm @ 2016-11-25 8:16 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Laurent Pinchart, Simon Horman, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-25 8:16 ` Magnus Damm
@ 2016-11-25 9:22 ` Geert Uytterhoeven
2016-12-08 13:28 ` Simon Horman
0 siblings, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2016-11-25 9:22 UTC (permalink / raw)
To: Magnus Damm; +Cc: Laurent Pinchart, Simon Horman, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-25 9:22 ` Geert Uytterhoeven
@ 2016-12-08 13:28 ` Simon Horman
2016-12-08 13:45 ` Geert Uytterhoeven
0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2016-12-08 13:28 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Magnus Damm, Laurent Pinchart, Linux-Renesas
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?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-12-08 13:28 ` Simon Horman
@ 2016-12-08 13:45 ` Geert Uytterhoeven
0 siblings, 0 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2016-12-08 13:45 UTC (permalink / raw)
To: Simon Horman; +Cc: Magnus Damm, Laurent Pinchart, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/4] arm64: dts: renesas: r8a7796: Add DU device to DT
2016-11-17 2:28 ` Magnus Damm
2016-11-17 8:31 ` Geert Uytterhoeven
@ 2016-11-17 9:12 ` Laurent Pinchart
1 sibling, 0 replies; 28+ messages in thread
From: Laurent Pinchart @ 2016-11-17 9:12 UTC (permalink / raw)
To: Magnus Damm; +Cc: Simon Horman, Geert Uytterhoeven, Linux-Renesas
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
^ permalink raw reply [flat|nested] 28+ messages in thread