[libcamera-devel] [PATCH 05/10] libcamera: request: Add support for fences

[Umang Jain]

Hi Jacopo,

On 10/28/21 4:45 PM, Jacopo Mondi wrote:
> Prepare the Request::Private class to handle fences by providing a
> storage vector and interface functions to handle the number of pending
> and expired fences.
>
> The intended user of the interface is the PipelineHandler class
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>   include/libcamera/internal/request.h | 21 +++++++
>   src/libcamera/request.cpp            | 89 +++++++++++++++++++++++++++-
>   2 files changed, 109 insertions(+), 1 deletion(-)
>
> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
> index df0cc014067e..2be4874756de 100644
> --- a/include/libcamera/internal/request.h
> +++ b/include/libcamera/internal/request.h
> @@ -7,8 +7,12 @@
>   #ifndef __LIBCAMERA_INTERNAL_REQUEST_H__
>   #define __LIBCAMERA_INTERNAL_REQUEST_H__
>   
> +#include <vector>
> +
>   #include <libcamera/request.h>
>   
> +#include <libcamera/internal/fence.h>
> +
>   namespace libcamera {
>   
>   class Camera;
> @@ -24,9 +28,26 @@ public:
>   
>   	Camera *camera() const { return camera_; }
>   
> +	unsigned int pendingFences() const { return pendingFences_; }
> +	unsigned int expiredFences() const { return expiredFences_; }
> +
> +	void reuse();
> +
> +	std::vector<Fence> &fences() { return fences_; }
> +	void addFence(Fence &&fence);
> +	void clearFences();
> +
> +	void fenceExpired();
> +	void fenceCompleted();
> +
>   private:
>   	Camera *camera_;
>   	bool cancelled_;
> +
> +	unsigned int pendingFences_ = 0;
> +	unsigned int expiredFences_ = 0;
> +
> +	std::vector<Fence> fences_;
>   };
>   
>   } /* namespace libcamera */
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 33fee1ac05ba..e88eee1fac36 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -63,6 +63,92 @@ Request::Private::~Private()
>    * request hasn't been queued
>    */
>   
> +/**
> + * \fn Request::Private::pendingFences()
> + * \brief Retrieve the number of pending fences
> + *
> + * A Fence is pending if has not yet been signalled or it has not expired yet.
> + *
> + * \return The number of pending fences
> + */
> +
> +/**
> + * \fn Request::Private::expiredFences()
> + * \brief Retrieve the number of expired fences
> + * \return The number of expired fences
> + */
> +
> +/**
> + * \brief Reset the request for reuse
> + */
> +void Request::Private::reuse()
> +{
> +	cancelled_ = false;
> +
> +	fences_.clear();
> +	pendingFences_ = 0;
> +	expiredFences_ = 0;
> +}
> +
> +/**
> + * \fn Request::Private::fences()
> + * \brief Retrieve a reference to the vector of pending fences
> + * \return A reference to the vector of pending fences
> + */
> +
> +/**
> + * \brief Move a Fence into the Request
> + *
> + * Move a Fence into the Request and increase the pending fences
> + * counter. The Fence is now stored into the Request and the original
> + * one passed in as parameter is reset.
> + *
> + * Once the Fence completes, either because it is signalled or because
> + * it has expired, the caller shall notify the Request about this by
> + * calling fenceCompleted() or fenceExpired();
> + */
> +void Request::Private::addFence(Fence &&fence)
> +{
> +	fences_.push_back(std::move(fence));

Drive-by comment, please let me know if I am wrong:

As per my understanding and your clarification, the fence is moved to a 
new Fence and pushed into fences_. I believe this new Fence will be 
disabled (by-default behaviour as stated in Fence class). So I don't 
think any of fenceCompleted() or fenceExpired() getting called, which 
seems problematic.

> +	pendingFences_++;
> +}
> +
> +/**
> + * \brief Release all Fences stored in the request
> + *
> + * Clear the vector of fences in the Request by releasing all of them.
> + * All Fences are closed and their file descriptors reset to 0.
> + */
> +void Request::Private::clearFences()
> +{
> +	ASSERT(!pendingFences_);
> +	fences_.clear();
> +}
> +
> +/**
> + * \brief Notify that a Fence has been signalled
> + *
> + * Notify to the Request that a Fence has completed. This function decrease the
> + * number of pending Fences in the request.
> + */
> +void Request::Private::fenceCompleted()
> +{
> +	pendingFences_--;
> +}
> +
> +/**
> + * \brief Notify that a Fence has expired
> + *
> + * Notify to the Request that a Fence has expired. This function decrease the
> + * number of pending Fences in the request and increase the number of expired
> + * ones.
> + */
> +void Request::Private::fenceExpired()
> +{
> +	expiredFences_++;
> +	pendingFences_--;
> +}
> +
>   /**
>    * \enum Request::Status
>    * Request completion status
> @@ -158,10 +244,11 @@ void Request::reuse(ReuseFlag flags)
>   
>   	sequence_ = 0;
>   	status_ = RequestPending;
> -	_d()->cancelled_ = false;
>   
>   	controls_->clear();
>   	metadata_->clear();
> +
> +	_d()->reuse();
>   }
>   
>   /**