[libcamera-devel] [PATCH v1 3/4] ipa: ipu3: Prevent over queuing of requests

[Umang Jain]

The IPAIPU3 processes each IPAFrameContext present in the FCQueue
ring buffer. If the application queues the requests at a pace where
IPAFrameContext(s) can get potentially over-written without getting
processed by the IPA, it would lead to un-desirable effects.

Hence, inspect the FCQueue if it is full and reject any further
queuing of requests. This requires changes to the IPAIPU3 mojom
interface. The IPAIPU3::queueRequest() cannot be [async] anymore
since we need to return a success/failure value.

The rejection by the IPA doesn't mean it cannot be queued
later. The request is still preserved in
IPU3CameraData::pendingRequests_. It shall be queued sequentially
on subsequent calls to  IPU3CameraData::queuePendingRequests().

Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
---
I am not super-happy with changing the interface and dropping [async]
but this is what I've for now. Would appreciate any other ideas...
---
 include/libcamera/ipa/ipu3.mojom     |  2 +-
 src/ipa/ipu3/ipu3.cpp                | 11 +++++++++--
 src/libcamera/pipeline/ipu3/ipu3.cpp | 10 +++++++++-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
index d1b1c6b8..3faffd9c 100644
--- a/include/libcamera/ipa/ipu3.mojom
+++ b/include/libcamera/ipa/ipu3.mojom
@@ -30,7 +30,7 @@ interface IPAIPU3Interface {
 	mapBuffers(array<libcamera.IPABuffer> buffers);
 	unmapBuffers(array<uint32> ids);
 
-	[async] queueRequest(uint32 frame, libcamera.ControlList controls);
+	queueRequest(uint32 frame, libcamera.ControlList controls) => (int32 ret);
 	[async] fillParamsBuffer(uint32 frame, uint32 bufferId);
 	[async] processStatsBuffer(uint32 frame, int64 frameTimestamp,
 				   uint32 bufferId, libcamera.ControlList sensorControls);
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 1d6ee515..63aa9b56 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -145,7 +145,7 @@ public:
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
 
-	void queueRequest(const uint32_t frame, const ControlList &controls) override;
+	int queueRequest(const uint32_t frame, const ControlList &controls) override;
 	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;
 	void processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,
 				const uint32_t bufferId,
@@ -616,11 +616,18 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
  *
  * Parse the request to handle any IPA-managed controls that were set from the
  * application such as manual sensor settings.
+ *
+ * \return 0 if successful, -EPERM otherwise.
  */
-void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)
+int IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)
 {
+	if (context_.frameContexts.isFull())
+		return -EPERM;
+
 	/* \todo Start processing for 'frame' based on 'controls'. */
 	*context_.frameContexts.get(frame) = { frame, controls };
+
+	return 0;
 }
 
 /**
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index fd989e61..d69e28d4 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -874,7 +874,15 @@ void IPU3CameraData::queuePendingRequests()
 
 		info->rawBuffer = rawBuffer;
 
-		ipa_->queueRequest(info->id, request->controls());
+		/*
+		 * Queueing request to the IPA can fail in cases like over-flowing
+		 * its queue. Hence, keep the request preserved in pendingRequests_
+		 * if a failure occurs.
+		 */
+		if (ipa_->queueRequest(info->id, request->controls()) != 0) {
+			frameInfos_.remove(info);
+			break;
+		}
 
 		pendingRequests_.pop();
 		processingRequests_.push(request);
-- 
2.31.1