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

  The rk3568 also features a RGA2 block. Add the necessary device tree
node.

Acked-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
  

Hello Michael,

Since we have the over-4GB problem now, should we mark this problem as a
TODO or something?

Shengyu
  

Hi,

On Sun, 22 Jan 2023 00:50:37 +0800, Shengyu Qu wrote:
> Since we have the over-4GB problem now, should we mark this problem as a
> TODO or something?

I am not really sure where to put such a TODO to make it visible for people
that are running into the issue and to make sure that it is removed once it is
fixed.

Maybe it would be better to add error handling to the rga_buf_map function to
fail if the address of the buffer that should be mapped has the upper 32 bit
set and print a warning. Furthermore, the driver would be able to skip the
buffer and prevent potential memory corruption caused by the erroneous
mapping.

Unfortunately, I don't have hardware that allows me to test this.

Michael
  

Hi Michael,

Seems we could use GFP_DMA32 flag to limit memory required by driver into

upper size range(actually using ZONE_DMA32 configured by device tree). Just

some driver modification needed. Maybe Nicolas could help testing? I would

like to fix this, but I don't have much free time these days.

Best regards,

Shengyu

> Hi,
>
> On Sun, 22 Jan 2023 00:50:37 +0800, Shengyu Qu wrote:
>> Since we have the over-4GB problem now, should we mark this problem as a
>> TODO or something?
> I am not really sure where to put such a TODO to make it visible for people
> that are running into the issue and to make sure that it is removed once it is
> fixed.
>
> Maybe it would be better to add error handling to the rga_buf_map function to
> fail if the address of the buffer that should be mapped has the upper 32 bit
> set and print a warning. Furthermore, the driver would be able to skip the
> buffer and prevent potential memory corruption caused by the erroneous
> mapping.
>
> Unfortunately, I don't have hardware that allows me to test this.
>
> Michael
  

Hi Shengyu,

On Fri, 17 Feb 2023 22:14:13 +0800, Shengyu Qu wrote:
> Seems we could use GFP_DMA32 flag to limit memory required by driver into
> upper size range(actually using ZONE_DMA32 configured by device tree). Just
> some driver modification needed. 

I don't think the GFP_DMA32 flag works with DmaBuf import. The buffer may be
allocated by some other driver that is able to address more than 4G and
imported into the RGA driver. In this case, limiting the allocations is not
enough, but we would still need error handling in the map function for buffers
that cannot be addressed by the RGA.

I guess we need both, a limit for the allocation and error checking for the
map.

Michael

> Maybe Nicolas could help testing? I would
> 
> like to fix this, but I don't have much free time these days.
> 
> Best regards,
> 
> Shengyu
> 
> > Hi,
> > 
> > On Sun, 22 Jan 2023 00:50:37 +0800, Shengyu Qu wrote:
> > > Since we have the over-4GB problem now, should we mark this problem as a
> > > TODO or something?
> > I am not really sure where to put such a TODO to make it visible for people
> > that are running into the issue and to make sure that it is removed once it is
> > fixed.
> > 
> > Maybe it would be better to add error handling to the rga_buf_map function to
> > fail if the address of the buffer that should be mapped has the upper 32 bit
> > set and print a warning. Furthermore, the driver would be able to skip the
> > buffer and prevent potential memory corruption caused by the erroneous
> > mapping.
> > 
> > Unfortunately, I don't have hardware that allows me to test this.
> > 
> > Michael
  

Hi, Michael,

> I don't think the GFP_DMA32 flag works with DmaBuf import. The buffer may be
> allocated by some other driver that is able to address more than 4G and
> imported into the RGA driver. In this case, limiting the allocations is not
> enough, but we would still need error handling in the map function for buffers
> that cannot be addressed by the RGA.
>
> I guess we need both, a limit for the allocation and error checking for the
> map.

Maybe you are right.. I haven't digged into v4l2-m2m API, so I'm not 
sure about

it. Seems we need others' help.

Shengyu
  

Hi folks,

On Fri, Jan 20, 2023 at 10:14:22AM +0100, Michael Tretter wrote:
> The rk3568 also features a RGA2 block. Add the necessary device tree
> node.
> 
> Acked-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>

Can this patch be merged via the media tree? I don't expect merging the
other one via a different tree being an issue either, so alternatively to
the 1st patch:

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
  

Hi,

Am Mittwoch, 17. Mai 2023, 20:39:08 CEST schrieb Sakari Ailus:
> Hi folks,
> 
> On Fri, Jan 20, 2023 at 10:14:22AM +0100, Michael Tretter wrote:
> > The rk3568 also features a RGA2 block. Add the necessary device tree
> > node.
> > 
> > Acked-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> 
> Can this patch be merged via the media tree? I don't expect merging the
> other one via a different tree being an issue either, so alternatively to
> the 1st patch:
> 
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

thanks for the Ack. To prevent conflicts with other additions to the
rk356x.dtsi file, I've picked now both patches for the rockchip tree.

Thanks
Heiko
  

On Sun, May 21, 2023 at 12:46:11PM +0200, Heiko Stübner wrote:
> Hi,
> 
> Am Mittwoch, 17. Mai 2023, 20:39:08 CEST schrieb Sakari Ailus:
> > Hi folks,
> > 
> > On Fri, Jan 20, 2023 at 10:14:22AM +0100, Michael Tretter wrote:
> > > The rk3568 also features a RGA2 block. Add the necessary device tree
> > > node.
> > > 
> > > Acked-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > 
> > Can this patch be merged via the media tree? I don't expect merging the
> > other one via a different tree being an issue either, so alternatively to
> > the 1st patch:
> > 
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> thanks for the Ack. To prevent conflicts with other additions to the
> rk356x.dtsi file, I've picked now both patches for the rockchip tree.

Thank you!