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

[Nicolas Frattaroli]

On Dienstag, 17. Januar 2023 13:41:14 CET Michael Tretter wrote:
> 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 at vanguardiasur.com.ar> wrote:
> > > > > On Mon, Nov 21, 2022 at 2:13 PM Shengyu Qu <wiagn233 at 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 at 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
> 

Sure,

Acked-by: Nicolas Frattaroli <frattaroli.nicolas at gmail.com>

Though I'm not sure how much weight my Acked-by carries here.

For applying the patch, yes that would be done by Heiko.

Cheers,
Nicolas Frattaroli