[PATCH v5 0/2] Add hantro g1 video decoder support for RK3588

[PATCH v5 0/2] Add hantro g1 video decoder support for RK3588

[Jianfeng Liu]

This is the v5 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 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/

[1]https://lore.kernel.org/all/20240118080602.9028-1-liujianfeng1994@gmail.com/

Jianfeng Liu (2):
  arm64: dts: rockchip: Add Hantro G1 VPU support for RK3588
  dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string

 .../bindings/media/rockchip-vpu.yaml          |  3 +++
 arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 21 +++++++++++++++++++
 2 files changed, 24 insertions(+)

-- 
2.34.1

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

[Jianfeng Liu]

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>
---
 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 ac5bd630f..de823f461 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -1072,6 +1072,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

[PATCH v5 2/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string

[Jianfeng Liu]

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

Re: [PATCH v5 2/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string

[Krzysztof Kozlowski]

On 13/04/2024 08:46, Jianfeng Liu wrote:
> 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>

This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.

Best regards,
Krzysztof

Re: Re: [PATCH v5 2/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string

[Jianfeng Liu]

Hi Krzysztof,

I'm sorry for my unkonwing about the kernel patching process. And I'm
sorry to let maintainers do extra work. Thank you for teaching me this.
I will do this right in future patches.

I did received a Acked-by tag from Conor in v4:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

I note it here in case someone forgets this tag.

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1874 bytes --]

Hi,

On Sat, Apr 13, 2024 at 02:46:07PM +0800, Jianfeng Liu 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>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  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 ac5bd630f..de823f461 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -1072,6 +1072,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
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 2/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

Hi,

On Sat, Apr 13, 2024 at 02:46:08PM +0800, Jianfeng Liu wrote:
> 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>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 2/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

On Saturday, 13 April 2024 17:57:09 CEST Jianfeng Liu wrote:
> I'm sorry for my unkonwing about the kernel patching process. And I'm
> sorry to let maintainers do extra work. Thank you for teaching me this.
> I will do this right in future patches.
> 
> I did received a Acked-by tag from Conor in v4:
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> I note it here in case someone forgets this tag.

I think it's beneficial to send a v6 with the following changes:
1) Make this dt-bindings patch the first in the series
2) Make sure you've collected all the tags you've received to all the patches
3) Specify the base commit

ad 1) I don't know if it's a hard rule, but I've seen a consistent pattern 
where the dt-binding changes come before those changes being applied to 
DeviceTree files. It also makes sense as when the dt-binding change hasn't been 
applied, then the DT file is technically invalid.

ad 2) You shouldn't make maintainers do extra work to get your patch(es) 
merged; you want to make their work as easy as possible. Thus you do the 
(extra) work and provide a new version of the patch(es).
Sending multiple versions in a single day is generally not recommended as you 
should give reviewers some time to do the review. But it should be fine now as 
several days have past without new reviews.

ad 3) The `git format-patch` command has a `--base=<commit>` parameter with 
which you can make explicit upon which commit the patch is based.
That works a lot better/easier then a textual description.

HTH

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 2/2] dt-bindings: media: rockchip-vpu: Add rk3588 vdpu121 compatible string

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2502 bytes --]

On Thu, Apr 18, 2024 at 11:19:56AM +0200, Diederik de Haas wrote:
> On Saturday, 13 April 2024 17:57:09 CEST Jianfeng Liu wrote:
> > I'm sorry for my unkonwing about the kernel patching process. And I'm
> > sorry to let maintainers do extra work. Thank you for teaching me this.
> > I will do this right in future patches.
> > 
> > I did received a Acked-by tag from Conor in v4:
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> > I note it here in case someone forgets this tag.
> 
> I think it's beneficial to send a v6 with the following changes:
> 1) Make this dt-bindings patch the first in the series
> 2) Make sure you've collected all the tags you've received to all the patches
> 3) Specify the base commit
> 
> ad 1) I don't know if it's a hard rule, but I've seen a consistent pattern 
> where the dt-binding changes come before those changes being applied to 
> DeviceTree files. It also makes sense as when the dt-binding change hasn't been 
> applied, then the DT file is technically invalid.

It is definitely preferred, since there is tooling that checks for
undocumented compatibles etc that would see spurious errors during
bisection, were that to be done. Generally I wouldn't suggest resending
for the order though if it were the only thing amiss.

> ad 2) You shouldn't make maintainers do extra work to get your patch(es) 
> merged; you want to make their work as easy as possible. Thus you do the 
> (extra) work and provide a new version of the patch(es).
> Sending multiple versions in a single day is generally not recommended as you 
> should give reviewers some time to do the review. But it should be fine now as 
> several days have past without new reviews.

I dunno, the best way to save our time is to not omit the tags in the
first place (or give a reason as to why you did) as we'll likely pull up
the previous version of the series to see if all comments were
addressed, if made by another maintainer (at least I do that and Krzysztof
must have here). In my workflow sending another ack takes much less time
than looking up previous versions and checking to see if things were
dealt with - probably that's in-part to me lacking automation for dfn:
lore search from mutt though..

> 
> ad 3) The `git format-patch` command has a `--base=<commit>` parameter with 
> which you can make explicit upon which commit the patch is based.
> That works a lot better/easier then a textual description.
> 
> HTH



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]