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

[Nicolas Dufresne]

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

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