* [PATCH v6 0/2] Add hantro g1 video decoder support for RK3588 @ 2024-04-18 11:10 Jianfeng Liu 2024-04-18 11:10 ` [PATCH v6 1/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string Jianfeng Liu 2024-04-18 11:10 ` [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 Jianfeng Liu 0 siblings, 2 replies; 6+ messages in thread From: Jianfeng Liu @ 2024-04-18 11:10 UTC (permalink / raw) To: linux-media, linux-rockchip, devicetree, linux-arm-kernel, linux-kernel Cc: ezequiel, p.zabel, mchehab, robh, krzk+dt, conor+dt, heiko, liujianfeng1994, sfr, sebastian.reichel, sigmaris, didi.debian This is the v6 version of this series adding hantro g1 video decoder support for RK3588. RK3588 has Hantro G1 video decoder known as VDPU121 in TRM of RK3588 which is capable to decode MPEG2/H.264/VP8 up to 1920x1088. This vpu ip is also found in RK3568. Test results from fluster can be found from thread of v3[1]. Changes in v6: - Apply dt-bindings first - Collect missing commit tags of old versions - Specify the base commit suggested by Diederik - Link to v5: https://lore.kernel.org/all/20240413064608.788561-1-liujianfeng1994@gmail.com/ Changes in v5: - Add missing interrupt-names to devicetree node - Rebase devicetree patch based on next-20240412 - Link to v4: https://lore.kernel.org/all/20240316071100.2419369-1-liujianfeng1994@gmail.com/ Changes in v4: - Change compatible string to rockchip,rk3588-vdpu121 - Link to v3: https://lore.kernel.org/all/20231231151112.3994194-1-liujianfeng1994@gmail.com/ Changes in v3: - Drop code in hantro_drv.c because hantro g1 vpu in rk3588 is compatible with the one in rk3568, only adding devicetree node should work. - Change devicetree node name to video-codec@fdb50000 to match the reg address. - Add dt-bindings rockchip,rk3588-vpu compatible with rockchip,rk3568-vpu - Link to v2: https://lore.kernel.org/all/20231228131617.3411561-1-liujianfeng1994@gmail.com Changes in v2: - Fix alphabetical order in patch1 and patch3 - Sort device tree node by bus-address - Drop rk3588_vpu_variant fron v1 because that is exactly the same as rk3568_vpu_variant - Link to v1: https://lore.kernel.org/all/20231227173911.3295410-1-liujianfeng1994@gmail.com [1]https://lore.kernel.org/all/20240118080602.9028-1-liujianfeng1994@gmail.com/ Jianfeng Liu (2): dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 .../bindings/media/rockchip-vpu.yaml | 3 +++ arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+) base-commit: 7b4f2bc91c15fdcf948bb2d9741a9d7d54303f8d -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v6 1/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string 2024-04-18 11:10 [PATCH v6 0/2] Add hantro g1 video decoder support for RK3588 Jianfeng Liu @ 2024-04-18 11:10 ` Jianfeng Liu 2024-04-18 11:10 ` [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 Jianfeng Liu 1 sibling, 0 replies; 6+ messages in thread From: Jianfeng Liu @ 2024-04-18 11:10 UTC (permalink / raw) To: linux-media, linux-rockchip, devicetree, linux-arm-kernel, linux-kernel Cc: ezequiel, p.zabel, mchehab, robh, krzk+dt, conor+dt, heiko, liujianfeng1994, sfr, sebastian.reichel, sigmaris, didi.debian, Conor Dooley Add Hantro G1 VPU compatible string for RK3588. RK3588 has the same Hantro G1 ip as RK3568, which are both known as VDPU121 in TRM of RK3568 and RK3588. Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com> Acked-by: Conor Dooley <conor.dooley@microchip.com> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- Documentation/devicetree/bindings/media/rockchip-vpu.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/media/rockchip-vpu.yaml b/Documentation/devicetree/bindings/media/rockchip-vpu.yaml index c57e1f488..4f667db91 100644 --- a/Documentation/devicetree/bindings/media/rockchip-vpu.yaml +++ b/Documentation/devicetree/bindings/media/rockchip-vpu.yaml @@ -31,6 +31,9 @@ properties: - items: - const: rockchip,rk3228-vpu - const: rockchip,rk3399-vpu + - items: + - const: rockchip,rk3588-vdpu121 + - const: rockchip,rk3568-vpu reg: maxItems: 1 -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 2024-04-18 11:10 [PATCH v6 0/2] Add hantro g1 video decoder support for RK3588 Jianfeng Liu 2024-04-18 11:10 ` [PATCH v6 1/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string Jianfeng Liu @ 2024-04-18 11:10 ` Jianfeng Liu 2024-04-19 17:28 ` Hugh Cole-Baker 1 sibling, 1 reply; 6+ messages in thread From: Jianfeng Liu @ 2024-04-18 11:10 UTC (permalink / raw) To: linux-media, linux-rockchip, devicetree, linux-arm-kernel, linux-kernel Cc: ezequiel, p.zabel, mchehab, robh, krzk+dt, conor+dt, heiko, liujianfeng1994, sfr, sebastian.reichel, sigmaris, didi.debian Enable Hantro G1 video decoder in RK3588's devicetree. Tested with FFmpeg v4l2_request code taken from [1] with MPEG2, H.264 and VP8 samples. [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/multimedia/ffmpeg/patches/v4l2-request/ffmpeg-001-v4l2-request.patch Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com> Tested-by: Hugh Cole-Baker <sigmaris@gmail.com> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi index b0a59ec51..b0817382f 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi @@ -1135,6 +1135,27 @@ power-domain@RK3588_PD_SDMMC { }; }; + vpu: video-codec@fdb50000 { + compatible = "rockchip,rk3588-vdpu121", "rockchip,rk3568-vpu"; + reg = <0x0 0xfdb50000 0x0 0x800>; + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>; + interrupt-names = "vdpu"; + clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>; + clock-names = "aclk", "hclk"; + iommus = <&vdpu_mmu>; + power-domains = <&power RK3588_PD_VDPU>; + }; + + vdpu_mmu: iommu@fdb50800 { + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu"; + reg = <0x0 0xfdb50800 0x0 0x40>; + interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH 0>; + clock-names = "aclk", "iface"; + clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>; + power-domains = <&power RK3588_PD_VDPU>; + #iommu-cells = <0>; + }; + av1d: video-codec@fdc70000 { compatible = "rockchip,rk3588-av1-vpu"; reg = <0x0 0xfdc70000 0x0 0x800>; -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 2024-04-18 11:10 ` [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 Jianfeng Liu @ 2024-04-19 17:28 ` Hugh Cole-Baker 2024-04-20 5:09 ` Jianfeng Liu 0 siblings, 1 reply; 6+ messages in thread From: Hugh Cole-Baker @ 2024-04-19 17:28 UTC (permalink / raw) To: Jianfeng Liu Cc: linux-media, linux-rockchip, devicetree, linux-arm-kernel, linux-kernel, ezequiel, p.zabel, mchehab, robh, krzk+dt, Conor Dooley, Heiko Stuebner, sfr, sebastian.reichel, didi.debian Hi Jianfeng, > On 18 Apr 2024, at 12:10, Jianfeng Liu <liujianfeng1994@gmail.com> wrote: > > Enable Hantro G1 video decoder in RK3588's devicetree. > > Tested with FFmpeg v4l2_request code taken from [1] > with MPEG2, H.264 and VP8 samples. > > [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/multimedia/ffmpeg/patches/v4l2-request/ffmpeg-001-v4l2-request.patch > > Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com> > Tested-by: Hugh Cole-Baker <sigmaris@gmail.com> > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > index b0a59ec51..b0817382f 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi > @@ -1135,6 +1135,27 @@ power-domain@RK3588_PD_SDMMC { > }; > }; > > + vpu: video-codec@fdb50000 { > + compatible = "rockchip,rk3588-vdpu121", "rockchip,rk3568-vpu"; > + reg = <0x0 0xfdb50000 0x0 0x800>; The register range at 0xfdb50000 length 0x800 includes "VEPU121 core0" encoder regs at offset 0 and "VDPU121" decoder regs at offset 0x400 (referring to the TRM v1.0 Part 1, section 5.5.1). So I think the "rockchip,rk3588-vdpu121" compatible isn't exactly correct to use for this entire device. IMO "rockchip,rk3588-vpu121" would be more appropriate if including both the decoder and encoder. It also raises the question of whether the decoder and encoder should be modeled in DT as one device like on RK3399, or separate devices. In the vendor DT [0] they are modeled as two devices but they share clocks, resets, IOMMU, and a "rockchip,taskqueue-node" value. I've tested the JPEG encoding functionality of this encoder with [1], and it seems to work, gstreamer produces a MJPEG video of the test pattern as expected. > + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>; > + interrupt-names = "vdpu"; > + clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>; > + clock-names = "aclk", "hclk"; > + iommus = <&vdpu_mmu>; > + power-domains = <&power RK3588_PD_VDPU>; > + }; > + > + vdpu_mmu: iommu@fdb50800 { > + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu"; > + reg = <0x0 0xfdb50800 0x0 0x40>; > + interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH 0>; > + clock-names = "aclk", "iface"; > + clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>; > + power-domains = <&power RK3588_PD_VDPU>; > + #iommu-cells = <0>; > + }; > + > av1d: video-codec@fdc70000 { > compatible = "rockchip,rk3588-av1-vpu"; > reg = <0x0 0xfdc70000 0x0 0x800>; > -- > 2.34.1 > [0]: https://github.com/friendlyarm/kernel-rockchip/blob/18fd1215fee01daef16b6ced1c0c3c3b83a4d8df/arch/arm64/boot/dts/rockchip/rk3588s.dtsi#L3630-L3683 [1]: https://github.com/sigmaris/linux/tree/rk3588-hantro-vpus with: gst-launch-1.0 videotestsrc pattern=ball flip=true ! v4l2jpegenc \ ! matroskamux ! filesink location=jpegtest.mkv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 2024-04-19 17:28 ` Hugh Cole-Baker @ 2024-04-20 5:09 ` Jianfeng Liu 2024-04-26 12:49 ` Nicolas Dufresne 0 siblings, 1 reply; 6+ messages in thread From: Jianfeng Liu @ 2024-04-20 5:09 UTC (permalink / raw) To: sigmaris Cc: conor+dt, devicetree, didi.debian, ezequiel, heiko, krzk+dt, linux-arm-kernel, linux-kernel, linux-media, linux-rockchip, liujianfeng1994, mchehab, p.zabel, robh, sebastian.reichel, sfr, nicolas, linkmauve Hi Hugh, Fri, 19 Apr 2024 18:28:01 +0100, Hugh Cole-Baker wrote: >The register range at 0xfdb50000 length 0x800 includes "VEPU121 core0" encoder >regs at offset 0 and "VDPU121" decoder regs at offset 0x400 (referring to the >TRM v1.0 Part 1, section 5.5.1). So I think the "rockchip,rk3588-vdpu121" >compatible isn't exactly correct to use for this entire device. There are five vepu121 cores for jpeg encoding. And Emmanuel is doing work on them[1]. And at the moment the driver doesn’t yet support exposing these cores all as a single video node to userspace, so Emmanuel only exposes one single core. >IMO "rockchip,rk3588-vpu121" would be more appropriate if including both the >decoder and encoder. It also raises the question of whether the decoder and >encoder should be modeled in DT as one device like on RK3399, or separate >devices. In the vendor DT [0] they are modeled as two devices but they share >clocks, resets, IOMMU, and a "rockchip,taskqueue-node" value. Now we have 5 jpeg enc cores, one from 0xfdb50000 and other four from 0xfdba00000. I tried to add a decoding only core 0xfb50400, but that does not work. So the vpu should be defined as one node in devicetree for both encoder and decoder like rk3399. This vpu121 should be exactly the same as the one in rk3399 which supports both encoding and decoding. But the current hantro driver has disabled h264 decoding since there is anthoer decoder rkvdec on rk3399. This vpu121 is the only decoder which supports h254 decoding on rk3588, so we can't just use the vpu_variant from rk3399. Maybe we can use rk3399_vpu_variant back when rkvdec2 on rk3588 is supported by mainline kernel. At the moment we can keep the compatible string same as the one from rk356x. Since there are already jpeg enc cores at 0xfdba0000, we can ignore the one at 0xfdb50000. When rkvdec2 is supported, I will change "rockchip,rk3588-vpu121" same as "rockchip,rk3399-vpu". And I think changing "rockchip,rk3588-vdpu121" to "rockchip,rk3588-vpu121" should match the hardware correctly. [1] https://lore.kernel.org/all/20240418141509.2485053-1-linkmauve@linkmauve.fr/ Best regards, Jianfeng ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 2024-04-20 5:09 ` Jianfeng Liu @ 2024-04-26 12:49 ` Nicolas Dufresne 0 siblings, 0 replies; 6+ messages in thread From: Nicolas Dufresne @ 2024-04-26 12:49 UTC (permalink / raw) To: Jianfeng Liu, sigmaris Cc: conor+dt, devicetree, didi.debian, ezequiel, heiko, krzk+dt, linux-arm-kernel, linux-kernel, linux-media, linux-rockchip, mchehab, p.zabel, robh, sebastian.reichel, sfr, linkmauve Le samedi 20 avril 2024 à 13:09 +0800, Jianfeng Liu a écrit : > Hi Hugh, > > Fri, 19 Apr 2024 18:28:01 +0100, Hugh Cole-Baker wrote: > > The register range at 0xfdb50000 length 0x800 includes "VEPU121 core0" encoder > > regs at offset 0 and "VDPU121" decoder regs at offset 0x400 (referring to the > > TRM v1.0 Part 1, section 5.5.1). So I think the "rockchip,rk3588-vdpu121" > > compatible isn't exactly correct to use for this entire device. > > There are five vepu121 cores for jpeg encoding. And Emmanuel is doing work on > them[1]. And at the moment the driver doesn’t yet support exposing these cores > all as a single video node to userspace, so Emmanuel only exposes one single > core. > > > IMO "rockchip,rk3588-vpu121" would be more appropriate if including both the > > decoder and encoder. It also raises the question of whether the decoder and > > encoder should be modeled in DT as one device like on RK3399, or separate > > devices. In the vendor DT [0] they are modeled as two devices but they share > > clocks, resets, IOMMU, and a "rockchip,taskqueue-node" value. > > Now we have 5 jpeg enc cores, one from 0xfdb50000 and other four from > 0xfdba00000. I tried to add a decoding only core 0xfb50400, but that does not > work. So the vpu should be defined as one node in devicetree for both encoder > and decoder like rk3399. > > This vpu121 should be exactly the same as the one in rk3399 which supports both > encoding and decoding. But the current hantro driver has disabled h264 decoding If its exactly the same combo as on rk3399, it have to be combined as they share the same internal memory. You'll notice this strange thing about both being 60fps FHD seperately and 30fps FHD concurrently, this is why. This leaves me with a feeling our understanding the of HW is far from perfect, we should be extra careful and circle back to Rockchip (ping Kever, he'll translate and CC the right person I suppose). Though, just exposing the decoder, and ignoring the encoder seems fine in the short term. Userspace will miss- behave though when we introduce the rkvdec2 decoder. As we'd like to change from trivial time multiplexing to proper core scheduling in the long term (for identical cores only), we have to keep in mind of the possible grouping, which will need to be something deducible from DT. Please keep that in mind when designing DT for this chip. > since there is anthoer decoder rkvdec on rk3399. This vpu121 is the only > decoder which supports h254 decoding on rk3588, so we can't just use the > vpu_variant from rk3399. Maybe we can use rk3399_vpu_variant back when rkvdec2 > on rk3588 is supported by mainline kernel. > > At the moment we can keep the compatible string same as the one from rk356x. > Since there are already jpeg enc cores at 0xfdba0000, we can ignore the one at > 0xfdb50000. When rkvdec2 is supported, I will change "rockchip,rk3588-vpu121" > same as "rockchip,rk3399-vpu". > > And I think changing "rockchip,rk3588-vdpu121" to "rockchip,rk3588-vpu121" > should match the hardware correctly. > > [1] https://lore.kernel.org/all/20240418141509.2485053-1-linkmauve@linkmauve.fr/ > > Best regards, > Jianfeng ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-26 12:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-18 11:10 [PATCH v6 0/2] Add hantro g1 video decoder support for RK3588 Jianfeng Liu 2024-04-18 11:10 ` [PATCH v6 1/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string Jianfeng Liu 2024-04-18 11:10 ` [PATCH v6 2/2] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588 Jianfeng Liu 2024-04-19 17:28 ` Hugh Cole-Baker 2024-04-20 5:09 ` Jianfeng Liu 2024-04-26 12:49 ` Nicolas Dufresne
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).