ARM: dts: sun8i: h3: orangepi-pc: Add CMA reserved memory node

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

Posted by Dmitry Osipenko 3 years, 6 months ago

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@collabora.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@40000000 {
+			compatible = "shared-dma-pool";
+			alloc-ranges = <0x40000000 0x40000000>;
+			size = <0x10000000>; /* 256MiB */
+			linux,cma-default;
+			reusable;
+		};
+	};
+
 	connector {
 		compatible = "hdmi-connector";
 		type = "a";
-- 
2.37.3

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

Posted by Clément Péron 3 years, 6 months ago

Hi Dmitry,

On Wed, 14 Sept 2022 at 17:12, Dmitry Osipenko
<dmitry.osipenko@collabora.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@collabora.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@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?

Regards,
Clement

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

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

Posted by Jernej Škrabec 3 years, 6 months ago

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

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

Posted by Dmitry Osipenko 3 years, 6 months ago

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

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

Posted by Jernej Škrabec 3 years, 5 months ago

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

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

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

Posted by Dmitry Osipenko 3 years, 5 months ago

On 10/13/22 01:17, Jernej Škrabec wrote:
> 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@collabora.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@collabora.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@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.
> 
> It still can be overriden using cma= kernel argument.

The point of the DT change is to make system usable out-of-the-box for
casual (non kernel developer) users.

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

128M wasn't enough for playing 1080p h265 big_bug_bunny. Practically it
doesn't matter how big CMA area is, as long as it's "shared".

In my experience 256M CMA is a good choice for devices with 1G RAM.
Bigger CMA may cause problems for initrd, depending on a bootlaoder version.

-- 
Best regards,
Dmitry