[libcamera-devel,v2,1/3] libcamera: Move DmaHeap to HeapAllocator as a base class
diff mbox series

Message ID 20230214093736.4067033-2-chenghaoyang@google.com
State Superseded
Headers show
Series
  • Add HeapAllocator
Related show

Commit Message

Harvey Yang Feb. 14, 2023, 9:37 a.m. UTC
As other components (like the WIP virtual pipeline handler) also need a
heap allocator, move DmaHeap from raspberry pi pipeline handler to a
general HeapAllocator as a base class.

Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 include/libcamera/dma_heap.h                  | 20 +++++++
 include/libcamera/heap.h                      | 27 ++++++++++
 include/libcamera/heap_allocator.h            | 30 +++++++++++
 include/libcamera/meson.build                 |  3 ++
 .../dma_heaps.cpp => dma_heap.cpp}            | 52 ++++++++++++-------
 src/libcamera/heap_allocator.cpp              | 43 +++++++++++++++
 src/libcamera/meson.build                     |  2 +
 .../pipeline/raspberrypi/dma_heaps.h          | 32 ------------
 .../pipeline/raspberrypi/meson.build          |  1 -
 .../pipeline/raspberrypi/raspberrypi.cpp      | 10 ++--
 10 files changed, 163 insertions(+), 57 deletions(-)
 create mode 100644 include/libcamera/dma_heap.h
 create mode 100644 include/libcamera/heap.h
 create mode 100644 include/libcamera/heap_allocator.h
 rename src/libcamera/{pipeline/raspberrypi/dma_heaps.cpp => dma_heap.cpp} (58%)
 create mode 100644 src/libcamera/heap_allocator.cpp
 delete mode 100644 src/libcamera/pipeline/raspberrypi/dma_heaps.h

Patch
diff mbox series

