Re: [PATCH 0/3] Add global CMA reserve area

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/30/24 2:09 AM, Vignesh Raghavendra wrote:


On 28/06/24 22:05, Randolph Sapp wrote:
On Fri Jun 28, 2024 at 10:57 AM CDT, Devarsh Thakkar wrote:
Hi Andrew, Vignesh,

On 24/06/24 22:03, Andrew Davis wrote:
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.


I disagree with this. The 12x decode for 480p is not an extreme use-case this
is something VPU is capable to run at optimum frame-rate (12x 1080p it can't)
and as the AM62A7 is meant to be AI + multimedia centric device, per the
device definition we were given the requirements to support a list of
multimedia use-cases which should work out of box and 12x decode for 480p was
one of them as device is very much capable of doing that with optimum
performance and I don't think it is right to change these requirements on the fly.

The AM62A7 board has 4 GiB of DDR and we have been using this CMA value since
more than a year, I have never heard anyone complain about out of memory or
CMA starvation and it suffices to requirements of *most use-cases*, but if for
some specific use-case it doesn't suffice, user can change it via kernel cmdline.

The kernelcmdline suggestion doesn't suffice out of box experience required,
we don't want to ask the user to reboot the board everytime they run out of CMA.


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

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.



Unlike above, this solution is independent of distro as it should be as we
want that all the supported multimedia use-cases should work out of box. This
solution is nothing illegal as CMA region carveouts are not a kernel
deprecated feature.

Right. I support this change for at least introducing a usable default. 32M of
CMA is barely enough to run glmark2 under Weston once everything's up and
running.

As I said before, the user or distro can still override the dt CMA block with
the cma kernel parameter if they aren't happy with the default block.
Unfortunately this is about the only way to have a usable default value to fall
back on.



Given the number of SoMs and non TI EVMs that are about to come out with
AM62A/P and AM67s, we need to provide a consistent way of being able to
support multimedia IPs out of the box. Modifying cmdline may not always
be feasible given distro defaults don't always provide a way to do so.


We need to keep thinking then. I empathize with desire to put
configuration in Device Tree. DT feels like a great spot for it,
it is ubiquitous on these boards and has a good bit of tooling around
it. We are already describing the hardware, why not configure it here
too? But the reason we do not want to go down that road is simple:
DT takes away use-case flexibility. A lack of flexibility is fine for
hardware which is unchanging, but not for configuration.

Device policy and configuration must be left to userspace.

It is not for us to decide how folks should use our hardware, and
that is what we are doing when we configure it in DT.

For configuration that must happen in early boot before userspace is
available (such as kernel stdout and memory carveouts) we have the
kernel cmdline. If we find something that cannot be done today though
the cmdline, then we should add that support to the cmdline, not
give up and just hide the configuration in DT.

What this series does is already available on the kernel cmdline.
Our bootloader can provide sane defaults on the cmdline today.
If the worry is that distros will override this default then
go fix those distros.

So I am inclined to queue first 2 patches unless there is another way t
achieve this.


Our lack of creativity in finding better solutions to this issue is
not an excuse to add more junk to DT..

Andrew

[...]





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux