From patchwork Tue Nov 10 00:27:09 2020 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: 10407 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 64D3FBE084 for ; Tue, 10 Nov 2020 00:28:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 40C9D630C2; Tue, 10 Nov 2020 01:28:05 +0100 (CET) Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 69E39630C0 for ; Tue, 10 Nov 2020 01:28:02 +0100 (CET) X-Halon-ID: c6e6ca33-22ea-11eb-bcc0-005056917f90 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id c6e6ca33-22ea-11eb-bcc0-005056917f90; Tue, 10 Nov 2020 01:22:12 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, naush@raspberrypi.com Date: Tue, 10 Nov 2020 01:27:09 +0100 Message-Id: <20201110002710.3233696-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201110002710.3233696-1-niklas.soderlund@ragnatech.se> References: <20201110002710.3233696-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 7/8] libcamera: pipeline: rkisp1: Use SOF event to warn about late parameters 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" In the Timeline approach the idea was to delay queuing buffers to the device until the IPA had a chance to prepare the parameters buffer. A check was still added to warn if the IPA queued buffers before the parameters buffer was filled in. This check happened much sooner then needed as the parameter buffers does not have to be ready when the buffer is queued but just before its consumed. As the pipeline now has true knowledge of each SOF we can move the check there and remove the delaying of queuing of buffers. This change really speeds up the IPA reactions as the delays used in the Timeline where approximated while with this change they are driven by events reported by the device. Signed-off-by: Niklas Söderlund --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 78 ++++++++---------------- 1 file changed, 24 insertions(+), 54 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 6b71fdd1db00460b..0879644af69fe7de 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -40,7 +40,6 @@ namespace libcamera { LOG_DEFINE_CATEGORY(RkISP1) class PipelineHandlerRkISP1; -class RkISP1ActionQueueBuffers; class RkISP1CameraData; enum RkISP1ActionType { @@ -203,7 +202,6 @@ private: PipelineHandler::cameraData(camera)); } - friend RkISP1ActionQueueBuffers; friend RkISP1CameraData; friend RkISP1Frames; @@ -214,6 +212,7 @@ private: void bufferReady(FrameBuffer *buffer); void paramReady(FrameBuffer *buffer); void statReady(FrameBuffer *buffer); + void frameStart(uint32_t sequence); int allocateBuffers(Camera *camera); int freeBuffers(Camera *camera); @@ -348,53 +347,6 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request) return nullptr; } -class RkISP1ActionQueueBuffers : public FrameAction -{ -public: - RkISP1ActionQueueBuffers(unsigned int frame, RkISP1CameraData *data, - PipelineHandlerRkISP1 *pipe) - : FrameAction(frame, QueueBuffers), data_(data), pipe_(pipe) - { - } - -protected: - void run() override - { - RkISP1FrameInfo *info = data_->frameInfo_.find(frame()); - if (!info) - LOG(RkISP1, Fatal) << "Frame not known"; - - /* - * \todo: If parameters are not filled a better method to handle - * the situation than queuing a buffer with unknown content - * should be used. - * - * It seems excessive to keep an internal zeroed scratch - * parameters buffer around as this should not happen unless the - * devices is under too much load. Perhaps failing the request - * and returning it to the application with an error code is - * better than queue it to hardware? - */ - if (!info->paramFilled) - LOG(RkISP1, Error) - << "Parameters not ready on time for frame " - << frame(); - - pipe_->param_->queueBuffer(info->paramBuffer); - pipe_->stat_->queueBuffer(info->statBuffer); - - if (info->mainPathBuffer) - pipe_->mainPath_.queueBuffer(info->mainPathBuffer); - - if (info->selfPathBuffer) - pipe_->selfPath_.queueBuffer(info->selfPathBuffer); - } - -private: - RkISP1CameraData *data_; - PipelineHandlerRkISP1 *pipe_; -}; - int RkISP1CameraData::loadIPA() { ipa_ = IPAManager::createIPA(pipe_, 1, 1); @@ -948,9 +900,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) op.controls = { request->controls() }; data->ipa_->processEvent(op); - data->timeline_.scheduleAction(std::make_unique(data->frame_, - data, - this)); + param_->queueBuffer(info->paramBuffer); + stat_->queueBuffer(info->statBuffer); + + if (info->mainPathBuffer) + mainPath_.queueBuffer(info->mainPathBuffer); + + if (info->selfPathBuffer) + selfPath_.queueBuffer(info->selfPathBuffer); data->frame_++; @@ -1088,6 +1045,7 @@ 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); /* @@ -1167,8 +1125,6 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) if (!info) return; - data->timeline_.bufferReady(buffer); - if (data->frame_ <= buffer->metadata().sequence) data->frame_ = buffer->metadata().sequence + 1; @@ -1178,6 +1134,20 @@ 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 */