mbox series

[0/3] Add global CMA reserve area

Message ID 20240613150902.2173582-1-devarsht@ti.com (mailing list archive)
Headers show
Series Add global CMA reserve area | expand

Message

Devarsh Thakkar June 13, 2024, 3:08 p.m. UTC
Add global CMA reserve area for AM62x, AM62A and AM62P SoCs.
These SoCs do not have MMU and hence require contiguous memory pool to
support various multimedia use-cases.

Brandon Brnich (1):
  arm64: dts: ti: k3-am62p5-sk: Reserve 576 MiB of global CMA

Devarsh Thakkar (2):
  arm64: dts: ti: k3-am62x-sk-common: Reserve 128MiB of global CMA
  arm64: dts: ti: k3-am62a7-sk: Reserve 576MiB of global CMA

 arch/arm64/boot/dts/ti/k3-am62a7-sk.dts        | 9 +++++++++
 arch/arm64/boot/dts/ti/k3-am62p5-sk.dts        | 7 +++++++
 arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 8 ++++++++
 3 files changed, 24 insertions(+)

Comments

Randolph Sapp June 14, 2024, 5:58 p.m. UTC | #1
On Thu Jun 13, 2024 at 10:08 AM CDT, Devarsh Thakkar wrote:
> Add global CMA reserve area for AM62x, AM62A and AM62P SoCs.
> These SoCs do not have MMU and hence require contiguous memory pool to
> support various multimedia use-cases.
>
> Brandon Brnich (1):
>   arm64: dts: ti: k3-am62p5-sk: Reserve 576 MiB of global CMA
>
> Devarsh Thakkar (2):
>   arm64: dts: ti: k3-am62x-sk-common: Reserve 128MiB of global CMA
>   arm64: dts: ti: k3-am62a7-sk: Reserve 576MiB of global CMA
>
>  arch/arm64/boot/dts/ti/k3-am62a7-sk.dts        | 9 +++++++++
>  arch/arm64/boot/dts/ti/k3-am62p5-sk.dts        | 7 +++++++
>  arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 8 ++++++++
>  3 files changed, 24 insertions(+)

I'm still a little torn about putting this allocation into the device tree
directly as the actual required allocation size depends on the task.

If it's allowed though, this series is fine for introducing those changes. This
uses the long-tested values we've been using on our tree for a bit now. The only
thing that's a little worrying is the missing range definitions for devices with
more than 32bits of addressable memory as Brandon has pointed out. Once that's
addressed:

Reviewed-by: Randolph Sapp <rs@ti.com>

Specifying these regions using the kernel cmdline parameter via u-boot was
brought up as a potential workaround. This is fine until you get into distro
boot methods which will almost certainly attempt to override those. I don't
know. Still a little odd. Curious how the community feels about it.

Technically the user or distro can still override it with the cmdline parameter
if necessary, so this may be the best way to have a useful default.
Andrew Davis June 24, 2024, 4:33 p.m. UTC | #2
On 6/14/24 12:58 PM, Randolph Sapp wrote:
> On Thu Jun 13, 2024 at 10:08 AM CDT, Devarsh Thakkar wrote:
>> Add global CMA reserve area for AM62x, AM62A and AM62P SoCs.
>> These SoCs do not have MMU and hence require contiguous memory pool to
>> support various multimedia use-cases.
>>
>> Brandon Brnich (1):
>>    arm64: dts: ti: k3-am62p5-sk: Reserve 576 MiB of global CMA
>>
>> Devarsh Thakkar (2):
>>    arm64: dts: ti: k3-am62x-sk-common: Reserve 128MiB of global CMA
>>    arm64: dts: ti: k3-am62a7-sk: Reserve 576MiB of global CMA
>>
>>   arch/arm64/boot/dts/ti/k3-am62a7-sk.dts        | 9 +++++++++
>>   arch/arm64/boot/dts/ti/k3-am62p5-sk.dts        | 7 +++++++
>>   arch/arm64/boot/dts/ti/k3-am62x-sk-common.dtsi | 8 ++++++++
>>   3 files changed, 24 insertions(+)
> 
> I'm still a little torn about putting this allocation into the device tree
> directly as the actual required allocation size depends on the task.
> 

That is the exact reason this does not belong in DT. For everyone *not*
using the most extreme case (12x decodes at the same time), this is
all wasted space. If one is running out of CMA, they can add more on
the kernel cmdline.

> If it's allowed though, this series is fine for introducing those changes. This
> uses the long-tested values we've been using on our tree for a bit now. The only
> thing that's a little worrying is the missing range definitions for devices with
> more than 32bits of addressable memory as Brandon has pointed out. Once that's
> addressed:
> 
> Reviewed-by: Randolph Sapp <rs@ti.com>
> 
> Specifying these regions using the kernel cmdline parameter via u-boot was
> brought up as a potential workaround. This is fine until you get into distro
> boot methods which will almost certainly attempt to override those. I don't
> know. Still a little odd. Curious how the community feels about it.
> 
> Technically the user or distro can still override it with the cmdline parameter
> if necessary, so this may be the best way to have a useful default.
> 

The most useful default is one that doesn't eat 576 MiB of memory "just in case".
Needing that much CMA is the exception case and should be the one that requires
adding something to the kernel cmdline.

If the kernel cmdline option does not work in some cases then we should
fix that instead of hard-coding a workaround here in DT. We are robbing
ourselves of a better solution here.

Andrew