Re: [PATCH v1] ARM: dts: sun8i: h3: orangepi-pc: Add CMA reserved memory node

[Jernej Škrabec]

Dne sreda, 14. september 2022 ob 17:33:27 CEST je Clément Péron napisal(a):
> Hi Dmitry,
> 
> On Wed, 14 Sept 2022 at 17:12, Dmitry Osipenko
> 
> <dmitry.osipenko@xxxxxxxxxxxxx> wrote:
> > Add 256MB CMA node to the Orange Pi PC board. This fixes memory allocation
> > failures for Cedrus video decoder on trying to play a 1080p video with
> > gstreamer.
> > 
> > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>
> > ---
> > 
> >  arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> > b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts index
> > b96e015f54ee..e655346a9fb4 100644
> > --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> > +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> > @@ -60,6 +60,20 @@ chosen {
> > 
> >                 stdout-path = "serial0:115200n8";
> >         
> >         };
> > 
> > +       reserved-memory {
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               ranges;
> > +
> > +               linux,cma@40000000 {
> > +                       compatible = "shared-dma-pool";
> > +                       alloc-ranges = <0x40000000 0x40000000>;
> > +                       size = <0x10000000>; /* 256MiB */
> > +                       linux,cma-default;
> > +                       reusable;
> > +               };
> > +       };
> > +
> 
> This change seems legit for all H3 boards and could be moved to the H3 dtsi,
> no?

That's true. However, there is a reason why this node doesn't exist. One or 
two H2+ boards (which use H3 dtsi) have only 256 MiB of RAM, so this can't 
work with them. A few H3 boards have 512 MiB of RAM, so you eat basically half 
of the RAM with that. Additionally, contrary to A20 and similar SoCs, which 
have such node, Cedrus can address whole RAM, so this is not strictly needed. 
It's better to leave this decision to distribution. Some don't care about 
Cedrus and some do a lot. Default size can be set via kernel config and it can 
be overriden by kernel argument if necessary.

Best regards,
Jernej

> 
> Regards,
> Clement
> 
> >         connector {
> >         
> >                 compatible = "hdmi-connector";
> >                 type = "a";
> > 
> > --
> > 2.37.3