[PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[Jianfeng Liu]

This is the v2 version of this series adding hantro g1 video decoder
support for rk3588.

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

Jianfeng Liu (3):
  media: verisilicon: Add support for Hantro G1 on RK3588
  arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588
  dt-bindings: media: rockchip-vpu: Add RK3588 compatible

 .../bindings/media/rockchip-vpu.yaml          |  1 +
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 20 +++++++++++++++++++
 .../media/platform/verisilicon/hantro_drv.c   |  1 +
 3 files changed, 22 insertions(+)

-- 
2.34.1

[PATCH v2 1/3] media: verisilicon: Add support for Hantro G1 on RK3588

[Jianfeng Liu]

This patch adds support for existing Hantro G1 video
decoder on RK3588.

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>
---
 drivers/media/platform/verisilicon/hantro_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index db3df6cc4..21334bb64 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -723,6 +723,7 @@ static const struct of_device_id of_hantro_match[] = {
 	{ .compatible = "rockchip,rk3568-vepu", .data = &rk3568_vepu_variant, },
 	{ .compatible = "rockchip,rk3568-vpu", .data = &rk3568_vpu_variant, },
 	{ .compatible = "rockchip,rk3588-av1-vpu", .data = &rk3588_vpu981_variant, },
+	{ .compatible = "rockchip,rk3588-vpu", .data = &rk3568_vpu_variant, },
 #endif
 #ifdef CONFIG_VIDEO_HANTRO_IMX8M
 	{ .compatible = "nxp,imx8mm-vpu-g1", .data = &imx8mm_vpu_g1_variant, },
-- 
2.34.1

[PATCH v2 2/3] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588

[Jianfeng Liu]

This patch enables 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>
---
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 5fb0baf8a..5da668184 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -640,6 +640,26 @@ i2c0: i2c@fd880000 {
 		status = "disabled";
 	};
 
+	vpu: video-codec@fdb50400 {
+		compatible = "rockchip,rk3588-vpu";
+		reg = <0x0 0xfdb50000 0x0 0x800>;
+		interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
+		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>;
+	};
+
 	vop: vop@fdd90000 {
 		compatible = "rockchip,rk3588-vop";
 		reg = <0x0 0xfdd90000 0x0 0x4200>, <0x0 0xfdd95000 0x0 0x1000>;
-- 
2.34.1

Re: [PATCH v2 2/3] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588

[Hugh Cole-Baker]

On Thu, 28 Dec 2023 at 13:17, Jianfeng Liu <liujianfeng1994@gmail.com> wrote:
>
> This patch enables 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>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 5fb0baf8a..5da668184 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -640,6 +640,26 @@ i2c0: i2c@fd880000 {
>                 status = "disabled";
>         };
>
> +       vpu: video-codec@fdb50400 {

The node name should be video-codec@fdb50000 to match the reg address.

> +               compatible = "rockchip,rk3588-vpu";
> +               reg = <0x0 0xfdb50000 0x0 0x800>;
> +               interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH 0>;
> +               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>;
> +       };
> +
>         vop: vop@fdd90000 {
>                 compatible = "rockchip,rk3588-vop";
>                 reg = <0x0 0xfdd90000 0x0 0x4200>, <0x0 0xfdd95000 0x0 0x1000>;
> --
> 2.34.1

Re: [PATCH v2 2/3] arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588

[amazingfate]

On Fri, 29 Dec 2023 11:02:08 +0000, Hugh Cole-Baker <sigmaris@gmail.com> wrote:
>The node name should be video-codec@fdb50000 to match the reg address.

Hi,
Thanks a lot for youre review, I will change the node name in v3.

Jianfeng

[PATCH v2 3/3] dt-bindings: media: rockchip-vpu: Add RK3588 compatible

[Jianfeng Liu]

Add compatible for rk3588 Hantro G1 vpu decoder.

Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
---
 Documentation/devicetree/bindings/media/rockchip-vpu.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rockchip-vpu.yaml b/Documentation/devicetree/bindings/media/rockchip-vpu.yaml
index c57e1f488..ba41446f6 100644
--- a/Documentation/devicetree/bindings/media/rockchip-vpu.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip-vpu.yaml
@@ -25,6 +25,7 @@ properties:
           - rockchip,px30-vpu
           - rockchip,rk3568-vpu
           - rockchip,rk3588-av1-vpu
+          - rockchip,rk3588-vpu
       - items:
           - const: rockchip,rk3188-vpu
           - const: rockchip,rk3066-vpu
-- 
2.34.1

Re: [PATCH v2 3/3] dt-bindings: media: rockchip-vpu: Add RK3588 compatible

[Krzysztof Kozlowski]

On 28/12/2023 14:16, Jianfeng Liu wrote:
> Add compatible for rk3588 Hantro G1 vpu decoder.
> 
> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> ---
>  Documentation/devicetree/bindings/media/rockchip-vpu.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip-vpu.yaml b/Documentation/devicetree/bindings/media/rockchip-vpu.yaml
> index c57e1f488..ba41446f6 100644
> --- a/Documentation/devicetree/bindings/media/rockchip-vpu.yaml
> +++ b/Documentation/devicetree/bindings/media/rockchip-vpu.yaml
> @@ -25,6 +25,7 @@ properties:
>            - rockchip,px30-vpu
>            - rockchip,rk3568-vpu
>            - rockchip,rk3588-av1-vpu
> +          - rockchip,rk3588-vpu

Your driver suggests it is compatible with 3568-vpu.

Also, nothing in commit msg explains me differences against existing
av1-vpu.

Best regards,
Krzysztof

Re: [PATCH v2 3/3] dt-bindings: media: rockchip-vpu: Add RK3588 compatible

[amazingfate]

Hi Krzysztof,

Sat, 30 Dec 2023 17:06:48 +0100, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>Your driver suggests it is compatible with 3568-vpu.
>
>Also, nothing in commit msg explains me differences against existing
>av1-vpu.

Yes, this vpu is exactly the same as the one on rk3568, so we can
make rockchip,rk3588-vpu compatible with rockchip,rk3568-vpu, just
like what we have done on rk3188/rk3066 and rk3228/rk3399.
I will update it in v3 patch.

Best regards,
Jianfeng

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[Hugh Cole-Baker]

On Thu, 28 Dec 2023 at 13:16, Jianfeng Liu <liujianfeng1994@gmail.com> wrote:
>
> This is the v2 version of this series adding hantro g1 video decoder
> support for rk3588.
>
> 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
>
> Jianfeng Liu (3):
>   media: verisilicon: Add support for Hantro G1 on RK3588
>   arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588
>   dt-bindings: media: rockchip-vpu: Add RK3588 compatible
>
>  .../bindings/media/rockchip-vpu.yaml          |  1 +
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 20 +++++++++++++++++++
>  .../media/platform/verisilicon/hantro_drv.c   |  1 +
>  3 files changed, 22 insertions(+)
>
> --
> 2.34.1

Tested H.264 and VP8 decode with Fluster on NanoPC T6;

H.264 JVT-AVC_V1 test suite:
  Ran 129/135 tests successfully.
  (matches FFmpeg-H.264 software decoder)
H.264 JVT-FR-EXT test suite:
  Ran 44/69 tests successfully
  (some failures caused by lack of support for Hi10P and 4:2:2)
VP8-TEST-VECTORS test suite:
  Ran 59/61 tests successfully
  (matches FFmpeg-VP8 software decoder)
Full results at:
https://gist.github.com/sigmaris/d3d8586bfced5ddc021aa9dab94d4ab8

Tested-by: Hugh Cole-Baker <sigmaris@gmail.com>

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[amazingfate]

Thanks for your test with fluster. I also tested on my rock-5b(rk3588)
and orangepi-3b(rk3566) which are sharing the same hantro g1 ip. I am
running on ubuntu jammy so I'm using gstreamer 1.20.1

rk3566 and rk3588 are sharing the same results:
JVT-AVC_V1 test suite with decoder GStreamer-H.264-V4L2SL-Gst1.0:
  Ran 112/135 tests successfully.
JVT-FR_EXT test suite with decoder GStreamer-H.264-V4L2SL-Gst1.0:
  Ran 27/69 tests successfully.
VP8-TEST-VECTORS test suite with decoder GStreamer-VP8-V4L2SL-Gst1.0:
  Ran 59/61 tests successfully.

H264 decoder test has less test case passing. I think that's because
of my low gstreamer version. If you have a rk356x board I guess you
will get the same result as rk3588. That should be the mainline support
status of hantro g1 decoder.

For ffmpeg at the moment fluster doesn't support v4l2-request decoder.
I tried Kwiboo's fork[1] but failed to pass tests with decoder
FFmpeg-H.264-V4L2-request. I can decode video with ffmpeg command like:
"ffmpeg -benchmark -hwaccel drm -hwaccel_output_format drm_prime -i Big_Buck_Bunny_1080_10s_30MB.mp4 -f null -"

[1] https://github.com/Kwiboo/fluster/tree/v4l2-request

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[Jonas Karlman]

Hi,

On 2023-12-30 10:52, amazingfate wrote:
> Thanks for your test with fluster. I also tested on my rock-5b(rk3588)
> and orangepi-3b(rk3566) which are sharing the same hantro g1 ip. I am
> running on ubuntu jammy so I'm using gstreamer 1.20.1
> 
> rk3566 and rk3588 are sharing the same results:
> JVT-AVC_V1 test suite with decoder GStreamer-H.264-V4L2SL-Gst1.0:
>   Ran 112/135 tests successfully.
> JVT-FR_EXT test suite with decoder GStreamer-H.264-V4L2SL-Gst1.0:
>   Ran 27/69 tests successfully.
> VP8-TEST-VECTORS test suite with decoder GStreamer-VP8-V4L2SL-Gst1.0:
>   Ran 59/61 tests successfully.
> 
> H264 decoder test has less test case passing. I think that's because
> of my low gstreamer version. If you have a rk356x board I guess you
> will get the same result as rk3588. That should be the mainline support
> status of hantro g1 decoder.
> 
> For ffmpeg at the moment fluster doesn't support v4l2-request decoder.
> I tried Kwiboo's fork[1] but failed to pass tests with decoder
> FFmpeg-H.264-V4L2-request. I can decode video with ffmpeg command like:
> "ffmpeg -benchmark -hwaccel drm -hwaccel_output_format drm_prime -i Big_Buck_Bunny_1080_10s_30MB.mp4 -f null -"

I have only tested this fork of fluster with ffmpeg 6.x, what version of
ffmpeg did you test with? I was expecting it to also work on ffmpeg 5.x.

Please also note that ffmpeg v4l2-request patches at [2] contain some
NV15/NV20 ffmpeg pix_fmt patches that fail ffmpeg tests. They are not
needed for decoding of 10-bit frames using rkvdec but they are required
to be able to run fluster test suite JVT-FR_EXT on rkvdec. (hantro g1 do
not support 10-bit frames on rk)

[2] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.1

Regards,
Jonas

> 
> [1] https://github.com/Kwiboo/fluster/tree/v4l2-request
> 

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[amazingfate]

Hi Jonas,

On Sat, 30 Dec 2023 12:49:38 +0100, Jonas Karlman <jonas@kwiboo.se> wrote:
>I have only tested this fork of fluster with ffmpeg 6.x, what version of
>ffmpeg did you test with? I was expecting it to also work on ffmpeg 5.x.

I am using ffmpeg 6.0 with v4l2-request patches from libreelec[1].
Ffmpeg v4l2 decoder in fluster fork is using ffmpeg args:
"-hwaccel_device /dev/dri/renderD128"
which make the test fall with hantro g1 on rk3588. After removing it I
can run tests by ffmpeg v4l2-request decoder. 

Rk3566 and rk3588 are sharing the same results:
JVT-AVC_V1 test suite with decoder FFmpeg-H.264-V4L2-request:
 Ran 127/135 tests successfully.
JVT-FR_EXT test suite with decoder FFmpeg-H.264-V4L2-request:
 Ran 44/69 tests successfully.
VP8-TEST-VECTORS test suite with decoder FFmpeg-VP8-V4L2-request:
 Ran 59/61 tests successfully.

[1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/multimedia/ffmpeg/patches/v4l2-request/ffmpeg-001-v4l2-request.patch

Regards,
Jianfeng

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[Jonas Karlman]

Hi Jianfeng,

On 2023-12-30 16:31, amazingfate wrote:
> Hi Jonas,
> 
> On Sat, 30 Dec 2023 12:49:38 +0100, Jonas Karlman <jonas@kwiboo.se> wrote:
>> I have only tested this fork of fluster with ffmpeg 6.x, what version of
>> ffmpeg did you test with? I was expecting it to also work on ffmpeg 5.x.
> 
> I am using ffmpeg 6.0 with v4l2-request patches from libreelec[1].

Great, that patch should match my v4l2-request-n6.0.1 branch [2],
and does not have any NV15/NV20 ffmpeg pix fmt patches that could
interfere.

[2] https://github.com/Kwiboo/FFmpeg/commits/v4l2-request-n6.0.1/

> Ffmpeg v4l2 decoder in fluster fork is using ffmpeg args:
> "-hwaccel_device /dev/dri/renderD128"
> which make the test fall with hantro g1 on rk3588. After removing it I
> can run tests by ffmpeg v4l2-request decoder. 

Thanks for confirming and testing again, and I fully understand why
"-hwaccel_device /dev/dri/renderD128" caused issues on rk3588 :-)

The commit "HACK: hwcontext_drm: do not require drm device" was required
to run ffmpeg without a hwaccel_device, and I tested fluster without it.

Regards,
Jonas

> 
> Rk3566 and rk3588 are sharing the same results:
> JVT-AVC_V1 test suite with decoder FFmpeg-H.264-V4L2-request:
>  Ran 127/135 tests successfully.
> JVT-FR_EXT test suite with decoder FFmpeg-H.264-V4L2-request:
>  Ran 44/69 tests successfully.
> VP8-TEST-VECTORS test suite with decoder FFmpeg-VP8-V4L2-request:
>  Ran 59/61 tests successfully.
> 
> [1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/multimedia/ffmpeg/patches/v4l2-request/ffmpeg-001-v4l2-request.patch
> 
> Regards,
> Jianfeng

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[Alex Bee]

Hi, Jianfeng
Am 28.12.23 um 14:16 schrieb Jianfeng Liu:
> This is the v2 version of this series adding hantro g1 video decoder
> support for rk3588.
> 
> 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
if the RK3568 and RK3588 variants match, patch [1/3] is not necessary. Just
document a additional compatible in a similar way it's being done for
rk3188/rk3066 or rk3228/rk3399.
If there are ever differences we don't know about yet, a additional variant
can still be added in the driver.

Alex


> - Link to v1: https://lore.kernel.org/all/20231227173911.3295410-1-liujianfeng1994@gmail.com
> 
> Jianfeng Liu (3):
>    media: verisilicon: Add support for Hantro G1 on RK3588
>    arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588
>    dt-bindings: media: rockchip-vpu: Add RK3588 compatible
> 
>   .../bindings/media/rockchip-vpu.yaml          |  1 +
>   arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 20 +++++++++++++++++++
>   .../media/platform/verisilicon/hantro_drv.c   |  1 +
>   3 files changed, 22 insertions(+)
> 

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[amazingfate]

Jianfeng

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[amazingfate]

>On Sat, 30 Dec 2023 10:46:42 +0100, Alex Bee <knaerzche@gmail.com> wrote:
>if the RK3568 and RK3588 variants match, patch [1/3] is not necessary. Just
>document a additional compatible in a similar way it's being done for
>rk3188/rk3066 or rk3228/rk3399.
>If there are ever differences we don't know about yet, a additional variant
>can still be added in the driver.

Hi Alex,
I did try to not touching hantro driver code and just enabling the node in
devicetree by a compatible property like:
compatible = "rockchip,rk3588-vpu", "rockchip,rk3568-vpu";
but the hantro g1 node is not binded by hantro driver. I guess that is
because hantro driver is already binded to the hantro av1 vpu on rk3588,
so I added patch[1/3] to make it work.

Jianfeng

Re: [PATCH v2 0/3] Add hantro g1 video decoder support for RK3588

[amazingfate]

Hi Alex,

On Sat, 30 Dec 2023 10:46:42 +0100, Alex Bee <knaerzche@gmail.com> wrote:
>if the RK3568 and RK3588 variants match, patch [1/3] is not necessary. Just
>document a additional compatible in a similar way it's being done for
>rk3188/rk3066 or rk3228/rk3399.
>If there are ever differences we don't know about yet, a additional variant
>can still be added in the driver.
You are right, this way works. I should have encounted a ccache issue which
made me running a devicetree I didn't want.

I will drop patch[1/3] and only add devicetree node and dt-binding in v3.

Best regards,
Jianfeng