[{"id":29682,"web_url":"https://patchwork.libcamera.org/comment/29682/","msgid":"<171710837376.372008.15357109803983653821@ping.linuxembedded.co.uk>","date":"2024-05-30T22:32:53","subject":"Re: [PATCH v2 3/5] libcamera: Rename DmaHeap class to\n\tDmaBufAllocator","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Hans de Goede (2024-05-30 18:15:58)\n> Users of the DmaHeap class really just want some way to allocate\n> dma-buffers from userspace. This can also be done by using /dev/udmabuf\n> instead of using /dev/dma_heap/*.\n> \n> Rename DmaHeap class to DmaBufAllocator in preparation of adding\n> /dev/udmabuf support.\n> \n> And update the DmaHeap class docs to match including replacing references\n> to \"dma-heap type\" with \"dma-buf provider\".\n> \n> This is a pure automated rename on the code ('s/DmaHeap/DmaBufAllocator/')\n> + file renames + doc updates. There are no functional changes.\n\nThat's all fine with me, and the patch overall itself.\n\nReviewed-by: Kieran Bingham \n\n> The DmaBufAllocator objects in vc4.cpp and software_isp.cpp are left named\n> dmaHeap_ to keep the changes to those 2 files to a minimum.\n> \n> \n> Signed-off-by: Hans de Goede \n> ---\n> .../libcamera/internal/dma_buf_allocator.h | 38 ++++\n> include/libcamera/internal/dma_heaps.h | 38 ----\n> include/libcamera/internal/meson.build | 2 +-\n> .../internal/software_isp/software_isp.h | 4 +-\n> src/libcamera/dma_buf_allocator.cpp | 159 +++++++++++++++++\n> src/libcamera/dma_heaps.cpp | 165 ------------------\n> src/libcamera/meson.build | 2 +-\n> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 +-\n\nNaush, The changes here to vc4.cpp are minimal, but even still, Are you\nhappy with the changes here? (And in fact the original class naming\ncomes from RPi too...)\n\n\n\n> src/libcamera/software_isp/software_isp.cpp | 5 +-\n> 9 files changed, 206 insertions(+), 211 deletions(-)\n> create mode 100644 include/libcamera/internal/dma_buf_allocator.h\n> delete mode 100644 include/libcamera/internal/dma_heaps.h\n> create mode 100644 src/libcamera/dma_buf_allocator.cpp\n> delete mode 100644 src/libcamera/dma_heaps.cpp\n> \n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> new file mode 100644\n> index 00000000..a881042e\n> --- /dev/null\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -0,0 +1,38 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi Ltd\n> + *\n> + * Helper class for dma-buf allocations.\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class DmaBufAllocator\n> +{\n> +public:\n> + enum class DmaBufAllocatorFlag {\n> + CmaHeap = 1 << 0,\n> + SystemHeap = 1 << 1,\n> + };\n> +\n> + using DmaBufAllocatorFlags = Flags;\n> +\n> + DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap);\n> + ~DmaBufAllocator();\n> + bool isValid() const { return providerHandle_.isValid(); }\n> + UniqueFD alloc(const char *name, std::size_t size);\n> +\n> +private:\n> + UniqueFD providerHandle_;\n> +};\n> +\n> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/dma_heaps.h b/include/libcamera/internal/dma_heaps.h\n> deleted file mode 100644\n> index f0a8aa5d..00000000\n> --- a/include/libcamera/internal/dma_heaps.h\n> +++ /dev/null\n> @@ -1,38 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2020, Raspberry Pi Ltd\n> - *\n> - * Helper class for dma-heap allocations.\n> - */\n> -\n> -#pragma once\n> -\n> -#include \n> -\n> -#include \n> -#include \n> -\n> -namespace libcamera {\n> -\n> -class DmaHeap\n> -{\n> -public:\n> - enum class DmaHeapFlag {\n> - Cma = 1 << 0,\n> - System = 1 << 1,\n> - };\n> -\n> - using DmaHeapFlags = Flags;\n> -\n> - DmaHeap(DmaHeapFlags flags = DmaHeapFlag::Cma);\n> - ~DmaHeap();\n> - bool isValid() const { return dmaHeapHandle_.isValid(); }\n> - UniqueFD alloc(const char *name, std::size_t size);\n> -\n> -private:\n> - UniqueFD dmaHeapHandle_;\n> -};\n> -\n> -LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaHeap::DmaHeapFlag)\n> -\n> -} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 160fdc37..9713ea1c 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -25,7 +25,7 @@ libcamera_internal_headers = files([\n> 'device_enumerator.h',\n> 'device_enumerator_sysfs.h',\n> 'device_enumerator_udev.h',\n> - 'dma_heaps.h',\n> + 'dma_buf_allocator.h',\n> 'formats.h',\n> 'framebuffer.h',\n> 'ipa_manager.h',\n> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> index 7e9fae6a..c5338c05 100644\n> --- a/include/libcamera/internal/software_isp/software_isp.h\n> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> @@ -27,7 +27,7 @@\n> #include \n> \n> #include \"libcamera/internal/camera_sensor.h\"\n> -#include \"libcamera/internal/dma_heaps.h\"\n> +#include \"libcamera/internal/dma_buf_allocator.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/shared_mem_object.h\"\n> #include \"libcamera/internal/software_isp/debayer_params.h\"\n> @@ -91,7 +91,7 @@ private:\n> Thread ispWorkerThread_;\n> SharedMemObject sharedParams_;\n> DebayerParams debayerParams_;\n> - DmaHeap dmaHeap_;\n> + DmaBufAllocator dmaHeap_;\n> \n> std::unique_ptr ipa_;\n> };\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> new file mode 100644\n> index 00000000..bc0d78ef\n> --- /dev/null\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -0,0 +1,159 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + * Copyright (C) 2020, Raspberry Pi Ltd\n> + *\n> + * Helper class for dma-buf allocations.\n> + */\n> +\n> +#include \"libcamera/internal/dma_buf_allocator.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +/**\n> + * \\file dma_buf_allocator.cpp\n> + * \\brief dma-buf allocator\n> + */\n> +\n> +namespace libcamera {\n> +\n> +#ifndef __DOXYGEN__\n> +struct DmaBufAllocatorInfo {\n> + DmaBufAllocator::DmaBufAllocatorFlag type;\n> + const char *deviceNodeName;\n> +};\n> +#endif\n> +\n> +static constexpr std::array providerInfos = { {\n> + /*\n> + * /dev/dma_heap/linux,cma is the CMA dma-heap. When the cma heap size is\n> + * specified on the kernel command line, this gets renamed to \"reserved\".\n> + */\n\nThat's a good move to be closer to the reference.\n\n> + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, \"/dev/dma_heap/linux,cma\" },\n> + { DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, \"/dev/dma_heap/reserved\" },\n> + { DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap, \"/dev/dma_heap/system\" },\n> +} };\n> +\n> +LOG_DEFINE_CATEGORY(DmaBufAllocator)\n> +\n> +/**\n> + * \\class DmaBufAllocator\n> + * \\brief Helper class for dma-buf allocations\n> + *\n> + * This class wraps a userspace dma-buf provider selected at construction time,\n> + * and exposes functions to allocate dma-buffers from this provider.\n> + *\n> + * Different providers may provide dma-buffers with different properties for\n> + * the underlying memory. Which providers are acceptable is specified through\n> + * the type argument passed to the DmaBufAllocator() constructor.\n> + */\n> +\n> +/**\n> + * \\enum DmaBufAllocator::DmaBufAllocatorFlag\n> + * \\brief Type of the dma-buf provider\n> + * \\var DmaBufAllocator::CmaHeap\n> + * \\brief Allocate from a CMA dma-heap, providing physically-contiguous memory\n> + * \\var DmaBufAllocator::SystemHeap\n> + * \\brief Allocate from the system dma-heap, using the page allocator\n> + */\n> +\n> +/**\n> + * \\typedef DmaBufAllocator::DmaBufAllocatorFlags\n> + * \\brief A bitwise combination of DmaBufAllocator::DmaBufAllocatorFlag values\n> + */\n> +\n> +/**\n> + * \\brief Construct a DmaBufAllocator of a given type\n> + * \\param[in] type The type(s) of the dma-buf providers to allocate from\n> + *\n> + * The dma-buf provider type is selected with the \\a type parameter, which defaults\n> + * to the CMA heap. If no provider of the given type can be accessed, the constructed\n> + * DmaBufAllocator instance is invalid as indicated by the isValid() function.\n> + *\n> + * Multiple types can be selected by combining type flags, in which case the\n> + * constructed DmaBufAllocator will match one of the types. If multiple requested\n> + * types can work on the system, which provider is used is undefined.\n> + */\n> +DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type)\n> +{\n> + for (const auto &info : providerInfos) {\n> + if (!(type & info.type))\n> + continue;\n> +\n> + int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0);\n> + if (ret < 0) {\n> + ret = errno;\n> + LOG(DmaBufAllocator, Debug)\n> + << \"Failed to open \" << info.deviceNodeName << \": \"\n> + << strerror(ret);\n> + continue;\n> + }\n> +\n> + LOG(DmaBufAllocator, Debug) << \"Using \" << info.deviceNodeName;\n> + providerHandle_ = UniqueFD(ret);\n> + break;\n> + }\n> +\n> + if (!providerHandle_.isValid())\n> + LOG(DmaBufAllocator, Error) << \"Could not open any dma-buf provider\";\n> +}\n> +\n> +/**\n> + * \\brief Destroy the DmaBufAllocator instance\n> + */\n> +DmaBufAllocator::~DmaBufAllocator() = default;\n> +\n> +/**\n> + * \\fn DmaBufAllocator::isValid()\n> + * \\brief Check if the DmaBufAllocator instance is valid\n> + * \\return True if the DmaBufAllocator is valid, false otherwise\n> + */\n> +\n> +/**\n> + * \\brief Allocate a dma-buf from the DmaBufAllocator\n> + * \\param [in] name The name to set for the allocated buffer\n> + * \\param [in] size The size of the buffer to allocate\n> + *\n> + * Allocates a dma-buf with read/write access.\n> + *\n> + * If the allocation fails, return an invalid UniqueFD.\n> + *\n> + * \\return The UniqueFD of the allocated buffer\n> + */\n> +UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> +{\n> + int ret;\n> +\n> + if (!name)\n> + return {};\n> +\n> + struct dma_heap_allocation_data alloc = {};\n> +\n> + alloc.len = size;\n> + alloc.fd_flags = O_CLOEXEC | O_RDWR;\n> +\n> + ret = ::ioctl(providerHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n> + if (ret < 0) {\n> + LOG(DmaBufAllocator, Error) << \"dma-heap allocation failure for \" << name;\n> + return {};\n> + }\n> +\n> + UniqueFD allocFd(alloc.fd);\n> + ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n> + if (ret < 0) {\n> + LOG(DmaBufAllocator, Error) << \"dma-heap naming failure for \" << name;\n> + return {};\n> + }\n> +\n> + return allocFd;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp\n> deleted file mode 100644\n> index d4cb880b..00000000\n> --- a/src/libcamera/dma_heaps.cpp\n> +++ /dev/null\n> @@ -1,165 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2020, Raspberry Pi Ltd\n> - *\n> - * Helper class for dma-heap allocations.\n> - */\n> -\n> -#include \"libcamera/internal/dma_heaps.h\"\n> -\n> -#include \n> -#include \n> -#include \n> -#include \n> -\n> -#include \n> -#include \n> -\n> -#include \n> -\n> -/**\n> - * \\file dma_heaps.cpp\n> - * \\brief dma-heap allocator\n> - */\n> -\n> -namespace libcamera {\n> -\n> -/*\n> - * /dev/dma_heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma\n> - * to only have to worry about importing.\n> - *\n> - * Annoyingly, should the cma heap size be specified on the kernel command line\n> - * instead of DT, the heap gets named \"reserved\" instead.\n> - */\n> -\n> -#ifndef __DOXYGEN__\n> -struct DmaHeapInfo {\n> - DmaHeap::DmaHeapFlag type;\n> - const char *deviceNodeName;\n> -};\n> -#endif\n> -\n> -static constexpr std::array heapInfos = { {\n> - { DmaHeap::DmaHeapFlag::Cma, \"/dev/dma_heap/linux,cma\" },\n> - { DmaHeap::DmaHeapFlag::Cma, \"/dev/dma_heap/reserved\" },\n> - { DmaHeap::DmaHeapFlag::System, \"/dev/dma_heap/system\" },\n> -} };\n> -\n> -LOG_DEFINE_CATEGORY(DmaHeap)\n> -\n> -/**\n> - * \\class DmaHeap\n> - * \\brief Helper class for dma-heap allocations\n> - *\n> - * DMA heaps are kernel devices that provide an API to allocate memory from\n> - * different pools called \"heaps\", wrap each allocated piece of memory in a\n> - * dmabuf object, and return the dmabuf file descriptor to userspace. Multiple\n> - * heaps can be provided by the system, with different properties for the\n> - * underlying memory.\n> - *\n> - * This class wraps a DMA heap selected at construction time, and exposes\n> - * functions to manage memory allocation.\n> - */\n> -\n> -/**\n> - * \\enum DmaHeap::DmaHeapFlag\n> - * \\brief Type of the dma-heap\n> - * \\var DmaHeap::Cma\n> - * \\brief Allocate from a CMA dma-heap, providing physically-contiguous memory\n> - * \\var DmaHeap::System\n> - * \\brief Allocate from the system dma-heap, using the page allocator\n> - */\n> -\n> -/**\n> - * \\typedef DmaHeap::DmaHeapFlags\n> - * \\brief A bitwise combination of DmaHeap::DmaHeapFlag values\n> - */\n> -\n> -/**\n> - * \\brief Construct a DmaHeap of a given type\n> - * \\param[in] type The type(s) of the dma-heap(s) to allocate from\n> - *\n> - * The DMA heap type is selected with the \\a type parameter, which defaults to\n> - * the CMA heap. If no heap of the given type can be accessed, the constructed\n> - * DmaHeap instance is invalid as indicated by the isValid() function.\n> - *\n> - * Multiple types can be selected by combining type flags, in which case the\n> - * constructed DmaHeap will match one of the types. If the system provides\n> - * multiple heaps that match the requested types, which heap is used is\n> - * undefined.\n> - */\n> -DmaHeap::DmaHeap(DmaHeapFlags type)\n> -{\n> - for (const auto &info : heapInfos) {\n> - if (!(type & info.type))\n> - continue;\n> -\n> - int ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0);\n> - if (ret < 0) {\n> - ret = errno;\n> - LOG(DmaHeap, Debug)\n> - << \"Failed to open \" << info.deviceNodeName << \": \"\n> - << strerror(ret);\n> - continue;\n> - }\n> -\n> - LOG(DmaHeap, Debug) << \"Using \" << info.deviceNodeName;\n> - dmaHeapHandle_ = UniqueFD(ret);\n> - break;\n> - }\n> -\n> - if (!dmaHeapHandle_.isValid())\n> - LOG(DmaHeap, Error) << \"Could not open any dmaHeap device\";\n> -}\n> -\n> -/**\n> - * \\brief Destroy the DmaHeap instance\n> - */\n> -DmaHeap::~DmaHeap() = default;\n> -\n> -/**\n> - * \\fn DmaHeap::isValid()\n> - * \\brief Check if the DmaHeap instance is valid\n> - * \\return True if the DmaHeap is valid, false otherwise\n> - */\n> -\n> -/**\n> - * \\brief Allocate a dma-buf from the DmaHeap\n> - * \\param [in] name The name to set for the allocated buffer\n> - * \\param [in] size The size of the buffer to allocate\n> - *\n> - * Allocates a dma-buf with read/write access.\n> - *\n> - * If the allocation fails, return an invalid UniqueFD.\n> - *\n> - * \\return The UniqueFD of the allocated buffer\n> - */\n> -UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n> -{\n> - int ret;\n> -\n> - if (!name)\n> - return {};\n> -\n> - struct dma_heap_allocation_data alloc = {};\n> -\n> - alloc.len = size;\n> - alloc.fd_flags = O_CLOEXEC | O_RDWR;\n> -\n> - ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n> - if (ret < 0) {\n> - LOG(DmaHeap, Error) << \"dmaHeap allocation failure for \" << name;\n> - return {};\n> - }\n> -\n> - UniqueFD allocFd(alloc.fd);\n> - ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n> - if (ret < 0) {\n> - LOG(DmaHeap, Error) << \"dmaHeap naming failure for \" << name;\n> - return {};\n> - }\n> -\n> - return allocFd;\n> -}\n> -\n> -} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index a3b12bc1..89504cee 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -15,7 +15,7 @@ libcamera_sources = files([\n> 'delayed_controls.cpp',\n> 'device_enumerator.cpp',\n> 'device_enumerator_sysfs.cpp',\n> - 'dma_heaps.cpp',\n> + 'dma_buf_allocator.cpp',\n> 'fence.cpp',\n> 'formats.cpp',\n> 'framebuffer.cpp',\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 37fb310f..4a89e35f 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -12,7 +12,7 @@\n> #include \n> \n> #include \"libcamera/internal/device_enumerator.h\"\n> -#include \"libcamera/internal/dma_heaps.h\"\n> +#include \"libcamera/internal/dma_buf_allocator.h\"\n> \n> #include \"../common/pipeline_base.h\"\n> #include \"../common/rpi_stream.h\"\n> @@ -86,7 +86,7 @@ public:\n> RPi::Device isp_;\n> \n> /* DMAHEAP allocation helper. */\n> - DmaHeap dmaHeap_;\n> + DmaBufAllocator dmaHeap_;\n> SharedFD lsTable_;\n> \n> struct Config {\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index c9b6be56..b5eab670 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -65,10 +65,11 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)\n> SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)\n> : debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10,\n> DebayerParams::kGain10, 0.5f, 0 },\n> - dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)\n> + dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |\n> + DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap)\n> {\n> if (!dmaHeap_.isValid()) {\n> - LOG(SoftwareIsp, Error) << \"Failed to create DmaHeap object\";\n> + LOG(SoftwareIsp, Error) << \"Failed to create DmaBufAllocator object\";\n> return;\n> }\n> \n> -- \n> 2.45.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CCD0EBDE6B\n\tfor ;\n\tThu, 30 May 2024 22:33:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A22D634B6;\n\tFri, 31 May 2024 00:32:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2AC4B6347E\n\tfor ;\n\tFri, 31 May 2024 00:32:57 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65B2AA06;\n\tFri, 31 May 2024 00:32:52 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UVVtrBH6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1717108372;\n\tbh=Lz9hu2TjAwAGEP/N97YoP3xT60ebneirgqsymesW9mY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=UVVtrBH6lOBoOEFPd5HQmPCPSwMM5zyZNwB0eKWmkUx8NcIRQIhWak2t76rRiUNWE\n\td39IBqDkegz7aoWjwW1JNVmeIMMowI/4Uh76LbNli2Rtw6WPnx4ouQOmBGHsOWphHm\n\tObHi6fCDAWjeigsM2MEsMwiuo3KG0pEVTp+eTwRI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240530171600.259495-4-hdegoede@redhat.com>","References":"<20240530171600.259495-1-hdegoede@redhat.com>\n\t<20240530171600.259495-4-hdegoede@redhat.com>","Subject":"Re: [PATCH v2 3/5] libcamera: Rename DmaHeap class to\n\tDmaBufAllocator","From":"Kieran Bingham ","Cc":"Andrey Konovalov ,\n\tPavel Machek ,\n\tBryan O'Donoghue , \n\tMilan Zamazal , Maxime Ripard , \n\tHans de Goede ","To":"Hans de Goede , libcamera-devel@lists.libcamera.org","Date":"Thu, 30 May 2024 23:32:53 +0100","Message-ID":"<171710837376.372008.15357109803983653821@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29711,"web_url":"https://patchwork.libcamera.org/comment/29711/","msgid":"<99aaa8d6-815a-41af-9ce0-4a9befa4873f@linaro.org>","date":"2024-05-31T14:51:08","subject":"Re: [PATCH v2 3/5] libcamera: Rename DmaHeap class to\n\tDmaBufAllocator","submitter":{"id":175,"url":"https://patchwork.libcamera.org/api/people/175/","name":"Bryan O'Donoghue","email":"bryan.odonoghue@linaro.org"},"content":"On 30/05/2024 18:15, Hans de Goede wrote:\n> Users of the DmaHeap class really just want some way to allocate\n> dma-buffers from userspace. This can also be done by using /dev/udmabuf\n> instead of using /dev/dma_heap/*.\n\nThis one fails to apply for me with @Milan's colour fixes applied.\n\nIts minor enough but one of you will need to rebase.\n\nhttps://lists.libcamera.org/pipermail/libcamera-devel/2024-May/042105.html\n\n---\nbod","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EF1E2BDE6B\n\tfor ;\n\tFri, 31 May 2024 14:51:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3B23E634B7;\n\tFri, 31 May 2024 16:51:11 +0200 (CEST)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD1CF634AF\n\tfor ;\n\tFri, 31 May 2024 16:51:09 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id\n\t38308e7fff4ca-2e95a75a90eso22888601fa.2\n\tfor ;\n\tFri, 31 May 2024 07:51:09 -0700 (PDT)","from [192.168.0.31] ([176.61.106.227])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4212b85c628sm28121525e9.25.2024.05.31.07.51.08\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tFri, 31 May 2024 07:51:08 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"wamNBrx7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1717167069; x=1717771869;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=+Aj3kytAZS+mooj3RA1WbD/f5d9BCw5r9RUvLrdV1mY=;\n\tb=wamNBrx7Gc/MhUhqlMMab4g8Vzy9Zp3vautx/D9l7G/VkPBvhKugD+FTBkLMqdK95w\n\ts4yI013vz054dLKodd1wqthUhI9oEhy1xhfTnLTTv3SesWag4EdZrnYzBOCvMjERr5rf\n\tXLjscer4A/aosV590WW8pNKndnBh7lf4GNJPA5JmpJoD2VR/zGex4ik1YtnnPfVfLGpV\n\tPPkj9m9ChrBYEo+gskak+D8/u6xjyjZatA9rR+uVy1hBPy0b3nswIImujMXwbHegHCEi\n\t1A1xZfQsB0UT/hXQBKkIkvReoqUzLfPXMDyN6pXY+ElBeGAQGPwF9IniBDqrYyYCswJ3\n\truTg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1717167069; x=1717771869;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=+Aj3kytAZS+mooj3RA1WbD/f5d9BCw5r9RUvLrdV1mY=;\n\tb=VLGqpRmYwDGijBZd8qg6W+BxXrOQCe0UkeGqtBSk5lV3zi4deFcrtOq6/aZ+Y6yxjM\n\tzDV03OJpQwqcsw+HJjYGYwMlzsIXxvHwfNHx300SDjrfmHFwHbltnZGBKa3GTxfUwUBJ\n\tAmQwF+aeA8/EcbTDwf4HYV5UlZmj+KljgoJH9p0gtgRvWdvSiWd6MuIwBXzHFZUBF/MV\n\tw2nxmJtu8FnwbcJYUoWKY+TN30GXMgzognED788NFoK/9fiVDM3zpWVLthEKDIYlKPJQ\n\tFUoUv9kk1/IqMKCKZXPdhqxql6+6VBoStnI2L0T0YmQhzFGhhPBzu2JuvIom5HbCjLYh\n\tsJ3g==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVmRZE7X/NW5Jt6Jb46F/ib0v6zGeIGrzUN5BNSjRmxTbQ2u5tI6F+LOrbVY65TuS8J5YB0FYb7w20ry2byipSBV1Ogq+AS7t9hpF0srMtRQkEWhA==","X-Gm-Message-State":"AOJu0YxeACvQM0Pd1NtM2jclL/c7SptGGX83y96xbdbrr9RJixbQARF7\n\teQpZUNzS1tM9PWQFDGwf2ssdvZxFPIFL5xTnOVC45Fc2FZMIjrMivoWIOcdL1SY=","X-Google-Smtp-Source":"AGHT+IGzmtmdqWcsGlnUimmz1uCyt/He+j3HLFyOIqX4xZ6b0lApLb1svDo1eO9aIMOmhSI+dzVumg==","X-Received":"by 2002:a05:651c:1402:b0:2ea:7def:46d0 with SMTP id\n\t38308e7fff4ca-2ea950c896emr12257871fa.9.1717167068833; \n\tFri, 31 May 2024 07:51:08 -0700 (PDT)","Message-ID":"<99aaa8d6-815a-41af-9ce0-4a9befa4873f@linaro.org>","Date":"Fri, 31 May 2024 15:51:08 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 3/5] libcamera: Rename DmaHeap class to\n\tDmaBufAllocator","To":"Hans de Goede , libcamera-devel@lists.libcamera.org","Cc":"Andrey Konovalov ,\n\tPavel Machek , Milan Zamazal ,\n\tMaxime Ripard ","References":"<20240530171600.259495-1-hdegoede@redhat.com>\n\t<20240530171600.259495-4-hdegoede@redhat.com>","Content-Language":"en-US","From":"Bryan O'Donoghue ","In-Reply-To":"<20240530171600.259495-4-hdegoede@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29730,"web_url":"https://patchwork.libcamera.org/comment/29730/","msgid":"<20240601232422.GE6683@pendragon.ideasonboard.com>","date":"2024-06-01T23:24:22","subject":"Re: [PATCH v2 3/5] libcamera: Rename DmaHeap class to\n\tDmaBufAllocator","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hans,\n\nThank you for the patch.\n\nOn Thu, May 30, 2024 at 07:15:58PM +0200, Hans de Goede wrote:\n> Users of the DmaHeap class really just want some way to allocate\n> dma-buffers from userspace. This can also be done by using /dev/udmabuf\n> instead of using /dev/dma_heap/*.\n> \n> Rename DmaHeap class to DmaBufAllocator in preparation of adding\n> /dev/udmabuf support.\n> \n> And update the DmaHeap class docs to match including replacing references\n> to \"dma-heap type\" with \"dma-buf provider\".\n> \n> This is a pure automated rename on the code ('s/DmaHeap/DmaBufAllocator/')\n> + file renames + doc updates. There are no functional changes.\n> \n> The DmaBufAllocator objects in vc4.cpp and software_isp.cpp are left named\n> dmaHeap_ to keep the changes to those 2 files to a minimum.\n> \n> Signed-off-by: Hans de Goede \n> ---\n> .../libcamera/internal/dma_buf_allocator.h | 38 ++++\n> include/libcamera/internal/dma_heaps.h | 38 ----\n> include/libcamera/internal/meson.build | 2 +-\n> .../internal/software_isp/software_isp.h | 4 +-\n> src/libcamera/dma_buf_allocator.cpp | 159 +++++++++++++++++\n> src/libcamera/dma_heaps.cpp | 165 ------------------\n> src/libcamera/meson.build | 2 +-\n> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 4 +-\n> src/libcamera/software_isp/software_isp.cpp | 5 +-\n> 9 files changed, 206 insertions(+), 211 deletions(-)\n> create mode 100644 include/libcamera/internal/dma_buf_allocator.h\n> delete mode 100644 include/libcamera/internal/dma_heaps.h\n> create mode 100644 src/libcamera/dma_buf_allocator.cpp\n> delete mode 100644 src/libcamera/dma_heaps.cpp\n> \n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> new file mode 100644\n> index 00000000..a881042e\n> --- /dev/null\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -0,0 +1,38 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi Ltd\n> + *\n> + * Helper class for dma-buf allocations.\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class DmaBufAllocator\n> +{\n> +public:\n> +\tenum class DmaBufAllocatorFlag {\n> +\t\tCmaHeap = 1 << 0,\n> +\t\tSystemHeap = 1 << 1,\n> +\t};\n> +\n> +\tusing DmaBufAllocatorFlags = Flags;\n> +\n> +\tDmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap);\n> +\t~DmaBufAllocator();\n> +\tbool isValid() const { return providerHandle_.isValid(); }\n> +\tUniqueFD alloc(const char *name, std::size_t size);\n> +\n> +private:\n> +\tUniqueFD providerHandle_;\n> +};\n> +\n> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaBufAllocator::DmaBufAllocatorFlag)\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/dma_heaps.h b/include/libcamera/internal/dma_heaps.h\n> deleted file mode 100644\n> index f0a8aa5d..00000000\n> --- a/include/libcamera/internal/dma_heaps.h\n> +++ /dev/null\n> @@ -1,38 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2020, Raspberry Pi Ltd\n> - *\n> - * Helper class for dma-heap allocations.\n> - */\n> -\n> -#pragma once\n> -\n> -#include \n> -\n> -#include \n> -#include \n> -\n> -namespace libcamera {\n> -\n> -class DmaHeap\n> -{\n> -public:\n> -\tenum class DmaHeapFlag {\n> -\t\tCma = 1 << 0,\n> -\t\tSystem = 1 << 1,\n> -\t};\n> -\n> -\tusing DmaHeapFlags = Flags;\n> -\n> -\tDmaHeap(DmaHeapFlags flags = DmaHeapFlag::Cma);\n> -\t~DmaHeap();\n> -\tbool isValid() const { return dmaHeapHandle_.isValid(); }\n> -\tUniqueFD alloc(const char *name, std::size_t size);\n> -\n> -private:\n> -\tUniqueFD dmaHeapHandle_;\n> -};\n> -\n> -LIBCAMERA_FLAGS_ENABLE_OPERATORS(DmaHeap::DmaHeapFlag)\n> -\n> -} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 160fdc37..9713ea1c 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -25,7 +25,7 @@ libcamera_internal_headers = files([\n> 'device_enumerator.h',\n> 'device_enumerator_sysfs.h',\n> 'device_enumerator_udev.h',\n> - 'dma_heaps.h',\n> + 'dma_buf_allocator.h',\n> 'formats.h',\n> 'framebuffer.h',\n> 'ipa_manager.h',\n> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> index 7e9fae6a..c5338c05 100644\n> --- a/include/libcamera/internal/software_isp/software_isp.h\n> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> @@ -27,7 +27,7 @@\n> #include \n> \n> #include \"libcamera/internal/camera_sensor.h\"\n> -#include \"libcamera/internal/dma_heaps.h\"\n> +#include \"libcamera/internal/dma_buf_allocator.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/shared_mem_object.h\"\n> #include \"libcamera/internal/software_isp/debayer_params.h\"\n> @@ -91,7 +91,7 @@ private:\n> \tThread ispWorkerThread_;\n> \tSharedMemObject sharedParams_;\n> \tDebayerParams debayerParams_;\n> -\tDmaHeap dmaHeap_;\n> +\tDmaBufAllocator dmaHeap_;\n> \n> \tstd::unique_ptr ipa_;\n> };\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> new file mode 100644\n> index 00000000..bc0d78ef\n> --- /dev/null\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -0,0 +1,159 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + * Copyright (C) 2020, Raspberry Pi Ltd\n> + *\n> + * Helper class for dma-buf allocations.\n> + */\n> +\n> +#include \"libcamera/internal/dma_buf_allocator.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +/**\n> + * \\file dma_buf_allocator.cpp\n> + * \\brief dma-buf allocator\n> + */\n> +\n> +namespace libcamera {\n> +\n> +#ifndef __DOXYGEN__\n> +struct DmaBufAllocatorInfo {\n> +\tDmaBufAllocator::DmaBufAllocatorFlag type;\n> +\tconst char *deviceNodeName;\n> +};\n> +#endif\n> +\n> +static constexpr std::array providerInfos = { {\n> +\t/*\n> +\t * /dev/dma_heap/linux,cma is the CMA dma-heap. When the cma heap size is\n> +\t * specified on the kernel command line, this gets renamed to \"reserved\".\n> +\t */\n> +\t{ DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, \"/dev/dma_heap/linux,cma\" },\n> +\t{ DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap, \"/dev/dma_heap/reserved\" },\n> +\t{ DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap, \"/dev/dma_heap/system\" },\n> +} };\n> +\n> +LOG_DEFINE_CATEGORY(DmaBufAllocator)\n> +\n> +/**\n> + * \\class DmaBufAllocator\n> + * \\brief Helper class for dma-buf allocations\n> + *\n> + * This class wraps a userspace dma-buf provider selected at construction time,\n> + * and exposes functions to allocate dma-buffers from this provider.\n> + *\n> + * Different providers may provide dma-buffers with different properties for\n> + * the underlying memory. Which providers are acceptable is specified through\n\ns/ / /\n\n> + * the type argument passed to the DmaBufAllocator() constructor.\n> + */\n> +\n> +/**\n> + * \\enum DmaBufAllocator::DmaBufAllocatorFlag\n> + * \\brief Type of the dma-buf provider\n> + * \\var DmaBufAllocator::CmaHeap\n> + * \\brief Allocate from a CMA dma-heap, providing physically-contiguous memory\n> + * \\var DmaBufAllocator::SystemHeap\n> + * \\brief Allocate from the system dma-heap, using the page allocator\n> + */\n> +\n> +/**\n> + * \\typedef DmaBufAllocator::DmaBufAllocatorFlags\n> + * \\brief A bitwise combination of DmaBufAllocator::DmaBufAllocatorFlag values\n> + */\n> +\n> +/**\n> + * \\brief Construct a DmaBufAllocator of a given type\n> + * \\param[in] type The type(s) of the dma-buf providers to allocate from\n> + *\n> + * The dma-buf provider type is selected with the \\a type parameter, which defaults\n> + * to the CMA heap. If no provider of the given type can be accessed, the constructed\n> + * DmaBufAllocator instance is invalid as indicated by the isValid() function.\n> + *\n> + * Multiple types can be selected by combining type flags, in which case the\n> + * constructed DmaBufAllocator will match one of the types. If multiple requested\n> + * types can work on the system, which provider is used is undefined.\n\nPlease wrap the text at 80 columns.\n\n> + */\n> +DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type)\n> +{\n> +\tfor (const auto &info : providerInfos) {\n> +\t\tif (!(type & info.type))\n> +\t\t\tcontinue;\n> +\n> +\t\tint ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0);\n> +\t\tif (ret < 0) {\n> +\t\t\tret = errno;\n> +\t\t\tLOG(DmaBufAllocator, Debug)\n> +\t\t\t\t<< \"Failed to open \" << info.deviceNodeName << \": \"\n> +\t\t\t\t<< strerror(ret);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tLOG(DmaBufAllocator, Debug) << \"Using \" << info.deviceNodeName;\n> +\t\tproviderHandle_ = UniqueFD(ret);\n> +\t\tbreak;\n> +\t}\n> +\n> +\tif (!providerHandle_.isValid())\n> +\t\tLOG(DmaBufAllocator, Error) << \"Could not open any dma-buf provider\";\n> +}\n> +\n> +/**\n> + * \\brief Destroy the DmaBufAllocator instance\n> + */\n> +DmaBufAllocator::~DmaBufAllocator() = default;\n> +\n> +/**\n> + * \\fn DmaBufAllocator::isValid()\n> + * \\brief Check if the DmaBufAllocator instance is valid\n> + * \\return True if the DmaBufAllocator is valid, false otherwise\n> + */\n> +\n> +/**\n> + * \\brief Allocate a dma-buf from the DmaBufAllocator\n> + * \\param [in] name The name to set for the allocated buffer\n> + * \\param [in] size The size of the buffer to allocate\n> + *\n> + * Allocates a dma-buf with read/write access.\n> + *\n> + * If the allocation fails, return an invalid UniqueFD.\n> + *\n> + * \\return The UniqueFD of the allocated buffer\n> + */\n> +UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)\n> +{\n> +\tint ret;\n> +\n> +\tif (!name)\n> +\t\treturn {};\n> +\n> +\tstruct dma_heap_allocation_data alloc = {};\n> +\n> +\talloc.len = size;\n> +\talloc.fd_flags = O_CLOEXEC | O_RDWR;\n> +\n> +\tret = ::ioctl(providerHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n> +\tif (ret < 0) {\n> +\t\tLOG(DmaBufAllocator, Error) << \"dma-heap allocation failure for \" << name;\n\n\t\tLOG(DmaBufAllocator, Error)\n\t\t\t<< \"dma-heap allocation failure for \" << name;\n\n> +\t\treturn {};\n> +\t}\n> +\n> +\tUniqueFD allocFd(alloc.fd);\n> +\tret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n> +\tif (ret < 0) {\n> +\t\tLOG(DmaBufAllocator, Error) << \"dma-heap naming failure for \" << name;\n\nSame here.\n\nReviewed-by: Laurent Pinchart \n\n> +\t\treturn {};\n> +\t}\n> +\n> +\treturn allocFd;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp\n> deleted file mode 100644\n> index d4cb880b..00000000\n> --- a/src/libcamera/dma_heaps.cpp\n> +++ /dev/null\n> @@ -1,165 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2020, Raspberry Pi Ltd\n> - *\n> - * Helper class for dma-heap allocations.\n> - */\n> -\n> -#include \"libcamera/internal/dma_heaps.h\"\n> -\n> -#include \n> -#include \n> -#include \n> -#include \n> -\n> -#include \n> -#include \n> -\n> -#include \n> -\n> -/**\n> - * \\file dma_heaps.cpp\n> - * \\brief dma-heap allocator\n> - */\n> -\n> -namespace libcamera {\n> -\n> -/*\n> - * /dev/dma_heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma\n> - * to only have to worry about importing.\n> - *\n> - * Annoyingly, should the cma heap size be specified on the kernel command line\n> - * instead of DT, the heap gets named \"reserved\" instead.\n> - */\n> -\n> -#ifndef __DOXYGEN__\n> -struct DmaHeapInfo {\n> -\tDmaHeap::DmaHeapFlag type;\n> -\tconst char *deviceNodeName;\n> -};\n> -#endif\n> -\n> -static constexpr std::array heapInfos = { {\n> -\t{ DmaHeap::DmaHeapFlag::Cma, \"/dev/dma_heap/linux,cma\" },\n> -\t{ DmaHeap::DmaHeapFlag::Cma, \"/dev/dma_heap/reserved\" },\n> -\t{ DmaHeap::DmaHeapFlag::System, \"/dev/dma_heap/system\" },\n> -} };\n> -\n> -LOG_DEFINE_CATEGORY(DmaHeap)\n> -\n> -/**\n> - * \\class DmaHeap\n> - * \\brief Helper class for dma-heap allocations\n> - *\n> - * DMA heaps are kernel devices that provide an API to allocate memory from\n> - * different pools called \"heaps\", wrap each allocated piece of memory in a\n> - * dmabuf object, and return the dmabuf file descriptor to userspace. Multiple\n> - * heaps can be provided by the system, with different properties for the\n> - * underlying memory.\n> - *\n> - * This class wraps a DMA heap selected at construction time, and exposes\n> - * functions to manage memory allocation.\n> - */\n> -\n> -/**\n> - * \\enum DmaHeap::DmaHeapFlag\n> - * \\brief Type of the dma-heap\n> - * \\var DmaHeap::Cma\n> - * \\brief Allocate from a CMA dma-heap, providing physically-contiguous memory\n> - * \\var DmaHeap::System\n> - * \\brief Allocate from the system dma-heap, using the page allocator\n> - */\n> -\n> -/**\n> - * \\typedef DmaHeap::DmaHeapFlags\n> - * \\brief A bitwise combination of DmaHeap::DmaHeapFlag values\n> - */\n> -\n> -/**\n> - * \\brief Construct a DmaHeap of a given type\n> - * \\param[in] type The type(s) of the dma-heap(s) to allocate from\n> - *\n> - * The DMA heap type is selected with the \\a type parameter, which defaults to\n> - * the CMA heap. If no heap of the given type can be accessed, the constructed\n> - * DmaHeap instance is invalid as indicated by the isValid() function.\n> - *\n> - * Multiple types can be selected by combining type flags, in which case the\n> - * constructed DmaHeap will match one of the types. If the system provides\n> - * multiple heaps that match the requested types, which heap is used is\n> - * undefined.\n> - */\n> -DmaHeap::DmaHeap(DmaHeapFlags type)\n> -{\n> -\tfor (const auto &info : heapInfos) {\n> -\t\tif (!(type & info.type))\n> -\t\t\tcontinue;\n> -\n> -\t\tint ret = ::open(info.deviceNodeName, O_RDWR | O_CLOEXEC, 0);\n> -\t\tif (ret < 0) {\n> -\t\t\tret = errno;\n> -\t\t\tLOG(DmaHeap, Debug)\n> -\t\t\t\t<< \"Failed to open \" << info.deviceNodeName << \": \"\n> -\t\t\t\t<< strerror(ret);\n> -\t\t\tcontinue;\n> -\t\t}\n> -\n> -\t\tLOG(DmaHeap, Debug) << \"Using \" << info.deviceNodeName;\n> -\t\tdmaHeapHandle_ = UniqueFD(ret);\n> -\t\tbreak;\n> -\t}\n> -\n> -\tif (!dmaHeapHandle_.isValid())\n> -\t\tLOG(DmaHeap, Error) << \"Could not open any dmaHeap device\";\n> -}\n> -\n> -/**\n> - * \\brief Destroy the DmaHeap instance\n> - */\n> -DmaHeap::~DmaHeap() = default;\n> -\n> -/**\n> - * \\fn DmaHeap::isValid()\n> - * \\brief Check if the DmaHeap instance is valid\n> - * \\return True if the DmaHeap is valid, false otherwise\n> - */\n> -\n> -/**\n> - * \\brief Allocate a dma-buf from the DmaHeap\n> - * \\param [in] name The name to set for the allocated buffer\n> - * \\param [in] size The size of the buffer to allocate\n> - *\n> - * Allocates a dma-buf with read/write access.\n> - *\n> - * If the allocation fails, return an invalid UniqueFD.\n> - *\n> - * \\return The UniqueFD of the allocated buffer\n> - */\n> -UniqueFD DmaHeap::alloc(const char *name, std::size_t size)\n> -{\n> -\tint ret;\n> -\n> -\tif (!name)\n> -\t\treturn {};\n> -\n> -\tstruct dma_heap_allocation_data alloc = {};\n> -\n> -\talloc.len = size;\n> -\talloc.fd_flags = O_CLOEXEC | O_RDWR;\n> -\n> -\tret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);\n> -\tif (ret < 0) {\n> -\t\tLOG(DmaHeap, Error) << \"dmaHeap allocation failure for \" << name;\n> -\t\treturn {};\n> -\t}\n> -\n> -\tUniqueFD allocFd(alloc.fd);\n> -\tret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);\n> -\tif (ret < 0) {\n> -\t\tLOG(DmaHeap, Error) << \"dmaHeap naming failure for \" << name;\n> -\t\treturn {};\n> -\t}\n> -\n> -\treturn allocFd;\n> -}\n> -\n> -} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index a3b12bc1..89504cee 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -15,7 +15,7 @@ libcamera_sources = files([\n> 'delayed_controls.cpp',\n> 'device_enumerator.cpp',\n> 'device_enumerator_sysfs.cpp',\n> - 'dma_heaps.cpp',\n> + 'dma_buf_allocator.cpp',\n> 'fence.cpp',\n> 'formats.cpp',\n> 'framebuffer.cpp',\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 37fb310f..4a89e35f 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -12,7 +12,7 @@\n> #include \n> \n> #include \"libcamera/internal/device_enumerator.h\"\n> -#include \"libcamera/internal/dma_heaps.h\"\n> +#include \"libcamera/internal/dma_buf_allocator.h\"\n> \n> #include \"../common/pipeline_base.h\"\n> #include \"../common/rpi_stream.h\"\n> @@ -86,7 +86,7 @@ public:\n> \tRPi::Device isp_;\n> \n> \t/* DMAHEAP allocation helper. */\n> -\tDmaHeap dmaHeap_;\n> +\tDmaBufAllocator dmaHeap_;\n> \tSharedFD lsTable_;\n> \n> \tstruct Config {\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index c9b6be56..b5eab670 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -65,10 +65,11 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)\n> SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)\n> \t: debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10,\n> \t\t\t DebayerParams::kGain10, 0.5f, 0 },\n> -\t dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)\n> +\t dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |\n> +\t\t DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap)\n> {\n> \tif (!dmaHeap_.isValid()) {\n> -\t\tLOG(SoftwareIsp, Error) << \"Failed to create DmaHeap object\";\n> +\t\tLOG(SoftwareIsp, Error) << \"Failed to create DmaBufAllocator object\";\n> \t\treturn;\n> \t}\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E76A5BDE6B\n\tfor ;\n\tSat, 1 Jun 2024 23:24:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1CF8D634CB;\n\tSun, 2 Jun 2024 01:24:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8295F634C5\n\tfor ;\n\tSun, 2 Jun 2024 01:24:36 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4F5071BA;\n\tSun, 2 Jun 2024 01:24:30 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"H+xOF6Kn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1717284270;\n\tbh=rMoj+7He9TT8yz67ybnwQwRgEwQlZZgLDqv3tQm1FHk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H+xOF6Knt0kRja5hmibxlYN+QrCxrpwRRYvawGR4pFOKoQuTxDiMMx/wo75CXU8KK\n\tE1mWdy7J5rgDOPWV5+EPXoAuhLEcSCACemTyQq3pP48eUnyx89NwpWdHxCM7XTSRSS\n\t3yGIm3AvufMBprrLl5gNfB7hLCI9OTut2d6K5AOA=","Date":"Sun, 2 Jun 2024 02:24:22 +0300","From":"Laurent Pinchart ","To":"Hans de Goede ","Cc":"libcamera-devel@lists.libcamera.org,\n\tAndrey Konovalov ,\n\tPavel Machek ,\n\tBryan O'Donoghue , \n\tMilan Zamazal , Maxime Ripard ","Subject":"Re: [PATCH v2 3/5] libcamera: Rename DmaHeap class to\n\tDmaBufAllocator","Message-ID":"<20240601232422.GE6683@pendragon.ideasonboard.com>","References":"<20240530171600.259495-1-hdegoede@redhat.com>\n\t<20240530171600.259495-4-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240530171600.259495-4-hdegoede@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]