From patchwork Mon Jan 18 14:06:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10884 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2A47DBD808 for ; Mon, 18 Jan 2021 14:06:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B24F068123; Mon, 18 Jan 2021 15:06:30 +0100 (CET) Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AE206010B for ; Mon, 18 Jan 2021 15:06:29 +0100 (CET) X-Halon-ID: 588cee81-5996-11eb-b73f-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 588cee81-5996-11eb-b73f-0050569116f7; Mon, 18 Jan 2021 15:06:27 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 18 Jan 2021 15:06:15 +0100 Message-Id: <20210118140615.2332504-1-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.30.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: Avoid race when processing parameter buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 where 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 have filled in the parameters buffer. Signed-off-by: Niklas Söderlund Tested-by: Sebastian Fricke Reviewed-by: Laurent Pinchart --- Hello, This patch is based on-top of '[PATCH v5 0/8] libcamera: Add helper for controls that take effect with a delay' as the change is much cleaner on-top of the removal of the Timeline. --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 41 ++++++++---------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 58ae8f98a5c683d5..e85979a719b7ced6 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(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 */