[{"id":22586,"web_url":"https://patchwork.libcamera.org/comment/22586/","msgid":"","date":"2022-04-05T22:53:23","subject":"Re: [libcamera-devel] [PATCH v4 2/6] ipa: ipu3: Inlink fillParams()\n\tin fillParamsBuffer()","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\ns/Inlink/Inline/ in the subject line (same for 3/6). It looks like I\nwrote \"inlinking\" instead of \"inlining\" in my review of v3, sorry about\nthat :-)\n\nOn Thu, Mar 31, 2022 at 10:00:54PM +0530, Umang Jain via libcamera-devel wrote:\n> Since we have moved away from switch/case on the operation ID,\n> there's little reason to split the operation in two functions.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 47 +++++++++++++++++--------------------------\n> 1 file changed, 18 insertions(+), 29 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 7779ad58..23a9033e 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -157,7 +157,6 @@ private:\n> \t\t\t ControlInfoMap *ipaControls);\n> \tvoid updateSessionConfiguration(const ControlInfoMap &sensorControls);\n> \n> -\tvoid fillParams(unsigned int frame, ipu3_uapi_params *params);\n> \tvoid parseStatistics(unsigned int frame,\n> \t\t\t int64_t frameTimestamp,\n> \t\t\t const ipu3_uapi_stats_3a *stats);\n> @@ -514,6 +513,9 @@ void IPAIPU3::unmapBuffers(const std::vector &ids)\n> * \\brief Fill and return a buffer with ISP processing parameters for a frame\n> * \\param[in] frame The frame number\n> * \\param[in] bufferId ID of the parameter buffer to fill\n> + *\n> + * Algorithms are expected to fill the IPU3 parameter buffer for the next\n> + * frame given their most recent processing of the ImgU statistics.\n> */\n> void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> {\n> @@ -527,7 +529,21 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> \tipu3_uapi_params *params =\n> \t\treinterpret_cast(mem.data());\n> \n> -\tfillParams(frame, params);\n> +\t/*\n> +\t * The incoming params buffer may contain uninitialised data, or the\n> +\t * parameters of previously queued frames. Clearing the entire buffer\n> +\t * may be an expensive operation, and the kernel will only read from\n> +\t * structures which have their associated use-flag set.\n> +\t *\n> +\t * It is the responsibility of the algorithms to set the use flags\n> +\t * accordingly for any data structure they update during prepare().\n> +\t */\n> +\tparams->use = {};\n> +\n> +\tfor (auto const &algo : algorithms_)\n> +\t\talgo->prepare(context_, params);\n> +\n> +\tparamsBufferReady.emit(frame);\n> }\n> \n> /**\n> @@ -570,33 +586,6 @@ void IPAIPU3::queueRequest(const uint32_t frame,\n> \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> }\n> \n> -/**\n> - * \\brief Fill the ImgU parameter buffer for the next frame\n> - * \\param[in] frame The number of the latest frame processed\n> - * \\param[out] params The parameter buffer to fill\n> - *\n> - * Algorithms are expected to fill the IPU3 parameter buffer for the next\n> - * frame given their most recent processing of the ImgU statistics.\n> - */\n> -void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> -{\n> -\t/*\n> -\t * The incoming params buffer may contain uninitialised data, or the\n> -\t * parameters of previously queued frames. Clearing the entire buffer\n> -\t * may be an expensive operation, and the kernel will only read from\n> -\t * structures which have their associated use-flag set.\n> -\t *\n> -\t * It is the responsibility of the algorithms to set the use flags\n> -\t * accordingly for any data structure they update during prepare().\n> -\t */\n> -\tparams->use = {};\n> -\n> -\tfor (auto const &algo : algorithms_)\n> -\t\talgo->prepare(context_, params);\n> -\n> -\tparamsBufferReady.emit(frame);\n> -}\n> -\n> /**\n> * \\brief Process the statistics generated by the ImgU\n> * \\param[in] frame The number of the latest frame processed","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 6BB41C3256\n\tfor ;\n\tTue, 5 Apr 2022 22:53:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3210665642;\n\tWed, 6 Apr 2022 00:53:28 +0200 (CEST)","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 DFC15633A5\n\tfor ;\n\tWed, 6 Apr 2022 00:53:26 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 99279482;\n\tWed, 6 Apr 2022 00:53:26 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649199208;\n\tbh=CtcbjugyLIijai/TTT9yhMcoAEDtyUMSJafT2sAEfXk=;\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=1SZYabCDEdm45pAdaYOdXd0Q45t2Unmv7UsuhlQeLDrNNWS9QhwA4sADvAR3LzSVG\n\tXgUpzheEqhUatXDBRpAfkxTDcmlncMLXclwFdYxjS4WlZfCcCDSTsJoyT/3aq+0wMt\n\t8oo+O3VbtWlhKYnT0JeciC0dx4rz0mwDVWwlXHfFPwWca1JgiEdFuES6thQ27AZAsJ\n\tFSH/IwSgFe376uxjTzcQcJAgO36Nv/UFV6RuaW+zDd+vCN5UqfGASu/9MMPXa6VQ0t\n\toSnql/turc/c7WrfEdIsI3H9tdl8rTsN/NgB9EdGf/vx8V6SXdy4+8H/W+bb+ys21U\n\tFfrRfQS1Dst+A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649199206;\n\tbh=CtcbjugyLIijai/TTT9yhMcoAEDtyUMSJafT2sAEfXk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OWPHj4Ha/d9+L3F/oOQmFSDul2WEjq8KNEdBnI+dsqVmsiajtvMKMsTHdOfhz+Gei\n\tCEpLcE0gmFTC/h8uAUKRYkCn26n0v6D2dtySVxsAfOepUmsO5H9TzyyMyBtdCECxCv\n\t58pl+1w1JTaV0+JwizXWq4mrhnVdfUcJkQojZJfg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OWPHj4Ha\"; dkim-atps=neutral","Date":"Wed, 6 Apr 2022 01:53:23 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220331163058.171418-1-umang.jain@ideasonboard.com>\n\t<20220331163058.171418-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220331163058.171418-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/6] ipa: ipu3: Inlink fillParams()\n\tin fillParamsBuffer()","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\" "}}]