The RGA2 on the Rockchip rk3568 is the same core as the RGA2 on the Rockchip
rk3288.
This series adds the necessary device tree binding and node in the device tree
to enable the RGA2 on the Rockchip rk3568.
I tested the driver with the GStreamer v4l2convert element on a Rock3 Model A
board.
This is a RESEND including the linux-media list, as Heiko asked for an
Acked-by from someone from media.
Michael
To: Jacob Chen <[email protected]>
To: Ezequiel Garcia <[email protected]>
To: Mauro Carvalho Chehab <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Heiko Stuebner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Nicolas Frattaroli <[email protected]>
Signed-off-by: Michael Tretter <[email protected]>
---
Michael Tretter (2):
media: dt-bindings: media: rockchip-rga: add rockchip,rk3568-rga
arm64: dts: rockchip: Add RGA2 support to rk356x
Documentation/devicetree/bindings/media/rockchip-rga.yaml | 4 +++-
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 11 +++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
---
base-commit: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
change-id: 20230119-rk3568-rga-d1b0cccc7b82
Best regards,
--
Michael Tretter <[email protected]>
The rk3568 also features a RGA2 block. Add the necessary device tree
node.
Acked-by: Nicolas Frattaroli <[email protected]>
Signed-off-by: Michael Tretter <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk356x.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 5706c3e24f0a..704b13f7f717 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -612,6 +612,17 @@ vdpu_mmu: iommu@fdea0800 {
#iommu-cells = <0>;
};
+ rga: rga@fdeb0000 {
+ compatible = "rockchip,rk3568-rga", "rockchip,rk3288-rga";
+ reg = <0x0 0xfdeb0000 0x0 0x180>;
+ interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru ACLK_RGA>, <&cru HCLK_RGA>, <&cru CLK_RGA_CORE>;
+ clock-names = "aclk", "hclk", "sclk";
+ resets = <&cru SRST_RGA_CORE>, <&cru SRST_A_RGA>, <&cru SRST_H_RGA>;
+ reset-names = "core", "axi", "ahb";
+ power-domains = <&power RK3568_PD_RGA>;
+ };
+
vepu: video-codec@fdee0000 {
compatible = "rockchip,rk3568-vepu";
reg = <0x0 0xfdee0000 0x0 0x800>;
--
2.30.2
Add a new compatible for the rk3568 Rockchip SoC, which also features an
RGA, which is called RGA2 in the TRM Part2. It is the same core as used
on the rk3288, which documents the same RGA2.
Specify a new compatible for the rk3568 to be able to handle unknown
SoC-specific differences in the driver.
Acked-by: Krzysztof Kozlowski <[email protected]>
Signed-off-by: Michael Tretter <[email protected]>
---
Documentation/devicetree/bindings/media/rockchip-rga.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/rockchip-rga.yaml b/Documentation/devicetree/bindings/media/rockchip-rga.yaml
index dd645ddccb07..ea2342222408 100644
--- a/Documentation/devicetree/bindings/media/rockchip-rga.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip-rga.yaml
@@ -21,7 +21,9 @@ properties:
- const: rockchip,rk3288-rga
- const: rockchip,rk3399-rga
- items:
- - const: rockchip,rk3228-rga
+ - enum:
+ - rockchip,rk3228-rga
+ - rockchip,rk3568-rga
- const: rockchip,rk3288-rga
reg:
--
2.30.2
Hello Michael,
Since we have the over-4GB problem now, should we mark this problem as a
TODO or something?
Shengyu
Hi Micheal,
Le vendredi 20 janvier 2023 à 10:14 +0100, Michael Tretter a écrit :
> The RGA2 on the Rockchip rk3568 is the same core as the RGA2 on the Rockchip
> rk3288.
>
> This series adds the necessary device tree binding and node in the device tree
> to enable the RGA2 on the Rockchip rk3568.
>
> I tested the driver with the GStreamer v4l2convert element on a Rock3 Model A
> board.
>
> This is a RESEND including the linux-media list, as Heiko asked for an
> Acked-by from someone from media.
I don't think there will be any concern about this on media side.
Acked-by: Nicolas Dufresne <[email protected]>
>
>
>
> Michael
>
> To: Jacob Chen <[email protected]>
> To: Ezequiel Garcia <[email protected]>
> To: Mauro Carvalho Chehab <[email protected]>
> To: Rob Herring <[email protected]>
> To: Krzysztof Kozlowski <[email protected]>
> To: Heiko Stuebner <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Nicolas Frattaroli <[email protected]>
> Signed-off-by: Michael Tretter <[email protected]>
>
> ---
> Michael Tretter (2):
> media: dt-bindings: media: rockchip-rga: add rockchip,rk3568-rga
> arm64: dts: rockchip: Add RGA2 support to rk356x
>
> Documentation/devicetree/bindings/media/rockchip-rga.yaml | 4 +++-
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 11 +++++++++++
> 2 files changed, 14 insertions(+), 1 deletion(-)
> ---
> base-commit: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
> change-id: 20230119-rk3568-rga-d1b0cccc7b82
>
> Best regards,
On Fri, Jan 20 2023 at 10:14:21 AM +0100, Michael Tretter
<[email protected]> wrote:
> Add a new compatible for the rk3568 Rockchip SoC, which also features
> an
> RGA, which is called RGA2 in the TRM Part2. It is the same core as
> used
> on the rk3288, which documents the same RGA2.
>
> Specify a new compatible for the rk3568 to be able to handle unknown
> SoC-specific differences in the driver.
>
> Acked-by: Krzysztof Kozlowski <[email protected]>
> Signed-off-by: Michael Tretter <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
> ---
> Documentation/devicetree/bindings/media/rockchip-rga.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git
> a/Documentation/devicetree/bindings/media/rockchip-rga.yaml
> b/Documentation/devicetree/bindings/media/rockchip-rga.yaml
> index dd645ddccb07..ea2342222408 100644
> --- a/Documentation/devicetree/bindings/media/rockchip-rga.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip-rga.yaml
> @@ -21,7 +21,9 @@ properties:
> - const: rockchip,rk3288-rga
> - const: rockchip,rk3399-rga
> - items:
> - - const: rockchip,rk3228-rga
> + - enum:
> + - rockchip,rk3228-rga
> + - rockchip,rk3568-rga
> - const: rockchip,rk3288-rga
>
> reg:
>
> --
> 2.30.2
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 <[email protected]>
> Signed-off-by: Michael Tretter <[email protected]>
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 <[email protected]>
--
Regards,
Sakari Ailus
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 <[email protected]>
> > Signed-off-by: Michael Tretter <[email protected]>
>
> 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 <[email protected]>
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 Fri, 20 Jan 2023 10:14:21 +0100, Michael Tretter wrote:
> The RGA2 on the Rockchip rk3568 is the same core as the RGA2 on the Rockchip
> rk3288.
>
> This series adds the necessary device tree binding and node in the device tree
> to enable the RGA2 on the Rockchip rk3568.
>
> I tested the driver with the GStreamer v4l2convert element on a Rock3 Model A
> board.
>
> [...]
Applied, thanks!
[1/2] media: dt-bindings: media: rockchip-rga: add rockchip,rk3568-rga
commit: 9b12ceb5a80d1fb45d293265de100e33b5843943
[2/2] arm64: dts: rockchip: Add RGA2 support to rk356x
commit: 0c3391f8bb06b744df521651534cd99e3d77e0a8
Best regards,
--
Heiko Stuebner <[email protected]>
On Sunday, 21 May 2023 12:44:58 CEST Heiko Stuebner wrote:
> On Fri, 20 Jan 2023 10:14:21 +0100, Michael Tretter wrote:
> > The RGA2 on the Rockchip rk3568 is the same core as the RGA2 on the
> > Rockchip rk3288.
> >
> > This series adds the necessary device tree binding and node in the device
> > tree to enable the RGA2 on the Rockchip rk3568.
> >
> > I tested the driver with the GStreamer v4l2convert element on a Rock3
> > Model A board.
> >
> > [...]
>
> Applied, thanks!
>
> [1/2] media: dt-bindings: media: rockchip-rga: add rockchip,rk3568-rga
> commit: 9b12ceb5a80d1fb45d293265de100e33b5843943
> [2/2] arm64: dts: rockchip: Add RGA2 support to rk356x
> commit: 0c3391f8bb06b744df521651534cd99e3d77e0a8
https://lore.kernel.org/all/TY3P286MB26115F60D273E840D36A610598CA9@TY3P286MB2611.JPNP286.PROD.OUTLOOK.COM/
indicated that there was a problem with device >= 4GB (RAM?):
> Since we have the over-4GB problem now, should we mark this problem as a
> TODO or something?
I thought that was the reason that these patches weren't picked up before?
I have no insight into this problem, so I can't comment on the technical
aspects, but I had made a note for myself 'locally' about it.
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 <[email protected]>
> > > Signed-off-by: Michael Tretter <[email protected]>
> >
> > 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 <[email protected]>
>
> 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!
--
Sakari Ailus
On Sun, 21 May 2023 21:32:51 +0200, Diederik de Haas wrote:
> On Sunday, 21 May 2023 12:44:58 CEST Heiko Stuebner wrote:
> > On Fri, 20 Jan 2023 10:14:21 +0100, Michael Tretter wrote:
> > > The RGA2 on the Rockchip rk3568 is the same core as the RGA2 on the
> > > Rockchip rk3288.
> > >
> > > This series adds the necessary device tree binding and node in the device
> > > tree to enable the RGA2 on the Rockchip rk3568.
> > >
> > > I tested the driver with the GStreamer v4l2convert element on a Rock3
> > > Model A board.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/2] media: dt-bindings: media: rockchip-rga: add rockchip,rk3568-rga
> > commit: 9b12ceb5a80d1fb45d293265de100e33b5843943
> > [2/2] arm64: dts: rockchip: Add RGA2 support to rk356x
> > commit: 0c3391f8bb06b744df521651534cd99e3d77e0a8
>
> https://lore.kernel.org/all/TY3P286MB26115F60D273E840D36A610598CA9@TY3P286MB2611.JPNP286.PROD.OUTLOOK.COM/
>
> indicated that there was a problem with device >= 4GB (RAM?):
> > Since we have the over-4GB problem now, should we mark this problem as a
> > TODO or something?
>
> I thought that was the reason that these patches weren't picked up before?
That's what I thought, too.
>
> I have no insight into this problem, so I can't comment on the technical
> aspects, but I had made a note for myself 'locally' about it.
Using the RGA2 with the driver in its current form on devices with more than 4
GB system memory may lead to memory corruption as buffer addresses are
silently truncated to 32 bits.
I'm not sure if that's actually a blocker for merging these patches.
Michael
On 2023-05-22 11:29, Michael Tretter wrote:
> On Sun, 21 May 2023 21:32:51 +0200, Diederik de Haas wrote:
>> On Sunday, 21 May 2023 12:44:58 CEST Heiko Stuebner wrote:
>>> On Fri, 20 Jan 2023 10:14:21 +0100, Michael Tretter wrote:
>>>> The RGA2 on the Rockchip rk3568 is the same core as the RGA2 on the
>>>> Rockchip rk3288.
>>>>
>>>> This series adds the necessary device tree binding and node in the device
>>>> tree to enable the RGA2 on the Rockchip rk3568.
>>>>
>>>> I tested the driver with the GStreamer v4l2convert element on a Rock3
>>>> Model A board.
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [1/2] media: dt-bindings: media: rockchip-rga: add rockchip,rk3568-rga
>>> commit: 9b12ceb5a80d1fb45d293265de100e33b5843943
>>> [2/2] arm64: dts: rockchip: Add RGA2 support to rk356x
>>> commit: 0c3391f8bb06b744df521651534cd99e3d77e0a8
>>
>> https://lore.kernel.org/all/TY3P286MB26115F60D273E840D36A610598CA9@TY3P286MB2611.JPNP286.PROD.OUTLOOK.COM/
>>
>> indicated that there was a problem with device >= 4GB (RAM?):
>>> Since we have the over-4GB problem now, should we mark this problem as a
>>> TODO or something?
>>
>> I thought that was the reason that these patches weren't picked up before?
>
> That's what I thought, too.
>
>>
>> I have no insight into this problem, so I can't comment on the technical
>> aspects, but I had made a note for myself 'locally' about it.
>
> Using the RGA2 with the driver in its current form on devices with more than 4
> GB system memory may lead to memory corruption as buffer addresses are
> silently truncated to 32 bits.
That's because the driver is completely broken and is not using the DMA
API anywhere near properly. The fact that it's been getting away with it
so far can be mostly put down to good luck and nobody using
CONFIG_DMA_API_DEBUG.
> I'm not sure if that's actually a blocker for merging these patches.
If anything, hopefully getting more SoC support merged might provide
more motivation for someone to fix the existing code :)
Robin.