linux-kernel - Re: [PATCH v2 4/4] arm64: dts: rockchip: Add rkvdec2 Video Decoder on rk3588(s)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 20 Jun 2024 09:31:27 -0400
From: Detlev Casanova <detlev.casanova@...labora.com>
To: Alex Bee <knaerzche@...il.com>, Jonas Karlman <jonas@...boo.se>
Cc: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Heiko Stuebner <heiko@...ech.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 Dragan Simic <dsimic@...jaro.org>, Diederik de Haas <didi.debian@...ow.org>,
 Andy Yan <andy.yan@...k-chips.com>,
 Boris Brezillon <boris.brezillon@...labora.com>,
 Hans Verkuil <hverkuil-cisco@...all.nl>,
 Daniel Almeida <daniel.almeida@...labora.com>,
 Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Benjamin Gaignard <benjamin.gaignard@...labora.com>,
 linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject:
 Re: [PATCH v2 4/4] arm64: dts: rockchip: Add rkvdec2 Video Decoder on
 rk3588(s)

Hi Jonas, Alex,

On Wednesday, June 19, 2024 2:06:40 P.M. EDT Jonas Karlman wrote:
> Hi Alex,
> 
> On 2024-06-19 19:19, Alex Bee wrote:
> > Am 19.06.24 um 17:28 schrieb Jonas Karlman:
> >> Hi Detlev,
> >> 
> >> On 2024-06-19 16:57, Detlev Casanova wrote:
> >>> Add the rkvdec2 Video Decoder to the RK3588s devicetree.
> >>> 
> >>> Signed-off-by: Detlev Casanova <detlev.casanova@...labora.com>
> >>> ---
> >>> 
> >>>   arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 50 +++++++++++++++++++++++
> >>>   1 file changed, 50 insertions(+)
> >>> 
> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> >>> b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi index
> >>> 6ac5ac8b48ab..7690632f57f1 100644
> >>> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> >>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> >>> @@ -2596,6 +2596,16 @@ system_sram2: sram@...01000 {
> >>> 
> >>>   		ranges = <0x0 0x0 0xff001000 0xef000>;
> >>>   		#address-cells = <1>;
> >>>   		#size-cells = <1>;
> >>> 
> >>> +
> >>> +		vdec0_sram: rkvdec-sram@0 {
> >>> +			reg = <0x0 0x78000>;
> >>> +			pool;
> >>> +		};
> >>> +
> >>> +		vdec1_sram: rkvdec-sram@1 {
> >>> +			reg = <0x78000 0x77000>;
> >>> +			pool;
> >>> +		};
> >>> 
> >>>   	};
> >>>   	
> >>>   	pinctrl: pinctrl {
> >>> 
> >>> @@ -2665,6 +2675,46 @@ gpio4: gpio@...50000 {
> >>> 
> >>>   			#interrupt-cells = <2>;
> >>>   		
> >>>   		};
> >>>   	
> >>>   	};
> >>> 
> >>> +
> >>> +	vdec0: video-decoder@...38100 {
> >>> +		compatible = "rockchip,rk3588-vdec";
> >>> +		reg = <0x0 0xfdc38100 0x0 0x500>;
> >>> +		interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> +		clocks = <&cru ACLK_RKVDEC0>, <&cru HCLK_RKVDEC0>, 
<&cru
> >>> CLK_RKVDEC0_CA>, +			 <&cru 
CLK_RKVDEC0_CORE>, <&cru
> >>> CLK_RKVDEC0_HEVC_CA>;
> >>> +		clock-names = "axi", "ahb", "cabac", "core", 
"hevc_cabac";
> >>> +		assigned-clocks = <&cru ACLK_RKVDEC0>, <&cru 
CLK_RKVDEC0_CORE>,
> >>> +				  <&cru CLK_RKVDEC0_CA>, <&cru 
CLK_RKVDEC0_HEVC_CA>;
> >>> +		assigned-clock-rates = <800000000>, <600000000>,
> >>> +				       <600000000>, <1000000000>;
> >>> +		resets = <&cru SRST_A_RKVDEC0>, <&cru SRST_H_RKVDEC0>, 
<&cru
> >>> SRST_RKVDEC0_CA>, +			 <&cru 
SRST_RKVDEC0_CORE>, <&cru
> >>> SRST_RKVDEC0_HEVC_CA>;
> >>> +		reset-names = "rst_axi", "rst_ahb", "rst_cabac",
> >>> +			      "rst_core", "rst_hevc_cabac";
> >>> +		power-domains = <&power RK3588_PD_RKVDEC0>;
> >>> +		sram = <&vdec0_sram>;
> >>> +		status = "okay";
> >>> +	};
> >>> +
> >>> +	vdec1: video-decoder@...40100 {
> >>> +		compatible = "rockchip,rk3588-vdec";
> >>> +		reg = <0x0 0xfdc40100 0x0 0x500>;
> >>> +		interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH 0>;
> >>> +		clocks = <&cru ACLK_RKVDEC1>, <&cru HCLK_RKVDEC1>, 
<&cru
> >>> CLK_RKVDEC1_CA>, +			 <&cru 
CLK_RKVDEC1_CORE>, <&cru
> >>> CLK_RKVDEC1_HEVC_CA>;
> >>> +		clock-names = "axi", "ahb", "cabac", "core", 
"hevc_cabac";
> >>> +		assigned-clocks = <&cru ACLK_RKVDEC1>, <&cru 
CLK_RKVDEC1_CORE>,
> >>> +				  <&cru CLK_RKVDEC1_CA>, <&cru 
CLK_RKVDEC1_HEVC_CA>;
> >>> +		assigned-clock-rates = <800000000>, <600000000>,
> >>> +				       <600000000>, <1000000000>;
> >>> +		resets = <&cru SRST_A_RKVDEC1>, <&cru SRST_H_RKVDEC1>, 
<&cru
> >>> SRST_RKVDEC1_CA>, +			 <&cru 
SRST_RKVDEC1_CORE>, <&cru
> >>> SRST_RKVDEC1_HEVC_CA>;
> >>> +		reset-names = "rst_axi", "rst_ahb", "rst_cabac",
> >>> +			      "rst_core", "rst_hevc_cabac";
> >>> +		power-domains = <&power RK3588_PD_RKVDEC1>;
> >>> +		sram = <&vdec1_sram>;
> >>> +		status = "okay";
> >>> +	};
> >> 
> >> This is still missing the iommus, please add the iommus, they should be
> >> 
> >> supported/same as the one used for e.g. VOP2:
> >>    compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> >> 
> >> The VOP2 MMUs does have one extra mmu_cfg_mode flag in AUTO_GATING,
> >> compared to the VDPU381 MMUs, however only the AV1D MMU should be
> >> special on RK3588.
> >> 
> >> Please add the iommus :-)
> > 
> > When looking add the vendor DT/iommu driver I'm seeing serval quirks
> > applied for vdec's iommus. Since it's rightly frowned upon adding such
> > boolean-quirk-properties to upstream devicetrees, we'd at least need
> > additional (fallback-) compatibles, even if it works with the iommu driver
> > as is (what I doubt, but haven't tested). We need to be able to apply
> > those
> > quirks later without changing the devicetree (as usual) and I'm sure RK
> > devs haven't added these quirks for the personal amusement.
> 
> Based on what I investigated the hw should work similar, and the quirks
> mostly seem related to optimizations and sw quirks, like do not zap each
> line, keep it alive even when pm runtime say it is not in use and other
> quirks that seem to be more of sw nature on how to best utilize the hw.

I did some testing with the IOMMU but unfortunately, I'm only getting page 
fault errors. This may be something I'm doing wrong, but it clearly needs more 
investigation.

> > If Detlev says
> > iommu is out of scope for this series (which is valid), I'd say it's fine
> > to leave them out for now (as no binding exists) and the HW works
> > (obviously) fine without them.
> 
> Sure, use of MMU can be added later.

I'd rather go for that for now. I'll add that IMMU support is missing in the 
TODO file.

> Regards,
> Jonas
> 
> >> Regards,
> >> Jonas
> >> 
> >>>   };
> >>>   
> >>>   #include "rk3588s-pinctrl.dtsi"


Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)