<div dir="ltr">Hi Kieran,<div><br></div><div>Sorry that I'm new to dma heap: I was trying to test virtual pipeline handler on dma heap</div><div>(or the udma heap I add in the following patch), while I find that my workstation doesn't</div><div>have the dma heap directory (`/dev/dma_heap` or `/dev/udmabuf`). I heard from Han-lin</div><div>that we might need to enable it in the linux kernel. Is that right? Is there a doc/tutorial that</div><div>I can follow?</div><div><br></div><div>Thanks!</div><div>Harvey</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 2, 2023 at 4:13 PM Harvey Yang <<a href="mailto:chenghaoyang@chromium.org">chenghaoyang@chromium.org</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">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} (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 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 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>
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>
- << strerror(ret);<br>
+ LOG(DmaHeap, Debug) << "Failed to open " << name << ": "<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 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 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 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 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 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 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 *>> 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 *unicam, MediaDevice *isp, Me<br>
{<br>
std::unique_ptr<RPiCameraData> data = 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 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 IPA. */<br>
+ /* Allocate the lens shading table via heapAllocator and pass to the IPA. */<br>
if (!lsTable_.isValid()) {<br>
- lsTable_ = SharedFD(dmaHeap_.alloc("ls_grid", ipa::RPi::MaxLsGridSize));<br>
+ lsTable_ = SharedFD(heapAllocator_.alloc("ls_grid", ipa::RPi::MaxLsGridSize));<br>
if (!lsTable_.isValid())<br>
return -ENOMEM;<br>
<br>
-- <br>
2.39.2.722.g9855ee24e9-goog<br>
<br>
</blockquote></div>