Patch Detail
Show a patch.
GET /api/1.1/patches/10281/?format=api
{ "id": 10281, "url": "https://patchwork.libcamera.org/api/1.1/patches/10281/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10281/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20201028010051.3830668-9-niklas.soderlund@ragnatech.se>", "date": "2020-10-28T01:00:50", "name": "[libcamera-devel,8/9] libcamera: pipeline: rkisp1: Use SOF event to warn about late parameters", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "fcfd42df7e51ce779a99c4338ff6e10cb1b2821f", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/1.1/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": { "id": 16, "url": "https://patchwork.libcamera.org/api/1.1/users/16/?format=api", "username": "neg", "first_name": "Niklas", "last_name": "Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "mbox": "https://patchwork.libcamera.org/patch/10281/mbox/", "series": [ { "id": 1423, "url": "https://patchwork.libcamera.org/api/1.1/series/1423/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1423", "date": "2020-10-28T01:00:42", "name": "libcamera: Add helper for controls that take effect with a delay", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1423/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10281/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10281/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 CC4F7C3B60\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 28 Oct 2020 01:01:20 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 823F462284;\n\tWed, 28 Oct 2020 02:01:20 +0100 (CET)", "from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net\n\t[195.74.38.229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 34DB26225C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 28 Oct 2020 02:01:18 +0100 (CET)", "from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de\n\t[79.202.36.88])\n\tby bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA\n\tid 15350d2e-18b9-11eb-954c-0050569116f7;\n\tWed, 28 Oct 2020 02:01:17 +0100 (CET)" ], "X-Halon-ID": "15350d2e-18b9-11eb-954c-0050569116f7", "Authorized-sender": "niklas.soderlund@fsdn.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "To": "libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com,\n\tnaush@raspberrypi.com", "Date": "Wed, 28 Oct 2020 02:00:50 +0100", "Message-Id": "<20201028010051.3830668-9-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.29.1", "In-Reply-To": "<20201028010051.3830668-1-niklas.soderlund@ragnatech.se>", "References": "<20201028010051.3830668-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH 8/9] libcamera: pipeline: rkisp1: Use SOF\n\tevent to warn about late parameters", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"utf-8\"", "Content-Transfer-Encoding": "base64", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "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 its\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 <niklas.soderlund@ragnatech.se>\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 d0a8be06656535a3..552c9ebbd5617e59 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -40,7 +40,6 @@ namespace libcamera {\n LOG_DEFINE_CATEGORY(RkISP1)\n \n class PipelineHandlerRkISP1;\n-class RkISP1ActionQueueBuffers;\n class RkISP1CameraData;\n \n enum RkISP1ActionType {\n@@ -203,7 +202,6 @@ private:\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n \n-\tfriend RkISP1ActionQueueBuffers;\n \tfriend RkISP1CameraData;\n \tfriend RkISP1Frames;\n \n@@ -214,6 +212,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@@ -348,53 +347,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@@ -948,9 +900,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<RkISP1ActionQueueBuffers>(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@@ -1088,6 +1045,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@@ -1167,8 +1125,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@@ -1178,6 +1134,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", "8/9" ] }