[libcamera-devel,v4,04/11] libcamera: framebuffer: Add Fence to FrameBuffer
diff mbox series

Message ID 20211201142936.107405-5-jacopo@jmondi.org
State Superseded
Delegated to: Jacopo Mondi
Headers show
Series
  • libcamera: Add support for Fence
Related show

Commit Message

Jacopo Mondi Dec. 1, 2021, 2:29 p.m. UTC
Add to the FrameBuffer::Private class a unique pointer to a
synchronization Fence.

The Fence will be used to signal the availability of the Framebuffer for
incoming data transfer.

The Fence will be associated to a FrameBuffer at Request::addBuffer()
time, and if correctly signalled, reset by the core at Request queue
time.

If a FrameBuffer completes with errors, due to a Fence wait failure, the
Fence will still be owned by the FrameBuffer and it is application
responsibility to correctly reset it before reusing the buffer.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/framebuffer.h          |  4 ++
 include/libcamera/internal/framebuffer.h |  8 ++++
 src/libcamera/framebuffer.cpp            | 58 ++++++++++++++++++++++++
 3 files changed, 70 insertions(+)

Comments

Laurent Pinchart Dec. 8, 2021, 9:41 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Wed, Dec 01, 2021 at 03:29:29PM +0100, Jacopo Mondi wrote:
> Add to the FrameBuffer::Private class a unique pointer to a
> synchronization Fence.

s/synchronization Fence/Fence/

