[{"id":22540,"web_url":"https://patchwork.libcamera.org/comment/22540/","msgid":"<20220401083116.GF3237525@pyrite.rasen.tech>","date":"2022-04-01T08:31:16","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Split queuing of\n\trequest and parameter filling","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Umang,\n\nOn Thu, Mar 31, 2022 at 10:27:24PM +0530, Umang Jain via libcamera-devel wrote:\n> Queuing of request (i.e. passing of controls to the IPA)\n> and filling of the parameters buffer are two separate operations.\n> Treat them as such by splitting them into two functions in\n> the rkisp1 IPA interface.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Paul Elder \n\n> ---\n> include/libcamera/ipa/rkisp1.mojom | 4 ++--\n> src/ipa/rkisp1/rkisp1.cpp | 11 ++++++++---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++--\n> 3 files changed, 13 insertions(+), 7 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index f50f1e11..e3537385 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -23,8 +23,8 @@ interface IPARkISP1Interface {\n> \tmapBuffers(array buffers);\n> \tunmapBuffers(array ids);\n> \n> -\t[async] queueRequest(uint32 frame, uint32 bufferId,\n> -\t\t\t libcamera.ControlList reqControls);\n> +\t[async] queueRequest(uint32 frame, libcamera.ControlList reqControls);\n> +\t[async] fillParamsBuffer(uint32 frame, uint32 bufferId);\n> \t[async] processStatsBuffer(uint32 frame, uint32 bufferId,\n> \t\t\t\t libcamera.ControlList sensorControls);\n> };\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 13ab1cdc..ef1f0d56 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -54,8 +54,8 @@ public:\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \n> -\tvoid queueRequest(const uint32_t frame, const uint32_t bufferId,\n> -\t\t\t const ControlList &controls) override;\n> +\tvoid queueRequest(const uint32_t frame, const ControlList &controls) override;\n> +\tvoid fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;\n> \tvoid processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> \t\t\t\tconst ControlList &sensorControls) override;\n> private:\n> @@ -235,8 +235,13 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,\n> +void IPARkISP1::queueRequest([[maybe_unused]] const uint32_t frame,\n> \t\t\t [[maybe_unused]] const ControlList &controls)\n> +{\n> +\t/* \\todo Start processing for 'frame' based on 'controls'. */\n> +}\n> +\n> +void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> {\n> \trkisp1_params_cfg *params =\n> \t\treinterpret_cast(\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index e6fc582b..1624e2ec 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -855,8 +855,9 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> \tif (!info)\n> \t\treturn -ENOENT;\n> \n> -\tdata->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),\n> -\t\t\t\t request->controls());\n> +\tdata->ipa_->queueRequest(data->frame_, request->controls());\n> +\tdata->ipa_->fillParamsBuffer(data->frame_, info->paramBuffer->cookie());\n> +\n> \tdata->frame_++;\n> \n> \treturn 0;\n> -- \n> 2.31.0\n>","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 DBA7FC3256\n\tfor ;\n\tFri, 1 Apr 2022 08:31:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0F750633A5;\n\tFri, 1 Apr 2022 10:31:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 40740604B9\n\tfor ;\n\tFri, 1 Apr 2022 10:31:25 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-148.catv02.itscom.jp\n\t[175.177.42.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D058B486;\n\tFri, 1 Apr 2022 10:31:23 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648801887;\n\tbh=d7CPXRgwQNkOWqFwrjiyfdomt0NPZF52AkKHsJhixEg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=dBAnDm1Ka1fE0eQSouIK5i7CVegbtKm7wrixrI8NUBHeIb7rju1E9uRFfgGFtxBYj\n\tyzR4lUR0rw8+ZpJLHQcNLnRaaUHSsf7flLgnYboa4OrltPVFIcdjL/n/2DdiNWEHnq\n\tWzmNPOp3M3wRdJx4pTwmggNj/QDEo3LlHCtLJjjMcCU8j8E9idMxW+DfN2HUSQzicI\n\tsp/xrg+Nmm8Lh8+p/0t4HNVCo0PgorAYjVI++6d7ol7ISVuvyd5iMhS/kYWS1HnRuh\n\tQzNmZbSSXzyxaJdHO2dQUKDaKtD957PxEpHT6+A3svl38rfYllckTZD37e33Qv9ZxM\n\tPZ7AGN8TlvTkA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648801884;\n\tbh=d7CPXRgwQNkOWqFwrjiyfdomt0NPZF52AkKHsJhixEg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Vws4r9opKE4TffSjXU5R2yMN/wQeKYoMs8zICC/RH/dhVBzitOEVJmjy1TAiJ/bE1\n\tOUYjdC4kYVk6C3ntCqNBEfFJTWfsywmAxiYbHrN2GOHzg8vrDJvh3BNL9Y/f0rr+/z\n\toLwPQcaWh6BWjKqZtvoDYbOnMz93IuV/vHjmHStA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Vws4r9op\"; dkim-atps=neutral","Date":"Fri, 1 Apr 2022 17:31:16 +0900","To":"Umang Jain ","Message-ID":"<20220401083116.GF3237525@pyrite.rasen.tech>","References":"<20220331165725.188364-1-umang.jain@ideasonboard.com>\n\t<20220331165725.188364-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220331165725.188364-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Split queuing of\n\trequest and parameter filling","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":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22547,"web_url":"https://patchwork.libcamera.org/comment/22547/","msgid":"","date":"2022-04-01T12:04:15","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Split queuing of\n\trequest and parameter filling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Thu, Mar 31, 2022 at 10:27:24PM +0530, Umang Jain via libcamera-devel wrote:\n> Queuing of request (i.e. passing of controls to the IPA)\n> and filling of the parameters buffer are two separate operations.\n> Treat them as such by splitting them into two functions in\n> the rkisp1 IPA interface.\n> \n> Signed-off-by: Umang Jain \n\nIt's a bit useless at the moment as queueRequest() is a stub, but it\nprepares for the future.\n\nReviewed-by: Laurent Pinchart \n\n> ---\n> include/libcamera/ipa/rkisp1.mojom | 4 ++--\n> src/ipa/rkisp1/rkisp1.cpp | 11 ++++++++---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++--\n> 3 files changed, 13 insertions(+), 7 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index f50f1e11..e3537385 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -23,8 +23,8 @@ interface IPARkISP1Interface {\n> \tmapBuffers(array buffers);\n> \tunmapBuffers(array ids);\n> \n> -\t[async] queueRequest(uint32 frame, uint32 bufferId,\n> -\t\t\t libcamera.ControlList reqControls);\n> +\t[async] queueRequest(uint32 frame, libcamera.ControlList reqControls);\n> +\t[async] fillParamsBuffer(uint32 frame, uint32 bufferId);\n> \t[async] processStatsBuffer(uint32 frame, uint32 bufferId,\n> \t\t\t\t libcamera.ControlList sensorControls);\n> };\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 13ab1cdc..ef1f0d56 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -54,8 +54,8 @@ public:\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \n> -\tvoid queueRequest(const uint32_t frame, const uint32_t bufferId,\n> -\t\t\t const ControlList &controls) override;\n> +\tvoid queueRequest(const uint32_t frame, const ControlList &controls) override;\n> +\tvoid fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;\n> \tvoid processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> \t\t\t\tconst ControlList &sensorControls) override;\n> private:\n> @@ -235,8 +235,13 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,\n> +void IPARkISP1::queueRequest([[maybe_unused]] const uint32_t frame,\n> \t\t\t [[maybe_unused]] const ControlList &controls)\n> +{\n> +\t/* \\todo Start processing for 'frame' based on 'controls'. */\n> +}\n> +\n> +void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> {\n> \trkisp1_params_cfg *params =\n> \t\treinterpret_cast(\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index e6fc582b..1624e2ec 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -855,8 +855,9 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> \tif (!info)\n> \t\treturn -ENOENT;\n> \n> -\tdata->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),\n> -\t\t\t\t request->controls());\n> +\tdata->ipa_->queueRequest(data->frame_, request->controls());\n> +\tdata->ipa_->fillParamsBuffer(data->frame_, info->paramBuffer->cookie());\n> +\n> \tdata->frame_++;\n>","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 561D2C0F1B\n\tfor ;\n\tFri, 1 Apr 2022 12:04:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 13D80633A7;\n\tFri, 1 Apr 2022 14:04:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3597633A6\n\tfor ;\n\tFri, 1 Apr 2022 14:04:17 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 673A0486;\n\tFri, 1 Apr 2022 14:04:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648814659;\n\tbh=LgUclr2D6lVlm1W2H4xju04sQbPw6+77QoBKgNcIiYU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=00pnC4EIvxsKgp0Jg3GWQrjaUxTURq93eK7hoSojRumW8Gii9AVxsZ2kWQGDk40Ga\n\teDq+LKh+405GAz75tmHceQMdpj5QptTU6QgcqIuUTlkWS5if0u0CLDww+eP0gIvSF2\n\tOe3zeXYtYUT91B4k/ZCPCCtjeFxjl4IYDiDmAl6QgFrQLMQk2W7XApYLMF1IZ1ipbC\n\tk0qKH0tfFxvEmC/lUtso3opwat8G13/5e0U34nM7clixSH7cFv+ildLD+ZsO66QsFq\n\tQ0Mn38FWgfGOly54HiBRo4u/ciUJRFomBwgL2+yBt71a1ihv0flhMtgDHvtvt7pEH7\n\t57HKQv5WfPZ5A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648814657;\n\tbh=LgUclr2D6lVlm1W2H4xju04sQbPw6+77QoBKgNcIiYU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eqs5SB2UrzhfjgMsFdDNg6Tz6dAW8Ph8HX/6DeTR4/OPZ6JhcYK/Jxw5DhM+qRqTN\n\tHa/Okk3QfeuQ7O0qcO0W+ir9l0LHdvqkVwi8KXs2s5yQzjJgObWENbZW6B7tpcrUUr\n\ttI4HlTcfNTMxvAvG53LXfdQ8/urVNdDr2sSL9ugM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"eqs5SB2U\"; dkim-atps=neutral","Date":"Fri, 1 Apr 2022 15:04:15 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220331165725.188364-1-umang.jain@ideasonboard.com>\n\t<20220331165725.188364-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220331165725.188364-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Split queuing of\n\trequest and parameter filling","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]