libcamera: pipeline: rkisp1: Avoid race when processing parameter buffers

When moving the pipeline away from the Timeline design it was discovered
that the design of queuing the buffers to the device as soon as possible
was not the best idea. The parameter buffers were queued to the device
before the IPA had processed them and this violates the V4L2 API.

Fix this by waiting to queue any buffer to the device until the IPA has
filled in the parameters buffer.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

1 files changed, 13 insertions, 28 deletions

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 58ae8f98..e85979a7 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -51,7 +51,6 @@ struct RkISP1FrameInfo {
 	FrameBuffer *mainPathBuffer;
 	FrameBuffer *selfPathBuffer;
 
-	bool paramFilled;
 	bool paramDequeued;
 	bool metadataProcessed;
 };
@@ -219,7 +218,6 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
 	info->mainPathBuffer = mainPathBuffer;
 	info->selfPathBuffer = selfPathBuffer;
 	info->statBuffer = statBuffer;
-	info->paramFilled = false;
 	info->paramDequeued = false;
 	info->metadataProcessed = false;
 
@@ -322,9 +320,20 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
 		break;
 	}
 	case RKISP1_IPA_ACTION_PARAM_FILLED: {
+		PipelineHandlerRkISP1 *pipe = static_cast<PipelineHandlerRkISP1 *>(pipe_);
 		RkISP1FrameInfo *info = frameInfo_.find(frame);
-		if (info)
-			info->paramFilled = true;
+		if (!info)
+			break;
+
+		pipe->param_->queueBuffer(info->paramBuffer);
+		pipe->stat_->queueBuffer(info->statBuffer);
+
+		if (info->mainPathBuffer)
+			mainPath_->queueBuffer(info->mainPathBuffer);
+
+		if (info->selfPathBuffer)
+			selfPath_->queueBuffer(info->selfPathBuffer);
+
 		break;
 	}
 	case RKISP1_IPA_ACTION_METADATA:
@@ -852,15 +861,6 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
 	op.controls = { request->controls() };
 	data->ipa_->processEvent(op);
 
-	param_->queueBuffer(info->paramBuffer);
-	stat_->queueBuffer(info->statBuffer);
-
-	if (info->mainPathBuffer)
-		mainPath_.queueBuffer(info->mainPathBuffer);
-
-	if (info->selfPathBuffer)
-		selfPath_.queueBuffer(info->selfPathBuffer);
-
 	data->frame_++;
 
 	return 0;
@@ -1009,7 +1009,6 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
 	selfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
 	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
-	isp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart);
 	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
 
 	/*
@@ -1097,20 +1096,6 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 	data->ipa_->processEvent(op);
 }
 
-void PipelineHandlerRkISP1::frameStart(uint32_t sequence)
-{
-	if (!activeCamera_)
-		return;
-
-	RkISP1CameraData *data = cameraData(activeCamera_);
-
-	RkISP1FrameInfo *info = data->frameInfo_.find(sequence);
-	if (!info || !info->paramFilled)
-		LOG(RkISP1, Info)
-			<< "Parameters not ready on time for frame "
-			<< sequence;
-}
-
 REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)
 
 } /* namespace libcamera */