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@xxxxxxxxxxxxxxxx> > >>>>>>>> > >>>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > >>>>>>> > >>>>>>> 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
