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@xxxxxxxxxxxxx>
> >>> ---
> >>>
> >>> 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@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,46 @@ gpio4: gpio@fec50000 {
> >>>
> >>> #interrupt-cells = <2>;
> >>>
> >>> };
> >>>
> >>> };
> >>>
> >>> +
> >>> + vdec0: video-decoder@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@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.
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"
Attachment:
signature.asc
Description: This is a digitally signed message part.
