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

[Dmitry Osipenko]

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@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.

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.

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

-- 
Best regards,
Dmitry