[libcamera-devel,v2,08/12] libcamera: request: Add Request::Private::prepare()
diff mbox series

Message ID 20211120111313.106621-9-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Add support for Fence
Related show

Commit Message

Jacopo Mondi Nov. 20, 2021, 11:13 a.m. UTC
Add a prepare() function to the Private Request representation.

The prepare() function is used by the PipelineHandler class to
prepare a Request to be queued to the hardware.

The current implementation of prepare() handles the fences associated
with the Framebuffers part of a Request. The function starts an event
notifier for each of those and notifies the Request as Ready to be queued
once all the fences have been signalled.

An optional timeout allows to interrupt blocked waits and notify the
Request as failed.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/internal/request.h |  21 ++++
 src/libcamera/request.cpp            | 150 +++++++++++++++++++++++++++
 2 files changed, 171 insertions(+)

Comments

Laurent Pinchart Nov. 21, 2021, 9:39 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Sat, Nov 20, 2021 at 12:13:09PM +0100, Jacopo Mondi wrote:
> Add a prepare() function to the Private Request representation.
> 
> The prepare() function is used by the PipelineHandler class to
> prepare a Request to be queued to the hardware.
> 
> The current implementation of prepare() handles the fences associated
> with the Framebuffers part of a Request. The function starts an event
> notifier for each of those and notifies the Request as Ready to be queued
> once all the fences have been signalled.
> 
> An optional timeout allows to interrupt blocked waits and notify the
> Request as failed.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/internal/request.h |  21 ++++
>  src/libcamera/request.cpp            | 150 +++++++++++++++++++++++++++
>  2 files changed, 171 insertions(+)
> 
> diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
> index 59bddde3a090..26b25fb12261 100644
> --- a/include/libcamera/internal/request.h
> +++ b/include/libcamera/internal/request.h
> @@ -7,10 +7,16 @@
>  #ifndef __LIBCAMERA_INTERNAL_REQUEST_H__
>  #define __LIBCAMERA_INTERNAL_REQUEST_H__
>  
> +#include <chrono>
>  #include <memory>
>  
> +#include <libcamera/base/event_notifier.h>
> +#include <libcamera/base/timer.h>
> +
>  #include <libcamera/request.h>
>  
> +using namespace std::chrono_literals;
> +
>  namespace libcamera {
>  
>  class Camera;
> @@ -21,6 +27,12 @@ class Request::Private : public Extensible::Private
>  	LIBCAMERA_DECLARE_PUBLIC(Request)
>  
>  public:
> +	enum class Status {
> +		Pending,
> +		Ready,
> +		Failed
> +	};
> +
>  	Private(Camera *camera);
>  	~Private();
>  
> @@ -29,21 +41,30 @@ public:
>  
>  	uint64_t cookie() const;
>  	Request::Status status() const;
> +	Status privateStatus() const { return status_; }

As mentioned in the cover letter, status and privateStatus isn't nice
indeed. I think one option to remove the privateStatus() could be to
call cancel() in Request::Private::timeout(), and then use
Request::status() in PipelineHandler::doQueueRequests(). There could be
other options too.

>  
>  	bool completeBuffer(FrameBuffer *buffer);
>  	void complete();
>  	void cancel();
>  	void reuse();
>  
> +	Status prepare(std::chrono::milliseconds timeout = 0ms);
> +	Signal<> prepared;
> +
>  	uint32_t sequence_ = 0;
>  
>  private:
>  	void _cancel();
> +	void notifierActivated(const std::unique_ptr<EventNotifier> &notifier);
> +	void timeout();
>  
>  	Camera *camera_;
>  	bool cancelled_;
> +	Status status_ = Status::Pending;
>  
>  	std::unordered_set<FrameBuffer *> pending_;
> +	std::unordered_set<std::unique_ptr<EventNotifier>> notifiers_;
> +	std::unique_ptr<Timer> timer_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 1d47698a6263..98f9719e5cf2 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -43,6 +43,22 @@ LOG_DEFINE_CATEGORY(Request)
>   * subclasses).
>   */
>  
> +/**
> + * \enum Request::Private::Status
> + * \brief Request private status
> + *
> + * The Request private status describes the lifecycle of the Request between
> + * the time it is queued to the Camera and the time it is queued to the device.
> + *
> + * A Request is created in Status::Pending state. Before actually queueing the
> + * Request the PipelineHandler base class prepare() the Request, whose status
> + * transitions to either Status::Ready or Status::Failed.
> + *
> + * \var Request::Private::Status::Pending
> + * \var Request::Private::Status::Ready
> + * \var Request::Private::Status::Failed
> + */
> +
>  /**
>   * \brief Create a Request::Private
>   * \param camera The Camera that creates the request
> @@ -95,6 +111,17 @@ Request::Status Request::Private::status() const
>  	return _o<Request>()->status();
>  }
>  
> +/**
> + * \fn Request::Private::privateStatus()
> + * \brief Retrieve the Request private status
> + *
> + * The private status, as described by the Request::Private:Status enumeration,
> + * describes the Request status between the time it is queued to the Camera and
> + * the time the Request is applied to the hardware.
> + *
> + * \return The Request private state
> + */
> +
>  /**
>   * \brief Complete a buffer for the request
>   * \param[in] buffer The buffer that has completed
> @@ -155,6 +182,8 @@ void Request::Private::_cancel()
>  
>  	cancelled_ = true;
>  	pending_.clear();
> +	notifiers_.clear();
> +	timer_.reset();
>  }
>  
>  /**
> @@ -182,8 +211,84 @@ void Request::Private::reuse()
>  {
>  	sequence_ = 0;
>  	cancelled_ = false;
> +	status_ = Status::Pending;
>  	pending_.clear();
> +	notifiers_.clear();
> +	timer_.reset();
> +}
> +
> +/**
> + * \brief Prepare the Request to be queued to the device
> + * \param[in] timeout Optional expiration timeout
> + *
> + * Prepare a Request to be queued to the hardware device it by ensuring it is

s/it by/by/

> + * ready for the incoming memory transfers.
> + *
> + * This currently means waiting on each frame buffer acquire fence to be
> + * signalled. An optional expiration timeout can be specified. If not all the
> + * fences have been signalled correctly before the timeout expires the Request
> + * is marked as Failed, otherwise it is set to the Ready state.
> + *
> + * \sa Request::Private::Status
> + *
> + * The function returns Status::Ready if all the prepare operations have been
> + * completed synchronously. If Status::Ready is returned the Request can be
> + * queued immediately and the prepared signal is not emitted. If instead the
> + * prepare operation requires to wait the completion of asynchronous events,
> + * such as fence notifications or timer expiration this function returns
> + * Status::Pending and the asynchronous event completion is notified by emitting
> + * the prepared signal.
> + *
> + * As we currently only handle fences, the function return Status::Ready if
> + * there are no fences to wait on. Status::Prepared is otherwise returned and
> + * the prepared signal is emitted when all fences have been signalled or the
> + * optional timeout has expired.
> + *
> + * The intended user of this function is the PipelineHandler base class, which
> + * 'prepares' a Request before queuing it to the hardware device.
> + *
> + * \return The Request status
> + */
> +Request::Private::Status Request::Private::prepare(std::chrono::milliseconds timeout)
> +{
> +	FrameBuffer::Private *bufferData;
> +
> +	/* Create and connect one notifier for each synchronization fence. */
> +	for (FrameBuffer *buffer : pending_) {
> +		bufferData = buffer->_d();
> +
> +		if (!bufferData->fence())
> +			continue;
> +
> +		int fenceFd = bufferData->fence()->fd().fd();
> +		notifiers_.emplace(new EventNotifier(fenceFd, EventNotifier::Read));

There's no bug in your code, but usage of new() indicates potential
memory leaks, so it's best to minimize its usage. I'd write

		notifiers_.insert(std::make_unique<EventNotifier>(fenceFd, EventNotifier::Read));

> +	}
> +
> +	if (notifiers_.empty()) {
> +		status_ = Status::Ready;
> +		return status_;
> +	}
> +
> +	for (const std::unique_ptr<EventNotifier> &notifier : notifiers_)
> +		notifier->activated.connect(this, [this, &notifier] {
> +							notifierActivated(notifier);
> +					        });
> +
> +	/* In case a timeout is specified, create a timer and set it up. */

Let's add

	 * The timer must be created here instead of in the Request constructor,
	 * in order to be bound to the pipeline handler thread.

> +	if (timeout != 0ms) {
> +		timer_ = std::make_unique<Timer>();
> +		timer_->timeout.connect(this, &Request::Private::timeout);
> +		timer_->start(timeout);
> +	}
> +
> +	return Status::Pending;
>  }
> +
> +/**
> + * \var Request::Private::prepared
> + * \brief Request preparation completed Signal
> + */
> +
>  /**
>   * \var Request::Private::sequence_
>   * \brief The request sequence number
> @@ -191,6 +296,51 @@ void Request::Private::reuse()
>   * \copydoc Request::sequence()
>   */
>  
> +void Request::Private::notifierActivated(const std::unique_ptr<EventNotifier> &notifier)
> +{
> +	auto it = notifiers_.find(notifier);
> +	ASSERT(it != notifiers_.end());
> +
> +	/* We need to close the fence if successfully signalled. */
> +	int fd = notifier->fd();
> +	bool found = false;
> +	for (FrameBuffer *buffer : pending_) {
> +		FrameBuffer::Private *bufferData = buffer->_d();
> +
> +		if (!bufferData->fence())
> +			continue;
> +
> +		if (bufferData->fence()->fd().fd() != fd)
> +			continue;

If you turned notifiers_ into a map indexed by FrameBuffer *, you could
pass the FrameBuffer pointer instead of the notifier to the
notifierActivated() function, and avoid this loop.

> +
> +		bufferData->closeFence();
> +		found = true;
> +		break;
> +	}
> +	ASSERT(found);
> +
> +	notifiers_.erase(it);
> +	if (!notifiers_.empty())
> +		return;
> +
> +	/* All fences completed, delete the timer and move to state Ready. */
> +	timer_.reset();
> +	status_ = Status::Ready;
> +	prepared.emit();
> +}
> +
> +void Request::Private::timeout()
> +{
> +	/* A timeout can only happen if there are fences not yet signalled. */
> +	ASSERT(!notifiers_.empty());
> +	notifiers_.clear();
> +
> +	LOG(Request, Error) << "Request prepare timeout";

Could you print the cookie to ease debugging ?

> +
> +	status_ = Status::Failed;
> +	prepared.emit();
> +}
> +
>  /**
>   * \enum Request::Status
>   * Request completion status
Jacopo Mondi Nov. 27, 2021, 8:50 a.m. UTC | #2
Hi Laurent

On Sun, Nov 21, 2021 at 11:39:22PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Sat, Nov 20, 2021 at 12:13:09PM +0100, Jacopo Mondi wrote:
> > Add a prepare() function to the Private Request representation.
> >
> > The prepare() function is used by the PipelineHandler class to
> > prepare a Request to be queued to the hardware.
> >
> > The current implementation of prepare() handles the fences associated
> > with the Framebuffers part of a Request. The function starts an event
> > notifier for each of those and notifies the Request as Ready to be queued
> > once all the fences have been signalled.
> >
> > An optional timeout allows to interrupt blocked waits and notify the
> > Request as failed.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  include/libcamera/internal/request.h |  21 ++++
> >  src/libcamera/request.cpp            | 150 +++++++++++++++++++++++++++
> >  2 files changed, 171 insertions(+)
> >
> > diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
> > index 59bddde3a090..26b25fb12261 100644
> > --- a/include/libcamera/internal/request.h
> > +++ b/include/libcamera/internal/request.h
> > @@ -7,10 +7,16 @@
> >  #ifndef __LIBCAMERA_INTERNAL_REQUEST_H__
> >  #define __LIBCAMERA_INTERNAL_REQUEST_H__
> >
> > +#include <chrono>
> >  #include <memory>
> >
> > +#include <libcamera/base/event_notifier.h>
> > +#include <libcamera/base/timer.h>
> > +
> >  #include <libcamera/request.h>
> >
> > +using namespace std::chrono_literals;
> > +
> >  namespace libcamera {
> >
> >  class Camera;
> > @@ -21,6 +27,12 @@ class Request::Private : public Extensible::Private
> >  	LIBCAMERA_DECLARE_PUBLIC(Request)
> >
> >  public:
> > +	enum class Status {
> > +		Pending,
> > +		Ready,
> > +		Failed
> > +	};
> > +
> >  	Private(Camera *camera);
> >  	~Private();
> >
> > @@ -29,21 +41,30 @@ public:
> >
> >  	uint64_t cookie() const;
> >  	Request::Status status() const;
> > +	Status privateStatus() const { return status_; }
>
> As mentioned in the cover letter, status and privateStatus isn't nice
> indeed. I think one option to remove the privateStatus() could be to
> call cancel() in Request::Private::timeout(), and then use
> Request::status() in PipelineHandler::doQueueRequests(). There could be
> other options too.

I didn't do so as I thought it could have caused out-of-order
completions but I was probably wrong as just buffes are completed in
error state by Request::cancel() not the request!

Anyway, I need somehow to keep the state of when a request has been
prepared (ie or fences signalled or timeout) to pass it to
queueRequestDevice.

Probably a single boolean prepared_ flag in Private would do but I
really dislike flags like this as keeping track of them is hell when
something goes wrong..

>
> >
> >  	bool completeBuffer(FrameBuffer *buffer);
> >  	void complete();
> >  	void cancel();
> >  	void reuse();
> >
> > +	Status prepare(std::chrono::milliseconds timeout = 0ms);
> > +	Signal<> prepared;
> > +
> >  	uint32_t sequence_ = 0;
> >
> >  private:
> >  	void _cancel();
> > +	void notifierActivated(const std::unique_ptr<EventNotifier> &notifier);
> > +	void timeout();
> >
> >  	Camera *camera_;
> >  	bool cancelled_;
> > +	Status status_ = Status::Pending;
> >
> >  	std::unordered_set<FrameBuffer *> pending_;
> > +	std::unordered_set<std::unique_ptr<EventNotifier>> notifiers_;
> > +	std::unique_ptr<Timer> timer_;
> >  };
> >
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index 1d47698a6263..98f9719e5cf2 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -43,6 +43,22 @@ LOG_DEFINE_CATEGORY(Request)
> >   * subclasses).
> >   */
> >
> > +/**
> > + * \enum Request::Private::Status
> > + * \brief Request private status
> > + *
> > + * The Request private status describes the lifecycle of the Request between
> > + * the time it is queued to the Camera and the time it is queued to the device.
> > + *
> > + * A Request is created in Status::Pending state. Before actually queueing the
> > + * Request the PipelineHandler base class prepare() the Request, whose status
> > + * transitions to either Status::Ready or Status::Failed.
> > + *
> > + * \var Request::Private::Status::Pending
> > + * \var Request::Private::Status::Ready
> > + * \var Request::Private::Status::Failed
> > + */
> > +
> >  /**
> >   * \brief Create a Request::Private
> >   * \param camera The Camera that creates the request
> > @@ -95,6 +111,17 @@ Request::Status Request::Private::status() const
> >  	return _o<Request>()->status();
> >  }
> >
> > +/**
> > + * \fn Request::Private::privateStatus()
> > + * \brief Retrieve the Request private status
> > + *
> > + * The private status, as described by the Request::Private:Status enumeration,
> > + * describes the Request status between the time it is queued to the Camera and
> > + * the time the Request is applied to the hardware.
> > + *
> > + * \return The Request private state
> > + */
> > +
> >  /**
> >   * \brief Complete a buffer for the request
> >   * \param[in] buffer The buffer that has completed
> > @@ -155,6 +182,8 @@ void Request::Private::_cancel()
> >
> >  	cancelled_ = true;
> >  	pending_.clear();
> > +	notifiers_.clear();
> > +	timer_.reset();
> >  }
> >
> >  /**
> > @@ -182,8 +211,84 @@ void Request::Private::reuse()
> >  {
> >  	sequence_ = 0;
> >  	cancelled_ = false;
> > +	status_ = Status::Pending;
> >  	pending_.clear();
> > +	notifiers_.clear();
> > +	timer_.reset();
> > +}
> > +
> > +/**
> > + * \brief Prepare the Request to be queued to the device
> > + * \param[in] timeout Optional expiration timeout
> > + *
> > + * Prepare a Request to be queued to the hardware device it by ensuring it is
>
> s/it by/by/
>
> > + * ready for the incoming memory transfers.
> > + *
> > + * This currently means waiting on each frame buffer acquire fence to be
> > + * signalled. An optional expiration timeout can be specified. If not all the
> > + * fences have been signalled correctly before the timeout expires the Request
> > + * is marked as Failed, otherwise it is set to the Ready state.
> > + *
> > + * \sa Request::Private::Status
> > + *
> > + * The function returns Status::Ready if all the prepare operations have been
> > + * completed synchronously. If Status::Ready is returned the Request can be
> > + * queued immediately and the prepared signal is not emitted. If instead the
> > + * prepare operation requires to wait the completion of asynchronous events,
> > + * such as fence notifications or timer expiration this function returns
> > + * Status::Pending and the asynchronous event completion is notified by emitting
> > + * the prepared signal.
> > + *
> > + * As we currently only handle fences, the function return Status::Ready if
> > + * there are no fences to wait on. Status::Prepared is otherwise returned and
> > + * the prepared signal is emitted when all fences have been signalled or the
> > + * optional timeout has expired.
> > + *
> > + * The intended user of this function is the PipelineHandler base class, which
> > + * 'prepares' a Request before queuing it to the hardware device.
> > + *
> > + * \return The Request status
> > + */
> > +Request::Private::Status Request::Private::prepare(std::chrono::milliseconds timeout)
> > +{
> > +	FrameBuffer::Private *bufferData;
> > +
> > +	/* Create and connect one notifier for each synchronization fence. */
> > +	for (FrameBuffer *buffer : pending_) {
> > +		bufferData = buffer->_d();
> > +
> > +		if (!bufferData->fence())
> > +			continue;
> > +
> > +		int fenceFd = bufferData->fence()->fd().fd();
> > +		notifiers_.emplace(new EventNotifier(fenceFd, EventNotifier::Read));
>
> There's no bug in your code, but usage of new() indicates potential
> memory leaks, so it's best to minimize its usage. I'd write
>
> 		notifiers_.insert(std::make_unique<EventNotifier>(fenceFd, EventNotifier::Read));
>
> > +	}
> > +
> > +	if (notifiers_.empty()) {
> > +		status_ = Status::Ready;
> > +		return status_;
> > +	}
> > +
> > +	for (const std::unique_ptr<EventNotifier> &notifier : notifiers_)
> > +		notifier->activated.connect(this, [this, &notifier] {
> > +							notifierActivated(notifier);
> > +					        });
> > +
> > +	/* In case a timeout is specified, create a timer and set it up. */
>
> Let's add
>
> 	 * The timer must be created here instead of in the Request constructor,
> 	 * in order to be bound to the pipeline handler thread.
>
> > +	if (timeout != 0ms) {
> > +		timer_ = std::make_unique<Timer>();
> > +		timer_->timeout.connect(this, &Request::Private::timeout);
> > +		timer_->start(timeout);
> > +	}
> > +
> > +	return Status::Pending;
> >  }
> > +
> > +/**
> > + * \var Request::Private::prepared
> > + * \brief Request preparation completed Signal
> > + */
> > +
> >  /**
> >   * \var Request::Private::sequence_
> >   * \brief The request sequence number
> > @@ -191,6 +296,51 @@ void Request::Private::reuse()
> >   * \copydoc Request::sequence()
> >   */
> >
> > +void Request::Private::notifierActivated(const std::unique_ptr<EventNotifier> &notifier)
> > +{
> > +	auto it = notifiers_.find(notifier);
> > +	ASSERT(it != notifiers_.end());
> > +
> > +	/* We need to close the fence if successfully signalled. */
> > +	int fd = notifier->fd();
> > +	bool found = false;
> > +	for (FrameBuffer *buffer : pending_) {
> > +		FrameBuffer::Private *bufferData = buffer->_d();
> > +
> > +		if (!bufferData->fence())
> > +			continue;
> > +
> > +		if (bufferData->fence()->fd().fd() != fd)
> > +			continue;
>
> If you turned notifiers_ into a map indexed by FrameBuffer *, you could
> pass the FrameBuffer pointer instead of the notifier to the
> notifierActivated() function, and avoid this loop.

That's a very nice idea!

>
> > +
> > +		bufferData->closeFence();
> > +		found = true;
> > +		break;
> > +	}
> > +	ASSERT(found);
> > +
> > +	notifiers_.erase(it);
> > +	if (!notifiers_.empty())
> > +		return;
> > +
> > +	/* All fences completed, delete the timer and move to state Ready. */
> > +	timer_.reset();
> > +	status_ = Status::Ready;
> > +	prepared.emit();
> > +}
> > +
> > +void Request::Private::timeout()
> > +{
> > +	/* A timeout can only happen if there are fences not yet signalled. */
> > +	ASSERT(!notifiers_.empty());
> > +	notifiers_.clear();
> > +
> > +	LOG(Request, Error) << "Request prepare timeout";
>
> Could you print the cookie to ease debugging ?
>

Indeed

> > +
> > +	status_ = Status::Failed;
> > +	prepared.emit();
> > +}
> > +
> >  /**
> >   * \enum Request::Status
> >   * Request completion status
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series

diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
index 59bddde3a090..26b25fb12261 100644
--- a/include/libcamera/internal/request.h
+++ b/include/libcamera/internal/request.h
@@ -7,10 +7,16 @@ 
 #ifndef __LIBCAMERA_INTERNAL_REQUEST_H__
 #define __LIBCAMERA_INTERNAL_REQUEST_H__
 
+#include <chrono>
 #include <memory>
 
+#include <libcamera/base/event_notifier.h>
+#include <libcamera/base/timer.h>
+
 #include <libcamera/request.h>
 
+using namespace std::chrono_literals;
+
 namespace libcamera {
 
 class Camera;
@@ -21,6 +27,12 @@  class Request::Private : public Extensible::Private
 	LIBCAMERA_DECLARE_PUBLIC(Request)
 
 public:
+	enum class Status {
+		Pending,
+		Ready,
+		Failed
+	};
+
 	Private(Camera *camera);
 	~Private();
 
@@ -29,21 +41,30 @@  public:
 
 	uint64_t cookie() const;
 	Request::Status status() const;
+	Status privateStatus() const { return status_; }
 
 	bool completeBuffer(FrameBuffer *buffer);
 	void complete();
 	void cancel();
 	void reuse();
 
+	Status prepare(std::chrono::milliseconds timeout = 0ms);
+	Signal<> prepared;
+
 	uint32_t sequence_ = 0;
 
 private:
 	void _cancel();
+	void notifierActivated(const std::unique_ptr<EventNotifier> &notifier);
+	void timeout();
 
 	Camera *camera_;
 	bool cancelled_;
+	Status status_ = Status::Pending;
 
 	std::unordered_set<FrameBuffer *> pending_;
+	std::unordered_set<std::unique_ptr<EventNotifier>> notifiers_;
+	std::unique_ptr<Timer> timer_;
 };
 
 } /* namespace libcamera */
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 1d47698a6263..98f9719e5cf2 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -43,6 +43,22 @@  LOG_DEFINE_CATEGORY(Request)
  * subclasses).
  */
 
