{"id":15524,"url":"https://patchwork.libcamera.org/api/patches/15524/?format=json","web_url":"https://patchwork.libcamera.org/patch/15524/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/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":"<20220323150057.121491-1-umang.jain@ideasonboard.com>","date":"2022-03-23T15:00:57","name":"[libcamera-devel,v1.1,2/2] ipa: rkisp1: Replace event-based ops with dedicated functions","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"6f4c09a0153021c4587528bd6aa5260205e2c4bb","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/?format=json","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"delegate":{"id":12,"url":"https://patchwork.libcamera.org/api/users/12/?format=json","username":"uajain","first_name":"Umang","last_name":"Jain","email":"umang.jain@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/patch/15524/mbox/","series":[{"id":2990,"url":"https://patchwork.libcamera.org/api/series/2990/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2990","date":"2022-03-23T15:00:57","name":null,"version":1,"mbox":"https://patchwork.libcamera.org/series/2990/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/15524/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/15524/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 D1600C0F1B\n\tfor ;\n\tWed, 23 Mar 2022 15:01:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1DC2E604DA;\n\tWed, 23 Mar 2022 16:01:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90867604C4\n\tfor ;\n\tWed, 23 Mar 2022 16:01:32 +0100 (CET)","from perceval.ideasonboard.com (unknown [103.251.226.192])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B8BF19DE;\n\tWed, 23 Mar 2022 16:01:30 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648047694;\n\tbh=8nsDZY+QTAM74c3zdtus9FxaMYU2UaoSlFsFmu0OHQ0=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=UZXhpSlc1V81mOsKY8aFKrRozkmhb0Sqrs1oU+fXY0a5bNoa7G1jjmxAM6nzJzwzk\n\tMe2PutLbQfAdmB8zusFz8YVnnTGCkD96m9f3Qpmolou+nFn9xIhuqTSObFyKHJ6pcX\n\tdHG17KeVrvVIatVIFvYpzzbztCj4LkXiJ6kiX/ekz2rybo5nG9+/s7KtZxTFKa119E\n\tU8MLLvJS7+w5INRsFM9AQiPORTWFKiGVcXzX+JGHdtcig6DvjgSJcCgs0LCz8GD3E8\n\thi6JwIisZWKd29qrVXb6M78nodU8ajwLBb1f9Ejl4VpGajN3lNxwI57QdVjQ3CEhEo\n\tWz/63U8yRuozw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648047692;\n\tbh=8nsDZY+QTAM74c3zdtus9FxaMYU2UaoSlFsFmu0OHQ0=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=JE5wGI1L+iCgLuOjVR2BPR+GIypTBAn/LMkeIOTeGdk4Wr/iLyFChNEfA9t8sV6fN\n\tfKS4KTGVLfwFiphUOsJ9A3s/XXPjNlVLh1hhiajjJ4reKG4VwiritYgBv/tTVo8lO3\n\tUxts3H0gR79jcy4RGjQWhi+rtlRwkeWIym0lw6yg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JE5wGI1L\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 23 Mar 2022 20:30:57 +0530","Message-Id":"<20220323150057.121491-1-umang.jain@ideasonboard.com>","X-Mailer":"git-send-email 2.31.1","In-Reply-To":"<20220308091520.34607-3-umang.jain@ideasonboard.com>","References":"<20220308091520.34607-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v1.1 2/2] ipa: rkisp1: Replace event-based\n\tops with dedicated functions","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The IPARkISP1Interface currently uses event-type based structures in\norder to communicate with the pipeline-handler (and vice-versa).\nReplace the event based structures with dedicated functions associated\nto each operation.\n\nThe translated naming scheme of operations to dedicated functions:\n ActionV4L2Set => setSensorControls\n ActionParamFilled => paramsBufferReady\n ActionMetadata => metdataReady\n EventSignalStatBuffer => processStatsBuffer()\n EventQueueRequest => queueRequest()\n\nThe lexical of IPARkISP1::metadataReady() will now conflict with the\nmetadataReady Signal being introduced in this patch as part of the\ninterface change. Hence, rename IPARkISP1::metadataReady() to\nIPARkISP1::prepareReady() to prevent the conflict.\n\nSigned-off-by: Umang Jain \nReviewed-by: Laurent Pinchart \nReviewed-by: Paul Elder \nTested-by: Paul Elder \n---\n include/libcamera/ipa/rkisp1.mojom | 31 ++-------\n src/ipa/rkisp1/rkisp1.cpp | 88 +++++++-----------------\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 71 +++++++------------\n 3 files changed, 59 insertions(+), 131 deletions(-)","diff":"diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\nindex c3a6d8e1..f50f1e11 100644\n--- a/include/libcamera/ipa/rkisp1.mojom\n+++ b/include/libcamera/ipa/rkisp1.mojom\n@@ -8,28 +8,6 @@ module ipa.rkisp1;\n \n import \"include/libcamera/ipa/core.mojom\";\n \n-enum RkISP1Operations {\n-\tActionV4L2Set = 1,\n-\tActionParamFilled = 2,\n-\tActionMetadata = 3,\n-\tEventSignalStatBuffer = 4,\n-\tEventQueueRequest = 5,\n-};\n-\n-struct RkISP1Event {\n-\tRkISP1Operations op;\n-\tuint32 frame;\n-\tuint32 bufferId;\n-\tlibcamera.ControlList controls;\n-\tlibcamera.ControlList sensorControls;\n-};\n-\n-struct RkISP1Action {\n-\tRkISP1Operations op;\n-\tlibcamera.ControlList controls;\n-\tlibcamera.ControlList sensorControls;\n-};\n-\n interface IPARkISP1Interface {\n \tinit(libcamera.IPASettings settings,\n \t uint32 hwRevision)\n@@ -45,9 +23,14 @@ interface IPARkISP1Interface {\n \tmapBuffers(array buffers);\n \tunmapBuffers(array ids);\n \n-\t[async] processEvent(RkISP1Event ev);\n+\t[async] queueRequest(uint32 frame, uint32 bufferId,\n+\t\t\t libcamera.ControlList reqControls);\n+\t[async] processStatsBuffer(uint32 frame, uint32 bufferId,\n+\t\t\t\t libcamera.ControlList sensorControls);\n };\n \n interface IPARkISP1EventInterface {\n-\tqueueFrameAction(uint32 frame, RkISP1Action action);\n+\tparamsBufferReady(uint32 frame);\n+\tsetSensorControls(uint32 frame, libcamera.ControlList sensorControls);\n+\tmetadataReady(uint32 frame, libcamera.ControlList metadata);\n };\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 129afddd..2225a84d 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -51,16 +51,14 @@ public:\n \t\t const std::map &entityControls) override;\n \tvoid mapBuffers(const std::vector &buffers) override;\n \tvoid unmapBuffers(const std::vector &ids) override;\n-\tvoid processEvent(const RkISP1Event &event) override;\n \n+\tvoid queueRequest(const uint32_t frame, const uint32_t bufferId,\n+\t\t\t const ControlList &controls) override;\n+\tvoid processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n+\t\t\t\tconst ControlList &sensorControls) override;\n private:\n-\tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n-\t\t\t const ControlList &controls);\n-\tvoid updateStatistics(unsigned int frame,\n-\t\t\t const rkisp1_stat_buffer *stats);\n-\n \tvoid setControls(unsigned int frame);\n-\tvoid metadataReady(unsigned int frame, unsigned int aeState);\n+\tvoid prepareMetadata(unsigned int frame, unsigned int aeState);\n \n \tstd::map buffers_;\n \tstd::map mappedBuffers_;\n@@ -231,60 +229,34 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n \t}\n }\n \n-void IPARkISP1::processEvent(const RkISP1Event &event)\n-{\n-\tswitch (event.op) {\n-\tcase EventSignalStatBuffer: {\n-\t\tunsigned int frame = event.frame;\n-\t\tunsigned int bufferId = event.bufferId;\n-\n-\t\tconst rkisp1_stat_buffer *stats =\n-\t\t\treinterpret_cast(\n-\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n-\n-\t\tcontext_.frameContext.sensor.exposure =\n-\t\t\tevent.sensorControls.get(V4L2_CID_EXPOSURE).get();\n-\t\tcontext_.frameContext.sensor.gain =\n-\t\t\tcamHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n-\n-\t\tupdateStatistics(frame, stats);\n-\t\tbreak;\n-\t}\n-\tcase EventQueueRequest: {\n-\t\tunsigned int frame = event.frame;\n-\t\tunsigned int bufferId = event.bufferId;\n-\n-\t\trkisp1_params_cfg *params =\n-\t\t\treinterpret_cast(\n-\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n-\n-\t\tqueueRequest(frame, params, event.controls);\n-\t\tbreak;\n-\t}\n-\tdefault:\n-\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.op;\n-\t\tbreak;\n-\t}\n-}\n-\n-void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n+void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,\n \t\t\t [[maybe_unused]] const ControlList &controls)\n {\n+\trkisp1_params_cfg *params =\n+\t\treinterpret_cast(\n+\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n+\n \t/* Prepare parameters buffer. */\n \tmemset(params, 0, sizeof(*params));\n \n \tfor (auto const &algo : algorithms_)\n \t\talgo->prepare(context_, params);\n \n-\tRkISP1Action op;\n-\top.op = ActionParamFilled;\n-\n-\tqueueFrameAction.emit(frame, op);\n+\tparamsBufferReady.emit(frame);\n }\n \n-void IPARkISP1::updateStatistics(unsigned int frame,\n-\t\t\t\t const rkisp1_stat_buffer *stats)\n+void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n+\t\t\t\t const ControlList &sensorControls)\n {\n+\tconst rkisp1_stat_buffer *stats =\n+\t\treinterpret_cast(\n+\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n+\n+\tcontext_.frameContext.sensor.exposure =\n+\t\tsensorControls.get(V4L2_CID_EXPOSURE).get();\n+\tcontext_.frameContext.sensor.gain =\n+\t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n+\n \tunsigned int aeState = 0;\n \n \tfor (auto const &algo : algorithms_)\n@@ -292,37 +264,29 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n \n \tsetControls(frame);\n \n-\tmetadataReady(frame, aeState);\n+\tprepareMetadata(frame, aeState);\n }\n \n void IPARkISP1::setControls(unsigned int frame)\n {\n-\tRkISP1Action op;\n-\top.op = ActionV4L2Set;\n-\n \tuint32_t exposure = context_.frameContext.agc.exposure;\n \tuint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n \n \tControlList ctrls(ctrls_);\n \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure));\n \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain));\n-\top.sensorControls = ctrls;\n \n-\tqueueFrameAction.emit(frame, op);\n+\tsetSensorControls.emit(frame, ctrls);\n }\n \n-void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n+void IPARkISP1::prepareMetadata(unsigned int frame, unsigned int aeState)\n {\n \tControlList ctrls(controls::controls);\n \n \tif (aeState)\n \t\tctrls.set(controls::AeLocked, aeState == 2);\n \n-\tRkISP1Action op;\n-\top.op = ActionMetadata;\n-\top.controls = ctrls;\n-\n-\tqueueFrameAction.emit(frame, op);\n+\tmetadataReady.emit(frame, ctrls);\n }\n \n } /* namespace ipa::rkisp1 */\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 8cca8a15..e6fc582b 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -104,8 +104,9 @@ public:\n \tstd::unique_ptr ipa_;\n \n private:\n-\tvoid queueFrameAction(unsigned int frame,\n-\t\t\t const ipa::rkisp1::RkISP1Action &action);\n+\tvoid paramFilled(unsigned int frame);\n+\tvoid setSensorControls(unsigned int frame,\n+\t\t\t const ControlList &sensorControls);\n \n \tvoid metadataReady(unsigned int frame, const ControlList &metadata);\n };\n@@ -316,8 +317,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n \tif (!ipa_)\n \t\treturn -ENOENT;\n \n-\tipa_->queueFrameAction.connect(this,\n-\t\t\t\t &RkISP1CameraData::queueFrameAction);\n+\tipa_->setSensorControls.connect(this, &RkISP1CameraData::setSensorControls);\n+\tipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);\n+\tipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);\n \n \tint ret = ipa_->init(IPASettings{ \"\", sensor_->model() }, hwRevision);\n \tif (ret < 0) {\n@@ -328,39 +330,27 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n \treturn 0;\n }\n \n-void RkISP1CameraData::queueFrameAction(unsigned int frame,\n-\t\t\t\t\tconst ipa::rkisp1::RkISP1Action &action)\n+void RkISP1CameraData::paramFilled(unsigned int frame)\n {\n-\tswitch (action.op) {\n-\tcase ipa::rkisp1::ActionV4L2Set: {\n-\t\tconst ControlList &controls = action.sensorControls;\n-\t\tdelayedCtrls_->push(controls);\n-\t\tbreak;\n-\t}\n-\tcase ipa::rkisp1::ActionParamFilled: {\n-\t\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n-\t\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n-\t\tif (!info)\n-\t\t\tbreak;\n+\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n+\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n+\tif (!info)\n+\t\treturn;\n \n-\t\tpipe->param_->queueBuffer(info->paramBuffer);\n-\t\tpipe->stat_->queueBuffer(info->statBuffer);\n+\tpipe->param_->queueBuffer(info->paramBuffer);\n+\tpipe->stat_->queueBuffer(info->statBuffer);\n \n-\t\tif (info->mainPathBuffer)\n-\t\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n+\tif (info->mainPathBuffer)\n+\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n \n-\t\tif (info->selfPathBuffer)\n-\t\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n+\tif (info->selfPathBuffer)\n+\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n+}\n \n-\t\tbreak;\n-\t}\n-\tcase ipa::rkisp1::ActionMetadata:\n-\t\tmetadataReady(frame, action.controls);\n-\t\tbreak;\n-\tdefault:\n-\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.op;\n-\t\tbreak;\n-\t}\n+void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n+\t\t\t\t\t const ControlList &sensorControls)\n+{\n+\tdelayedCtrls_->push(sensorControls);\n }\n \n void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n@@ -865,13 +855,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n \tif (!info)\n \t\treturn -ENOENT;\n \n-\tipa::rkisp1::RkISP1Event ev;\n-\tev.op = ipa::rkisp1::EventQueueRequest;\n-\tev.frame = data->frame_;\n-\tev.bufferId = info->paramBuffer->cookie();\n-\tev.controls = request->controls();\n-\tdata->ipa_->processEvent(ev);\n-\n+\tdata->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),\n+\t\t\t\t request->controls());\n \tdata->frame_++;\n \n \treturn 0;\n@@ -1120,12 +1105,8 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \tif (data->frame_ <= buffer->metadata().sequence)\n \t\tdata->frame_ = buffer->metadata().sequence + 1;\n \n-\tipa::rkisp1::RkISP1Event ev;\n-\tev.op = ipa::rkisp1::EventSignalStatBuffer;\n-\tev.frame = info->frame;\n-\tev.bufferId = info->statBuffer->cookie();\n-\tev.sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);\n-\tdata->ipa_->processEvent(ev);\n+\tdata->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),\n+\t\t\t\t data->delayedCtrls_->get(buffer->metadata().sequence));\n }\n \n REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)\n","prefixes":["libcamera-devel","v1.1","2/2"]}