[v12,1/7] libcamera: add DmaBufAllocator::exportBuffers()
diff mbox series

Message ID 20240910044834.2477701-2-chenghaoyang@google.com
State Superseded
Headers show
Series
  • Add VirtualPipelineHandler
Related show

Commit Message

Harvey Yang Sept. 10, 2024, 4:40 a.m. UTC 
Add a helper function exportBuffers in DmaBufAllocator to make it easier
to use.

It'll be used in Virtual Pipeline Handler and SoftwareIsp.

Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .../libcamera/internal/dma_buf_allocator.h    | 13 +++++
 src/libcamera/dma_buf_allocator.cpp           | 58 +++++++++++++++++++
 2 files changed, 71 insertions(+)

Comments

Jacopo Mondi Sept. 26, 2024, 10:26 a.m. UTC | #1 
Hi Harvey

On Tue, Sep 10, 2024 at 04:40:14AM GMT, Harvey Yang wrote:
> Add a helper function exportBuffers in DmaBufAllocator to make it easier
> to use.
>
> It'll be used in Virtual Pipeline Handler and SoftwareIsp.
>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  .../libcamera/internal/dma_buf_allocator.h    | 13 +++++
>  src/libcamera/dma_buf_allocator.cpp           | 58 +++++++++++++++++++
>  2 files changed, 71 insertions(+)
>
> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> index d2a0a0d1..66d3b419 100644
> --- a/include/libcamera/internal/dma_buf_allocator.h
> +++ b/include/libcamera/internal/dma_buf_allocator.h
> @@ -7,11 +7,17 @@
>
>  #pragma once
>
> +#include <memory>
> +#include <string>
> +#include <vector>
> +
>  #include <libcamera/base/flags.h>
>  #include <libcamera/base/unique_fd.h>
>
>  namespace libcamera {
>
> +class FrameBuffer;
> +
>  class DmaBufAllocator
>  {
>  public:
> @@ -28,7 +34,14 @@ public:
>  	bool isValid() const { return providerHandle_.isValid(); }
>  	UniqueFD alloc(const char *name, std::size_t size);
>
> +	int exportBuffers(unsigned int count,
> +			  const std::vector<unsigned int> &frameSize,
> +			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> +
>  private:
> +	std::unique_ptr<FrameBuffer> createBuffer(
> +		std::string name, const std::vector<unsigned int> &frameSizes);
> +
>  	UniqueFD allocFromHeap(const char *name, std::size_t size);
>  	UniqueFD allocFromUDmaBuf(const char *name, std::size_t size);
>  	UniqueFD providerHandle_;
> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> index be6efb89..a49d1c60 100644
> --- a/src/libcamera/dma_buf_allocator.cpp
> +++ b/src/libcamera/dma_buf_allocator.cpp
> @@ -23,6 +23,10 @@
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/memfd.h>
>
> +#include <libcamera/framebuffer.h>
> +
> +#include "libcamera/internal/formats.h"
> +
>  /**
>   * \file dma_buf_allocator.cpp
>   * \brief dma-buf allocator
> @@ -205,4 +209,58 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
>  		return allocFromHeap(name, size);
>  }
>
> +/**
> + * \brief Allocate and export buffers from the DmaBufAllocator
> + * \param[in] count The number of requested FrameBuffers
> + * \param[in] frameSizes The sizes of planes in each FrameBuffer
> + * \param[out] buffers Array of buffers successfully allocated
> + *
> + * Planes in a FrameBuffer are allocated with a single dma buf.
> + * \todo Add the option to allocate each plane with a dma buf respectively.
> + *
> + * \return The number of allocated buffers on success or a negative error code
> + * otherwise
> + */
> +int DmaBufAllocator::exportBuffers(unsigned int count,
> +				   const std::vector<unsigned int> &frameSizes,
> +				   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	for (unsigned int i = 0; i < count; ++i) {
> +		std::unique_ptr<FrameBuffer> buffer =
> +			createBuffer("frame-" + std::to_string(i), frameSizes);
> +		if (!buffer) {
> +			LOG(DmaBufAllocator, Error) << "Unable to create buffer";
> +
> +			buffers->clear();
> +			return -EINVAL;
> +		}
> +
> +		buffers->push_back(std::move(buffer));
> +	}
> +
> +	return count;
> +}
> +
> +std::unique_ptr<FrameBuffer>
> +DmaBufAllocator::createBuffer(std::string name,
> +			      const std::vector<unsigned int> &frameSizes)
> +{
> +	std::vector<FrameBuffer::Plane> planes;
> +
> +	unsigned int bufferSize = 0, offset = 0;
> +	for (auto frameSize : frameSizes)
> +		bufferSize += frameSize;
> +
> +	SharedFD fd(alloc(name.c_str(), bufferSize));
> +	if (!fd.isValid())
> +		return nullptr;
> +
> +	for (auto frameSize : frameSizes) {
> +		planes.emplace_back(FrameBuffer::Plane{ fd, offset, frameSize });
> +		offset += frameSize;
> +	}
> +
> +	return std::make_unique<FrameBuffer>(planes);
> +}
> +
>  } /* namespace libcamera */
> --
> 2.46.0.598.g6f2099f65c-goog
>
Jacopo Mondi Sept. 26, 2024, 10:28 a.m. UTC | #2 
One little nit

On Tue, Sep 10, 2024 at 04:40:14AM GMT, Harvey Yang wrote:
> Add a helper function exportBuffers in DmaBufAllocator to make it easier
> to use.
>
> It'll be used in Virtual Pipeline Handler and SoftwareIsp.
>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  .../libcamera/internal/dma_buf_allocator.h    | 13 +++++
>  src/libcamera/dma_buf_allocator.cpp           | 58 +++++++++++++++++++
>  2 files changed, 71 insertions(+)
>
> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> index d2a0a0d1..66d3b419 100644
> --- a/include/libcamera/internal/dma_buf_allocator.h
> +++ b/include/libcamera/internal/dma_buf_allocator.h
> @@ -7,11 +7,17 @@
>
>  #pragma once
>
> +#include <memory>
> +#include <string>
> +#include <vector>
> +
>  #include <libcamera/base/flags.h>
>  #include <libcamera/base/unique_fd.h>
>
>  namespace libcamera {
>
> +class FrameBuffer;
> +
>  class DmaBufAllocator
>  {
>  public:
> @@ -28,7 +34,14 @@ public:
>  	bool isValid() const { return providerHandle_.isValid(); }
>  	UniqueFD alloc(const char *name, std::size_t size);
>
> +	int exportBuffers(unsigned int count,
> +			  const std::vector<unsigned int> &frameSize,
> +			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> +
>  private:
> +	std::unique_ptr<FrameBuffer> createBuffer(
> +		std::string name, const std::vector<unsigned int> &frameSizes);
> +
>  	UniqueFD allocFromHeap(const char *name, std::size_t size);
>  	UniqueFD allocFromUDmaBuf(const char *name, std::size_t size);
>  	UniqueFD providerHandle_;
> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> index be6efb89..a49d1c60 100644
> --- a/src/libcamera/dma_buf_allocator.cpp
> +++ b/src/libcamera/dma_buf_allocator.cpp
> @@ -23,6 +23,10 @@
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/memfd.h>
>
> +#include <libcamera/framebuffer.h>
> +
> +#include "libcamera/internal/formats.h"

What is this used for ?

You need to include <libcamera/base/shared_fh.h> as well

With the above fixed/clarified you can retain my tag

> +
>  /**
>   * \file dma_buf_allocator.cpp
>   * \brief dma-buf allocator
> @@ -205,4 +209,58 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
>  		return allocFromHeap(name, size);
>  }
>
> +/**
> + * \brief Allocate and export buffers from the DmaBufAllocator
> + * \param[in] count The number of requested FrameBuffers
> + * \param[in] frameSizes The sizes of planes in each FrameBuffer
> + * \param[out] buffers Array of buffers successfully allocated
> + *
> + * Planes in a FrameBuffer are allocated with a single dma buf.
> + * \todo Add the option to allocate each plane with a dma buf respectively.
> + *
> + * \return The number of allocated buffers on success or a negative error code
> + * otherwise
> + */
> +int DmaBufAllocator::exportBuffers(unsigned int count,
> +				   const std::vector<unsigned int> &frameSizes,
> +				   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	for (unsigned int i = 0; i < count; ++i) {
> +		std::unique_ptr<FrameBuffer> buffer =
> +			createBuffer("frame-" + std::to_string(i), frameSizes);
> +		if (!buffer) {
> +			LOG(DmaBufAllocator, Error) << "Unable to create buffer";
> +
> +			buffers->clear();
> +			return -EINVAL;
> +		}
> +
> +		buffers->push_back(std::move(buffer));
> +	}
> +
> +	return count;
> +}
> +
> +std::unique_ptr<FrameBuffer>
> +DmaBufAllocator::createBuffer(std::string name,
> +			      const std::vector<unsigned int> &frameSizes)
> +{
> +	std::vector<FrameBuffer::Plane> planes;
> +
> +	unsigned int bufferSize = 0, offset = 0;
> +	for (auto frameSize : frameSizes)
> +		bufferSize += frameSize;
> +
> +	SharedFD fd(alloc(name.c_str(), bufferSize));
> +	if (!fd.isValid())
> +		return nullptr;
> +
> +	for (auto frameSize : frameSizes) {
> +		planes.emplace_back(FrameBuffer::Plane{ fd, offset, frameSize });
> +		offset += frameSize;
> +	}
> +
> +	return std::make_unique<FrameBuffer>(planes);
> +}
> +
>  } /* namespace libcamera */
> --
> 2.46.0.598.g6f2099f65c-goog
>
Cheng-Hao Yang Sept. 26, 2024, 4:32 p.m. UTC | #3 
Thanks Jacopo,

On Thu, Sep 26, 2024 at 6:29 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> One little nit
>
> On Tue, Sep 10, 2024 at 04:40:14AM GMT, Harvey Yang wrote:
> > Add a helper function exportBuffers in DmaBufAllocator to make it easier
> > to use.
> >
> > It'll be used in Virtual Pipeline Handler and SoftwareIsp.
> >
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  .../libcamera/internal/dma_buf_allocator.h    | 13 +++++
> >  src/libcamera/dma_buf_allocator.cpp           | 58 +++++++++++++++++++
> >  2 files changed, 71 insertions(+)
> >
> > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> > index d2a0a0d1..66d3b419 100644
> > --- a/include/libcamera/internal/dma_buf_allocator.h
> > +++ b/include/libcamera/internal/dma_buf_allocator.h
> > @@ -7,11 +7,17 @@
> >
> >  #pragma once
> >
> > +#include <memory>
> > +#include <string>
> > +#include <vector>
> > +
> >  #include <libcamera/base/flags.h>
> >  #include <libcamera/base/unique_fd.h>
> >
> >  namespace libcamera {
> >
> > +class FrameBuffer;
> > +
> >  class DmaBufAllocator
> >  {
> >  public:
> > @@ -28,7 +34,14 @@ public:
> >       bool isValid() const { return providerHandle_.isValid(); }
> >       UniqueFD alloc(const char *name, std::size_t size);
> >
> > +     int exportBuffers(unsigned int count,
> > +                       const std::vector<unsigned int> &frameSize,
> > +                       std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> > +
> >  private:
> > +     std::unique_ptr<FrameBuffer> createBuffer(
> > +             std::string name, const std::vector<unsigned int> &frameSizes);
> > +
> >       UniqueFD allocFromHeap(const char *name, std::size_t size);
> >       UniqueFD allocFromUDmaBuf(const char *name, std::size_t size);
> >       UniqueFD providerHandle_;
> > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> > index be6efb89..a49d1c60 100644
> > --- a/src/libcamera/dma_buf_allocator.cpp
> > +++ b/src/libcamera/dma_buf_allocator.cpp
> > @@ -23,6 +23,10 @@
> >  #include <libcamera/base/log.h>
> >  #include <libcamera/base/memfd.h>
> >
> > +#include <libcamera/framebuffer.h>
> > +
> > +#include "libcamera/internal/formats.h"
>
> What is this used for ?
>
> You need to include <libcamera/base/shared_fh.h> as well
>
> With the above fixed/clarified you can retain my tag

Both done.


>
> > +
> >  /**
> >   * \file dma_buf_allocator.cpp
> >   * \brief dma-buf allocator
> > @@ -205,4 +209,58 @@ UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
> >               return allocFromHeap(name, size);
> >  }
> >
> > +/**
> > + * \brief Allocate and export buffers from the DmaBufAllocator
> > + * \param[in] count The number of requested FrameBuffers
> > + * \param[in] frameSizes The sizes of planes in each FrameBuffer
> > + * \param[out] buffers Array of buffers successfully allocated
> > + *
> > + * Planes in a FrameBuffer are allocated with a single dma buf.
> > + * \todo Add the option to allocate each plane with a dma buf respectively.
> > + *
> > + * \return The number of allocated buffers on success or a negative error code
> > + * otherwise
> > + */
> > +int DmaBufAllocator::exportBuffers(unsigned int count,
> > +                                const std::vector<unsigned int> &frameSizes,
> > +                                std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +{
> > +     for (unsigned int i = 0; i < count; ++i) {
> > +             std::unique_ptr<FrameBuffer> buffer =
> > +                     createBuffer("frame-" + std::to_string(i), frameSizes);
> > +             if (!buffer) {
> > +                     LOG(DmaBufAllocator, Error) << "Unable to create buffer";
> > +
> > +                     buffers->clear();
> > +                     return -EINVAL;
> > +             }
> > +
> > +             buffers->push_back(std::move(buffer));
> > +     }
> > +
> > +     return count;
> > +}
> > +
> > +std::unique_ptr<FrameBuffer>
> > +DmaBufAllocator::createBuffer(std::string name,
> > +                           const std::vector<unsigned int> &frameSizes)
> > +{
> > +     std::vector<FrameBuffer::Plane> planes;
> > +
> > +     unsigned int bufferSize = 0, offset = 0;
> > +     for (auto frameSize : frameSizes)
> > +             bufferSize += frameSize;
> > +
> > +     SharedFD fd(alloc(name.c_str(), bufferSize));
> > +     if (!fd.isValid())
> > +             return nullptr;
> > +
> > +     for (auto frameSize : frameSizes) {
> > +             planes.emplace_back(FrameBuffer::Plane{ fd, offset, frameSize });
> > +             offset += frameSize;
> > +     }
> > +
> > +     return std::make_unique<FrameBuffer>(planes);
> > +}
> > +
> >  } /* namespace libcamera */
> > --
> > 2.46.0.598.g6f2099f65c-goog
> >



--
BR,
Harvey Yang

Patch
diff mbox series 

diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
index d2a0a0d1..66d3b419 100644
--- a/include/libcamera/internal/dma_buf_allocator.h
+++ b/include/libcamera/internal/dma_buf_allocator.h
@@ -7,11 +7,17 @@ 
 
 #pragma once
 
+#include <memory>
+#include <string>
+#include <vector>
+
 #include <libcamera/base/flags.h>
 #include <libcamera/base/unique_fd.h>
 
 namespace libcamera {
 
+class FrameBuffer;
+
 class DmaBufAllocator
 {
 public:
@@ -28,7 +34,14 @@  public:
 	bool isValid() const { return providerHandle_.isValid(); }
 	UniqueFD alloc(const char *name, std::size_t size);
 
+	int exportBuffers(unsigned int count,
+			  const std::vector<unsigned int> &frameSize,
+			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
+
 private:
+	std::unique_ptr<FrameBuffer> createBuffer(
+		std::string name, const std::vector<unsigned int> &frameSizes);
+
 	UniqueFD allocFromHeap(const char *name, std::size_t size);
 	UniqueFD allocFromUDmaBuf(const char *name, std::size_t size);
 	UniqueFD providerHandle_;
diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
index be6efb89..a49d1c60 100644
--- a/src/libcamera/dma_buf_allocator.cpp
+++ b/src/libcamera/dma_buf_allocator.cpp
@@ -23,6 +23,10 @@ 
 #include <libcamera/base/log.h>
 #include <libcamera/base/memfd.h>
 
+#include <libcamera/framebuffer.h>
+
+#include "libcamera/internal/formats.h"
+
 /**
  * \file dma_buf_allocator.cpp
  * \brief dma-buf allocator
@@ -205,4 +209,58 @@  UniqueFD DmaBufAllocator::alloc(const char *name, std::size_t size)
 		return allocFromHeap(name, size);
 }
 
+/**
+ * \brief Allocate and export buffers from the DmaBufAllocator
+ * \param[in] count The number of requested FrameBuffers
+ * \param[in] frameSizes The sizes of planes in each FrameBuffer
+ * \param[out] buffers Array of buffers successfully allocated
+ *
+ * Planes in a FrameBuffer are allocated with a single dma buf.
+ * \todo Add the option to allocate each plane with a dma buf respectively.
+ *
+ * \return The number of allocated buffers on success or a negative error code
+ * otherwise
+ */
+int DmaBufAllocator::exportBuffers(unsigned int count,
+				   const std::vector<unsigned int> &frameSizes,
+				   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+{
+	for (unsigned int i = 0; i < count; ++i) {
+		std::unique_ptr<FrameBuffer> buffer =
+			createBuffer("frame-" + std::to_string(i), frameSizes);
+		if (!buffer) {
+			LOG(DmaBufAllocator, Error) << "Unable to create buffer";
+
+			buffers->clear();
+			return -EINVAL;
+		}
+
+		buffers->push_back(std::move(buffer));
+	}
+
+	return count;
+}
+
+std::unique_ptr<FrameBuffer>
+DmaBufAllocator::createBuffer(std::string name,
+			      const std::vector<unsigned int> &frameSizes)
+{
+	std::vector<FrameBuffer::Plane> planes;
+
+	unsigned int bufferSize = 0, offset = 0;
+	for (auto frameSize : frameSizes)
+		bufferSize += frameSize;
+
+	SharedFD fd(alloc(name.c_str(), bufferSize));
+	if (!fd.isValid())
+		return nullptr;
+
+	for (auto frameSize : frameSizes) {
+		planes.emplace_back(FrameBuffer::Plane{ fd, offset, frameSize });
+		offset += frameSize;
+	}
+
+	return std::make_unique<FrameBuffer>(planes);
+}
+
 } /* namespace libcamera */