[libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move dma_heaps.[h, cpp] to common directories
Naushir Patuck
naush at raspberrypi.com
Mon Jan 15 10:08:42 CET 2024
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>
>
> >
> > 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