+/**
+ * \enum Request::Private::Status
+ * \brief Request private status
+ *
+ * The Request private status describes the lifecycle of the Request between
+ * the time it is queued to the Camera and the time it is queued to the device.
+ *
+ * A Request is created in Status::Pending state. Before actually queueing the
+ * Request the PipelineHandler base class prepare() the Request, whose status
+ * transitions to either Status::Ready or Status::Failed.
+ *
+ * \var Request::Private::Status::Pending
+ * \var Request::Private::Status::Ready
+ * \var Request::Private::Status::Failed
+ */
+
 /**
  * \brief Create a Request::Private
  * \param camera The Camera that creates the request
@@ -95,6 +111,17 @@  Request::Status Request::Private::status() const
 	return _o<Request>()->status();
 }
 
+/**
+ * \fn Request::Private::privateStatus()
+ * \brief Retrieve the Request private status
+ *
+ * The private status, as described by the Request::Private:Status enumeration,
+ * describes the Request status between the time it is queued to the Camera and
+ * the time the Request is applied to the hardware.
+ *
+ * \return The Request private state
+ */
+
 /**
  * \brief Complete a buffer for the request
  * \param[in] buffer The buffer that has completed
@@ -155,6 +182,8 @@  void Request::Private::_cancel()
 
 	cancelled_ = true;
 	pending_.clear();
+	notifiers_.clear();
+	timer_.reset();
 }
 
 /**
@@ -182,8 +211,84 @@  void Request::Private::reuse()
 {
 	sequence_ = 0;
 	cancelled_ = false;
+	status_ = Status::Pending;
 	pending_.clear();
+	notifiers_.clear();
+	timer_.reset();
+}
+
+/**
+ * \brief Prepare the Request to be queued to the device
+ * \param[in] timeout Optional expiration timeout
+ *
+ * Prepare a Request to be queued to the hardware device it by ensuring it is
+ * ready for the incoming memory transfers.
+ *
+ * This currently means waiting on each frame buffer acquire fence to be
+ * signalled. An optional expiration timeout can be specified. If not all the
+ * fences have been signalled correctly before the timeout expires the Request
+ * is marked as Failed, otherwise it is set to the Ready state.
+ *
+ * \sa Request::Private::Status
+ *
+ * The function returns Status::Ready if all the prepare operations have been
+ * completed synchronously. If Status::Ready is returned the Request can be
+ * queued immediately and the prepared signal is not emitted. If instead the
+ * prepare operation requires to wait the completion of asynchronous events,
+ * such as fence notifications or timer expiration this function returns
+ * Status::Pending and the asynchronous event completion is notified by emitting
+ * the prepared signal.
+ *
+ * As we currently only handle fences, the function return Status::Ready if
+ * there are no fences to wait on. Status::Prepared is otherwise returned and
+ * the prepared signal is emitted when all fences have been signalled or the
+ * optional timeout has expired.
+ *
+ * The intended user of this function is the PipelineHandler base class, which
+ * 'prepares' a Request before queuing it to the hardware device.
+ *
+ * \return The Request status
+ */
+Request::Private::Status Request::Private::prepare(std::chrono::milliseconds timeout)
+{
+	FrameBuffer::Private *bufferData;
+
+	/* Create and connect one notifier for each synchronization fence. */
+	for (FrameBuffer *buffer : pending_) {
+		bufferData = buffer->_d();
+
+		if (!bufferData->fence())
+			continue;
+
+		int fenceFd = bufferData->fence()->fd().fd();
+		notifiers_.emplace(new EventNotifier(fenceFd, EventNotifier::Read));
+	}
+
+	if (notifiers_.empty()) {
+		status_ = Status::Ready;
+		return status_;
+	}
+
+	for (const std::unique_ptr<EventNotifier> &notifier : notifiers_)
+		notifier->activated.connect(this, [this, &notifier] {
+							notifierActivated(notifier);
+					        });
+
+	/* In case a timeout is specified, create a timer and set it up. */
+	if (timeout != 0ms) {
+		timer_ = std::make_unique<Timer>();
+		timer_->timeout.connect(this, &Request::Private::timeout);
+		timer_->start(timeout);
+	}
+
+	return Status::Pending;
 }
