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

[Jacopo Mondi]

Hi Umang,

On Wed, Nov 10, 2021 at 02:51:44PM +0530, Umang Jain wrote:
> 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.
>

No, that's by design :)

Fences will be activated only when the request is queued to the camera.

> > +	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();
> >   }
> >   /**