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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2057585.bB369e8A3T@kista>
Date:   Thu, 13 Oct 2022 00:17:32 +0200
From:   Jernej Škrabec <jernej.skrabec@...il.com>
To:     Clément Péron <peron.clem@...il.com>,
        Dmitry Osipenko <dmitry.osipenko@...labora.com>
Cc:     Chen-Yu Tsai <wens@...e.org>, Samuel Holland <samuel@...lland.org>,
        linux-sunxi@...ts.linux.dev,
        linux-kernel <linux-kernel@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>
Subject: Re: Re: [PATCH v1] ARM: dts: sun8i: h3: orangepi-pc: Add CMA reserved memory node

Hi,

Dne četrtek, 15. september 2022 ob 11:01:36 CEST je Dmitry Osipenko 
napisal(a):
> Hi,
> 
> On 9/14/22 21:34, Jernej Škrabec wrote:
> > 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@...labora.com> 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@...labora.com>
> >>> ---
> >>> 
> >>>  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@...00000 {
> >>> +                       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.
> 
> It's a "reusable" CMA, hence it won't be eaten. System is free to use
> the reusable CMA. In practice, CMA may get populated with a pinned pages
> over time, hence system will work fine, but CMA will get fragmented and
> this may cause problems for a larger CMA allocations.
> 
> The main problem with 512M boards should be that they may not have a
> suitable area for 256M CMA because it should be only either a low or
> high part of the memory that might be busy at a boot time, and then
> kernel will fail to boot.
> 
> > 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.
> 
> In my experience generic distributions usually don't care about
> particular boards/devices. They ship a multiplatform kernel using
> default config that has 64M for CMA and Cedrus doesn't work well with that.

It still can be overriden using cma= kernel argument.

> 
> BTW, the sunxi_defconfig doesn't specify CMA size at all, so it defaults
> to 16M.

That can be easily changed, someone just need to send patch. Historically, 
from A20 times, default choice for CMA size was 96 MiB. Good choice, and in my 
opinion also maximum, is 128 MiB. That's enough for 1080p. 256 MiB is really 
only needed for 4k content, which only H3 and its variants support, from what 
I can tell.

Best regards,
Jernej