<div dir="ltr">Hi Kieran,<div><br></div><div>Thanks for pointing it out and so sorry about that. Updated in v2.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Feb 9, 2023 at 10:54 PM Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Harvey,<br>
<br>
Thanks for looking at this to make it easier to get allocations<br>
throughout libcamera.<br>
<br>
Quoting Harvey Yang via libcamera-devel (2023-02-08 09:59:21)<br>
> If DmaHeap is not valid, fall back to UdmaHeap to allocate buffers.<br>
> <br>
> Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> ---<br>
> include/libcamera/meson.build | 1 +<br>
> include/libcamera/udma_heap.h | 20 ++++++<br>
> src/libcamera/heap_allocator.cpp | 3 +<br>
> src/libcamera/meson.build | 1 +<br>
> src/libcamera/udma_heap.cpp | 117 +++++++++++++++++++++++++++++++<br>
> 5 files changed, 142 insertions(+)<br>
> create mode 100644 include/libcamera/udma_heap.h<br>
> create mode 100644 src/libcamera/udma_heap.cpp<br>
> <br>
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build<br>
> index f486630a..a1414a57 100644<br>
> --- a/include/libcamera/meson.build<br>
> +++ b/include/libcamera/meson.build<br>
> @@ -19,6 +19,7 @@ libcamera_public_headers = files([<br>
> 'request.h',<br>
> 'stream.h',<br>
> 'transform.h',<br>
> + 'udma_heap.h',<br>
> ])<br>
> <br>
> subdir('base')<br>
> diff --git a/include/libcamera/udma_heap.h b/include/libcamera/udma_heap.h<br>
> new file mode 100644<br>
> index 00000000..b5b86922<br>
> --- /dev/null<br>
> +++ b/include/libcamera/udma_heap.h<br>
> @@ -0,0 +1,20 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2023, Google Inc.<br>
<br>
I have the same comment as from Dave in regards to Copyright.<br>
<br>
I expect this is highly derived from the work I commenced, at<br>
<br>
<a href="https://github.com/kbingham/libcamera/commit/8d79af53eaf461bab246c2fdeb1d9fdcbb288743" rel="noreferrer" target="_blank">https://github.com/kbingham/libcamera/commit/8d79af53eaf461bab246c2fdeb1d9fdcbb288743</a><br>
<br>
So please keep some form of attribution to Ideas on Board Oy.<br>
<br>
<br>
> + *<br>
> + * udma_heap.h - Udma Heap implementation.<br>
> + */<br>
> +<br>
> +#include <libcamera/heap.h><br>
> +<br>
> +namespace libcamera {<br>
> +<br>
> +class UdmaHeap : public Heap<br>
> +{<br>
> +public:<br>
> + UdmaHeap();<br>
> + ~UdmaHeap();<br>
> + UniqueFD alloc(const char *name, std::size_t size) override;<br>
> +};<br>
> +<br>
> +} /* namespace libcamera */<br>
> diff --git a/src/libcamera/heap_allocator.cpp b/src/libcamera/heap_allocator.cpp<br>
> index 594b1d6a..179c2c21 100644<br>
> --- a/src/libcamera/heap_allocator.cpp<br>
> +++ b/src/libcamera/heap_allocator.cpp<br>
> @@ -18,6 +18,7 @@<br>
> #include <libcamera/base/log.h><br>
> <br>
> #include <libcamera/dma_heap.h><br>
> +#include <libcamera/udma_heap.h><br>
> <br>
> namespace libcamera {<br>
> <br>
> @@ -26,6 +27,8 @@ LOG_DEFINE_CATEGORY(HeapAllocator)<br>
> HeapAllocator::HeapAllocator()<br>
> {<br>
> heap_ = std::make_unique<DmaHeap>();<br>
> + if (!isValid())<br>
> + heap_ = std::make_unique<UdmaHeap>();<br>
> }<br>
> <br>
> HeapAllocator::~HeapAllocator() = default;<br>
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build<br>
> index ee586c0d..bce26d4b 100644<br>
> --- a/src/libcamera/meson.build<br>
> +++ b/src/libcamera/meson.build<br>
> @@ -45,6 +45,7 @@ libcamera_sources = files([<br>
> 'stream.cpp',<br>
> 'sysfs.cpp',<br>
> 'transform.cpp',<br>
> + 'udma_heap.cpp',<br>
> 'v4l2_device.cpp',<br>
> 'v4l2_pixelformat.cpp',<br>
> 'v4l2_subdevice.cpp',<br>
> diff --git a/src/libcamera/udma_heap.cpp b/src/libcamera/udma_heap.cpp<br>
> new file mode 100644<br>
> index 00000000..2a470f6b<br>
> --- /dev/null<br>
> +++ b/src/libcamera/udma_heap.cpp<br>
> @@ -0,0 +1,117 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2023, Google Inc.<br>
<br>
And here of course.<br>
<br>
<br>
> + *<br>
> + * dma_heap.h - Dma Heap implementation.<br>
> + */<br>
> +<br>
> +#include <libcamera/udma_heap.h><br>
> +<br>
> +#include <fcntl.h><br>
> +#include <sys/ioctl.h><br>
> +#include <sys/mman.h><br>
> +#include <sys/stat.h><br>
> +#include <sys/types.h><br>
> +#include <unistd.h><br>
> +<br>
> +#include <linux/udmabuf.h><br>
> +<br>
> +#include <libcamera/base/log.h><br>
> +<br>
> +namespace libcamera {<br>
> +<br>
> +LOG_DEFINE_CATEGORY(UdmaHeap)<br>
> +<br>
> +UdmaHeap::UdmaHeap()<br>
> +{<br>
> + int ret = ::open("/dev/udmabuf", O_RDWR, 0);<br>
> + if (ret < 0) {<br>
> + ret = errno;<br>
> + LOG(UdmaHeap, Error)<br>
> + << "Failed to open allocator: " << strerror(ret);<br>
> +<br>
> + if (ret == EACCES) {<br>
> + LOG(UdmaHeap, Info)<br>
> + << "Consider making /dev/udmabuf accessible by the video group";<br>
> + LOG(UdmaHeap, Info)<br>
> + << "Alternatively, add your user to the kvm group.";<br>
> + }<br>
> +<br>
> + } else {<br>
> + handle_ = UniqueFD(ret);<br>
> + }<br>
> +}<br>
> +<br>
> +UdmaHeap::~UdmaHeap() = default;<br>
> +<br>
> +UniqueFD UdmaHeap::alloc(const char *name, std::size_t size)<br>
> +{<br>
> + if (!isValid()) {<br>
> + LOG(UdmaHeap, Fatal) << "Allocation attempted without allocator" << name;<br>
> + return {};<br>
> + }<br>
> +<br>
> + int ret;<br>
> +<br>
> + ret = memfd_create(name, MFD_ALLOW_SEALING);<br>
> + if (ret < 0) {<br>
> + ret = errno;<br>
> + LOG(UdmaHeap, Error)<br>
> + << "Failed to allocate memfd storage: "<br>
> + << strerror(ret);<br>
> + return {};<br>
> + }<br>
> +<br>
> + UniqueFD memfd(ret);<br>
> +<br>
> + ret = ftruncate(memfd.get(), size);<br>
> + if (ret < 0) {<br>
> + ret = errno;<br>
> + LOG(UdmaHeap, Error)<br>
> + << "Failed to set memfd size: " << strerror(ret);<br>
> + return {};<br>
> + }<br>
> +<br>
> + /* UdmaHeap Buffers *must* have the F_SEAL_SHRINK seal */<br>
> + ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);<br>
> + if (ret < 0) {<br>
> + ret = errno;<br>
> + LOG(UdmaHeap, Error)<br>
> + << "Failed to seal the memfd: " << strerror(ret);<br>
> + return {};<br>
> + }<br>
> +<br>
> + struct udmabuf_create create;<br>
> +<br>
> + create.memfd = memfd.get();<br>
> + create.flags = UDMABUF_FLAGS_CLOEXEC;<br>
> + create.offset = 0;<br>
> + create.size = size;<br>
> +<br>
> + ret = ::ioctl(handle_.get(), UDMABUF_CREATE, &create);<br>
> + if (ret < 0) {<br>
> + ret = errno;<br>
> + LOG(UdmaHeap, Error)<br>
> + << "Failed to allocate " << size << " bytes: "<br>
> + << strerror(ret);<br>
> + return {};<br>
> + }<br>
> +<br>
> + /* The underlying memfd is kept as as a reference in the kernel */<br>
> + UniqueFD uDma(ret);<br>
> +<br>
> + if (create.size != size)<br>
> + LOG(UdmaHeap, Warning)<br>
> + << "Allocated " << create.size << " bytes instead of "<br>
> + << size << " bytes";<br>
> +<br>
> + /* Fail if not suitable, the allocation will be free'd by UniqueFD */<br>
> + if (create.size < size)<br>
> + return {};<br>
> +<br>
> + LOG(UdmaHeap, Debug) << "Allocated " << create.size << " bytes";<br>
> +<br>
> + return uDma;<br>
> +}<br>
> +<br>
> +} /* namespace libcamera */<br>
> -- <br>
> 2.39.1.519.gcb327c4b5f-goog<br>
><br>
</blockquote></div>