From patchwork Mon Jun 3 11:12:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 20190 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 535C9BD87C for ; Mon, 3 Jun 2024 11:13:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5D71D634DA; Mon, 3 Jun 2024 13:13:14 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="Cv+6fk8N"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F1AB4634D4 for ; Mon, 3 Jun 2024 13:13:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1717413188; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7V+BzSGggSPMDiPg8WVdHhZsPuiD3di1IdKyySBk/CM=; b=Cv+6fk8NEJC/shTrbzGFznsw0Gb3NLhk/Z6f576mWKLO0ekEwqRDUf+GECbsSVcaSFcZ19 49j8ooHG6TV+87bN/Ci5NbITnfXYBtq9RoHL/glUJRB1qqbAotWv3C+azinx7oMuKaNyZm Z67Yk1K/3iXJkL3Pg7RdsQWCku+gAco= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-428-6RJuC_jVMyaRpqvjipnvIQ-1; Mon, 03 Jun 2024 07:13:03 -0400 X-MC-Unique: 6RJuC_jVMyaRpqvjipnvIQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A85D33806703; Mon, 3 Jun 2024 11:13:02 +0000 (UTC) Received: from x1.localdomain.com (unknown [10.39.193.39]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F1A72870; Mon, 3 Jun 2024 11:13:01 +0000 (UTC) From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Maxime Ripard , Milan Zamazal , Hans de Goede , Kieran Bingham , Bryan O'Donoghue , Laurent Pinchart Subject: [PATCH v3 1/3] libcamera: Rename DmaHeap class to DmaBufAllocator Date: Mon, 3 Jun 2024 13:12:57 +0200 Message-ID: <20240603111259.54321-2-hdegoede@redhat.com> In-Reply-To: <20240603111259.54321-1-hdegoede@redhat.com> References: <20240603111259.54321-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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. Reviewed-by: Kieran Bingham Tested-by: Bryan O'Donoghue # Lenovo-x13s Reviewed-by: Laurent Pinchart Signed-off-by: Hans de Goede --- Changes in v3: - Wrap comments and error message logging at 80 chars Changes in v2: - New patch in v2 of this series --- .../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 | 162 +++++++++++++++++ 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, 209 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 + +#include +#include + +namespace libcamera { + +class DmaBufAllocator +{ +public: + enum class DmaBufAllocatorFlag { + CmaHeap = 1 << 0, + SystemHeap = 1 << 1, + }; + + using DmaBufAllocatorFlags = Flags; + + 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 - -#include -#include - -namespace libcamera { - -class DmaHeap -{ -public: - enum class DmaHeapFlag { - Cma = 1 << 0, - System = 1 << 1, - }; - - using DmaHeapFlags = Flags; - - 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 #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 sharedParams_; DebayerParams debayerParams_; - DmaHeap dmaHeap_; + DmaBufAllocator dmaHeap_; std::unique_ptr ipa_; }; diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp new file mode 100644 index 00000000..af512512 --- /dev/null +++ b/src/libcamera/dma_buf_allocator.cpp @@ -0,0 +1,162 @@ +/* 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 +#include +#include +#include + +#include +#include + +#include + +/** + * \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 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 -#include -#include -#include - -#include -#include - -#include - -/** - * \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 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 #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_; /* 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 0efe1d4e..034727c5 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -66,7 +66,8 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) * handler */ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) - : dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) + : dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap | + DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap) { /* * debayerParams_ must be initialized because the initial value is used for @@ -86,7 +87,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) } if (!dmaHeap_.isValid()) { - LOG(SoftwareIsp, Error) << "Failed to create DmaHeap object"; + LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object"; return; }