[5/7] arm64: dts: allwinner: sun50i-h6: Add missing SRAM region for video engine

The SRAM C region contains a partial alias to the VE SRAM already
referenced by the video engine. To avoid access through this alias
window, the region should also be claimed by the video engine.

Add a reference to the SRAM C region to the video engine node.

Fixes: b542570e5605 ("arm64: dts: allwinner: h6: Add Video Engine node")
Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Dne torek, 5. maj 2026 ob 15:48:09 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> The SRAM C region contains a partial alias to the VE SRAM already
> referenced by the video engine. To avoid access through this alias
> window, the region should also be claimed by the video engine.
> 
> Add a reference to the SRAM C region to the video engine node.
> 
> Fixes: b542570e5605 ("arm64: dts: allwinner: h6: Add Video Engine node")
> Signed-off-by: Chen-Yu Tsai <wens@kernel.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej
  

Hi Chen-Yu,

On Tue 05 May 26, 21:48, Chen-Yu Tsai wrote:
> The SRAM C region contains a partial alias to the VE SRAM already
> referenced by the video engine. To avoid access through this alias
> window, the region should also be claimed by the video engine.
> 
> Add a reference to the SRAM C region to the video engine node.

It feels very weird to have a reference to the DE2 SRAM region in the
VE node. It seems unlikely that the same region would be used by both DE2 and
VE and I am pretty sure can have both running at the same time without
overstepping.

From what I can see we have so far assumed that the SRAM C and SRAM C1
are two different physical SRAM areas, but this is most likely not the case.
My guess would be that SRAM C1 is actually a part of SRAM C and the DE2 is
using another part of it. The syscon block probably allows switching access
to these different parts of SRAM C.

Also the sram_c1 node implies it's 2 MiB, which sounds quite unlikely.

All the best,

Paul

> 
> Fixes: b542570e5605 ("arm64: dts: allwinner: h6: Add Video Engine node")
> Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
> ---
>  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 72ce1a75647b..88c6e3e105c0 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -205,7 +205,7 @@ video-codec@1c0e000 {
>  			clock-names = "ahb", "mod", "ram";
>  			resets = <&ccu RST_BUS_VE>;
>  			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> -			allwinner,sram = <&ve_sram 1>;
> +			allwinner,sram = <&ve_sram 1>, <&de2_sram 1>;
>  			iommus = <&iommu 3>;
>  		};
>  
> -- 
> 2.47.3
>
  

On Tue, May 5, 2026 at 10:35 PM Paul Kocialkowski <paulk@sys-base.io> wrote:
>
> Hi Chen-Yu,
>
> On Tue 05 May 26, 21:48, Chen-Yu Tsai wrote:
> > The SRAM C region contains a partial alias to the VE SRAM already
> > referenced by the video engine. To avoid access through this alias
> > window, the region should also be claimed by the video engine.
> >
> > Add a reference to the SRAM C region to the video engine node.
>
> It feels very weird to have a reference to the DE2 SRAM region in the
> VE node. It seems unlikely that the same region would be used by both DE2 and
> VE and I am pretty sure can have both running at the same time without
> overstepping.
>
> From what I can see we have so far assumed that the SRAM C and SRAM C1
> are two different physical SRAM areas, but this is most likely not the case.
> My guess would be that SRAM C1 is actually a part of SRAM C and the DE2 is

No. It's the opposite. 0x28000 maps to the same SRAM block at 0x1a00000.
If you write to one, you see the results in both. However the alias at
028000 can be disabled while retaining access to 0x1a00000. I don't
remember how much of the alias points to VE SRAM (0x1a00000). Maybe it
was the whole alias on the H6.

Also, I can't remember if this was tested on the H6 or H616, but the
VE indeed writes to VE SRAM (0x1a00000), so that part is indeed used
by the video engine.

> using another part of it. The syscon block probably allows switching access
> to these different parts of SRAM C.

>From what I've seen in the vendor BSP, both drivers toggle the switch.
I believe the goal is to prevent the CPU from having access, rather than
either peripheral claiming it as its own. And that is also what our SRAM
driver does. It claims the SRAM from the CPU. That's all. The toggle bit
pattern used by both drivers is the same.

> Also the sram_c1 node implies it's 2 MiB, which sounds quite unlikely.

That is what the memory map says. Calling it SRAM C1 is probably wrong.


ChenYu

> All the best,
>
> Paul
>
> >
> > Fixes: b542570e5605 ("arm64: dts: allwinner: h6: Add Video Engine node")
> > Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
> > ---
> >  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 72ce1a75647b..88c6e3e105c0 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > @@ -205,7 +205,7 @@ video-codec@1c0e000 {
> >                       clock-names = "ahb", "mod", "ram";
> >                       resets = <&ccu RST_BUS_VE>;
> >                       interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > -                     allwinner,sram = <&ve_sram 1>;
> > +                     allwinner,sram = <&ve_sram 1>, <&de2_sram 1>;
> >                       iommus = <&iommu 3>;
> >               };
> >
> > --
> > 2.47.3
> >
>
> --
> Paul Kocialkowski,
>
> Independent contractor - sys-base - https://www.sys-base.io/
> Free software developer - https://www.paulk.fr/
>
> Expert in multimedia, graphics and embedded hardware support with Linux.