<div dir="ltr">Thanks Kieran for the input. I found it difficult to enable dma/udma in Google's linux distribution though, so I tested it in my personal Arch linux instead (udmabuf is available).<div><br></div><div>However, I found that udmabuf only works if the requested size is power of 2 (ex: 4194304 for 1920*1080). Is it expected, or there's something wrong with my linux udmabuf support?</div><div><br></div><div>BR,<br>Harvey</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 2, 2023 at 8:01 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">Quoting Cheng-Hao Yang (2023-03-02 08:25:00)<br>
> Hi Kieran,<br>
> <br>
> Sorry that I'm new to dma heap: I was trying to test virtual pipeline<br>
> handler on dma heap<br>
> (or the udma heap I add in the following patch), while I find that my<br>
> workstation doesn't<br>
> have the dma heap directory (`/dev/dma_heap` or `/dev/udmabuf`). I heard<br>
> from Han-lin<br>
> that we might need to enable it in the linux kernel. Is that right? Is<br>
> there a doc/tutorial that<br>
> I can follow?<br>
<br>
It could be kernel specific indeed. I guess it's up to your<br>
distribution?<br>
<br>
UDMABUF<br>
- <a href="https://github.com/torvalds/linux/blob/ee3f96b164688dae21e2466a57f2e806b64e8a37/drivers/dma-buf/Kconfig#L33" rel="noreferrer" target="_blank">https://github.com/torvalds/linux/blob/ee3f96b164688dae21e2466a57f2e806b64e8a37/drivers/dma-buf/Kconfig#L33</a><br>
<br>
DMABUF_HEAPS<br>
- <a href="https://github.com/torvalds/linux/blob/ee3f96b164688dae21e2466a57f2e806b64e8a37/drivers/dma-buf/Kconfig#L68" rel="noreferrer" target="_blank">https://github.com/torvalds/linux/blob/ee3f96b164688dae21e2466a57f2e806b64e8a37/drivers/dma-buf/Kconfig#L68</a><br>
<br>
<br>
--<br>
Regards<br>
<br>
Kieran<br>
<br>
<br>
<br>
> <br>
> Thanks!<br>
> Harvey<br>
> <br>
> <br>
> <br>
> On Thu, Mar 2, 2023 at 4:13 PM Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> wrote:<br>
> <br>
> > As other components (like the WIP virtual pipeline handler) also need a<br>
> > heap allocator, move DmaHeap from raspberry pi pipeline handler to a<br>
> > general HeapAllocator as a base class.<br>
> ><br>
> > Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> > ---<br>
> > include/libcamera/dma_heap.h | 20 +++++++++<br>
> > include/libcamera/heap.h | 27 ++++++++++++<br>
> > include/libcamera/heap_allocator.h | 30 +++++++++++++<br>
> > include/libcamera/meson.build | 3 ++<br>
> > .../dma_heaps.cpp => dma_heap.cpp} | 35 +++++++--------<br>
> > src/libcamera/heap_allocator.cpp | 43 +++++++++++++++++++<br>
> > src/libcamera/meson.build | 2 +<br>
> > .../pipeline/raspberrypi/dma_heaps.h | 32 --------------<br>
> > .../pipeline/raspberrypi/meson.build | 1 -<br>
> > .../pipeline/raspberrypi/raspberrypi.cpp | 10 ++---<br>
> > 10 files changed, 146 insertions(+), 57 deletions(-)<br>
> > create mode 100644 include/libcamera/dma_heap.h<br>
> > create mode 100644 include/libcamera/heap.h<br>
> > create mode 100644 include/libcamera/heap_allocator.h<br>
> > rename src/libcamera/{pipeline/raspberrypi/dma_heaps.cpp => dma_heap.cpp}<br>
> > (69%)<br>
> > create mode 100644 src/libcamera/heap_allocator.cpp<br>
> > delete mode 100644 src/libcamera/pipeline/raspberrypi/dma_heaps.h<br>
> ><br>
> > diff --git a/include/libcamera/dma_heap.h b/include/libcamera/dma_heap.h<br>
> > new file mode 100644<br>
> > index 00000000..a663c317<br>
> > --- /dev/null<br>
> > +++ b/include/libcamera/dma_heap.h<br>
> > @@ -0,0 +1,20 @@<br>
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> > +/*<br>
> > + * Copyright (C) 2020, Raspberry Pi Ltd<br>
> > + *<br>
> > + * dma_heap.h - Dma Heap implementation.<br>
> > + */<br>
> > +<br>
> > +#include <libcamera/heap.h><br>
> > +<br>
> > +namespace libcamera {<br>
> > +<br>
> > +class DmaHeap : public Heap<br>
> > +{<br>
> > +public:<br>
> > + DmaHeap();<br>
> > + ~DmaHeap();<br>
> > + UniqueFD alloc(const char *name, std::size_t size) override;<br>
> > +};<br>
> > +<br>
> > +} /* namespace libcamera */<br>
> > diff --git a/include/libcamera/heap.h b/include/libcamera/heap.h<br>
> > new file mode 100644<br>
> > index 00000000..c49a2ac3<br>
> > --- /dev/null<br>
> > +++ b/include/libcamera/heap.h<br>
> > @@ -0,0 +1,27 @@<br>
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> > +/*<br>
> > + * Copyright (C) 2023, Google Inc.<br>
> > + *<br>
> > + * heap.h - Heap interface.<br>
> > + */<br>
> > +<br>
> > +#pragma once<br>
> > +<br>
> > +#include <stddef.h><br>
> > +<br>
> > +#include <libcamera/base/unique_fd.h><br>
> > +<br>
> > +namespace libcamera {<br>
> > +<br>
> > +class Heap<br>
> > +{<br>
> > +public:<br>
> > + virtual ~Heap() = default;<br>
> > + bool isValid() const { return handle_.isValid(); }<br>
> > + virtual UniqueFD alloc(const char *name, std::size_t size) = 0;<br>
> > +<br>
> > +protected:<br>
> > + UniqueFD handle_;<br>
> > +};<br>
> > +<br>
> > +} /* namespace libcamera */<br>
> > diff --git a/include/libcamera/heap_allocator.h<br>
> > b/include/libcamera/heap_allocator.h<br>
> > new file mode 100644<br>
> > index 00000000..cd7ed1a3<br>
> > --- /dev/null<br>
> > +++ b/include/libcamera/heap_allocator.h<br>
> > @@ -0,0 +1,30 @@<br>
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> > +/*<br>
> > + * Copyright (C) 2023, Google Inc.<br>
> > + *<br>
> > + * heap_allocator.h - Helper class for heap buffer allocations.<br>
> > + */<br>
> > +<br>
> > +#pragma once<br>
> > +<br>
> > +#include <stddef.h><br>
> > +<br>
> > +#include <libcamera/base/unique_fd.h><br>
> > +<br>
> > +#include <libcamera/heap.h><br>
> > +<br>
> > +namespace libcamera {<br>
> > +<br>
> > +class HeapAllocator<br>
> > +{<br>
> > +public:<br>
> > + HeapAllocator();<br>
> > + ~HeapAllocator();<br>
> > + bool isValid() const { return heap_->isValid(); }<br>
> > + UniqueFD alloc(const char *name, std::size_t size);<br>
> > +<br>
> > +private:<br>
> > + std::unique_ptr<Heap> heap_;<br>
> > +};<br>
> > +<br>
> > +} /* namespace libcamera */<br>
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build<br>
> > index 408b7acf..f486630a 100644<br>
> > --- a/include/libcamera/meson.build<br>
> > +++ b/include/libcamera/meson.build<br>
> > @@ -7,10 +7,13 @@ libcamera_public_headers = files([<br>
> > 'camera_manager.h',<br>
> > 'color_space.h',<br>
> > 'controls.h',<br>
> > + 'dma_heap.h',<br>
> > 'fence.h',<br>
> > 'framebuffer.h',<br>
> > 'framebuffer_allocator.h',<br>
> > 'geometry.h',<br>
> > + 'heap.h',<br>
> > + 'heap_allocator.h',<br>
> > 'logging.h',<br>
> > 'pixel_format.h',<br>
> > 'request.h',<br>
> > diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp<br>
> > b/src/libcamera/dma_heap.cpp<br>
> > similarity index 69%<br>
> > rename from src/libcamera/pipeline/raspberrypi/dma_heaps.cpp<br>
> > rename to src/libcamera/dma_heap.cpp<br>
> > index 6b644406..02415975 100644<br>
> > --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp<br>
> > +++ b/src/libcamera/dma_heap.cpp<br>
> > @@ -2,18 +2,19 @@<br>
> > /*<br>
> > * Copyright (C) 2020, Raspberry Pi Ltd<br>
> > *<br>
> > - * dma_heaps.h - Helper class for dma-heap allocations.<br>
> > + * dma_heap.h - Dma Heap implementation.<br>
> > */<br>
> ><br>
> > -#include "dma_heaps.h"<br>
> > +#include <libcamera/dma_heap.h><br>
> ><br>
> > #include <array><br>
> > #include <fcntl.h><br>
> > -#include <linux/dma-buf.h><br>
> > -#include <linux/dma-heap.h><br>
> > #include <sys/ioctl.h><br>
> > #include <unistd.h><br>
> ><br>
> > +#include <linux/dma-buf.h><br>
> > +#include <linux/dma-heap.h><br>
> > +<br>
> > #include <libcamera/base/log.h><br>
> ><br>
> > /*<br>
> > @@ -30,9 +31,7 @@ static constexpr std::array<const char *, 2> heapNames =<br>
> > {<br>
> ><br>
> > namespace libcamera {<br>
> ><br>
> > -LOG_DECLARE_CATEGORY(RPI)<br>
> > -<br>
> > -namespace RPi {<br>
> > +LOG_DEFINE_CATEGORY(DmaHeap)<br>
> ><br>
> > DmaHeap::DmaHeap()<br>
> > {<br>
> > @@ -40,17 +39,17 @@ DmaHeap::DmaHeap()<br>
> > int ret = ::open(name, O_RDWR, 0);<br>
> > if (ret < 0) {<br>
> > ret = errno;<br>
> > - LOG(RPI, Debug) << "Failed to open " << name << ":<br>
> > "<br>
> > - << strerror(ret);<br>
> > + LOG(DmaHeap, Debug) << "Failed to open " << name<br>
> > << ": "<br>
> > + << strerror(ret);<br>
> > continue;<br>
> > }<br>
> ><br>
> > - dmaHeapHandle_ = UniqueFD(ret);<br>
> > + handle_ = UniqueFD(ret);<br>
> > break;<br>
> > }<br>
> ><br>
> > - if (!dmaHeapHandle_.isValid())<br>
> > - LOG(RPI, Error) << "Could not open any dmaHeap device";<br>
> > + if (!handle_.isValid())<br>
> > + LOG(DmaHeap, Error) << "Could not open any dmaHeap device";<br>
> > }<br>
> ><br>
> > DmaHeap::~DmaHeap() = default;<br>
> > @@ -67,24 +66,22 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t<br>
> > size)<br>
> > alloc.len = size;<br>
> > alloc.fd_flags = O_CLOEXEC | O_RDWR;<br>
> ><br>
> > - ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);<br>
> > + ret = ::ioctl(handle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);<br>
> > if (ret < 0) {<br>
> > - LOG(RPI, Error) << "dmaHeap allocation failure for "<br>
> > - << name;<br>
> > + LOG(DmaHeap, Error) << "dmaHeap allocation failure for "<br>
> > + << name;<br>
> > return {};<br>
> > }<br>
> ><br>
> > UniqueFD allocFd(alloc.fd);<br>
> > ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);<br>
> > if (ret < 0) {<br>
> > - LOG(RPI, Error) << "dmaHeap naming failure for "<br>
> > - << name;<br>
> > + LOG(DmaHeap, Error) << "dmaHeap naming failure for "<br>
> > + << name;<br>
> > return {};<br>
> > }<br>
> ><br>
> > return allocFd;<br>
> > }<br>
> ><br>
> > -} /* namespace RPi */<br>
> > -<br>
> > } /* namespace libcamera */<br>
> > diff --git a/src/libcamera/heap_allocator.cpp<br>
> > b/src/libcamera/heap_allocator.cpp<br>
> > new file mode 100644<br>
> > index 00000000..594b1d6a<br>
> > --- /dev/null<br>
> > +++ b/src/libcamera/heap_allocator.cpp<br>
> > @@ -0,0 +1,43 @@<br>
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> > +/*<br>
> > + * Copyright (C) 2023, Google Inc.<br>
> > + *<br>
> > + * heap_allocator.cpp - Helper class for heap buffer allocations.<br>
> > + */<br>
> > +<br>
> > +#include <libcamera/heap_allocator.h><br>
> > +<br>
> > +#include <array><br>
> > +#include <fcntl.h><br>
> > +#include <sys/ioctl.h><br>
> > +#include <unistd.h><br>
> > +<br>
> > +#include <linux/dma-buf.h><br>
> > +#include <linux/dma-heap.h><br>
> > +<br>
> > +#include <libcamera/base/log.h><br>
> > +<br>
> > +#include <libcamera/dma_heap.h><br>
> > +<br>
> > +namespace libcamera {<br>
> > +<br>
> > +LOG_DEFINE_CATEGORY(HeapAllocator)<br>
> > +<br>
> > +HeapAllocator::HeapAllocator()<br>
> > +{<br>
> > + heap_ = std::make_unique<DmaHeap>();<br>
> > +}<br>
> > +<br>
> > +HeapAllocator::~HeapAllocator() = default;<br>
> > +<br>
> > +UniqueFD HeapAllocator::alloc(const char *name, std::size_t size)<br>
> > +{<br>
> > + if (!isValid()) {<br>
> > + LOG(HeapAllocator, Fatal) << "Allocation attempted without<br>
> > allocator" << name;<br>
> > + return {};<br>
> > + }<br>
> > +<br>
> > + return heap_->alloc(name, size);<br>
> > +}<br>
> > +<br>
> > +} /* namespace libcamera */<br>
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build<br>
> > index 9869bfe7..ee586c0d 100644<br>
> > --- a/src/libcamera/meson.build<br>
> > +++ b/src/libcamera/meson.build<br>
> > @@ -17,11 +17,13 @@ libcamera_sources = files([<br>
> > 'delayed_controls.cpp',<br>
> > 'device_enumerator.cpp',<br>
> > 'device_enumerator_sysfs.cpp',<br>
> > + 'dma_heap.cpp',<br>
> > 'fence.cpp',<br>
> > 'formats.cpp',<br>
> > 'framebuffer.cpp',<br>
> > 'framebuffer_allocator.cpp',<br>
> > 'geometry.cpp',<br>
> > + 'heap_allocator.cpp',<br>
> > 'ipa_controls.cpp',<br>
> > 'ipa_data_serializer.cpp',<br>
> > 'ipa_interface.cpp',<br>
> > diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h<br>
> > b/src/libcamera/pipeline/raspberrypi/dma_heaps.h<br>
> > deleted file mode 100644<br>
> > index 0a4a8d86..00000000<br>
> > --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.h<br>
> > +++ /dev/null<br>
> > @@ -1,32 +0,0 @@<br>
> > -/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> > -/*<br>
> > - * Copyright (C) 2020, Raspberry Pi Ltd<br>
> > - *<br>
> > - * dma_heaps.h - Helper class for dma-heap allocations.<br>
> > - */<br>
> > -<br>
> > -#pragma once<br>
> > -<br>
> > -#include <stddef.h><br>
> > -<br>
> > -#include <libcamera/base/unique_fd.h><br>
> > -<br>
> > -namespace libcamera {<br>
> > -<br>
> > -namespace RPi {<br>
> > -<br>
> > -class DmaHeap<br>
> > -{<br>
> > -public:<br>
> > - DmaHeap();<br>
> > - ~DmaHeap();<br>
> > - bool isValid() const { return dmaHeapHandle_.isValid(); }<br>
> > - UniqueFD alloc(const char *name, std::size_t size);<br>
> > -<br>
> > -private:<br>
> > - UniqueFD dmaHeapHandle_;<br>
> > -};<br>
> > -<br>
> > -} /* namespace RPi */<br>
> > -<br>
> > -} /* namespace libcamera */<br>
> > diff --git a/src/libcamera/pipeline/raspberrypi/meson.build<br>
> > b/src/libcamera/pipeline/raspberrypi/meson.build<br>
> > index 59e8fb18..7322f0e8 100644<br>
> > --- a/src/libcamera/pipeline/raspberrypi/meson.build<br>
> > +++ b/src/libcamera/pipeline/raspberrypi/meson.build<br>
> > @@ -2,7 +2,6 @@<br>
> ><br>
> > libcamera_sources += files([<br>
> > 'delayed_controls.cpp',<br>
> > - 'dma_heaps.cpp',<br>
> > 'raspberrypi.cpp',<br>
> > 'rpi_stream.cpp',<br>
> > ])<br>
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > index 84120954..b7e0d031 100644<br>
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > @@ -21,6 +21,7 @@<br>
> > #include <libcamera/camera.h><br>
> > #include <libcamera/control_ids.h><br>
> > #include <libcamera/formats.h><br>
> > +#include <libcamera/heap_allocator.h><br>
> > #include <libcamera/ipa/raspberrypi_ipa_interface.h><br>
> > #include <libcamera/ipa/raspberrypi_ipa_proxy.h><br>
> > #include <libcamera/logging.h><br>
> > @@ -44,7 +45,6 @@<br>
> > #include "libcamera/internal/yaml_parser.h"<br>
> ><br>
> > #include "delayed_controls.h"<br>
> > -#include "dma_heaps.h"<br>
> > #include "rpi_stream.h"<br>
> ><br>
> > using namespace std::chrono_literals;<br>
> > @@ -245,7 +245,7 @@ public:<br>
> > std::vector<std::pair<std::unique_ptr<V4L2Subdevice>, MediaLink<br>
> > *>> bridgeDevices_;<br>
> ><br>
> > /* DMAHEAP allocation helper. */<br>
> > - RPi::DmaHeap dmaHeap_;<br>
> > + HeapAllocator heapAllocator_;<br>
> > SharedFD lsTable_;<br>
> ><br>
> > std::unique_ptr<RPi::DelayedControls> delayedCtrls_;<br>
> > @@ -1293,7 +1293,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice<br>
> > *unicam, MediaDevice *isp, Me<br>
> > {<br>
> > std::unique_ptr<RPiCameraData> data =<br>
> > std::make_unique<RPiCameraData>(this);<br>
> ><br>
> > - if (!data->dmaHeap_.isValid())<br>
> > + if (!data->heapAllocator_.isValid())<br>
> > return -ENOMEM;<br>
> ><br>
> > MediaEntity *unicamImage = unicam->getEntityByName("unicam-image");<br>
> > @@ -1680,9 +1680,9 @@ int RPiCameraData::configureIPA(const<br>
> > CameraConfiguration *config, ipa::RPi::IPA<br>
> > /* Always send the user transform to the IPA. */<br>
> > ipaConfig.transform = static_cast<unsigned int>(config->transform);<br>
> ><br>
> > - /* Allocate the lens shading table via dmaHeap and pass to the<br>
> > IPA. */<br>
> > + /* Allocate the lens shading table via heapAllocator and pass to<br>
> > the IPA. */<br>
> > if (!lsTable_.isValid()) {<br>
> > - lsTable_ = SharedFD(dmaHeap_.alloc("ls_grid",<br>
> > ipa::RPi::MaxLsGridSize));<br>
> > + lsTable_ = SharedFD(heapAllocator_.alloc("ls_grid",<br>
> > ipa::RPi::MaxLsGridSize));<br>
> > if (!lsTable_.isValid())<br>
> > return -ENOMEM;<br>
> ><br>
> > --<br>
> > 2.39.2.722.g9855ee24e9-goog<br>
> ><br>
> ><br>
</blockquote></div>