{"id":10880,"url":"https://patchwork.libcamera.org/api/1.1/patches/10880/?format=json","web_url":"https://patchwork.libcamera.org/patch/10880/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210116114805.905397-8-niklas.soderlund@ragnatech.se>","date":"2021-01-16T11:48:04","name":"[libcamera-devel,v5,7/8] libcamera: pipeline: rkisp1: Use SOF event to warn about late parameters","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"fcfd42df7e51ce779a99c4338ff6e10cb1b2821f","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/1.1/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":{"id":16,"url":"https://patchwork.libcamera.org/api/1.1/users/16/?format=json","username":"neg","first_name":"Niklas","last_name":"Söderlund","email":"niklas.soderlund@ragnatech.se"},"mbox":"https://patchwork.libcamera.org/patch/10880/mbox/","series":[{"id":1579,"url":"https://patchwork.libcamera.org/api/1.1/series/1579/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1579","date":"2021-01-16T11:47:57","name":"libcamera: Add helper for controls that take effect with a delay","version":5,"mbox":"https://patchwork.libcamera.org/series/1579/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/10880/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/10880/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8A70EC0F1D\n\tfor ;\n\tSat, 16 Jan 2021 11:48:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D60268129;\n\tSat, 16 Jan 2021 12:48:25 +0100 (CET)","from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net\n\t[195.74.38.228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12D7E6810C\n\tfor ;\n\tSat, 16 Jan 2021 12:48:23 +0100 (CET)","from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de\n\t[79.202.36.88])\n\tby bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid bbc2f532-57f0-11eb-a076-005056917f90;\n\tSat, 16 Jan 2021 12:48:22 +0100 (CET)"],"X-Halon-ID":"bbc2f532-57f0-11eb-a076-005056917f90","Authorized-sender":"niklas.soderlund@fsdn.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Date":"Sat, 16 Jan 2021 12:48:04 +0100","Message-Id":"<20210116114805.905397-8-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.30.0","In-Reply-To":"<20210116114805.905397-1-niklas.soderlund@ragnatech.se>","References":"<20210116114805.905397-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v5 7/8] libcamera: pipeline: rkisp1: Use\n\tSOF 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":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"In the Timeline approach the idea was to delay queuing buffers to the\ndevice until the IPA had a chance to prepare the parameters buffer. A\ncheck was still added to warn if the IPA queued buffers before the\nparameters buffer was filled in.\n\nThis check happened much sooner then needed as the parameter buffers\ndoes not have to be ready when the buffer is queued but just before it's\nconsumed. As the pipeline now has true knowledge of each SOF we can move\nthe check there and remove the delaying of queuing of buffers.\n\nThis change really speeds up the IPA reactions as the delays used in the\nTimeline where approximated while with this change they are driven by\nevents reported by the device.\n\nSigned-off-by: Niklas Söderlund \nReviewed-by: Jacopo Mondi \n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 78 ++++++++----------------\n 1 file changed, 24 insertions(+), 54 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 116d205b85cc146c..ab555cf23a7b564b 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -41,7 +41,6 @@ namespace libcamera {\n LOG_DEFINE_CATEGORY(RkISP1)\n \n class PipelineHandlerRkISP1;\n-class RkISP1ActionQueueBuffers;\n class RkISP1CameraData;\n \n enum RkISP1ActionType {\n@@ -197,7 +196,6 @@ private:\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n \n-\tfriend RkISP1ActionQueueBuffers;\n \tfriend RkISP1CameraData;\n \tfriend RkISP1Frames;\n \n@@ -208,6 +206,7 @@ private:\n \tvoid bufferReady(FrameBuffer *buffer);\n \tvoid paramReady(FrameBuffer *buffer);\n \tvoid statReady(FrameBuffer *buffer);\n+\tvoid frameStart(uint32_t sequence);\n \n \tint allocateBuffers(Camera *camera);\n \tint freeBuffers(Camera *camera);\n@@ -342,53 +341,6 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request)\n \treturn nullptr;\n }\n \n-class RkISP1ActionQueueBuffers : public FrameAction\n-{\n-public:\n-\tRkISP1ActionQueueBuffers(unsigned int frame, RkISP1CameraData *data,\n-\t\t\t\t PipelineHandlerRkISP1 *pipe)\n-\t\t: FrameAction(frame, QueueBuffers), data_(data), pipe_(pipe)\n-\t{\n-\t}\n-\n-protected:\n-\tvoid run() override\n-\t{\n-\t\tRkISP1FrameInfo *info = data_->frameInfo_.find(frame());\n-\t\tif (!info)\n-\t\t\tLOG(RkISP1, Fatal) << \"Frame not known\";\n-\n-\t\t/*\n-\t\t * \\todo: If parameters are not filled a better method to handle\n-\t\t * the situation than queuing a buffer with unknown content\n-\t\t * should be used.\n-\t\t *\n-\t\t * It seems excessive to keep an internal zeroed scratch\n-\t\t * parameters buffer around as this should not happen unless the\n-\t\t * devices is under too much load. Perhaps failing the request\n-\t\t * and returning it to the application with an error code is\n-\t\t * better than queue it to hardware?\n-\t\t */\n-\t\tif (!info->paramFilled)\n-\t\t\tLOG(RkISP1, Error)\n-\t\t\t\t<< \"Parameters not ready on time for frame \"\n-\t\t\t\t<< frame();\n-\n-\t\tpipe_->param_->queueBuffer(info->paramBuffer);\n-\t\tpipe_->stat_->queueBuffer(info->statBuffer);\n-\n-\t\tif (info->mainPathBuffer)\n-\t\t\tpipe_->mainPath_.queueBuffer(info->mainPathBuffer);\n-\n-\t\tif (info->selfPathBuffer)\n-\t\t\tpipe_->selfPath_.queueBuffer(info->selfPathBuffer);\n-\t}\n-\n-private:\n-\tRkISP1CameraData *data_;\n-\tPipelineHandlerRkISP1 *pipe_;\n-};\n-\n int RkISP1CameraData::loadIPA()\n {\n \tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n@@ -945,9 +897,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n \top.controls = { request->controls() };\n \tdata->ipa_->processEvent(op);\n \n-\tdata->timeline_.scheduleAction(std::make_unique(data->frame_,\n-\t\t\t\t\t\t\t\t\t\t data,\n-\t\t\t\t\t\t\t\t\t\t this));\n+\tparam_->queueBuffer(info->paramBuffer);\n+\tstat_->queueBuffer(info->statBuffer);\n+\n+\tif (info->mainPathBuffer)\n+\t\tmainPath_.queueBuffer(info->mainPathBuffer);\n+\n+\tif (info->selfPathBuffer)\n+\t\tselfPath_.queueBuffer(info->selfPathBuffer);\n \n \tdata->frame_++;\n \n@@ -1097,6 +1054,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \tmainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);\n \tselfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);\n \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n+\tisp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart);\n \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n \n \t/*\n@@ -1175,8 +1133,6 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \tif (!info)\n \t\treturn;\n \n-\tdata->timeline_.bufferReady(buffer);\n-\n \tif (data->frame_ <= buffer->metadata().sequence)\n \t\tdata->frame_ = buffer->metadata().sequence + 1;\n \n@@ -1186,6 +1142,20 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \tdata->ipa_->processEvent(op);\n }\n \n+void PipelineHandlerRkISP1::frameStart(uint32_t sequence)\n+{\n+\tif (!activeCamera_)\n+\t\treturn;\n+\n+\tRkISP1CameraData *data = cameraData(activeCamera_);\n+\n+\tRkISP1FrameInfo *info = data->frameInfo_.find(sequence);\n+\tif (!info || !info->paramFilled)\n+\t\tLOG(RkISP1, Info)\n+\t\t\t<< \"Parameters not ready on time for frame \"\n+\t\t\t<< sequence;\n+}\n+\n REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)\n \n } /* namespace libcamera */\n","prefixes":["libcamera-devel","v5","7/8"]}