[{"id":21046,"web_url":"https://patchwork.libcamera.org/comment/21046/","msgid":"","date":"2021-11-19T13:54:05","subject":"Re: [libcamera-devel] [PATCH v1 7/8] ipa: rkisp1: agc: Introduce\n\tprepare call","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Fri, Nov 19, 2021 at 12:16:53PM +0100, Jean-Michel Hautbois wrote:\n> When a new parameter buffer needs to be queued, we need to specify which\n> algorithm is activated or not in the ISP. Add a simple prepare function\n> in AGC for that, which may later evolve to take the exposure locking\n> into account. For that function to be called, we also need to add the\n> loop on the algorithms in IPARkISP1::queueRequest.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 7 +++++++\n> src/ipa/rkisp1/algorithms/agc.h | 1 +\n> src/ipa/rkisp1/rkisp1.cpp | 12 +++---------\n> 3 files changed, 11 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 3d4a17ff..2b7c6fda 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -244,6 +244,13 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)\n> \tframeCount_++;\n> }\n> \n> +void Agc::prepare([[maybe_unused]] IPAContext &context,\n> +\t\t rkisp1_params_cfg *params)\n> +{\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;\n\nThere would be a tiny performance improvement if you set the\nRKISP1_CIF_ISP_MODULE_AEC flag in module_en_update only when the AEC\nconfiguration changed. Currently, that would be on the first frame only,\nas we don't support disabling AE anymore after this patch. It's so tiny\nthat it doesn't matter much at the moment, but once we implement support\nfor manual exposure, it would be nice to improve this.\n\nReviewed-by: Laurent Pinchart \n\n> +}\n> +\n> } /* namespace ipa::rkisp1::algorithms */\n> \n> } /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index fbb8ea98..934a455e 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -28,6 +28,7 @@ public:\n> \t~Agc() = default;\n> \n> \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> \tvoid process(IPAContext &context, const rkisp1_stat_buffer *stats) override;\n> \n> private:\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 1783f91f..b1bfb8b6 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -271,19 +271,13 @@ void IPARkISP1::processEvent(const RkISP1Event &event)\n> }\n> \n> void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> -\t\t\t const ControlList &controls)\n> +\t\t\t [[maybe_unused]] const ControlList &controls)\n> {\n> \t/* Prepare parameters buffer. */\n> \tmemset(params, 0, sizeof(*params));\n> \n> -\t/* Auto Exposure on/off. */\n> -\tif (controls.contains(controls::AeEnable)) {\n> -\t\tautoExposure_ = controls.get(controls::AeEnable);\n> -\t\tif (autoExposure_)\n> -\t\t\tparams->module_ens = RKISP1_CIF_ISP_MODULE_AEC;\n> -\n> -\t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n> -\t}\n> +\tfor (auto const &algo : algorithms_)\n> +\t\talgo->prepare(context_, params);\n> \n> \tRkISP1Action op;\n> \top.op = ActionParamFilled;","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 1232ABDB13\n\tfor ;\n\tFri, 19 Nov 2021 13:54:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 775E160371;\n\tFri, 19 Nov 2021 14:54:29 +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 5F3A8600B5\n\tfor ;\n\tFri, 19 Nov 2021 14:54:28 +0100 (CET)","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 D73861959;\n\tFri, 19 Nov 2021 14:54:27 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ez/1bRDU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637330068;\n\tbh=j2fKRSO0CwS+iKHO7THr2ftiW9kxq0JW2wqemNxduF4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ez/1bRDUKPal/xUNupPftzocBOz5QnRQyXn99hbnjOa+rnt4Y5Fa0HZDsDFwqOv8H\n\tXgby40ph2zYkVSMt4A1T4NEssi/EhiWq03GFuiw0jnIFrw3/SwaDQhRUhxu4s5LsPT\n\tVDdZRayOQuhXMJq+/kdpFCH0y2/4e66/CZ36hvy4=","Date":"Fri, 19 Nov 2021 15:54:05 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-8-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211119111654.68445-8-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 7/8] ipa: rkisp1: agc: Introduce\n\tprepare call","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]