Hi Detlev, On 2024-06-25 19:40, Detlev Casanova wrote: > Hi Jonas, > > On Thursday, June 20, 2024 11:00:49 A.M. EDT Jonas Karlman wrote: >> Hi Detlev, >> >> On 2024-06-20 16:19, Detlev Casanova wrote: >>> Add the rkvdec2 Video Decoder to the RK3588s devicetree. >>> >>> Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx> >>> --- >>> >>> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 48 +++++++++++++++++++++++ >>> 1 file changed, 48 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi >>> b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi index >>> 6ac5ac8b48ab..9c44c99125b4 100644 >>> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi >>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi >>> @@ -2596,6 +2596,16 @@ system_sram2: sram@ff001000 { >>> >>> 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,44 @@ gpio4: gpio@fec50000 { >>> >>> #interrupt-cells = <2>; >>> >>> }; >>> >>> }; >>> >>> + >>> + vdec0: video-decoder@fdc38100 { >> >> This and the vdec1 node should probably be added between >> >> pmu: power-management@fd8d8000 >> >> and >> >> av1d: video-codec@fdc70000 >> >> to follow reg order. >> >> Also I am wondering if the nodes should be named >> >> video-codec@fdc38000 >> >> and >> >> video-codec@fdc40000 >> >> to match "1.1 Address Mapping" in TRM and the actual base address for >> the VDPU381 IP and video-codec is used for other codec nodes. >> >>> + compatible = "rockchip,rk3588-vdec"; >>> + reg = <0x0 0xfdc38100 0x0 0x500>; >> >> For existing rkvdec1 devices the cache regs is also included in the >> range, should cache regs also be included for rkvdec2?, e.g.: >> >> reg = <0x0 0xfdc38100 0x0 0x600>; >> >> And maybe it also should include the link list regs, e.g.: >> >> reg = <0x0 0xfdc38000 0x0 0x700>; >> >> or possible: >> >> reg = <0x0 0xfdc38000 0x0 0x100>, >> <0x0 0xfdc38100 0x0 0x500>, >> <0x0 0xfdc38600 0x0 0x100>; >> >> Something like that may be a better description of the hw. > > Would it make sense to also add reg-names then ? > reg-names = "link", "function", "cache"; This sounds like a good idea and matches TRM: