[libcamera-devel] [PATCH 2/2] libcamera: pipeline-handler: Consider max in-flight requests constraint

[Umang Jain]

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 (for e.g. the IPA).

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 PipelineHandler
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 and RkISP1 pipeline handlers. This will
prevent over-writing of frame contexts (i.e. FCQueue) in these two
pipeline handlers.

Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
---
 include/libcamera/internal/pipeline_handler.h |  4 ++
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  1 +
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  1 +
 src/libcamera/pipeline_handler.cpp            | 51 ++++++++++++++++++-
 4 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index ec4f662d..83f8bd9f 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -70,6 +70,7 @@ public:
 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;
@@ -97,6 +98,9 @@ private:
 	Mutex lock_;
 	unsigned int useCount_ LIBCAMERA_TSA_GUARDED_BY(lock_);
 
+	uint32_t maxQueueRequests_;
+	uint32_t requestsQueueCounter_;
+
 	friend class PipelineHandlerFactoryBase;
 };
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index e4d79ea4..d1d42f78 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -422,6 +422,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
 PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)
 	: PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr)
 {
+	setMaxQueueRequests(ipa::ipu3::kMaxFrameContexts);
 }
 
 std::unique_ptr<CameraConfiguration>
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index eb9ad65c..a48adba9 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -599,6 +599,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
 	: PipelineHandler(manager), hasSelfPath_(true)
 {
+	setMaxQueueRequests(ipa::rkisp1::kMaxFrameContexts);
 }
 
 std::unique_ptr<CameraConfiguration>
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index cfade490..103f9db0 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -67,7 +67,8 @@ LOG_DEFINE_CATEGORY(Pipeline)
  * through the PipelineHandlerFactoryBase::create() function.
  */
 PipelineHandler::PipelineHandler(CameraManager *manager)
-	: manager_(manager), useCount_(0)
+	: manager_(manager), useCount_(0), maxQueueRequests_(0),
+	  requestsQueueCounter_(0)
 {
 }
 
@@ -428,6 +429,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_) {
@@ -455,6 +459,10 @@ void PipelineHandler::doQueueRequests()
 		if (!request->_d()->prepared_)
 			break;
 
+		if (maxQueueRequests_ &&
+		    requestsQueueCounter_ >= maxQueueRequests_)
+			break;
+
 		doQueueRequest(request);
 		waitingRequests_.pop();
 	}
@@ -531,6 +539,9 @@ void PipelineHandler::completeRequest(Request *request)
 		ASSERT(!req->hasPendingBuffers());
 		data->queuedRequests_.pop_front();
 		camera->requestComplete(req);
+
+		if (maxQueueRequests_)
+			requestsQueueCounter_--;
 	}
 }
 
@@ -647,6 +658,44 @@ 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.
+ */
+
+/**
+ * \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.38.1