[libcamera-devel] [PATCH v2 03/12] libcamera: framebuffer: private: Add Fence

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Sat, Nov 20, 2021 at 12:13:04PM +0100, Jacopo Mondi wrote:
> Add to the FrameBuffer::Private class a uniquely owned Fence instance.
> 
> The Fence will be used to signal the availability of the Framebuffer to
> incoming data transfer.
> 
> Add a set of function to set, cancel and retrieve a refence to the
> Fence.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/framebuffer.h          |  3 +++
>  include/libcamera/internal/framebuffer.h |  9 ++++++++
>  src/libcamera/framebuffer.cpp            | 26 ++++++++++++++++++++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
> index 7f2f176af691..aa2a1f5193c7 100644
> --- a/include/libcamera/framebuffer.h
> +++ b/include/libcamera/framebuffer.h
> @@ -19,6 +19,7 @@
>  
>  namespace libcamera {
>  
> +class Fence;
>  class Request;
>  
>  struct FrameMetadata {
> @@ -66,6 +67,8 @@ public:
>  	unsigned int cookie() const { return cookie_; }
>  	void setCookie(unsigned int cookie) { cookie_ = cookie; }
>  
> +	Fence *fence() const;
> +
>  	void cancel() { metadata_.status = FrameMetadata::FrameCancelled; }
>  
>  private:
> diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h
> index cd33c295466e..9e1699a6875d 100644
> --- a/include/libcamera/internal/framebuffer.h
> +++ b/include/libcamera/internal/framebuffer.h
> @@ -7,10 +7,14 @@
>  #ifndef __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__
>  #define __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__
>  
> +#include <memory>
> +
>  #include <libcamera/base/class.h>
>  
>  #include <libcamera/framebuffer.h>
>  
> +#include <libcamera/internal/fence.h>
> +
>  namespace libcamera {
>  
>  class FrameBuffer::Private : public Extensible::Private
> @@ -23,8 +27,13 @@ public:
>  	void setRequest(Request *request) { request_ = request; }
>  	bool isContiguous() const { return isContiguous_; }
>  
> +	void setFence(std::unique_ptr<Fence> &&fence) { fence_ = std::move(fence); }
> +	void closeFence() { fence_.reset(); }

I think I'd call this resetFence(). Another option would be to call
setFence(nullptr) instead, and drop the function. I think resetFence()
would be better, I'm thinking it could return the
std::unique_ptr<Fence>, in order to extract the fence from the
FrameBuffer when the request completes. We would need a resetFence()
function in the FrameBuffer class too, which could replace the existing
fence() function. I'll comment further on that in subsequent patches.

> +	Fence *fence() const { return fence_.get(); }
> +
>  private:
>  	Request *request_;
> +	std::unique_ptr<Fence> fence_;
>  	bool isContiguous_;
>  };
>  
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 337ea1155a38..63c4ce6ab450 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -126,6 +126,23 @@ FrameBuffer::Private::Private()
>   * handlers, it is called by the pipeline handlers themselves.
>   */
>  
> +/**
> + * \fn FrameBuffer::Private::setFence()
> + * \brief Set the synchronization fence for this buffer
> + * \param[in] fence The synchronization fence
> + */
> +
> +/**
> + * \fn FrameBuffer::Private::closeFence()
> + * \brief Close the synchronization fence for this buffer
> + */
> +
> +/**
> + * \fn FrameBuffer::Private::fence()
> + * \brief Retrieve a pointer to the synchronization fence of this buffer
> + * \return A pointer to the buffer fence, nullptr if the buffer has not fence
> + */
> +
>  /**
>   * \fn FrameBuffer::Private::isContiguous()
>   * \brief Check if the frame buffer stores planes contiguously in memory
> @@ -305,6 +322,15 @@ Request *FrameBuffer::request() const
>   * libcamera core never modifies the buffer cookie.
>   */
>  
> +/**
> + * \brief Retrieve a reference to the Fence associated with this Framebuffer
> + * \return A pointer to the Fence, if set, nullptr otherwise
> + */
> +Fence *FrameBuffer::fence() const
> +{
> +	return _d()->fence();
> +}
> +
>  /**
>   * \fn FrameBuffer::cancel()
>   * \brief Marks the buffer as cancelled

-- 
Regards,

Laurent Pinchart