| Message ID | 1476740091-7011-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | 1a349bbbbb1c1a689a6e0c3f1f974802fdd32115 |
| Delegated to: | Simon Horman |
| Headers | show |
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
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.
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.
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
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
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
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 >
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
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.
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
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.
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 ?
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
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
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
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.
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
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
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
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?
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
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 { + }; + }; + }; + }; }; };
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(+)