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

[Kieran Bingham]

Quoting Jacopo Mondi (2021-10-28 12:15:15)
> 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));
> +       pendingFences_++;

Hrm ... I was expecting signals to be connected here ... but I guess
that happens elsewhere...


> +}
> +
> +/**
> + * \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_--;

Are all these event driven counters thread safe? They may be, just seems
a potential risk area...


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