[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5790441.DvuYhMxLoT@arisu>
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)
Powered by blists - more mailing lists