[{"id":22588,"web_url":"https://patchwork.libcamera.org/comment/22588/","msgid":"<YkzJXTqZmskGWgOY@pendragon.ideasonboard.com>","date":"2022-04-05T22:57:33","subject":"Re: [libcamera-devel] [IPAIPU3 PATCH v4 4/6] ipu3: Use the new\n\tIPAIPU3 interface","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:00:56PM +0530, Umang Jain via libcamera-devel wrote:\n> Use the new IPAIPU3 interface that migrates the event-based ops\n> to dedication functions.\n> \n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>  ipu3.cpp | 102 ++++++++++++++++++++++---------------------------------\n>  1 file changed, 41 insertions(+), 61 deletions(-)\n> \n> diff --git a/ipu3.cpp b/ipu3.cpp\n> index 6ab86e0..289cc77 100644\n> --- a/ipu3.cpp\n> +++ b/ipu3.cpp\n> @@ -49,13 +49,17 @@ public:\n>  \n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> -\tvoid processEvent(const IPU3Event &event) override;\n> +\n> +\tvoid fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;\n> +\tvoid processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,\n> +\t\t\t\tconst uint32_t bufferId,\n> +\t\t\t\tconst ControlList &sensorControls) override;\n> +\tvoid queueRequest(const uint32_t frame, const ControlList &controls) override;\n\nAs for the libcamera IPA, I would sort this in the order in which\noperations will be called.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n>  \n>  private:\n>  \tvoid updateControls(const IPACameraSensorInfo &sensorInfo,\n>  \t\t\t    const ControlInfoMap &sensorControls,\n>  \t\t\t    ControlInfoMap *ipaControls);\n> -\tvoid processControls(unsigned int frame, const ControlList &metadata);\n>  \tvoid runAiq(unsigned int frame);\n>  \tvoid fillParams(unsigned int frame, ipu3_uapi_params *params);\n>  \tvoid parseStatistics(unsigned int frame,\n> @@ -321,54 +325,41 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)\n>  \t}\n>  }\n>  \n> -void IPAIPU3::processEvent(const IPU3Event &event)\n> +void IPAIPU3::queueRequest([[maybe_unused]] unsigned int frame,\n> +\t\t\t   [[maybe_unused]] const ControlList &controls)\n>  {\n> -\tswitch (event.op) {\n> -\tcase EventProcessControls: {\n> -\t\tprocessControls(event.frame, event.controls);\n> -\t\tbreak;\n> -\t}\n> -\tcase EventStatReady: {\n> -\t\tauto it = buffers_.find(event.bufferId);\n> -\t\tif (it == buffers_.end()) {\n> -\t\t\tLOG(IPAIPU3, Error) << \"Could not find stats buffer!\";\n> -\t\t\treturn;\n> -\t\t}\n> -\n> -\t\tSpan<uint8_t> mem = it->second.maps()[0];\n> -\t\tconst ipu3_uapi_stats_3a *stats =\n> -\t\t\treinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());\n> -\n> -\t\tparseStatistics(event.frame,\n> -\t\t\t\tevent.frameTimestamp,\n> -\t\t\t\tstats,\n> -\t\t\t\tevent.sensorControls);\n> -\t\tbreak;\n> -\t}\n> -\tcase EventFillParams: {\n> -\t\tauto it = buffers_.find(event.bufferId);\n> -\t\tif (it == buffers_.end()) {\n> -\t\t\tLOG(IPAIPU3, Error) << \"Could not find param buffer!\";\n> -\t\t\treturn;\n> -\t\t}\n> -\n> -\t\tSpan<uint8_t> mem = it->second.maps()[0];\n> -\t\tipu3_uapi_params *params =\n> -\t\t\treinterpret_cast<ipu3_uapi_params *>(mem.data());\n> -\n> -\t\tfillParams(event.frame, params);\n> -\t\tbreak;\n> -\t}\n> -\tdefault:\n> -\t\tLOG(IPAIPU3, Error) << \"Unknown event \" << event.op;\n> -\t\tbreak;\n> +\t/* \\todo Start processing for 'frame' based on 'controls'. */\n> +}\n> +\n> +void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> +{\n> +\tauto it = buffers_.find(bufferId);\n> +\tif (it == buffers_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Could not find params buffer\";\n> +\t\treturn;\n>  \t}\n> +\n> +\tSpan<uint8_t> mem = it->second.maps()[0];\n> +\tipu3_uapi_params *params =\n> +\t\treinterpret_cast<ipu3_uapi_params *>(mem.data());\n> +\n> +\tfillParams(frame, params);\n>  }\n>  \n> -void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,\n> -\t\t\t      [[maybe_unused]] const ControlList &controls)\n> +void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,\n> +\t\t\t\t const uint32_t bufferId, const ControlList &sensorControls)\n>  {\n> -\t/* \\todo Start processing for 'frame' based on 'controls'. */\n> +\tauto it = buffers_.find(bufferId);\n> +\tif (it == buffers_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Could not find stats buffer\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tSpan<uint8_t> mem = it->second.maps()[0];\n> +\tconst ipu3_uapi_stats_3a *stats =\n> +\t\treinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());\n> +\n> +\tparseStatistics(frame, frameTimestamp, stats, sensorControls);\n>  }\n>  \n>  void IPAIPU3::runAiq([[maybe_unused]] unsigned int frame)\n> @@ -424,10 +415,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n>  \n>  \tsetControls(frame);\n>  \n> -\tIPU3Action op;\n> -\top.op = ActionParamFilled;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tparamsBufferReady.emit(frame);\n>  }\n>  \n>  void IPAIPU3::parseStatistics(unsigned int frame,\n> @@ -488,27 +476,19 @@ void IPAIPU3::parseStatistics(unsigned int frame,\n>  \t\t\t\t(sensorInfo_.pixelRate / 1e6);\n>  \tctrls.set(controls::FrameDuration, frameDuration);\n>  \n> -\tIPU3Action op;\n> -\top.op = ActionMetadataReady;\n> -\top.controls = ctrls;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tmetadataReady.emit(frame, ctrls);\n>  }\n>  \n>  void IPAIPU3::setControls(unsigned int frame)\n>  {\n> -\tIPU3Action op;\n> -\top.op = ActionSetSensorControls;\n> -\n>  \tControlList sensorCtrls(ctrls_);\n>  \tsensorCtrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n>  \tsensorCtrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n>  \n> -\top.sensorControls = sensorCtrls;\n> -\n> -\top.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n> +\tControlList lensCtrls(ctrls_);\n> +\tlensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n>  \n> -\tqueueFrameAction.emit(frame, op);\n> +\tsetSensorControls.emit(frame, sensorCtrls, lensCtrls);\n>  }\n>  \n>  } /* namespace ipa::ipu3 */","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 E91C6C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Apr 2022 22:57:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54C776563F;\n\tWed,  6 Apr 2022 00:57:40 +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 667B1633A5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 Apr 2022 00:57:38 +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 3860F482;\n\tWed,  6 Apr 2022 00:57:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649199460;\n\tbh=JRJ5xWKS9w+Mw+1IwzYYhEwMaGESnN/V64lD1zSBdHM=;\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=3+rvwcM4b4nVWQN6tikCwySik0P9yiu9Hiw7a/HdfcQUohBmc5wViq+jDHT1YTjVS\n\tXeT2Foxrc2cd0khpnLfYmNbwohG0+/Odhfq3HH/8F7lWITdzKEDN86fFbSQd5Vcq3c\n\tp3Kb/9kfHlSagHLXfDxzTRV6rtyhZ4x5RCbG1VuQ8Yv83uHi6svhU0j0xGC56pZhQp\n\t2uxYcejKHwVfocw2dtjpfbi1JudkVn/MRsSXE1tUcBATA8+zZw68ueDYcp02JDz2Ie\n\tyemipz4uoohDYzLMkjQXNG3tnBlsJZfmBoM1PGIiDrW5kHs7M5kd6tP77u/lmdRziu\n\tmoaXYQqjs162w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649199457;\n\tbh=JRJ5xWKS9w+Mw+1IwzYYhEwMaGESnN/V64lD1zSBdHM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hbsX9eKT2F94Qz7sm5WHK/u29AFlzQ1KHJ2oHX/jw7YFqlRfMdOe11upVKt8XfPVc\n\tQLaiXT3rJLcE2p+VAUN0MFuJnozlKh1jb5h3u6yCS3DIL32aAM1RIYwUp2wnKyCAWw\n\tldS6CUwdVkyhWYi1ozqNtR+JwGx8QqX3PH7RYPek="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hbsX9eKT\"; dkim-atps=neutral","Date":"Wed, 6 Apr 2022 01:57:33 +0300","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<YkzJXTqZmskGWgOY@pendragon.ideasonboard.com>","References":"<20220331163058.171418-1-umang.jain@ideasonboard.com>\n\t<20220331163058.171418-5-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220331163058.171418-5-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [IPAIPU3 PATCH v4 4/6] ipu3: Use the new\n\tIPAIPU3 interface","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]