[libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move dma_heaps.[h, cpp] to common directories
Hans de Goede
hdegoede at redhat.com
Tue Feb 6 21:33:10 CET 2024
Hi,
On 1/15/24 10:08, Naushir Patuck wrote:
> Hi all,
>
> On Sat, 13 Jan 2024 at 22:34, Kieran Bingham via libcamera-devel
> <libcamera-devel at lists.libcamera.org> wrote:
>>
>> Quoting Hans de Goede via libcamera-devel (2024-01-13 14:22:02)
>>> From: Andrey Konovalov <andrey.konovalov at linaro.org>
>>>
>>> DmaHeap class is useful outside the RPi pipeline handler too.
>>>
>>> Move dma_heaps.h and dma_heaps.cpp to common directories. Update
>>> the build files and RPi vc4 pipeline handler accordingly.
>>
>> This looks fine to me, but I'd like to see an Acked-by: or Reviewed-by:
>> from Raspberry Pi to move this.
>>
>> Naush, David, - any objections to promoting the DmaHeap code to core
>> code?
>
> No objections from me.
>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
>
>>
>> The next patch also extends it with a new feature, but I don't believe
>> it impacts the RPi code base specifically.
>>
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Kieran, Naush, I'm adding your Reviewed-by to v3 of this series
to indicate that you are ok with the move, but note that Andrey
has added documentation to the DmaHeap code for v3. So you
may want to review the new documentation parts when I post v3.
Regards,
Hans
>>
>>>
>>> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org> # sc8280xp Lenovo x13s
>>> Tested-by: Pavel Machek <pavel at ucw.cz>
>>> ---
>>> .../libcamera/internal}/dma_heaps.h | 4 ----
>>> include/libcamera/internal/meson.build | 1 +
>>> .../{pipeline/rpi/vc4 => }/dma_heaps.cpp | 18 +++++++-----------
>>> src/libcamera/meson.build | 1 +
>>> src/libcamera/pipeline/rpi/vc4/meson.build | 1 -
>>> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 5 ++---
>>> 6 files changed, 11 insertions(+), 19 deletions(-)
>>> rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (92%)
>>> rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (83%)
>>>
>>> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h b/include/libcamera/internal/dma_heaps.h
>>> similarity index 92%
>>> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.h
>>> rename to include/libcamera/internal/dma_heaps.h
>>> index 0a4a8d86..cff8f140 100644
>>> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.h
>>> +++ b/include/libcamera/internal/dma_heaps.h
>>> @@ -13,8 +13,6 @@
>>>
>>> namespace libcamera {
>>>
>>> -namespace RPi {
>>> -
>>> class DmaHeap
>>> {
>>> public:
>>> @@ -27,6 +25,4 @@ private:
>>> UniqueFD dmaHeapHandle_;
>>> };
>>>
>>> -} /* namespace RPi */
>>> -
>>> } /* namespace libcamera */
>>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
>>> index 7f1f3440..33eb0fb3 100644
>>> --- a/include/libcamera/internal/meson.build
>>> +++ b/include/libcamera/internal/meson.build
>>> @@ -25,6 +25,7 @@ libcamera_internal_headers = files([
>>> 'device_enumerator.h',
>>> 'device_enumerator_sysfs.h',
>>> 'device_enumerator_udev.h',
>>> + 'dma_heaps.h',
>>> 'formats.h',
>>> 'framebuffer.h',
>>> 'ipa_manager.h',
>>> diff --git a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp b/src/libcamera/dma_heaps.cpp
>>> similarity index 83%
>>> rename from src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
>>> rename to src/libcamera/dma_heaps.cpp
>>> index 317b1fc1..7444d9c2 100644
>>> --- a/src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
>>> +++ b/src/libcamera/dma_heaps.cpp
>>> @@ -5,8 +5,6 @@
>>> * dma_heaps.h - Helper class for dma-heap allocations.
>>> */
>>>
>>> -#include "dma_heaps.h"
>>> -
>>> #include <array>
>>> #include <fcntl.h>
>>> #include <linux/dma-buf.h>
>>> @@ -16,6 +14,8 @@
>>>
>>> #include <libcamera/base/log.h>
>>>
>>> +#include "libcamera/internal/dma_heaps.h"
>>> +
>>> /*
>>> * /dev/dma-heap/linux,cma is the dma-heap allocator, which allows dmaheap-cma
>>> * to only have to worry about importing.
>>> @@ -30,9 +30,7 @@ static constexpr std::array<const char *, 2> heapNames = {
>>>
>>> namespace libcamera {
>>>
>>> -LOG_DECLARE_CATEGORY(RPI)
>>> -
>>> -namespace RPi {
>>> +LOG_DEFINE_CATEGORY(DmaHeap)
>>>
>>> DmaHeap::DmaHeap()
>>> {
>>> @@ -40,7 +38,7 @@ DmaHeap::DmaHeap()
>>> int ret = ::open(name, O_RDWR | O_CLOEXEC, 0);
>>> if (ret < 0) {
>>> ret = errno;
>>> - LOG(RPI, Debug) << "Failed to open " << name << ": "
>>> + LOG(DmaHeap, Debug) << "Failed to open " << name << ": "
>>> << strerror(ret);
>>> continue;
>>> }
>>> @@ -50,7 +48,7 @@ DmaHeap::DmaHeap()
>>> }
>>>
>>> if (!dmaHeapHandle_.isValid())
>>> - LOG(RPI, Error) << "Could not open any dmaHeap device";
>>> + LOG(DmaHeap, Error) << "Could not open any dmaHeap device";
>>> }
>>>
>>> DmaHeap::~DmaHeap() = default;
>>> @@ -69,7 +67,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)
>>>
>>> ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc);
>>> if (ret < 0) {
>>> - LOG(RPI, Error) << "dmaHeap allocation failure for "
>>> + LOG(DmaHeap, Error) << "dmaHeap allocation failure for "
>>> << name;
>>> return {};
>>> }
>>> @@ -77,7 +75,7 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)
>>> UniqueFD allocFd(alloc.fd);
>>> ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name);
>>> if (ret < 0) {
>>> - LOG(RPI, Error) << "dmaHeap naming failure for "
>>> + LOG(DmaHeap, Error) << "dmaHeap naming failure for "
>>> << name;
>>> return {};
>>> }
>>> @@ -85,6 +83,4 @@ UniqueFD DmaHeap::alloc(const char *name, std::size_t size)
>>> return allocFd;
>>> }
>>>
>>> -} /* namespace RPi */
>>> -
>>> } /* namespace libcamera */
>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>>> index 45f63e93..3c5e43df 100644
>>> --- a/src/libcamera/meson.build
>>> +++ b/src/libcamera/meson.build
>>> @@ -17,6 +17,7 @@ libcamera_sources = files([
>>> 'delayed_controls.cpp',
>>> 'device_enumerator.cpp',
>>> 'device_enumerator_sysfs.cpp',
>>> + 'dma_heaps.cpp',
>>> 'fence.cpp',
>>> 'formats.cpp',
>>> 'framebuffer.cpp',
>>> diff --git a/src/libcamera/pipeline/rpi/vc4/meson.build b/src/libcamera/pipeline/rpi/vc4/meson.build
>>> index cdb049c5..386e2296 100644
>>> --- a/src/libcamera/pipeline/rpi/vc4/meson.build
>>> +++ b/src/libcamera/pipeline/rpi/vc4/meson.build
>>> @@ -1,7 +1,6 @@
>>> # SPDX-License-Identifier: CC0-1.0
>>>
>>> libcamera_sources += files([
>>> - 'dma_heaps.cpp',
>>> 'vc4.cpp',
>>> ])
>>>
>>> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
>>> index 26102ea7..3a42e75e 100644
>>> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
>>> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
>>> @@ -12,12 +12,11 @@
>>> #include <libcamera/formats.h>
>>>
>>> #include "libcamera/internal/device_enumerator.h"
>>> +#include "libcamera/internal/dma_heaps.h"
>>>
>>> #include "../common/pipeline_base.h"
>>> #include "../common/rpi_stream.h"
>>>
>>> -#include "dma_heaps.h"
>>> -
>>> using namespace std::chrono_literals;
>>>
>>> namespace libcamera {
>>> @@ -87,7 +86,7 @@ public:
>>> RPi::Device<Isp, 4> isp_;
>>>
>>> /* DMAHEAP allocation helper. */
>>> - RPi::DmaHeap dmaHeap_;
>>> + DmaHeap dmaHeap_;
>>> SharedFD lsTable_;
>>>
>>> struct Config {
>>> --
>>> 2.43.0
>>>
>
More information about the libcamera-devel
mailing list