mbox series

[RESEND,0/2] media: rockchip: rga: Add rk3568 support

Message ID 20230119-rk3568-rga-v1-0-43d4d14365e6@pengutronix.de (mailing list archive)
Headers show
Series media: rockchip: rga: Add rk3568 support | expand

Message

Michael Tretter Jan. 20, 2023, 9:14 a.m. UTC
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 <jacob-chen@iotwrt.com>
To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
To: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Heiko Stuebner <heiko@sntech.de>
Cc: linux-media@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>

---
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,

Comments

Nicolas Dufresne Jan. 23, 2023, 7:49 p.m. UTC | #1
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 <nicolas.dufresne@collabora.com>

> 
> 
> 
> Michael
> 
> To: Jacob Chen <jacob-chen@iotwrt.com>
> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> To: Mauro Carvalho Chehab <mchehab@kernel.org>
> To: Rob Herring <robh+dt@kernel.org>
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> To: Heiko Stuebner <heiko@sntech.de>
> Cc: linux-media@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Cc: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> 
> ---
> 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,
Heiko Stuebner May 21, 2023, 10:44 a.m. UTC | #2
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,
Diederik de Haas May 21, 2023, 7:32 p.m. UTC | #3
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.
Michael Tretter May 22, 2023, 10:29 a.m. UTC | #4
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
Robin Murphy May 23, 2023, 11:13 a.m. UTC | #5
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.