| Message ID | 20240530171600.259495-4-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Hans de Goede (2024-05-30 18:15:58) > Users of the DmaHeap class really just want some way to allocate > dma-buffers from userspace. This can also be done by using /dev/udmabuf > instead of using /dev/dma_heap/*. > > Rename DmaHeap class to DmaBufAllocator in preparation of adding > /dev/udmabuf support. > > And update the DmaHeap class docs to match including replacing references > to "dma-heap type" with "dma-buf provider". > > This is a pure automated rename on the code ('s/DmaHeap/DmaBufAllocator/') > + file renames + doc updates. There are no functional changes. That's all fine with me, and the patch overall itself. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > The DmaBufAllocator objects in vc4.cpp and software_isp.cpp are left named > dmaHeap_ to keep the changes to those 2 files to a minimum. > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > .../libcamera/internal/dma_buf_allocator.h | 38 ++++ > include/libcamera/internal/dma_heaps.h | 38 ---- > include/libcamera/internal/meson.build | 2 +- > .../internal/software_isp/software_isp.h | 4 +- > src/libcamera/dma_buf_allocator.cpp | 159 +++++++++++++++++ > src/libcamera/dma_heaps.cpp | 165 ------------------ > src/libcamera/meson.build | 2 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 +- Naush, The changes here to vc4.cpp are minimal, but even still, Are you happy with the changes here? (And in fact the original class naming comes from RPi too...) > src/libcamera/software_isp/software_isp.cpp | 5 +- > 9 files changed, 206 insertions(+), 211 deletions(-) > create mode 100644 include/libcamera/internal/dma_buf_allocator.h > delete mode 100644 include/libcamera/internal/dma_heaps.h > create mode 100644 src/libcamera/dma_buf_allocator.cpp > delete mode 100644 src/libcamera/dma_heaps.cpp > > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h > new file mode 100644 > index 00000000..a881042e > --- /dev/null > +++ b/include/libcamera/internal/dma_buf_allocator.h > @@ -0,0 +1,38 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2020, Raspberry Pi Ltd > + * > + * Helper class for dma-buf allocations. > + */ > + > +#pragma once > + > +#include <stddef.h> > + > +#include <libcamera/base/flags.h> > +#include <libcamera/base/unique_fd.h> > + > +namespace libcamera { > + > +class DmaBufAllocator > +{ > +public: > + enum class DmaBufAllocatorFlag { > + CmaHeap = 1 << 0, > + SystemHeap = 1 << 1, > + }; > + > + using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>; > + > + DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap); > + ~DmaBufAllocator(); > + bool isValid() const { return providerHandle_.isValid(); } > + UniqueFD alloc(const char *name, std::size_t size); > + > +private: > + UniqueFD providerHandle_; > +}; > + > +LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag) > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/internal/dma_heaps.h b/include/libcamera/internal/dma_heaps.h > deleted file mode 100644 > index f0a8aa5d..00000000 > --- a/include/libcamera/internal/dma_heaps.h > +++ /dev/null > @@ -1,38 +0,0 @@ > -/* SPDX-License-Identifier: LGPL-2.1-or-later */ > -/* > - * Copyright (C) 2020, Raspberry Pi Ltd > - * > - * Helper class for dma-heap allocations. > - */ > - > -#pragma once > - > -#include <stddef.h> > - > -#include <libcamera/base/flags.h> > -#include <libcamera/base/unique_fd.h> > - > -namespace libcamera { > - > -class DmaHeap > -{ > -public: > - enum class DmaHeapFlag { > - Cma = 1 << 0, > - System = 1 << 1, > - }; > - > - using DmaHeapFlags = Flags<DmaHeapFlag>; > - > - DmaHeap(DmaHeapFlags flags = DmaHeapFlag::Cma); > - ~DmaHeap(); > - bool isValid() const { return dmaHeapHandle_.isValid(); } > - UniqueFD alloc(const char *name, std::size_t size); > - > -private: > - UniqueFD dmaHeapHandle_; > -}; > - > -LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaHeap::DmaHeapFlag) > - > -} /* namespace libcamera */ > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 160fdc37..9713ea1c 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -25,7 +25,7 @@ libcamera_internal_headers = files([ > 'device_enumerator.h', > 'device_enumerator_sysfs.h', > 'device_enumerator_udev.h', > - 'dma_heaps.h', > + 'dma_buf_allocator.h', > 'formats.h', > 'framebuffer.h', > 'ipa_manager.h', > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h > index 7e9fae6a..c5338c05 100644 > --- a/include/libcamera/internal/software_isp/software_isp.h > +++ b/include/libcamera/internal/software_isp/software_isp.h > @@ -27,7 +27,7 @@ > #include <libcamera/ipa/soft_ipa_proxy.h> > > #include "libcamera/internal/camera_sensor.h" > -#include "libcamera/internal/dma_heaps.h" > +#include "libcamera/internal/dma_buf_allocator.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/shared_mem_object.h" > #include "libcamera/internal/software_isp/debayer_params.h" > @@ -91,7 +91,7 @@ private: > Thread ispWorkerThread_; > SharedMemObject<DebayerParams> sharedParams_; > DebayerParams debayerParams_; > - DmaHeap dmaHeap_; > + DmaBufAllocator dmaHeap_; > > std::unique_ptr<ipa::soft::IPAProxySoft> ipa_; > }; > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp > new file mode 100644 > index 00000000..bc0d78ef > --- /dev/null > +++ b/src/libcamera/dma_buf_allocator.cpp > @@ -0,0 +1,159 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Red Hat Inc. > + * Copyright (C) 2020, Raspberry Pi Ltd > + * > + * Helper class for dma-buf allocations. > + */ > + > +#include "libcamera/internal/dma_buf_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> > + > +/** > + * \file dma_buf_allocator.cpp > + * \brief dma-buf allocator > + */ > + > +namespace libcamera { > + > +#ifndef __DOXYGEN__ > +struct DmaBufAllocatorInfo { > + DmaBufAllocator::DmaBufAllocatorFlag type; > + const char *deviceNodeName; > +}; > +#endif > + > +static constexpr std::array<DmaBufAllocatorInfo, 3> 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". > + */ That's a good move to be closer to the reference. > + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/linux,cma" }, > + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/reserved" }, > + { DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap, "/dev/dma_heap/system" }, > +} }; > + > +LOG_DEFINE_CATEGORY(DmaBufAllocator) > + > +/** > + * \class DmaBufAllocator > + * \brief Helper class for dma-buf allocations > + * > + * This class wraps a userspace dma-buf provider selected at construction time, > + * and exposes functions to allocate dma-buffers from this provider. > + * > + * Different providers may provide dma-buffers with different properties for > + * the underlying memory. Which providers are acceptable is specified through > + * the type argument passed to the DmaBufAllocator() constructor. > + */ > + > +/** > + * \enum DmaBufAllocator::DmaBufAllocatorFlag > + * \brief Type of the dma-buf provider > + * \var DmaBufAllocator::CmaHeap > + * \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 > + */ > + > +/** > + * \typedef DmaBufAllocator::DmaBufAllocatorFlags > + * \brief A bitwise combination of DmaBufAllocator::DmaBufAllocatorFlag values > + */ > + > +/** > + * \brief Construct a DmaBufAllocator of a given type > + * \param[in] type The type(s) of the dma-buf providers to allocate from > + * > + * The dma-buf provider type is selected with the \a type parameter, which defaults > + * to the CMA heap. If no provider of the given type can be accessed, the constructed > + * DmaBufAllocator instance is invalid as indicated by the isValid() function. > + * > + * Multiple types can be selected by combining type flags, in which case the > + * constructed DmaBufAllocator will match one of the types. If multiple requested > + * types can work on the system, which provider is used is undefined. > + */ > +DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type) > +{ > + for (const auto &info : providerInfos) { > + if (!(type & info.type)) > + continue; > + > + int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0); > + if (ret < 0) { > + ret = errno; > + LOG(DmaBufAllocator, Debug) > + << "Failed to open " << info.deviceNodeName << ": " > + << strerror(ret); > + continue; > + } > + > + LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName; > + providerHandle_ = UniqueFD(ret); > + break; > + } > + > + if (!providerHandle_.isValid()) > + LOG(DmaBufAllocator, Error) << "Could not open any dma-buf provider"; > +} > + > +/** > + * \brief Destroy the DmaBufAllocator instance > + */ > +DmaBufAllocator::~DmaBufAllocator() = default; > + > +/** > + * \fn DmaBufAllocator::isValid() > + * \brief Check if the DmaBufAllocator instance is valid > + * \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) > +{ > + int ret; > + > + if (!name) > + return {}; > + > + struct dma_heap_allocation_data alloc = {}; > + > + alloc.len = size; > + alloc.fd_flags = O_CLOEXEC | O_RDWR; > + > + ret = ::ioctl(providerHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc); > + if (ret < 0) { > + LOG(DmaBufAllocator, Error) << "dma-heap allocation failure for " << name; > + return {}; > + } > + > + UniqueFD allocFd(alloc.fd); > + ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name); > + if (ret < 0) { > + LOG(DmaBufAllocator, Error) << "dma-heap naming failure for " << name; > + return {}; > + } > + > + return allocFd; > +} > + > +} /* namespace libcamera */ > diff --git a/src/libcamera/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp > deleted file mode 100644 > index d4cb880b..00000000 > --- a/src/libcamera/dma_heaps.cpp > +++ /dev/null > @@ -1,165 +0,0 @@ > -/* SPDX-License-Identifier: LGPL-2.1-or-later */ > -/* > - * Copyright (C) 2020, Raspberry Pi Ltd > - * > - * Helper class for dma-heap allocations. > - */ > - > -#include "libcamera/internal/dma_heaps.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> > - > -/** > - * \file dma_heaps.cpp > - * \brief dma-heap allocator > - */ > - > -namespace libcamera { > - > -/* > - * /dev/dma_heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma > - * to only have to worry about importing. > - * > - * Annoyingly, should the cma heap size be specified on the kernel command line > - * instead of DT, the heap gets named "reserved" instead. > - */ > - > -#ifndef __DOXYGEN__ > -struct DmaHeapInfo { > - DmaHeap::DmaHeapFlag type; > - const char *deviceNodeName; > -}; > -#endif > - > -static constexpr std::array<DmaHeapInfo, 3> heapInfos = { { > - { DmaHeap::DmaHeapFlag::Cma, "/dev/dma_heap/linux,cma" }, > - { DmaHeap::DmaHeapFlag::Cma, "/dev/dma_heap/reserved" }, > - { DmaHeap::DmaHeapFlag::System, "/dev/dma_heap/system" }, > -} }; > - > -LOG_DEFINE_CATEGORY(DmaHeap) > - > -/** > - * \class DmaHeap > - * \brief Helper class for dma-heap allocations > - * > - * DMA heaps are kernel devices that provide an API to allocate memory from > - * different pools called "heaps", wrap each allocated piece of memory in a > - * dmabuf object, and return the dmabuf file descriptor to userspace. Multiple > - * heaps can be provided by the system, with different properties for the > - * underlying memory. > - * > - * This class wraps a DMA heap selected at construction time, and exposes > - * functions to manage memory allocation. > - */ > - > -/** > - * \enum DmaHeap::DmaHeapFlag > - * \brief Type of the dma-heap > - * \var DmaHeap::Cma > - * \brief Allocate from a CMA dma-heap, providing physically-contiguous memory > - * \var DmaHeap::System > - * \brief Allocate from the system dma-heap, using the page allocator > - */ > - > -/** > - * \typedef DmaHeap::DmaHeapFlags > - * \brief A bitwise combination of DmaHeap::DmaHeapFlag values > - */ > - > -/** > - * \brief Construct a DmaHeap of a given type > - * \param[in] type The type(s) of the dma-heap(s) to allocate from > - * > - * The DMA heap type is selected with the \a type parameter, which defaults to > - * the CMA heap. If no heap of the given type can be accessed, the constructed > - * DmaHeap instance is invalid as indicated by the isValid() function. > - * > - * Multiple types can be selected by combining type flags, in which case the > - * constructed DmaHeap will match one of the types. If the system provides > - * multiple heaps that match the requested types, which heap is used is > - * undefined. > - */ > -DmaHeap::DmaHeap(DmaHeapFlags type) > -{ > - for (const auto &info : heapInfos) { > - if (!(type & info.type)) > - continue; > - > - int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0); > - if (ret < 0) { > - ret = errno; > - LOG(DmaHeap, Debug) > - << "Failed to open " << info.deviceNodeName << ": " > - << strerror(ret); > - continue; > - } > - > - LOG(DmaHeap, Debug) << "Using " << info.deviceNodeName; > - dmaHeapHandle_ = UniqueFD(ret); > - break; > - } > - > - if (!dmaHeapHandle_.isValid()) > - LOG(DmaHeap, Error) << "Could not open any dmaHeap device"; > -} > - > -/** > - * \brief Destroy the DmaHeap instance > - */ > -DmaHeap::~DmaHeap() = default; > - > -/** > - * \fn DmaHeap::isValid() > - * \brief Check if the DmaHeap instance is valid > - * \return True if the DmaHeap is valid, false otherwise > - */ > - > -/** > - * \brief Allocate a dma-buf from the DmaHeap > - * \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 DmaHeap::alloc(const char *name, std::size_t size) > -{ > - int ret; > - > - if (!name) > - return {}; > - > - struct dma_heap_allocation_data alloc = {}; > - > - alloc.len = size; > - alloc.fd_flags = O_CLOEXEC | O_RDWR; > - > - ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc); > - if (ret < 0) { > - 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(DmaHeap, Error) << "dmaHeap naming failure for " << name; > - return {}; > - } > - > - return allocFd; > -} > - > -} /* namespace libcamera */ > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index a3b12bc1..89504cee 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -15,7 +15,7 @@ libcamera_sources = files([ > 'delayed_controls.cpp', > 'device_enumerator.cpp', > 'device_enumerator_sysfs.cpp', > - 'dma_heaps.cpp', > + 'dma_buf_allocator.cpp', > 'fence.cpp', > 'formats.cpp', > 'framebuffer.cpp', > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > index 37fb310f..4a89e35f 100644 > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > @@ -12,7 +12,7 @@ > #include <libcamera/formats.h> > > #include "libcamera/internal/device_enumerator.h" > -#include "libcamera/internal/dma_heaps.h" > +#include "libcamera/internal/dma_buf_allocator.h" > > #include "../common/pipeline_base.h" > #include "../common/rpi_stream.h" > @@ -86,7 +86,7 @@ public: > RPi::Device<Isp, 4> isp_; > > /* DMAHEAP allocation helper. */ > - DmaHeap dmaHeap_; > + DmaBufAllocator dmaHeap_; > SharedFD lsTable_; > > struct Config { > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp > index c9b6be56..b5eab670 100644 > --- a/src/libcamera/software_isp/software_isp.cpp > +++ b/src/libcamera/software_isp/software_isp.cpp > @@ -65,10 +65,11 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) > SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) > : debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, > DebayerParams::kGain10, 0.5f, 0 }, > - dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) > + dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap | > + DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap) > { > if (!dmaHeap_.isValid()) { > - LOG(SoftwareIsp, Error) << "Failed to create DmaHeap object"; > + LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object"; > return; > } > > -- > 2.45.1 >
On 30/05/2024 18:15, Hans de Goede wrote: > Users of the DmaHeap class really just want some way to allocate > dma-buffers from userspace. This can also be done by using /dev/udmabuf > instead of using /dev/dma_heap/*. This one fails to apply for me with @Milan's colour fixes applied. Its minor enough but one of you will need to rebase. https://lists.libcamera.org/pipermail/libcamera-devel/2024-May/042105.html --- bod
Hi Hans, Thank you for the patch. On Thu, May 30, 2024 at 07:15:58PM +0200, Hans de Goede wrote: > Users of the DmaHeap class really just want some way to allocate > dma-buffers from userspace. This can also be done by using /dev/udmabuf > instead of using /dev/dma_heap/*. > > Rename DmaHeap class to DmaBufAllocator in preparation of adding > /dev/udmabuf support. > > And update the DmaHeap class docs to match including replacing references > to "dma-heap type" with "dma-buf provider". > > This is a pure automated rename on the code ('s/DmaHeap/DmaBufAllocator/') > + file renames + doc updates. There are no functional changes. > > The DmaBufAllocator objects in vc4.cpp and software_isp.cpp are left named > dmaHeap_ to keep the changes to those 2 files to a minimum. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > .../libcamera/internal/dma_buf_allocator.h | 38 ++++ > include/libcamera/internal/dma_heaps.h | 38 ---- > include/libcamera/internal/meson.build | 2 +- > .../internal/software_isp/software_isp.h | 4 +- > src/libcamera/dma_buf_allocator.cpp | 159 +++++++++++++++++ > src/libcamera/dma_heaps.cpp | 165 ------------------ > src/libcamera/meson.build | 2 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 +- > src/libcamera/software_isp/software_isp.cpp | 5 +- > 9 files changed, 206 insertions(+), 211 deletions(-) > create mode 100644 include/libcamera/internal/dma_buf_allocator.h > delete mode 100644 include/libcamera/internal/dma_heaps.h > create mode 100644 src/libcamera/dma_buf_allocator.cpp > delete mode 100644 src/libcamera/dma_heaps.cpp > > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h > new file mode 100644 > index 00000000..a881042e > --- /dev/null > +++ b/include/libcamera/internal/dma_buf_allocator.h > @@ -0,0 +1,38 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2020, Raspberry Pi Ltd > + * > + * Helper class for dma-buf allocations. > + */ > + > +#pragma once > + > +#include <stddef.h> > + > +#include <libcamera/base/flags.h> > +#include <libcamera/base/unique_fd.h> > + > +namespace libcamera { > + > +class DmaBufAllocator > +{ > +public: > + enum class DmaBufAllocatorFlag { > + CmaHeap = 1 << 0, > + SystemHeap = 1 << 1, > + }; > + > + using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>; > + > + DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap); > + ~DmaBufAllocator(); > + bool isValid() const { return providerHandle_.isValid(); } > + UniqueFD alloc(const char *name, std::size_t size); > + > +private: > + UniqueFD providerHandle_; > +}; > + > +LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag) > + > +} /* namespace libcamera */ > diff --git a/include/libcamera/internal/dma_heaps.h b/include/libcamera/internal/dma_heaps.h > deleted file mode 100644 > index f0a8aa5d..00000000 > --- a/include/libcamera/internal/dma_heaps.h > +++ /dev/null > @@ -1,38 +0,0 @@ > -/* SPDX-License-Identifier: LGPL-2.1-or-later */ > -/* > - * Copyright (C) 2020, Raspberry Pi Ltd > - * > - * Helper class for dma-heap allocations. > - */ > - > -#pragma once > - > -#include <stddef.h> > - > -#include <libcamera/base/flags.h> > -#include <libcamera/base/unique_fd.h> > - > -namespace libcamera { > - > -class DmaHeap > -{ > -public: > - enum class DmaHeapFlag { > - Cma = 1 << 0, > - System = 1 << 1, > - }; > - > - using DmaHeapFlags = Flags<DmaHeapFlag>; > - > - DmaHeap(DmaHeapFlags flags = DmaHeapFlag::Cma); > - ~DmaHeap(); > - bool isValid() const { return dmaHeapHandle_.isValid(); } > - UniqueFD alloc(const char *name, std::size_t size); > - > -private: > - UniqueFD dmaHeapHandle_; > -}; > - > -LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaHeap::DmaHeapFlag) > - > -} /* namespace libcamera */ > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 160fdc37..9713ea1c 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -25,7 +25,7 @@ libcamera_internal_headers = files([ > 'device_enumerator.h', > 'device_enumerator_sysfs.h', > 'device_enumerator_udev.h', > - 'dma_heaps.h', > + 'dma_buf_allocator.h', > 'formats.h', > 'framebuffer.h', > 'ipa_manager.h', > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h > index 7e9fae6a..c5338c05 100644 > --- a/include/libcamera/internal/software_isp/software_isp.h > +++ b/include/libcamera/internal/software_isp/software_isp.h > @@ -27,7 +27,7 @@ > #include <libcamera/ipa/soft_ipa_proxy.h> > > #include "libcamera/internal/camera_sensor.h" > -#include "libcamera/internal/dma_heaps.h" > +#include "libcamera/internal/dma_buf_allocator.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/shared_mem_object.h" > #include "libcamera/internal/software_isp/debayer_params.h" > @@ -91,7 +91,7 @@ private: > Thread ispWorkerThread_; > SharedMemObject<DebayerParams> sharedParams_; > DebayerParams debayerParams_; > - DmaHeap dmaHeap_; > + DmaBufAllocator dmaHeap_; > > std::unique_ptr<ipa::soft::IPAProxySoft> ipa_; > }; > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp > new file mode 100644 > index 00000000..bc0d78ef > --- /dev/null > +++ b/src/libcamera/dma_buf_allocator.cpp > @@ -0,0 +1,159 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Red Hat Inc. > + * Copyright (C) 2020, Raspberry Pi Ltd > + * > + * Helper class for dma-buf allocations. > + */ > + > +#include "libcamera/internal/dma_buf_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> > + > +/** > + * \file dma_buf_allocator.cpp > + * \brief dma-buf allocator > + */ > + > +namespace libcamera { > + > +#ifndef __DOXYGEN__ > +struct DmaBufAllocatorInfo { > + DmaBufAllocator::DmaBufAllocatorFlag type; > + const char *deviceNodeName; > +}; > +#endif > + > +static constexpr std::array<DmaBufAllocatorInfo, 3> 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". > + */ > + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/linux,cma" }, > + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/reserved" }, > + { DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap, "/dev/dma_heap/system" }, > +} }; > + > +LOG_DEFINE_CATEGORY(DmaBufAllocator) > + > +/** > + * \class DmaBufAllocator > + * \brief Helper class for dma-buf allocations > + * > + * This class wraps a userspace dma-buf provider selected at construction time, > + * and exposes functions to allocate dma-buffers from this provider. > + * > + * Different providers may provide dma-buffers with different properties for > + * the underlying memory. Which providers are acceptable is specified through s/ / / > + * the type argument passed to the DmaBufAllocator() constructor. > + */ > + > +/** > + * \enum DmaBufAllocator::DmaBufAllocatorFlag > + * \brief Type of the dma-buf provider > + * \var DmaBufAllocator::CmaHeap > + * \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 > + */ > + > +/** > + * \typedef DmaBufAllocator::DmaBufAllocatorFlags > + * \brief A bitwise combination of DmaBufAllocator::DmaBufAllocatorFlag values > + */ > + > +/** > + * \brief Construct a DmaBufAllocator of a given type > + * \param[in] type The type(s) of the dma-buf providers to allocate from > + * > + * The dma-buf provider type is selected with the \a type parameter, which defaults > + * to the CMA heap. If no provider of the given type can be accessed, the constructed > + * DmaBufAllocator instance is invalid as indicated by the isValid() function. > + * > + * Multiple types can be selected by combining type flags, in which case the > + * constructed DmaBufAllocator will match one of the types. If multiple requested > + * types can work on the system, which provider is used is undefined. Please wrap the text at 80 columns. > + */ > +DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type) > +{ > + for (const auto &info : providerInfos) { > + if (!(type & info.type)) > + continue; > + > + int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0); > + if (ret < 0) { > + ret = errno; > + LOG(DmaBufAllocator, Debug) > + << "Failed to open " << info.deviceNodeName << ": " > + << strerror(ret); > + continue; > + } > + > + LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName; > + providerHandle_ = UniqueFD(ret); > + break; > + } > + > + if (!providerHandle_.isValid()) > + LOG(DmaBufAllocator, Error) << "Could not open any dma-buf provider"; > +} > + > +/** > + * \brief Destroy the DmaBufAllocator instance > + */ > +DmaBufAllocator::~DmaBufAllocator() = default; > + > +/** > + * \fn DmaBufAllocator::isValid() > + * \brief Check if the DmaBufAllocator instance is valid > + * \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) > +{ > + int ret; > + > + if (!name) > + return {}; > + > + struct dma_heap_allocation_data alloc = {}; > + > + alloc.len = size; > + alloc.fd_flags = O_CLOEXEC | O_RDWR; > + > + ret = ::ioctl(providerHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc); > + if (ret < 0) { > + LOG(DmaBufAllocator, Error) << "dma-heap allocation failure for " << name; LOG(DmaBufAllocator, Error) << "dma-heap allocation failure for " << name; > + return {}; > + } > + > + UniqueFD allocFd(alloc.fd); > + ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name); > + if (ret < 0) { > + LOG(DmaBufAllocator, Error) << "dma-heap naming failure for " << name; Same here. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + return {}; > + } > + > + return allocFd; > +} > + > +} /* namespace libcamera */ > diff --git a/src/libcamera/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp > deleted file mode 100644 > index d4cb880b..00000000 > --- a/src/libcamera/dma_heaps.cpp > +++ /dev/null > @@ -1,165 +0,0 @@ > -/* SPDX-License-Identifier: LGPL-2.1-or-later */ > -/* > - * Copyright (C) 2020, Raspberry Pi Ltd > - * > - * Helper class for dma-heap allocations. > - */ > - > -#include "libcamera/internal/dma_heaps.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> > - > -/** > - * \file dma_heaps.cpp > - * \brief dma-heap allocator > - */ > - > -namespace libcamera { > - > -/* > - * /dev/dma_heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma > - * to only have to worry about importing. > - * > - * Annoyingly, should the cma heap size be specified on the kernel command line > - * instead of DT, the heap gets named "reserved" instead. > - */ > - > -#ifndef __DOXYGEN__ > -struct DmaHeapInfo { > - DmaHeap::DmaHeapFlag type; > - const char *deviceNodeName; > -}; > -#endif > - > -static constexpr std::array<DmaHeapInfo, 3> heapInfos = { { > - { DmaHeap::DmaHeapFlag::Cma, "/dev/dma_heap/linux,cma" }, > - { DmaHeap::DmaHeapFlag::Cma, "/dev/dma_heap/reserved" }, > - { DmaHeap::DmaHeapFlag::System, "/dev/dma_heap/system" }, > -} }; > - > -LOG_DEFINE_CATEGORY(DmaHeap) > - > -/** > - * \class DmaHeap > - * \brief Helper class for dma-heap allocations > - * > - * DMA heaps are kernel devices that provide an API to allocate memory from > - * different pools called "heaps", wrap each allocated piece of memory in a > - * dmabuf object, and return the dmabuf file descriptor to userspace. Multiple > - * heaps can be provided by the system, with different properties for the > - * underlying memory. > - * > - * This class wraps a DMA heap selected at construction time, and exposes > - * functions to manage memory allocation. > - */ > - > -/** > - * \enum DmaHeap::DmaHeapFlag > - * \brief Type of the dma-heap > - * \var DmaHeap::Cma > - * \brief Allocate from a CMA dma-heap, providing physically-contiguous memory > - * \var DmaHeap::System > - * \brief Allocate from the system dma-heap, using the page allocator > - */ > - > -/** > - * \typedef DmaHeap::DmaHeapFlags > - * \brief A bitwise combination of DmaHeap::DmaHeapFlag values > - */ > - > -/** > - * \brief Construct a DmaHeap of a given type > - * \param[in] type The type(s) of the dma-heap(s) to allocate from > - * > - * The DMA heap type is selected with the \a type parameter, which defaults to > - * the CMA heap. If no heap of the given type can be accessed, the constructed > - * DmaHeap instance is invalid as indicated by the isValid() function. > - * > - * Multiple types can be selected by combining type flags, in which case the > - * constructed DmaHeap will match one of the types. If the system provides > - * multiple heaps that match the requested types, which heap is used is > - * undefined. > - */ > -DmaHeap::DmaHeap(DmaHeapFlags type) > -{ > - for (const auto &info : heapInfos) { > - if (!(type & info.type)) > - continue; > - > - int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0); > - if (ret < 0) { > - ret = errno; > - LOG(DmaHeap, Debug) > - << "Failed to open " << info.deviceNodeName << ": " > - << strerror(ret); > - continue; > - } > - > - LOG(DmaHeap, Debug) << "Using " << info.deviceNodeName; > - dmaHeapHandle_ = UniqueFD(ret); > - break; > - } > - > - if (!dmaHeapHandle_.isValid()) > - LOG(DmaHeap, Error) << "Could not open any dmaHeap device"; > -} > - > -/** > - * \brief Destroy the DmaHeap instance > - */ > -DmaHeap::~DmaHeap() = default; > - > -/** > - * \fn DmaHeap::isValid() > - * \brief Check if the DmaHeap instance is valid > - * \return True if the DmaHeap is valid, false otherwise > - */ > - > -/** > - * \brief Allocate a dma-buf from the DmaHeap > - * \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 DmaHeap::alloc(const char *name, std::size_t size) > -{ > - int ret; > - > - if (!name) > - return {}; > - > - struct dma_heap_allocation_data alloc = {}; > - > - alloc.len = size; > - alloc.fd_flags = O_CLOEXEC | O_RDWR; > - > - ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc); > - if (ret < 0) { > - 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(DmaHeap, Error) << "dmaHeap naming failure for " << name; > - return {}; > - } > - > - return allocFd; > -} > - > -} /* namespace libcamera */ > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index a3b12bc1..89504cee 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -15,7 +15,7 @@ libcamera_sources = files([ > 'delayed_controls.cpp', > 'device_enumerator.cpp', > 'device_enumerator_sysfs.cpp', > - 'dma_heaps.cpp', > + 'dma_buf_allocator.cpp', > 'fence.cpp', > 'formats.cpp', > 'framebuffer.cpp', > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > index 37fb310f..4a89e35f 100644 > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > @@ -12,7 +12,7 @@ > #include <libcamera/formats.h> > > #include "libcamera/internal/device_enumerator.h" > -#include "libcamera/internal/dma_heaps.h" > +#include "libcamera/internal/dma_buf_allocator.h" > > #include "../common/pipeline_base.h" > #include "../common/rpi_stream.h" > @@ -86,7 +86,7 @@ public: > RPi::Device<Isp, 4> isp_; > > /* DMAHEAP allocation helper. */ > - DmaHeap dmaHeap_; > + DmaBufAllocator dmaHeap_; > SharedFD lsTable_; > > struct Config { > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp > index c9b6be56..b5eab670 100644 > --- a/src/libcamera/software_isp/software_isp.cpp > +++ b/src/libcamera/software_isp/software_isp.cpp > @@ -65,10 +65,11 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) > SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) > : debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, > DebayerParams::kGain10, 0.5f, 0 }, > - dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) > + dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap | > + DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap) > { > if (!dmaHeap_.isValid()) { > - LOG(SoftwareIsp, Error) << "Failed to create DmaHeap object"; > + LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object"; > return; > } >
diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h new file mode 100644 index 00000000..a881042e --- /dev/null +++ b/include/libcamera/internal/dma_buf_allocator.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Raspberry Pi Ltd + * + * Helper class for dma-buf allocations. + */ + +#pragma once + +#include <stddef.h> + +#include <libcamera/base/flags.h> +#include <libcamera/base/unique_fd.h> + +namespace libcamera { + +class DmaBufAllocator +{ +public: + enum class DmaBufAllocatorFlag { + CmaHeap = 1 << 0, + SystemHeap = 1 << 1, + }; + + using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>; + + DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap); + ~DmaBufAllocator(); + bool isValid() const { return providerHandle_.isValid(); } + UniqueFD alloc(const char *name, std::size_t size); + +private: + UniqueFD providerHandle_; +}; + +LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag) + +} /* namespace libcamera */ diff --git a/include/libcamera/internal/dma_heaps.h b/include/libcamera/internal/dma_heaps.h deleted file mode 100644 index f0a8aa5d..00000000 --- a/include/libcamera/internal/dma_heaps.h +++ /dev/null @@ -1,38 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2020, Raspberry Pi Ltd - * - * Helper class for dma-heap allocations. - */ - -#pragma once - -#include <stddef.h> - -#include <libcamera/base/flags.h> -#include <libcamera/base/unique_fd.h> - -namespace libcamera { - -class DmaHeap -{ -public: - enum class DmaHeapFlag { - Cma = 1 << 0, - System = 1 << 1, - }; - - using DmaHeapFlags = Flags<DmaHeapFlag>; - - DmaHeap(DmaHeapFlags flags = DmaHeapFlag::Cma); - ~DmaHeap(); - bool isValid() const { return dmaHeapHandle_.isValid(); } - UniqueFD alloc(const char *name, std::size_t size); - -private: - UniqueFD dmaHeapHandle_; -}; - -LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaHeap::DmaHeapFlag) - -} /* namespace libcamera */ diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 160fdc37..9713ea1c 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -25,7 +25,7 @@ libcamera_internal_headers = files([ 'device_enumerator.h', 'device_enumerator_sysfs.h', 'device_enumerator_udev.h', - 'dma_heaps.h', + 'dma_buf_allocator.h', 'formats.h', 'framebuffer.h', 'ipa_manager.h', diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h index 7e9fae6a..c5338c05 100644 --- a/include/libcamera/internal/software_isp/software_isp.h +++ b/include/libcamera/internal/software_isp/software_isp.h @@ -27,7 +27,7 @@ #include <libcamera/ipa/soft_ipa_proxy.h> #include "libcamera/internal/camera_sensor.h" -#include "libcamera/internal/dma_heaps.h" +#include "libcamera/internal/dma_buf_allocator.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/shared_mem_object.h" #include "libcamera/internal/software_isp/debayer_params.h" @@ -91,7 +91,7 @@ private: Thread ispWorkerThread_; SharedMemObject<DebayerParams> sharedParams_; DebayerParams debayerParams_; - DmaHeap dmaHeap_; + DmaBufAllocator dmaHeap_; std::unique_ptr<ipa::soft::IPAProxySoft> ipa_; }; diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp new file mode 100644 index 00000000..bc0d78ef --- /dev/null +++ b/src/libcamera/dma_buf_allocator.cpp @@ -0,0 +1,159 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Red Hat Inc. + * Copyright (C) 2020, Raspberry Pi Ltd + * + * Helper class for dma-buf allocations. + */ + +#include "libcamera/internal/dma_buf_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> + +/** + * \file dma_buf_allocator.cpp + * \brief dma-buf allocator + */ + +namespace libcamera { + +#ifndef __DOXYGEN__ +struct DmaBufAllocatorInfo { + DmaBufAllocator::DmaBufAllocatorFlag type; + const char *deviceNodeName; +}; +#endif + +static constexpr std::array<DmaBufAllocatorInfo, 3> 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". + */ + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/linux,cma" }, + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, "/dev/dma_heap/reserved" }, + { DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap, "/dev/dma_heap/system" }, +} }; + +LOG_DEFINE_CATEGORY(DmaBufAllocator) + +/** + * \class DmaBufAllocator + * \brief Helper class for dma-buf allocations + * + * This class wraps a userspace dma-buf provider selected at construction time, + * and exposes functions to allocate dma-buffers from this provider. + * + * Different providers may provide dma-buffers with different properties for + * the underlying memory. Which providers are acceptable is specified through + * the type argument passed to the DmaBufAllocator() constructor. + */ + +/** + * \enum DmaBufAllocator::DmaBufAllocatorFlag + * \brief Type of the dma-buf provider + * \var DmaBufAllocator::CmaHeap + * \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 + */ + +/** + * \typedef DmaBufAllocator::DmaBufAllocatorFlags + * \brief A bitwise combination of DmaBufAllocator::DmaBufAllocatorFlag values + */ + +/** + * \brief Construct a DmaBufAllocator of a given type + * \param[in] type The type(s) of the dma-buf providers to allocate from + * + * The dma-buf provider type is selected with the \a type parameter, which defaults + * to the CMA heap. If no provider of the given type can be accessed, the constructed + * DmaBufAllocator instance is invalid as indicated by the isValid() function. + * + * Multiple types can be selected by combining type flags, in which case the + * constructed DmaBufAllocator will match one of the types. If multiple requested + * types can work on the system, which provider is used is undefined. + */ +DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type) +{ + for (const auto &info : providerInfos) { + if (!(type & info.type)) + continue; + + int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0); + if (ret < 0) { + ret = errno; + LOG(DmaBufAllocator, Debug) + << "Failed to open " << info.deviceNodeName << ": " + << strerror(ret); + continue; + } + + LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName; + providerHandle_ = UniqueFD(ret); + break; + } + + if (!providerHandle_.isValid()) + LOG(DmaBufAllocator, Error) << "Could not open any dma-buf provider"; +} + +/** + * \brief Destroy the DmaBufAllocator instance + */ +DmaBufAllocator::~DmaBufAllocator() = default; + +/** + * \fn DmaBufAllocator::isValid() + * \brief Check if the DmaBufAllocator instance is valid + * \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) +{ + int ret; + + if (!name) + return {}; + + struct dma_heap_allocation_data alloc = {}; + + alloc.len = size; + alloc.fd_flags = O_CLOEXEC | O_RDWR; + + ret = ::ioctl(providerHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc); + if (ret < 0) { + LOG(DmaBufAllocator, Error) << "dma-heap allocation failure for " << name; + return {}; + } + + UniqueFD allocFd(alloc.fd); + ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name); + if (ret < 0) { + LOG(DmaBufAllocator, Error) << "dma-heap naming failure for " << name; + return {}; + } + + return allocFd; +} + +} /* namespace libcamera */ diff --git a/src/libcamera/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp deleted file mode 100644 index d4cb880b..00000000 --- a/src/libcamera/dma_heaps.cpp +++ /dev/null @@ -1,165 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2020, Raspberry Pi Ltd - * - * Helper class for dma-heap allocations. - */ - -#include "libcamera/internal/dma_heaps.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> - -/** - * \file dma_heaps.cpp - * \brief dma-heap allocator - */ - -namespace libcamera { - -/* - * /dev/dma_heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma - * to only have to worry about importing. - * - * Annoyingly, should the cma heap size be specified on the kernel command line - * instead of DT, the heap gets named "reserved" instead. - */ - -#ifndef __DOXYGEN__ -struct DmaHeapInfo { - DmaHeap::DmaHeapFlag type; - const char *deviceNodeName; -}; -#endif - -static constexpr std::array<DmaHeapInfo, 3> heapInfos = { { - { DmaHeap::DmaHeapFlag::Cma, "/dev/dma_heap/linux,cma" }, - { DmaHeap::DmaHeapFlag::Cma, "/dev/dma_heap/reserved" }, - { DmaHeap::DmaHeapFlag::System, "/dev/dma_heap/system" }, -} }; - -LOG_DEFINE_CATEGORY(DmaHeap) - -/** - * \class DmaHeap - * \brief Helper class for dma-heap allocations - * - * DMA heaps are kernel devices that provide an API to allocate memory from - * different pools called "heaps", wrap each allocated piece of memory in a - * dmabuf object, and return the dmabuf file descriptor to userspace. Multiple - * heaps can be provided by the system, with different properties for the - * underlying memory. - * - * This class wraps a DMA heap selected at construction time, and exposes - * functions to manage memory allocation. - */ - -/** - * \enum DmaHeap::DmaHeapFlag - * \brief Type of the dma-heap - * \var DmaHeap::Cma - * \brief Allocate from a CMA dma-heap, providing physically-contiguous memory - * \var DmaHeap::System - * \brief Allocate from the system dma-heap, using the page allocator - */ - -/** - * \typedef DmaHeap::DmaHeapFlags - * \brief A bitwise combination of DmaHeap::DmaHeapFlag values - */ - -/** - * \brief Construct a DmaHeap of a given type - * \param[in] type The type(s) of the dma-heap(s) to allocate from - * - * The DMA heap type is selected with the \a type parameter, which defaults to - * the CMA heap. If no heap of the given type can be accessed, the constructed - * DmaHeap instance is invalid as indicated by the isValid() function. - * - * Multiple types can be selected by combining type flags, in which case the - * constructed DmaHeap will match one of the types. If the system provides - * multiple heaps that match the requested types, which heap is used is - * undefined. - */ -DmaHeap::DmaHeap(DmaHeapFlags type) -{ - for (const auto &info : heapInfos) { - if (!(type & info.type)) - continue; - - int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0); - if (ret < 0) { - ret = errno; - LOG(DmaHeap, Debug) - << "Failed to open " << info.deviceNodeName << ": " - << strerror(ret); - continue; - } - - LOG(DmaHeap, Debug) << "Using " << info.deviceNodeName; - dmaHeapHandle_ = UniqueFD(ret); - break; - } - - if (!dmaHeapHandle_.isValid()) - LOG(DmaHeap, Error) << "Could not open any dmaHeap device"; -} - -/** - * \brief Destroy the DmaHeap instance - */ -DmaHeap::~DmaHeap() = default; - -/** - * \fn DmaHeap::isValid() - * \brief Check if the DmaHeap instance is valid - * \return True if the DmaHeap is valid, false otherwise - */ - -/** - * \brief Allocate a dma-buf from the DmaHeap - * \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 DmaHeap::alloc(const char *name, std::size_t size) -{ - int ret; - - if (!name) - return {}; - - struct dma_heap_allocation_data alloc = {}; - - alloc.len = size; - alloc.fd_flags = O_CLOEXEC | O_RDWR; - - ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc); - if (ret < 0) { - 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(DmaHeap, Error) << "dmaHeap naming failure for " << name; - return {}; - } - - return allocFd; -} - -} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index a3b12bc1..89504cee 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -15,7 +15,7 @@ libcamera_sources = files([ 'delayed_controls.cpp', 'device_enumerator.cpp', 'device_enumerator_sysfs.cpp', - 'dma_heaps.cpp', + 'dma_buf_allocator.cpp', 'fence.cpp', 'formats.cpp', 'framebuffer.cpp', diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index 37fb310f..4a89e35f 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -12,7 +12,7 @@ #include <libcamera/formats.h> #include "libcamera/internal/device_enumerator.h" -#include "libcamera/internal/dma_heaps.h" +#include "libcamera/internal/dma_buf_allocator.h" #include "../common/pipeline_base.h" #include "../common/rpi_stream.h" @@ -86,7 +86,7 @@ public: RPi::Device<Isp, 4> isp_; /* DMAHEAP allocation helper. */ - DmaHeap dmaHeap_; + DmaBufAllocator dmaHeap_; SharedFD lsTable_; struct Config { diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index c9b6be56..b5eab670 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -65,10 +65,11 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) : debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, DebayerParams::kGain10, 0.5f, 0 }, - dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) + dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap | + DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap) { if (!dmaHeap_.isValid()) { - LOG(SoftwareIsp, Error) << "Failed to create DmaHeap object"; + LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object"; return; }
Users of the DmaHeap class really just want some way to allocate dma-buffers from userspace. This can also be done by using /dev/udmabuf instead of using /dev/dma_heap/*. Rename DmaHeap class to DmaBufAllocator in preparation of adding /dev/udmabuf support. And update the DmaHeap class docs to match including replacing references to "dma-heap type" with "dma-buf provider". This is a pure automated rename on the code ('s/DmaHeap/DmaBufAllocator/') + file renames + doc updates. There are no functional changes. The DmaBufAllocator objects in vc4.cpp and software_isp.cpp are left named dmaHeap_ to keep the changes to those 2 files to a minimum. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- .../libcamera/internal/dma_buf_allocator.h | 38 ++++ include/libcamera/internal/dma_heaps.h | 38 ---- include/libcamera/internal/meson.build | 2 +- .../internal/software_isp/software_isp.h | 4 +- src/libcamera/dma_buf_allocator.cpp | 159 +++++++++++++++++ src/libcamera/dma_heaps.cpp | 165 ------------------ src/libcamera/meson.build | 2 +- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 +- src/libcamera/software_isp/software_isp.cpp | 5 +- 9 files changed, 206 insertions(+), 211 deletions(-) create mode 100644 include/libcamera/internal/dma_buf_allocator.h delete mode 100644 include/libcamera/internal/dma_heaps.h create mode 100644 src/libcamera/dma_buf_allocator.cpp delete mode 100644 src/libcamera/dma_heaps.cpp