From 185fe3d4b4d1264b2d72f8c92d17e248092efc46 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Fri, 12 Jul 2019 17:17:35 +0300
Subject: libcamera: pipeline_handler: Simplify request completion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

libcamera guarantees that requests complete in sequence. This
requirement is currently pushed down to pipeline handlers. Three out of
four of our pipeline handlers implement that requirement based on the
sole assumption that buffers will always complete in sequeuence, while
the IPU3 pipeline handler implements a more complex logic.

It turns out that the logic can be moved to the base PipelineHandler
class with support from the Request class. Do so to simplify the
pipeline handlers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 10 ++--------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp |  4 +---
 src/libcamera/pipeline/uvcvideo.cpp      |  2 +-
 src/libcamera/pipeline/vimc.cpp          |  2 +-
 src/libcamera/pipeline_handler.cpp       | 24 ++++++++++++++++--------
 5 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index e4efb972..1abd20e5 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -927,14 +927,8 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
 		/* Request not completed yet, return here. */
 		return;
 
-	/* Complete the pending requests in queueing order. */
-	while (1) {
-		request = queuedRequests_.front();
-		if (request->hasPendingBuffers())
-			break;
-
-		pipe_->completeRequest(camera_, request);
-	}
+	/* Mark the request as complete. */
+	pipe_->completeRequest(camera_, request);
 }
 
 /**
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4a5898d2..38323643 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -491,9 +491,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)
 {
 	ASSERT(activeCamera_);
-
-	RkISP1CameraData *data = cameraData(activeCamera_);
-	Request *request = data->queuedRequests_.front();
+	Request *request = buffer->request();
 
 	completeBuffer(activeCamera_, request, buffer);
 	completeRequest(activeCamera_, request);
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 72313cfd..387d71ef 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -382,7 +382,7 @@ int UVCCameraData::init(MediaEntity *entity)
 
 void UVCCameraData::bufferReady(Buffer *buffer)
 {
-	Request *request = queuedRequests_.front();
+	Request *request = buffer->request();
 
 	pipe_->completeBuffer(camera_, request, buffer);
 	pipe_->completeRequest(camera_, request);
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index f8a58be0..ff2529c9 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -376,7 +376,7 @@ int VimcCameraData::init(MediaDevice *media)
 
 void VimcCameraData::bufferReady(Buffer *buffer)
 {
-	Request *request = queuedRequests_.front();
+	Request *request = buffer->request();
 
 	pipe_->completeBuffer(camera_, request, buffer);
 	pipe_->completeRequest(camera_, request);
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 0283e4e5..c6092002 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -391,8 +391,9 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)
  * This method shall be called by pipeline handlers to signal completion of the
  * \a buffer part of the \a request. It notifies applications of buffer
  * completion and updates the request's internal buffer tracking. The request
- * is not completed automatically when the last buffer completes, pipeline
- * handlers shall complete requests explicitly with completeRequest().
+ * is not completed automatically when the last buffer completes to give
+ * pipeline handlers a chance to perform any operation that may still be
+ * needed. They shall complete requests explicitly with completeRequest().
  *
  * \return True if all buffers contained in the request have completed, false
  * otherwise
@@ -413,17 +414,24 @@ bool PipelineHandler::completeBuffer(Camera *camera, Request *request,
  * request has completed. The request is deleted and shall not be accessed once
  * this method returns.
  *
- * The pipeline handler shall ensure that requests complete in the same order
- * they are submitted.
+ * This method ensures that requests will be returned to the application in
+ * submission order, the pipeline handler may call it on any complete request
+ * without any ordering constraint.
  */
 void PipelineHandler::completeRequest(Camera *camera, Request *request)
 {
+	request->complete(Request::RequestComplete);
+
 	CameraData *data = cameraData(camera);
-	ASSERT(request == data->queuedRequests_.front());
-	data->queuedRequests_.pop_front();
 
-	request->complete(Request::RequestComplete);
-	camera->requestComplete(request);
+	while (!data->queuedRequests_.empty()) {
+		request = data->queuedRequests_.front();
+		if (request->hasPendingBuffers())
+			break;
+
+		data->queuedRequests_.pop_front();
+		camera->requestComplete(request);
+	}
 }
 
 /**
-- 
cgit v1.2.1