[{"id":22651,"web_url":"https://patchwork.libcamera.org/comment/22651/","msgid":"<164936966265.22830.2313762450154896674@Monstersaurus>","date":"2022-04-07T22:14:22","subject":"Re: [libcamera-devel] [PATCH v5 2/6] ipa: ipu3: Inline fillParams()\n\tin fillParamsBuffer()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain via libcamera-devel (2022-04-06 15:17:05)\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> Reviewed-by: Laurent Pinchart \n\n\nReviewed-by: Kieran Bingham \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 9a59f80f..8c9a20f5 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -156,7 +156,6 @@ private:\n> ControlInfoMap *ipaControls);\n> void updateSessionConfiguration(const ControlInfoMap &sensorControls);\n> \n> - void fillParams(unsigned int frame, ipu3_uapi_params *params);\n> void parseStatistics(unsigned int frame,\n> int64_t frameTimestamp,\n> const ipu3_uapi_stats_3a *stats);\n> @@ -513,6 +512,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> @@ -526,7 +528,21 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> ipu3_uapi_params *params =\n> reinterpret_cast(mem.data());\n> \n> - fillParams(frame, params);\n> + /*\n> + * The incoming params buffer may contain uninitialised data, or the\n> + * parameters of previously queued frames. Clearing the entire buffer\n> + * may be an expensive operation, and the kernel will only read from\n> + * structures which have their associated use-flag set.\n> + *\n> + * It is the responsibility of the algorithms to set the use flags\n> + * accordingly for any data structure they update during prepare().\n> + */\n> + params->use = {};\n> +\n> + for (auto const &algo : algorithms_)\n> + algo->prepare(context_, params);\n> +\n> + paramsBufferReady.emit(frame);\n> }\n> \n> /**\n> @@ -569,33 +585,6 @@ void IPAIPU3::queueRequest(const uint32_t frame,\n> /* \\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> - /*\n> - * The incoming params buffer may contain uninitialised data, or the\n> - * parameters of previously queued frames. Clearing the entire buffer\n> - * may be an expensive operation, and the kernel will only read from\n> - * structures which have their associated use-flag set.\n> - *\n> - * It is the responsibility of the algorithms to set the use flags\n> - * accordingly for any data structure they update during prepare().\n> - */\n> - params->use = {};\n> -\n> - for (auto const &algo : algorithms_)\n> - algo->prepare(context_, params);\n> -\n> - paramsBufferReady.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\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 3556FC3256\n\tfor ;\n\tThu, 7 Apr 2022 22:14:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9334F65641;\n\tFri, 8 Apr 2022 00:14: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 7D09C65640\n\tfor ;\n\tFri, 8 Apr 2022 00:14:25 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 19207499;\n\tFri, 8 Apr 2022 00:14:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649369667;\n\tbh=h7BolUu1J2iFhvyIFA3JnogW+glVpJC/glBykCZkWJw=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=w2c5+lMRprVxJRiyx9iJeLiZ2tMNX5A1LBQ5UU4dSXEWbL8iLSH6Ww7lBC7cRi34R\n\tpmNeON7eDcbZUrz8e92uW8aA7ViPuW6wdFjxL7RVTzdJ+V3dw7ZKKROTc3gxMLleas\n\tW9iCL8NrYLGPTIyAs/JPAEC1H0hlxv9KeUnmEUdhOLtClH9oDUj8/kZfvk9ORlNw38\n\tJ6pKVGYjqv0m0RNRk7N6Yk2gEkddg4qM2LkDzZbh7zMEesOieSS6x7nIdafH3U2RRa\n\tN99yA4xjThva9MgLOwlbsLk6O5+QmCHhsZsWzP4pPvmVXOpkqjp8q5GktEqJflKLj0\n\twD8Z+adqgKXHQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649369665;\n\tbh=h7BolUu1J2iFhvyIFA3JnogW+glVpJC/glBykCZkWJw=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=S1E3e13hlquwyE1RmW5o2nJVnMhpCv0qrH2YpVLNCJUL13IvLdm7ld3HubvdF4V+f\n\tbRLZQ2rAGfQX9jWnM6jRf2unmyb2p4IFTs8j6QCnylX4LMR+hk4ZKlC4RHATi0wCez\n\tC2HnhAZK5qVo1z/SG4v6qgfKICXKpBN2TeynN1EA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"S1E3e13h\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220406141709.164794-3-umang.jain@ideasonboard.com>","References":"<20220406141709.164794-1-umang.jain@ideasonboard.com>\n\t<20220406141709.164794-3-umang.jain@ideasonboard.com>","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 07 Apr 2022 23:14:22 +0100","Message-ID":"<164936966265.22830.2313762450154896674@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v5 2/6] ipa: ipu3: Inline 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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22660,"web_url":"https://patchwork.libcamera.org/comment/22660/","msgid":"<20220408070232.GN3237525@pyrite.rasen.tech>","date":"2022-04-08T07:02:32","subject":"Re: [libcamera-devel] [PATCH v5 2/6] ipa: ipu3: Inline fillParams()\n\tin fillParamsBuffer()","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 Wed, Apr 06, 2022 at 07:47:05PM +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> Reviewed-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \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 9a59f80f..8c9a20f5 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -156,7 +156,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> @@ -513,6 +512,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> @@ -526,7 +528,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> @@ -569,33 +585,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\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 A8F19C0F1B\n\tfor ;\n\tFri, 8 Apr 2022 07:02:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7133D65644;\n\tFri, 8 Apr 2022 09:02:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BB4F61FBA\n\tfor ;\n\tFri, 8 Apr 2022 09:02:40 +0200 (CEST)","from pyrite.rasen.tech (softbank036240056250.bbtec.net\n\t[36.240.56.250])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 17EBF499;\n\tFri, 8 Apr 2022 09:02:38 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649401361;\n\tbh=AQEXHvYSOhoDVwfJ7JFsdyMascJmH4FBZVQo31bl518=;\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=Ves/Drm47vK3EGjR7l5p/GDvTolwW75aKs2g6wsRWX1oKek2YCoC7M72NouM7IcvK\n\tj8bhcuXQeCZDmpxDW9eZ7qzFeEk6ivGLm2i6bROvYDRviwL/bDbo8aMs35Y5Sa2NfD\n\tDPfIaWcAegaNX+oCL5vO3mgpDDNb/GagMZfu6gFaILqUtzXBES2gM43gM8F8PGkuAX\n\temebgCwo2DC8lZTI0Df02QNxWa5+QI2J7y+yabHhP8KyAuEFs8JpsCDfMF/kaHxJgb\n\tpLosgr7OAElIqK98p4xMW/T82LBBBq+OG47Gx3HMtz6M7X5bnhRSSVN8Y+pkUKXL5e\n\tXQQiG/wLLsJHg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649401360;\n\tbh=AQEXHvYSOhoDVwfJ7JFsdyMascJmH4FBZVQo31bl518=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NTPMEk2jHyVg8lBS2Iog7PcKe3tZ6vE02k6BIGS2SRFZ/qlPHtaXj3hW697cVS0ne\n\t6EgRb8a5tanlyonFNIVUKcmop1zUHb+HViAzWLGAKunGG1xbDSHmzyHmQPutMNEa6z\n\tT5d66dy14MXBgb5sNYAojL/tKRYUOTYigJazA824="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NTPMEk2j\"; dkim-atps=neutral","Date":"Fri, 8 Apr 2022 16:02:32 +0900","To":"Umang Jain ","Message-ID":"<20220408070232.GN3237525@pyrite.rasen.tech>","References":"<20220406141709.164794-1-umang.jain@ideasonboard.com>\n\t<20220406141709.164794-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220406141709.164794-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/6] ipa: ipu3: Inline 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":"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\" "}}]