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

[Jonas Karlman]

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 at collabora.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 at ff001000 {
>>>   		ranges = <0x0 0x0 0xff001000 0xef000>;
>>>   		#address-cells = <1>;
>>>   		#size-cells = <1>;
>>> +
>>> +		vdec0_sram: rkvdec-sram at 0 {
>>> +			reg = <0x0 0x78000>;
>>> +			pool;
>>> +		};
>>> +
>>> +		vdec1_sram: rkvdec-sram at 1 {
>>> +			reg = <0x78000 0x77000>;
>>> +			pool;
>>> +		};
>>>   	};
>>>   
>>>   	pinctrl: pinctrl {
>>> @@ -2665,6 +2675,46 @@ gpio4: gpio at fec50000 {
>>>   			#interrupt-cells = <2>;
>>>   		};
>>>   	};
>>> +
>>> +	vdec0: video-decoder at fdc38100 {
>>> +		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 at fdc40100 {
>>> +		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.

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

Regards,
Jonas

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