[PATCH v2 4/5] libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri May 31 00:36:08 CEST 2024
Quoting Hans de Goede (2024-05-30 18:15:59)
> The dma-buf allocator currently allocates from CMA and system heaps.
>
> Extend the dma-buf allocator to support allocating dma-buffers by creating
> memfd-s and turning those into dma-buffers using /dev/udmabuf.
>
> The buffers allocated through memfd/udmabuf are not suitable for zero-copy
> buffer sharing with other devices.
>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> Changes in v2:
> - Reword the commit message
> - Add a new DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf type for udmabuf
> - Drop unnecessary size != size check
> - Reword log messages to be more like the DMA heap alloc path
> - Move UniqueFD(ret) up so as to not leak the fd on errors
> ---
> .../libcamera/internal/dma_buf_allocator.h | 4 +
> src/libcamera/dma_buf_allocator.cpp | 111 +++++++++++++++---
> 2 files changed, 100 insertions(+), 15 deletions(-)
>
> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> index a881042e..36ec1696 100644
> --- a/include/libcamera/internal/dma_buf_allocator.h
> +++ b/include/libcamera/internal/dma_buf_allocator.h
> @@ -20,6 +20,7 @@ public:
> enum class DmaBufAllocatorFlag {
> CmaHeap = 1 << 0,
> SystemHeap = 1 << 1,
> + UDmaBuf = 1 << 2,
> };
>
> using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>;
> @@ -30,7 +31,10 @@ public:
> UniqueFD alloc(const char *name, std::size_t size);
>
> private:
> + UniqueFD allocFromHeap(const char *name, std::size_t size);
> + UniqueFD allocFromUDmaBuf(const char *name, std::size_t size);
> UniqueFD providerHandle_;
> + DmaBufAllocatorFlag type_;
> };
>
> LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)
> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> index bc0d78ef..369a419a 100644
> --- a/src/libcamera/dma_buf_allocator.cpp
> +++ b/src/libcamera/dma_buf_allocator.cpp
> @@ -11,10 +11,14 @@
> #include <array>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> #include <unistd.h>
>
> #include <linux/dma-buf.h>
> #include <linux/dma-heap.h>
> +#include <linux/udmabuf.h>
>
> #include <libcamera/base/log.h>
>
> @@ -32,7 +36,7 @@ struct DmaBufAllocatorInfo {
> };
> #endif
>
> -static constexpr std::array<DmaBufAllocatorInfo, 3> providerInfos = { {
> +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".
> @@ -40,6 +44,7 @@ static constexpr std::array<DmaBufAllocatorInfo, 3> providerInfos = { {
> { 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)
> @@ -63,6 +68,8 @@ LOG_DEFINE_CATEGORY(DmaBufAllocator)
> * \brief Allocate from a CMA dma-heap, providing physically-contiguous memory
> * \var DmaBufAllocator::SystemHeap
> * \brief Allocate from the system dma-heap, using the page allocator
> + * \var DmaBufAllocator::UDmaBuf
> + * \brief Allocate using a memfd + /dev/udmabuf
> */
>
> /**
> @@ -99,6 +106,7 @@ DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type)
>
> LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName;
> providerHandle_ = UniqueFD(ret);
> + type_ = info.type;
> break;
> }
>
> @@ -117,25 +125,76 @@ DmaBufAllocator::~DmaBufAllocator() = default;
> * \return True if the DmaBufAllocator is valid, false otherwise
> */
>
> -/**
> - * \brief Allocate a dma-buf from the DmaBufAllocator
> - * \param [in] name The name to set for the allocated buffer
> - * \param [in] size The size of the buffer to allocate
> - *
> - * Allocates a dma-buf with read/write access.
> - *
> - * If the allocation fails, return an invalid UniqueFD.
> - *
> - * \return The UniqueFD of the allocated buffer
> - */
> -UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
> +UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)
> {
> - int ret;
> + /* Size must be a multiple of the page-size round it up. */
> + std::size_t pageMask = sysconf(_SC_PAGESIZE) - 1;
> + size = (size + pageMask) & ~pageMask;
>
> - if (!name)
> + int ret = memfd_create(name, MFD_ALLOW_SEALING);
> + if (ret < 0) {
> + ret = errno;
> + LOG(DmaBufAllocator, Error)
> + << "Failed to allocate memfd storage for " << name
> + << ": " << strerror(ret);
> return {};
> + }
>
> + UniqueFD memfd(ret);
> +
> + ret = ftruncate(memfd.get(), size);
> + if (ret < 0) {
> + ret = errno;
> + LOG(DmaBufAllocator, Error)
> + << "Failed to set memfd size for " << name
> + << ": " << strerror(ret);
> + return {};
> + }
> +
> + /* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */
> + ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);
> + if (ret < 0) {
> + ret = errno;
> + LOG(DmaBufAllocator, Error)
> + << "Failed to seal the memfd for " << name
> + << ": " << strerror(ret);
> + return {};
> + }
> +
> + struct udmabuf_create create;
> +
> + create.memfd = memfd.get();
> + create.flags = UDMABUF_FLAGS_CLOEXEC;
> + create.offset = 0;
> + create.size = size;
> +
> + ret = ::ioctl(providerHandle_.get(), UDMABUF_CREATE, &create);
> + if (ret < 0) {
> + ret = errno;
> + LOG(DmaBufAllocator, Error)
> + << "Failed to allocate " << size << " bytes for "
> + << name << ": " << strerror(ret);
> + return {};
> + }
> +
> + /* The underlying memfd is kept as as a reference in the kernel */
> + UniqueFD uDma(ret);
> +
> + /* Fail if not suitable, the allocation will be free'd by UniqueFD */
> + if (create.size < size) {
> + LOG(DmaBufAllocator, Error)
> + << "UDMABUF_CREATE for " << name << " allocated "
> + << create.size << " bytes instead of " << size << " bytes";
> + return {};
> + }
> +
> + return uDma;
> +}
> +
> +UniqueFD DmaBufAllocator::allocFromHeap(const char *name, std::size_t size)
> +{
> struct dma_heap_allocation_data alloc = {};
> + int ret;
>
> alloc.len = size;
> alloc.fd_flags = O_CLOEXEC | O_RDWR;
> @@ -156,4 +215,26 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
> return allocFd;
> }
>
> +/**
> + * \brief Allocate a dma-buf from the DmaBufAllocator
> + * \param [in] name The name to set for the allocated buffer
> + * \param [in] size The size of the buffer to allocate
> + *
> + * Allocates a dma-buf with read/write access.
> + *
> + * If the allocation fails, return an invalid UniqueFD.
> + *
> + * \return The UniqueFD of the allocated buffer
> + */
> +UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
> +{
> + if (!name)
> + return {};
> +
> + if (type_ == DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> + return allocFromUDmaBuf(name, size);
> + else
> + return allocFromHeap(name, size);
> +}
> +
> } /* namespace libcamera */
> --
> 2.45.1
>
More information about the libcamera-devel
mailing list