[libcamera-devel] [PATCH v3 3/4] libcamera: pipeline-handler: Consider max in-flight requests constraint
Jacopo Mondi
jacopo at jmondi.org
Mon Jun 20 12:42:32 CEST 2022
Hi Umang
to be honest I was expecting this to be much simpler, with a the
size of the FCQueue being a compile-time constant (exposed as camera
property) and the ph base class counting the number of in-flight
requests against it.
I might have missed the use case for run-time configuration of the
FCQueue size...
On Mon, Jun 20, 2022 at 02:10:23AM +0530, Umang Jain via libcamera-devel wrote:
> This patch introduces a new constraint in the pipeline handler base
> class which allows various derived pipeline handlers to set and
> restrict the number of maximum in-flight requests that can be queued to
> the underlying components.
>
> The pipeline handler is now equipped with the responsibility of not to
> over queue the requests to the underlying layers. The derived
> pipeline handler (or even IPA) can also have various kind of requests
> queue(arrays, queues or ring-buffer) hence, it might be an issue where
> the application queues the requests at a rate where these kind of
> queues can over-flow. The patch ensures that the base pipeline-handler
> will never let the requests overflow to underlying layers, once the
> derived pipeline handler sets its maximum capacity of handling
> in-flight requests using PipelineHandler::setMaxQueueRequests().
>
> The queue request management introduced in the pipeline handler base
> class is now used by the IPU3 pipeline handler.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> include/libcamera/internal/pipeline_handler.h | 6 ++
> include/libcamera/ipa/ipu3.mojom | 1 +
> src/ipa/ipu3/ipa_context.cpp | 18 ++++--
> src/ipa/ipu3/ipa_context.h | 5 +-
> src/ipa/ipu3/ipu3.cpp | 3 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++
> src/libcamera/pipeline_handler.cpp | 61 ++++++++++++++++++-
> 7 files changed, 94 insertions(+), 9 deletions(-)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c3e4c258..5a319011 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -67,9 +67,12 @@ public:
>
> const char *name() const { return name_; }
>
> + uint32_t maxQueueRequests() { return maxQueueRequests_; }
> +
> protected:
> void registerCamera(std::shared_ptr<Camera> camera);
> void hotplugMediaDevice(MediaDevice *media);
> + void setMaxQueueRequests(uint32_t maxRequests);
>
> virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
> virtual void stopDevice(Camera *camera) = 0;
> @@ -88,6 +91,9 @@ private:
>
> std::queue<Request *> waitingRequests_;
>
> + uint32_t maxQueueRequests_;
> + uint32_t requestsQueueCounter_;
> +
> const char *name_;
>
> Mutex lock_;
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index d1b1c6b8..3e7bbc6e 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -14,6 +14,7 @@ struct IPAConfigInfo {
> libcamera.ControlInfoMap lensControls;
> libcamera.Size bdsOutputSize;
> libcamera.Size iif;
> + uint32 maxQueueRequests;
> };
>
> interface IPAIPU3Interface {
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 60c58b97..564f8e6c 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -248,7 +248,7 @@ IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)
> * \brief FCQueue constructor
> */
> FCQueue::FCQueue()
> - : latestFC_(0), oldestFC_(0), isFull_(false)
> + : latestFC_(0), oldestFC_(0), size_(0), isFull_(false)
> {
> }
>
> @@ -269,7 +269,7 @@ FCQueue::FCQueue()
> IPAFrameContext *FCQueue::get(uint32_t frame)
> {
> if (latestFC_ && frame <= latestFC_) {
> - IPAFrameContext &frameContext = this->at(frame % kMaxFrameContexts);
> + IPAFrameContext &frameContext = this->at(frame % size_);
> if (frame != frameContext.frame)
> LOG(IPAIPU3, Warning)
> << "Got wrong frame context for frame " << frame;
> @@ -287,10 +287,10 @@ IPAFrameContext *FCQueue::get(uint32_t frame)
> latestFC_ = frame;
>
> /* Check if the queue is full to avoid over-queueing later */
> - if (oldestFC_ && latestFC_ - oldestFC_ >= kMaxFrameContexts - 1)
> + if (oldestFC_ && latestFC_ - oldestFC_ >= size_ - 1)
> isFull_ = true;
>
> - return &this->at(latestFC_ % kMaxFrameContexts);
> + return &this->at(latestFC_ % size_);
> }
> }
>
> @@ -329,6 +329,7 @@ void FCQueue::completeFrame(uint32_t frame)
> */
> void FCQueue::setSize(uint32_t size)
> {
> + size_ = size;
> resize(size);
> }
>
> @@ -338,6 +339,12 @@ void FCQueue::setSize(uint32_t size)
> * \return True of FCQueue is full, False otherwise
> */
>
> +/**
> + * \fn FCQueue::size()
> + * \brief The number of IPAFrameContext(s) FCQueue handles
> + * \return Returns the size of FCQueue
> + */
> +
> /**
> * \brief Clear the FCQueue by resetting all the entries in the ring-buffer
> */
> @@ -345,6 +352,9 @@ void FCQueue::reset()
> {
> clear();
>
> + if (size_)
> + setSize(size_);
> +
> isFull_ = false;
>
> latestFC_ = 0;
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 52a0b5e7..0da2ad26 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -21,9 +21,6 @@ namespace libcamera {
>
> namespace ipa::ipu3 {
>
> -/* Maximum number of frame contexts to be held */
> -static constexpr uint32_t kMaxFrameContexts = 16;
> -
> struct IPASessionConfiguration {
> struct {
> ipu3_uapi_grid_config bdsGrid;
> @@ -100,10 +97,12 @@ public:
>
> IPAFrameContext *get(uint32_t frame);
> bool isFull() { return isFull_; }
> + uint32_t size() { return size_; }
>
> private:
> uint32_t latestFC_;
> uint32_t oldestFC_;
> + uint32_t size_;
>
> bool isFull_;
> };
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 15070fa0..3218df49 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -458,8 +458,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>
> /* Clean IPAActiveState at each reconfiguration. */
> context_.activeState = {};
> +
> context_.frameContexts.reset();
> - context_.frameContexts.setSize(kMaxFrameContexts);
> + context_.frameContexts.setSize(configInfo.maxQueueRequests);
>
> if (!validateSensorControls()) {
> LOG(IPAIPU3, Error) << "Sensor control validation failed.";
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index b7dda282..d599fb95 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -672,6 +672,15 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> configInfo.bdsOutputSize = config->imguConfig().bds;
> configInfo.iif = config->imguConfig().iif;
>
> + /*
> + * \todo Find a better position where maxQueueRequests can be
> + * defined and fetched. Maybe in IPA Interface? Camera property?
> + * Not sure.
> + */
> + uint32_t maxQueueRequests = 16;
> + configInfo.maxQueueRequests = maxQueueRequests;
> + setMaxQueueRequests(maxQueueRequests);
> +
> ret = data->ipa_->configure(configInfo, &data->ipaControls_);
> if (ret) {
> LOG(IPU3, Error) << "Failed to configure IPA: "
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 7ebd76ad..08ec28de 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -68,7 +68,8 @@ LOG_DEFINE_CATEGORY(Pipeline)
> * respective factories.
> */
> PipelineHandler::PipelineHandler(CameraManager *manager)
> - : manager_(manager), lockOwner_(false)
> + : manager_(manager), maxQueueRequests_(0), requestsQueueCounter_(0),
> + lockOwner_(false)
> {
> }
>
> @@ -312,6 +313,9 @@ void PipelineHandler::stop(Camera *camera)
> /* Make sure no requests are pending. */
> Camera::Private *data = camera->_d();
> ASSERT(data->queuedRequests_.empty());
> +
> + maxQueueRequests_ = 0;
> + requestsQueueCounter_ = 0;
> }
>
> /**
> @@ -397,6 +401,9 @@ void PipelineHandler::doQueueRequest(Request *request)
> Camera::Private *data = camera->_d();
> data->queuedRequests_.push_back(request);
>
> + if (maxQueueRequests_)
> + requestsQueueCounter_++;
> +
> request->_d()->sequence_ = data->requestSequence_++;
>
> if (request->_d()->cancelled_) {
> @@ -424,6 +431,10 @@ void PipelineHandler::doQueueRequests()
> if (!request->_d()->prepared_)
> break;
>
> + if (maxQueueRequests_ &&
> + requestsQueueCounter_ >= maxQueueRequests_)
> + break;
> +
> doQueueRequest(request);
> waitingRequests_.pop();
> }
> @@ -500,6 +511,9 @@ void PipelineHandler::completeRequest(Request *request)
> ASSERT(!req->hasPendingBuffers());
> data->queuedRequests_.pop_front();
> camera->requestComplete(req);
> +
> + if (maxQueueRequests_)
> + requestsQueueCounter_--;
> }
> }
>
> @@ -616,6 +630,51 @@ void PipelineHandler::disconnect()
> * constant for the whole lifetime of the pipeline handler.
> */
>
> +/**
> + * \var PipelineHandler::maxQueueRequests_
> + * \brief Maximum number of in-flight requests that can be queued
> + *
> + * A hardware can handle a certain number of maximum requests at a given
> + * point. If such a constraint exists, set maxQueueRequests_ via
> + * \a setMaxQueueRequests() in the derived pipeline handler.
> + *
> + * The derived pipeline handler can choose not to define such constraint as
> + * well. In that case, the derived pipeline handler can avoid setting
> + * \a setMaxQueueReqeuests(), hence \a maxQueueRequests_ and
> + * \a requestsQueueCounter_ will be 0.
> + */
> +
> +/**
> + * \var PipelineHandler::requestsQueueCounter_
> + * \brief Number of requests queued to the underlying hardware
> + *
> + * If \a setMaxQueueRequests() is set by the derived pipeline handler,
> + * requestsQueueCounter_ reflects the number of requests queued
> + * to the underlying hardware by the pipeline handler.
> + */
> +
> +/**
> + * \fn PipelineHandler::maxQueueRequests()
> + * \brief Retrieve the maximum number of in-flight requests handled by the
> + * underlying hardware
> + * \return Number of requests that can be queued to underlying hardware
> + */
> +
> +/**
> + * \brief Sets the maximum number of requests that can be queued
> + * \param[in] maxRequests Maximum number of in-flight requests
> + *
> + * A hardware can handle a certain number of requests at a given point.
> + * This function sets the maximum number of in-flight requests that can
> + * be queued to the hardware by the pipeline handler. Each derived pipeline
> + * handler should set the maximum number of in-flight requests it can handle
> + * at a given point using this function, if at all such a constraint exists.
> + */
> +void PipelineHandler::setMaxQueueRequests(uint32_t maxRequests)
> +{
> + maxQueueRequests_ = maxRequests;
> +}
> +
> /**
> * \fn PipelineHandler::name()
> * \brief Retrieve the pipeline handler name
> --
> 2.31.1
>
More information about the libcamera-devel
mailing list