diff --git a/include/libcamera/dma_heap.h b/include/libcamera/dma_heap.h
new file mode 100644
index 00000000..a663c317
--- /dev/null
+++ b/include/libcamera/dma_heap.h
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2020, Raspberry Pi Ltd
+ *
+ * dma_heap.h - Dma Heap implementation.
+ */
+
+#include <libcamera/heap.h>
+
+namespace libcamera {
+
+class DmaHeap : public Heap
+{
+public:
+	DmaHeap();
+	~DmaHeap();
+	UniqueFD alloc(const char *name, std::size_t size) override;
+};
+
+} /* namespace libcamera */
diff --git a/include/libcamera/heap.h b/include/libcamera/heap.h
new file mode 100644
index 00000000..c49a2ac3
--- /dev/null
+++ b/include/libcamera/heap.h
@@ -0,0 +1,27 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2023, Google Inc.
+ *
+ * heap.h - Heap interface.
+ */
+
+#pragma once
+
+#include <stddef.h>
+
+#include <libcamera/base/unique_fd.h>
+
+namespace libcamera {
+
+class Heap
+{
+public:
+	virtual ~Heap() = default;
+	bool isValid() const { return handle_.isValid(); }
+	virtual UniqueFD alloc(const char *name, std::size_t size) = 0;
+
+protected:
+	UniqueFD handle_;
+};
+
+} /* namespace libcamera */
diff --git a/include/libcamera/heap_allocator.h b/include/libcamera/heap_allocator.h
new file mode 100644
index 00000000..cd7ed1a3
--- /dev/null
+++ b/include/libcamera/heap_allocator.h
@@ -0,0 +1,30 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2023, Google Inc.
+ *
+ * heap_allocator.h - Helper class for heap buffer allocations.
+ */
+
+#pragma once
+
+#include <stddef.h>
+
+#include <libcamera/base/unique_fd.h>
+
+#include <libcamera/heap.h>
+
+namespace libcamera {
+
+class HeapAllocator
+{
+public:
+	HeapAllocator();
+	~HeapAllocator();
+	bool isValid() const { return heap_->isValid(); }
+	UniqueFD alloc(const char *name, std::size_t size);
+
+private:
+	std::unique_ptr<Heap> heap_;
+};
+
+} /* namespace libcamera */
diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
index 408b7acf..f486630a 100644
--- a/include/libcamera/meson.build
+++ b/include/libcamera/meson.build
@@ -7,10 +7,13 @@  libcamera_public_headers = files([
     'camera_manager.h',
     'color_space.h',
     'controls.h',
+    'dma_heap.h',
     'fence.h',
     'framebuffer.h',
     'framebuffer_allocator.h',
     'geometry.h',
+    'heap.h',
+    'heap_allocator.h',
     'logging.h',
     'pixel_format.h',
     'request.h',
diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/dma_heap.cpp
similarity index 58%
rename from src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
rename to src/libcamera/dma_heap.cpp
index 6b644406..38f603b0 100644
--- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp
+++ b/src/libcamera/dma_heap.cpp
@@ -2,18 +2,19 @@ 
 /*
  * Copyright (C) 2020, Raspberry Pi Ltd
  *
- * dma_heaps.h - Helper class for dma-heap allocations.
+ * dma_heap.h - Dma Heap implementation.
  */
 
-#include "dma_heaps.h"
+#include <libcamera/dma_heap.h>
 
 #include <array>
 #include <fcntl.h>
-#include <linux/dma-buf.h>
-#include <linux/dma-heap.h>
 #include <sys/ioctl.h>
 #include <unistd.h>
 
+#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
+
 #include <libcamera/base/log.h>
 
 /*
@@ -30,9 +31,7 @@  static constexpr std::array<const char *, 2> heapNames = {
 
 namespace libcamera {
 
-LOG_DECLARE_CATEGORY(RPI)
-
-namespace RPi {
+LOG_DEFINE_CATEGORY(DmaHeap)
 
 DmaHeap::DmaHeap()
 {
@@ -40,17 +39,34 @@  DmaHeap::DmaHeap()
 		int ret = ::open(name, O_RDWR, 0);
 		if (ret < 0) {
 			ret = errno;
-			LOG(RPI, Debug) << "Failed to open " << name << ": "
-					<< strerror(ret);
+			LOG(DmaHeap, Debug) << "Failed to open " << name << ": "
+					    << strerror(ret);
 			continue;
 		}
 
-		dmaHeapHandle_ = UniqueFD(ret);
+		handle_ = UniqueFD(ret);
 		break;
 	}
 
-	if (!dmaHeapHandle_.isValid())
-		LOG(RPI, Error) << "Could not open any dmaHeap device";
+	if (!handle_.isValid())
+		LOG(DmaHeap, Error) << "Could not open any dmaHeap device";
+
+	int ret = ::open("/dev/udmabuf", O_RDWR, 0);
+	if (ret < 0) {
+		ret = errno;
+		LOG(DmaHeap, Error)
+			<< "Failed to open allocator: " << strerror(ret);
+
+		if (ret == EACCES) {
+			LOG(DmaHeap, Info)
+				<< "Consider making /dev/udmabuf accessible by the video group";
+			LOG(DmaHeap, Info)
+				<< "Alternatively, add your user to the kvm group.";
+		}
+
+	} else {
+		handle_ = UniqueFD(ret);
+	}
 }
 
 DmaHeap::~DmaHeap() = default;
@@ -67,24 +83,22 @@  UniqueFD DmaHeap::alloc(const char *name, std::size_t size)
 	alloc.len = size;
 	alloc.fd_flags = O_CLOEXEC | O_RDWR;
 
-	ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);
+	ret = ::ioctl(handle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);
 	if (ret < 0) {
-		LOG(RPI, Error) << "dmaHeap allocation failure for "
-				<< name;
+		LOG(DmaHeap, Error) << "dmaHeap allocation failure for "
+				    << name;
 		return {};
 	}
 
 	UniqueFD allocFd(alloc.fd);
 	ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);
 	if (ret < 0) {
-		LOG(RPI, Error) << "dmaHeap naming failure for "
-				<< name;
+		LOG(DmaHeap, Error) << "dmaHeap naming failure for "
+				    << name;
 		return {};
 	}
 
 	return allocFd;
 }
 
-} /* namespace RPi */
-
 } /* namespace libcamera */
diff --git a/src/libcamera/heap_allocator.cpp b/src/libcamera/heap_allocator.cpp
new file mode 100644
index 00000000..594b1d6a
--- /dev/null
+++ b/src/libcamera/heap_allocator.cpp
@@ -0,0 +1,43 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2023, Google Inc.
+ *
+ * heap_allocator.cpp - Helper class for heap buffer allocations.
+ */
+
+#include <libcamera/heap_allocator.h>
+
+#include <array>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+
+#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
+
+#include <libcamera/base/log.h>
+
+#include <libcamera/dma_heap.h>
+
+namespace libcamera {
+
+LOG_DEFINE_CATEGORY(HeapAllocator)
+
+HeapAllocator::HeapAllocator()
+{
+	heap_ = std::make_unique<DmaHeap>();
+}
+
+HeapAllocator::~HeapAllocator() = default;
+
+UniqueFD HeapAllocator::alloc(const char *name, std::size_t size)
+{
+	if (!isValid()) {
+		LOG(HeapAllocator, Fatal) << "Allocation attempted without allocator" << name;
+		return {};
+	}
+
+	return heap_->alloc(name, size);
+}
+
+} /* namespace libcamera */
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index 9869bfe7..ee586c0d 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -17,11 +17,13 @@  libcamera_sources = files([
     'delayed_controls.cpp',
     'device_enumerator.cpp',
     'device_enumerator_sysfs.cpp',
+    'dma_heap.cpp',
     'fence.cpp',
     'formats.cpp',
     'framebuffer.cpp',
     'framebuffer_allocator.cpp',
     'geometry.cpp',
+    'heap_allocator.cpp',
     'ipa_controls.cpp',
     'ipa_data_serializer.cpp',
     'ipa_interface.cpp',
diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/raspberrypi/dma_heaps.h
deleted file mode 100644
index 0a4a8d86..00000000
--- a/src/libcamera/pipeline/raspberrypi/dma_heaps.h
+++ /dev/null
@@ -1,32 +0,0 @@ 
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2020, Raspberry Pi Ltd
- *
- * dma_heaps.h - Helper class for dma-heap allocations.
- */
-
-#pragma once
-
-#include <stddef.h>
-
-#include <libcamera/base/unique_fd.h>
-
-namespace libcamera {
-
-namespace RPi {
-
-class DmaHeap
-{
-public:
-	DmaHeap();
-	~DmaHeap();
-	bool isValid() const { return dmaHeapHandle_.isValid(); }
-	UniqueFD alloc(const char *name, std::size_t size);
-
-private:
-	UniqueFD dmaHeapHandle_;
-};
-
-} /* namespace RPi */
-
-} /* namespace libcamera */
diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build
index 6064a3f0..42b96286 100644
--- a/src/libcamera/pipeline/raspberrypi/meson.build
+++ b/src/libcamera/pipeline/raspberrypi/meson.build
@@ -2,7 +2,6 @@ 
 
 libcamera_sources += files([
     'delayed_controls.cpp',
-    'dma_heaps.cpp',
     'raspberrypi.cpp',
     'rpi_stream.cpp',
 ])
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 8569df17..18325faa 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -20,6 +20,7 @@ 
 #include <libcamera/camera.h>
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
+#include <libcamera/heap_allocator.h>
 #include <libcamera/ipa/raspberrypi_ipa_interface.h>
 #include <libcamera/ipa/raspberrypi_ipa_proxy.h>
 #include <libcamera/logging.h>
@@ -41,7 +42,6 @@ 
 #include "libcamera/internal/v4l2_videodevice.h"
 
 #include "delayed_controls.h"
-#include "dma_heaps.h"
 #include "rpi_stream.h"
 
 using namespace std::chrono_literals;
@@ -240,7 +240,7 @@  public:
 	std::vector<std::pair<std::unique_ptr<V4L2Subdevice>, MediaLink *>> bridgeDevices_;
 
 	/* DMAHEAP allocation helper. */
-	RPi::DmaHeap dmaHeap_;
+	HeapAllocator heapAllocator_;
 	SharedFD lsTable_;
 
 	std::unique_ptr<RPi::DelayedControls> delayedCtrls_;
@@ -1206,7 +1206,7 @@  int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me
 {
 	std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
 
-	if (!data->dmaHeap_.isValid())
+	if (!data->heapAllocator_.isValid())
 		return -ENOMEM;
 
 	MediaEntity *unicamImage = unicam->getEntityByName("unicam-image");
@@ -1595,9 +1595,9 @@  int RPiCameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::IPA
 	/* Always send the user transform to the IPA. */
 	ipaConfig.transform = static_cast<unsigned int>(config->transform);
 
-	/* Allocate the lens shading table via dmaHeap and pass to the IPA. */
+	/* Allocate the lens shading table via heapAllocator and pass to the IPA. */
 	if (!lsTable_.isValid()) {
-		lsTable_ = SharedFD(dmaHeap_.alloc("ls_grid", ipa::RPi::MaxLsGridSize));
+		lsTable_ = SharedFD(heapAllocator_.alloc("ls_grid", ipa::RPi::MaxLsGridSize));
 		if (!lsTable_.isValid())
 			return -ENOMEM;