[libcamera-devel] [PATCH v2 02/18] libcamera: internal: Move dma_heaps.[h, cpp] to common directories

Jacopo Mondi jacopo.mondi at ideasonboard.com
Wed Jan 31 10:23:42 CET 2024


Or

On Wed, Jan 31, 2024 at 10:21:20AM +0100, Jacopo Mondi wrote:
> Hi Hans
>
> On Sat, Jan 13, 2024 at 03:22:02PM +0100, Hans de Goede via libcamera-devel wrote:
> > 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.
> >
> > 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);
>
> minor nit: please re-align, here and in below occurrences
>
> 			LOG(DmaHeap, Debug) << "Failed to open " << name << ": "
> 					    << strerror(ret);
>

Now that I've read Laurent's review which I missed due to not
up-to-date notmuch index, do whatever you prefer among the several
proposed solutions :)

Sorry for the noise

> >  			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