Re: [PATCH 2/2] arm64: dts: rockchip: Add RGA2 support to rk356x

[Michael Tretter <m.tretter@pengutronix.de>]

On Wed, 23 Nov 2022 10:28:41 +0100, Michael Tretter wrote:
> On Tue, 22 Nov 2022 20:47:49 +0100, Nicolas Frattaroli wrote:
> > On Dienstag, 22. November 2022 00:10:25 CET Peter Geis wrote:
> > > On Mon, Nov 21, 2022 at 12:34 PM Ezequiel Garcia
> > > <ezequiel@vanguardiasur.com.ar> wrote:
> > > > On Mon, Nov 21, 2022 at 2:13 PM Shengyu Qu <wiagn233@outlook.com> wrote:
> > > > > I remember someone said that rga2 has only 32bit address space but not
> > > > > having a mmu or mmu needs to be configured. Better ask by yourself in
> > > > > pine64 discord since I'm not sure about that.
> > > > > 于 2022年11月22日 GMT+08:00 上午12:41:16, Michael Tretter <m.tretter@pengutronix.de> 写到:
> > > > > >On Tue, 22 Nov 2022 00:01:28 +0800, Shengyu Qu wrote:
> > > > > >> Someone from pine64 discord found that RGA2 doesn't work properly on over
> > > > > >>
> > > > > >> 4GB memory RK3568 devices. Are you sure current driver works now?
> > > > > >
> > > > > >I am absolutely sure that the driver works on a 2GB ROCK3A board.
> > > > > >Unfortunately, I don't have a device with 4GB or more memory and I cannot test
> > > > > >the driver with such a device.
> > > > > >
> > > > > >As the documentation for the RGA2 is the same in the TRM of the rk3288 and
> > > > > >rk3568, I would guess that the driver doesn't work on a rk3288 with more than
> > > > > >4GB (if there is such a thing) either and the driver needs to be fixed for
> > > > > >both SoCs.
> > > > > >
> > > >
> > > > In any case, if there's some kind of issue it must be in the driver,
> > > > and not in the device tree binding (i.e. this patchset).
> > > 
> > > An unfortunate number of Rockchip drivers break on rk356x boards with
> > > more than 4GB of ram. I've found requesting memory allocations with
> > > the GFP_DMA flag solves the problem, as the kernel only allocates
> > > 32bit addresses for DMA on rk356x. This is similar to the bug with the
> > > ITS MSI allocations.
> > 
> > the problem in this case at least partly seems to be down to the driver
> > assuming 32 bit addresses, see e.g. [1] (if I understand the code
> > correctly) and [2]. When I asked in the #armlinux IRC on Libera.chat
> > about this, Robin Murphy remarked:
> > 
> >   <robmur01> yeesh, the virt_to_phys/dma_sync abuse is even worse
> >   - in that particular instance I'd be inclined to replace
> >   {src,dst}_mmu_pages with proper coherent DMA buffers
> > 
> > So the driver in general needs some cleanup, which contributes to the
> > problem.
> 
> As far as I understand the 4GB are a hardware limitation.
> 
> According to the rk3568 TRM, the RGA2_MMU_SRC_BASE has 28 bits for the upper
> 28 bits of the address of the MMU TLB. Thus the MMU TLB must be located within
> 4GB memory.
> 
> And within the MMU TLB, the addresses are 32 bit as well (unless I am missing
> something important). Unfortunately, I couldn't find any documentation for the
> TLB. The downstream driver writes only 32 bit addresses to the TLB as well.
> Thus, I assume that all video buffers must be located within 4GB memory, too.
> 
> While I agree that the driver needs some cleanup and may use correct types for
> handling the bit size limitation, I think this isn't some driver limitation.
> Maybe the driver should set the dma mask to 32 bits and should be explicitly
> aware of the 32 bit limit.
> 
> > 
> > But as was said, this isn't a problem with the device tree, so it should
> > still make it in. It just means that the driver is broken on 8GB RK356x.

Nicolas: Is this your Acked-By?

Who would pick up the patch? Heiko?

Michael