> The Fence will be used to signal the availability of the Framebuffer for
> incoming data transfer.
> 
> The Fence will be associated to a FrameBuffer at Request::addBuffer()
> time, and if correctly signalled, reset by the core at Request queue
> time.
> 
> If a FrameBuffer completes with errors, due to a Fence wait failure, the
> Fence will still be owned by the FrameBuffer and it is application
> responsibility to correctly reset it before reusing the buffer.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/framebuffer.h          |  4 ++
>  include/libcamera/internal/framebuffer.h |  8 ++++
>  src/libcamera/framebuffer.cpp            | 58 ++++++++++++++++++++++++
>  3 files changed, 70 insertions(+)
> 
> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
> index 357bbe189551..39ebbdaebf47 100644
> --- a/include/libcamera/framebuffer.h
> +++ b/include/libcamera/framebuffer.h
> @@ -9,6 +9,7 @@
>  
>  #include <assert.h>
>  #include <limits>
> +#include <memory>
>  #include <stdint.h>
>  #include <vector>
>  
> @@ -18,6 +19,7 @@
>  
>  namespace libcamera {
>  
> +class Fence;
>  class Request;
>  
>  struct FrameMetadata {
> @@ -65,6 +67,8 @@ public:
>  	unsigned int cookie() const { return cookie_; }
>  	void setCookie(unsigned int cookie) { cookie_ = cookie; }
>  
> +	std::unique_ptr<Fence> releaseFence();
> +
>  	void cancel() { metadata_.status = FrameMetadata::FrameCancelled; }
>  
>  private:
> diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h
> index 908b478985e7..fc9f4a2fc768 100644
> --- a/include/libcamera/internal/framebuffer.h
> +++ b/include/libcamera/internal/framebuffer.h
> @@ -7,8 +7,12 @@
>  
>  #pragma once
>  
> +#include <memory>
> +#include <utility>
> +
>  #include <libcamera/base/class.h>
>  
> +#include <libcamera/fence.h>
>  #include <libcamera/framebuffer.h>
>  
>  namespace libcamera {
> @@ -23,7 +27,11 @@ public:
>  	void setRequest(Request *request) { request_ = request; }
>  	bool isContiguous() const { return isContiguous_; }
>  
> +	Fence *fence() const { return fence_.get(); }
> +	void setFence(std::unique_ptr<Fence> fence) { fence_ = std::move(fence); }
> +
>  private:
> +	std::unique_ptr<Fence> fence_;
>  	Request *request_;
>  	bool isContiguous_;
>  };
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 701212f3ae21..a2cb0486e14d 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -140,6 +140,45 @@ FrameBuffer::Private::Private()
>   * \return True if the planes are stored contiguously in memory, false otherwise
>   */
>  
> +/**
> + * \fn FrameBuffer::Private::fence()
> + * \brief Retrieve a const pointer to the Fence
> + *
> + * This function does only return a reference to the the fence and does not
> + * change its ownership. The fence is stored in the FrameBuffer and can only be
> + * reset with FrameBuffer::releaseFence() in case the buffer has completed with
> + * error due to a Fence wait failure.
> + *
> + * If buffer with a Fence completes with errors due to a failure in handling
> + * the fence, applications are responsible for releasing the Fence before
> + * calling Request::addBuffer() again.
> + *
> + * \sa Request::addBuffer()
> + *
> + * \return A const pointer to the Fence if any, nullptr otherwise
> + */
> +
> +/**
> + * \fn FrameBuffer::Private::setFence()
> + * \brief Move a \a fence in this buffer
> + * \param[in] fence The synchronization fence

s/synchronization fence/fence/

> + *
> + * This function associates a Fence with this Framebuffer. The intended caller
> + * is the Request::addBuffer() function.
> + *
> + * Once a FrameBuffer is associated with a Fence, the FrameBuffer will only be
> + * made available to the hardware device once the synchronization Fence has been

s/synchronization Fence/Fence/

> + * correctly signalled.
> + *
> + * \sa Request::prepare()
> + *
> + * If the FrameBuffer completes successfully the core releases the Fence and the
> + * Buffer can be reused immediately. If handling of the Fence fails during the
> + * request preparation, the Fence is not reset and is left in the FrameBuffer.

s/reset/released/ maybe ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + * It is applications responsibility to correctly release the fence and handle
> + * it opportunely before using the buffer again.
> + */
> +
>  /**
>   * \class FrameBuffer
>   * \brief Frame buffer data and its associated dynamic metadata
> @@ -329,6 +368,25 @@ Request *FrameBuffer::request() const
>   * libcamera core never modifies the buffer cookie.
>   */
>  
> +/**
> + * \brief Extract the Fence associated with this Framebuffer
> + *
> + * This function moves the buffer's fence ownership to the caller.
> + * After the fence has been released, calling this function always return
> + * nullptr.
> + *
> + * If buffer with a Fence completes with errors due to a failure in handling
> + * the fence, applications are responsible for releasing the Fence before
> + * calling Request::addBuffer() again.
> + *
> + * \return A unique pointer to the Fence if set, or nullptr if the fence has
> + * been released already
> + */
> +std::unique_ptr<Fence> FrameBuffer::releaseFence()
> +{
> +	return std::move(_d()->fence_);
> +}
> +
>  /**
>   * \fn FrameBuffer::cancel()
>   * \brief Marks the buffer as cancelled

Patch
diff mbox series

diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
index 357bbe189551..39ebbdaebf47 100644
--- a/include/libcamera/framebuffer.h
+++ b/include/libcamera/framebuffer.h
@@ -9,6 +9,7 @@ 
 
 #include <assert.h>
 #include <limits>
+#include <memory>
 #include <stdint.h>
 #include <vector>
 
@@ -18,6 +19,7 @@ 
 
 namespace libcamera {
 
+class Fence;
 class Request;
 
 struct FrameMetadata {
@@ -65,6 +67,8 @@  public:
 	unsigned int cookie() const { return cookie_; }
 	void setCookie(unsigned int cookie) { cookie_ = cookie; }
 
+	std::unique_ptr<Fence> releaseFence();
+
 	void cancel() { metadata_.status = FrameMetadata::FrameCancelled; }
 
 private:
diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h
index 908b478985e7..fc9f4a2fc768 100644
--- a/include/libcamera/internal/framebuffer.h
+++ b/include/libcamera/internal/framebuffer.h
@@ -7,8 +7,12 @@ 
 
 #pragma once
 
+#include <memory>
+#include <utility>
+
 #include <libcamera/base/class.h>
 
+#include <libcamera/fence.h>
 #include <libcamera/framebuffer.h>
 
 namespace libcamera {
@@ -23,7 +27,11 @@  public:
 	void setRequest(Request *request) { request_ = request; }
 	bool isContiguous() const { return isContiguous_; }
 
+	Fence *fence() const { return fence_.get(); }
+	void setFence(std::unique_ptr<Fence> fence) { fence_ = std::move(fence); }
+
 private:
+	std::unique_ptr<Fence> fence_;
 	Request *request_;
 	bool isContiguous_;
 };
diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
index 701212f3ae21..a2cb0486e14d 100644
--- a/src/libcamera/framebuffer.cpp
+++ b/src/libcamera/framebuffer.cpp
@@ -140,6 +140,45 @@  FrameBuffer::Private::Private()
  * \return True if the planes are stored contiguously in memory, false otherwise
  */
 
+/**
+ * \fn FrameBuffer::Private::fence()
+ * \brief Retrieve a const pointer to the Fence
+ *
+ * This function does only return a reference to the the fence and does not
+ * change its ownership. The fence is stored in the FrameBuffer and can only be
+ * reset with FrameBuffer::releaseFence() in case the buffer has completed with
+ * error due to a Fence wait failure.
+ *
+ * If buffer with a Fence completes with errors due to a failure in handling
+ * the fence, applications are responsible for releasing the Fence before
+ * calling Request::addBuffer() again.
+ *
+ * \sa Request::addBuffer()
+ *
+ * \return A const pointer to the Fence if any, nullptr otherwise
+ */
+
+/**
+ * \fn FrameBuffer::Private::setFence()
+ * \brief Move a \a fence in this buffer
+ * \param[in] fence The synchronization fence
+ *
+ * This function associates a Fence with this Framebuffer. The intended caller
+ * is the Request::addBuffer() function.
+ *
+ * Once a FrameBuffer is associated with a Fence, the FrameBuffer will only be
+ * made available to the hardware device once the synchronization Fence has been
+ * correctly signalled.
+ *
+ * \sa Request::prepare()
+ *
+ * If the FrameBuffer completes successfully the core releases the Fence and the
+ * Buffer can be reused immediately. If handling of the Fence fails during the
+ * request preparation, the Fence is not reset and is left in the FrameBuffer.
+ * It is applications responsibility to correctly release the fence and handle
+ * it opportunely before using the buffer again.
+ */
+
 /**
  * \class FrameBuffer
  * \brief Frame buffer data and its associated dynamic metadata
@@ -329,6 +368,25 @@  Request *FrameBuffer::request() const
  * libcamera core never modifies the buffer cookie.
  */
 
+/**
+ * \brief Extract the Fence associated with this Framebuffer
+ *
+ * This function moves the buffer's fence ownership to the caller.
+ * After the fence has been released, calling this function always return
+ * nullptr.
+ *
+ * If buffer with a Fence completes with errors due to a failure in handling
+ * the fence, applications are responsible for releasing the Fence before
+ * calling Request::addBuffer() again.
+ *
+ * \return A unique pointer to the Fence if set, or nullptr if the fence has
+ * been released already
+ */
+std::unique_ptr<Fence> FrameBuffer::releaseFence()
+{
+	return std::move(_d()->fence_);
+}
+
 /**
  * \fn FrameBuffer::cancel()
  * \brief Marks the buffer as cancelled