2022-11-16 19:44:53

by Jernej Škrabec

[permalink] [raw]
Subject: [PATCH 0/2] arm64: allwinner: h6: Add second IOMMU reference to Cedrus

It turns out that Cedrus on H6 actually uses two IOMMU channels. Manual
mentiones two IOMMU channels, but it doesn't specify which is used when.
Page faults were also observed from both.

This series updates binding to allow up to two IOMMU channels and fixes
H6 DTSI.

Please take a look.

Best regards,
Jernej

Jernej Skrabec (2):
media: dt-bindings: allwinner: video-engine: Fix number of IOMMU
channels
arm64: dts: allwinner: h6: Fix Cedrus IOMMU channels

.../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 2 +-
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

--
2.38.1



2022-11-16 19:46:59

by Jernej Škrabec

[permalink] [raw]
Subject: [PATCH 1/2] media: dt-bindings: allwinner: video-engine: Fix number of IOMMU channels

Cedrus (video engine) on Allwinner H6 actually uses two IOMMU channel,
not just one. However, Cedrus on SoCs like D1 only uses one channel.

Allow up to 2 IOMMU channels.

Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage")
Signed-off-by: Jernej Skrabec <[email protected]>
---
.../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
index 541325f900a1..257bb372d166 100644
--- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
+++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
@@ -55,7 +55,7 @@ properties:
description: Phandle to the device SRAM

iommus:
- maxItems: 1
+ maxItems: 2

memory-region:
maxItems: 1
--
2.38.1


2022-11-16 20:27:59

by Jernej Škrabec

[permalink] [raw]
Subject: [PATCH 2/2] arm64: dts: allwinner: h6: Fix Cedrus IOMMU channels

Cedrus H6 actually uses two IOMMU channels. During development page
faults from both were observed. Documentation also lists both of them
to be connected to Cedrus, but it doesn't make clear which is used for
what.

Add second IOMMU channel.

Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage")
Signed-off-by: Jernej Skrabec <[email protected]>
---
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 53f6660656ac..7bff054a9bdf 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -172,7 +172,7 @@ video-codec@1c0e000 {
resets = <&ccu RST_BUS_VE>;
interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
allwinner,sram = <&ve_sram 1>;
- iommus = <&iommu 3>;
+ iommus = <&iommu 1>, <&iommu 3>;
};

gpu: gpu@1800000 {
--
2.38.1


2022-11-16 23:29:08

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: allwinner: h6: Fix Cedrus IOMMU channels

On 11/16/22 13:31, Jernej Skrabec wrote:
> Cedrus H6 actually uses two IOMMU channels. During development page
> faults from both were observed. Documentation also lists both of them
> to be connected to Cedrus, but it doesn't make clear which is used for
> what.
>
> Add second IOMMU channel.
>
> Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage")
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Samuel Holland <[email protected]>


2022-11-16 23:40:53

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH 1/2] media: dt-bindings: allwinner: video-engine: Fix number of IOMMU channels

On 11/16/22 13:31, Jernej Skrabec wrote:
> Cedrus (video engine) on Allwinner H6 actually uses two IOMMU channel,
> not just one. However, Cedrus on SoCs like D1 only uses one channel.
>
> Allow up to 2 IOMMU channels.
>
> Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage")
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> .../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
> index 541325f900a1..257bb372d166 100644
> --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
> +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
> @@ -55,7 +55,7 @@ properties:
> description: Phandle to the device SRAM
>
> iommus:
> - maxItems: 1
> + maxItems: 2

You need to add minItems: 1, or else this will require two items.

Regards,
Samuel

>
> memory-region:
> maxItems: 1


2022-11-16 23:56:04

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/2] media: dt-bindings: allwinner: video-engine: Fix number of IOMMU channels


On Wed, 16 Nov 2022 20:31:04 +0100, Jernej Skrabec wrote:
> Cedrus (video engine) on Allwinner H6 actually uses two IOMMU channel,
> not just one. However, Cedrus on SoCs like D1 only uses one channel.
>
> Allow up to 2 IOMMU channels.
>
> Fixes: 62a8ccf3a248 ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage")
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> .../bindings/media/allwinner,sun4i-a10-video-engine.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/


video-codec@1c0e000: iommus: [[10, 3]] is too short
arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-lite2.dtb
arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dtb

video-codec@1c0e000: iommus: [[12, 3]] is too short
arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dtb
arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dtb
arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dtb
arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dtb
arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6.dtb
arch/arm64/boot/dts/allwinner/sun50i-h6-tanix-tx6-mini.dtb