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. So I am inclined to queue first 2 patches unless there is another way t achieve this. [...] -- Regards Vignesh