+
+/**
+ * \var Request::Private::prepared
+ * \brief Request preparation completed Signal
+ */
+
 /**
  * \var Request::Private::sequence_
  * \brief The request sequence number
@@ -191,6 +296,51 @@  void Request::Private::reuse()
  * \copydoc Request::sequence()
  */
 
+void Request::Private::notifierActivated(const std::unique_ptr<EventNotifier> &notifier)
+{
+	auto it = notifiers_.find(notifier);
+	ASSERT(it != notifiers_.end());
+
+	/* We need to close the fence if successfully signalled. */
+	int fd = notifier->fd();
+	bool found = false;
+	for (FrameBuffer *buffer : pending_) {
+		FrameBuffer::Private *bufferData = buffer->_d();
+
+		if (!bufferData->fence())
+			continue;
+
+		if (bufferData->fence()->fd().fd() != fd)
+			continue;
+
+		bufferData->closeFence();
+		found = true;
+		break;
+	}
+	ASSERT(found);
+
+	notifiers_.erase(it);
+	if (!notifiers_.empty())
+		return;
+
+	/* All fences completed, delete the timer and move to state Ready. */
+	timer_.reset();
+	status_ = Status::Ready;
+	prepared.emit();
+}
+
+void Request::Private::timeout()
+{
+	/* A timeout can only happen if there are fences not yet signalled. */
+	ASSERT(!notifiers_.empty());
+	notifiers_.clear();
+
+	LOG(Request, Error) << "Request prepare timeout";
+
+	status_ = Status::Failed;
+	prepared.emit();
+}
+
 /**
  * \enum Request::Status
  * Request completion status