[PATCH v3 2/3] libcamera: DmaBufAllocator: Support allocating from /dev/udmabuf

Hans de Goede hdegoede at redhat.com
Mon Jun 3 13:12:58 CEST 2024


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>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org> # Lenovo-x13s
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
Changes in v3:
- Style fixes to some comments
- Drop the unnecessary checking of the created dma-buf size

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           | 101 +++++++++++++++---
 2 files changed, 90 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 af512512..5ae51721 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
  */
 
 /**
@@ -100,6 +107,7 @@ DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type)
 
 		LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName;
 		providerHandle_ = UniqueFD(ret);
+		type_ = info.type;
 		break;
 	}
 
@@ -118,25 +126,66 @@ 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 create dma buf for " << name
+			<< ": " << strerror(ret);
+		return {};
+	}
+
+	/* The underlying memfd is kept as as a reference in the kernel. */
+	return UniqueFD(ret);
+}
+
+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;
@@ -159,4 +208,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