[PATCH 03/27] libcamera: dma_buf_allocator: Favour udmabuf over cma heap allocations

[Kieran Bingham]

Quoting Nicolas Dufresne (2025-04-23 13:50:03)
> Le mercredi 23 avril 2025 à 10:36 +0100, Kieran Bingham a écrit :
> > Quoting Laurent Pinchart (2025-04-22 23:22:31)
> > > Hi Bryan,
> > > 
> > > Thank you for the patch.
> > > 
> > > On Tue, Apr 22, 2025 at 10:58:56PM +0100, Bryan O'Donoghue wrote:
> > > > When /dev/dma_heap/linux,cma or /dev/dma_heap/system exist currently we
> > > > favour allocation from this type of heap over /dev/udmabuf.
> > > > 
> > > > We ought to favour udmabuf though
> > > > 
> > > > - udmabuf is the preferred method by various distros for security reasons
> > > > - Contiguous memory is a scarce resource
> > > > 
> > > > Change the ordering of the allocator lookup so that the udmabuf lookup
> > > > comes first.
> > > 
> > > This means that on a system where CMA allocation is possible from
> > > userspace, the buffers allocated by libcamera for the virtual pipeline
> > > handler and ISP won't be shareable without copies with a consumer that
> > > requires contiguous memory (e.g. most KMS devices on Arm platforms).
> > > Isn't that an issue ? It seems to even count as a regression.
> > > 
> > 
> > On my X13s, everything works better for me on udmabuf. If we try to use
> > the cma allocator things fail.
> > 
> > That /could/ be a separate topic/issue - but at least might explain some
> > of the rationale behind this.
> > 
> > I usually end up with something like this on my system.
> 
> Did you mean to add a link ? I'm quite curious, since here on Meteor

No, sorry - In the text above, I meant "I usually end up with something
like ... 'this patch' ... on my system". Meaning I configure my x13s to
prefer udmabuf over cma because cma allocations frequently fail for me.

It's not clear if it's a resource leak, or indeterminate usage by other
components - (hence might be something else to look into) but udmabuf
always works on softisp for x13s so far, so just use it.


> Lake GPU, passing over anything that has been touched by the CPU
> results in a big mess. This used to only happen when passing these to
> the display controller, now the GPU also does not handle the CPU
> coherent memory properly.

even if the CPU only 'reads' the data ?

For GPU ISP - the only access the CPU should do is read the data to
generate some statistics. And even that - I would hope in the future
will be turned into operations performed by the GPU ...

--
Kiran


> 
> Nicolas
> 
> > 
> > 
> > > > Fixes: ea4baaacc325 ("libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf")
> > > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org>
> > > > ---
> > > >  src/libcamera/dma_buf_allocator.cpp | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> > > > index d8c62dd6..722ffd46 100644
> > > > --- a/src/libcamera/dma_buf_allocator.cpp
> > > > +++ b/src/libcamera/dma_buf_allocator.cpp
> > > > @@ -45,10 +45,10 @@ static constexpr std::array<DmaBufAllocatorInfo, 4> providerInfos = { {
> > > >        * /dev/dma_heap/linux,cma is the CMA dma-heap. When the cma heap size is
> > > >        * specified on the kernel command line, this gets renamed to "reserved".
> > > >        */
> > > > +     { DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf, "/dev/udmabuf" },
> > > >       { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/linux,cma" },
> > > >       { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/reserved" },
> > > >       { DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap, "/dev/dma_heap/system" },
> > > > -     { DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf, "/dev/udmabuf" },
> > > >  } };
> > > >  
> > > >  LOG_DEFINE_CATEGORY(DmaBufAllocator)
> > > 
> > > -- 
> > > Regards,
> > > 
> > > Laurent Pinchart