[{"id":21029,"web_url":"https://patchwork.libcamera.org/comment/21029/","msgid":"<163732220339.1089182.2201697410917034354@Monstersaurus>","date":"2021-11-19T11:43:23","subject":"Re: [libcamera-devel] [PATCH v1 1/8] ipa: rkisp1: Pass IPASettings\n\tat init call","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-19 11:16:47)\n> When the IPA will be initialized, it will need to know the sensor model\n\n/will be/is/\n\n> used in order to properly call CameraSensorHelper for the analogue gain.\n> Modify the init definition in the pipeline handler and in the IPA as\n> well as the mojo interface to pass a IPASettings.\n\n/a /the /\n\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> include/libcamera/ipa/rkisp1.mojom | 4 +++-\n> src/ipa/rkisp1/rkisp1.cpp | 5 +++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++----\n> 3 files changed, 11 insertions(+), 7 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index cae757ea..a6991d4f 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -29,7 +29,9 @@ struct RkISP1Action {\n> };\n> \n> interface IPARkISP1Interface {\n> - init(uint32 hwRevision) => (int32 ret);\n> + init(libcamera.IPASettings settings,\n> + uint32 hwRevision)\n> + => (int32 ret);\n> start() => (int32 ret);\n> stop();\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index bf2c13b6..7ecbf8ae 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -34,7 +34,7 @@ namespace ipa::rkisp1 {\n> class IPARkISP1 : public IPARkISP1Interface\n> {\n> public:\n> - int init(unsigned int hwRevision) override;\n> + int init(const IPASettings &settings, unsigned int hwRevision) override;\n> int start() override;\n> void stop() override {}\n> \n> @@ -75,7 +75,8 @@ private:\n> unsigned int hwHistogramWeightGridsSize_;\n> };\n> \n> -int IPARkISP1::init(unsigned int hwRevision)\n> +int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n> + unsigned int hwRevision)\n> {\n> /* \\todo Add support for other revisions */\n> switch (hwRevision) {\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 98008862..db8856d3 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -88,7 +88,7 @@ public:\n> }\n> \n> PipelineHandlerRkISP1 *pipe();\n> - int loadIPA(unsigned int hwRevision);\n> + int loadIPA(const IPASettings &settings, unsigned int hwRevision);\n> \n> Stream mainPathStream_;\n> Stream selfPathStream_;\n> @@ -310,7 +310,7 @@ PipelineHandlerRkISP1 *RkISP1CameraData::pipe()\n> return static_cast(Camera::Private::pipe());\n> }\n> \n> -int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> +int RkISP1CameraData::loadIPA(const IPASettings &settings, unsigned int hwRevision)\n> {\n> ipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> if (!ipa_)\n> @@ -319,7 +319,7 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> ipa_->queueFrameAction.connect(this,\n> &RkISP1CameraData::queueFrameAction);\n> \n> - int ret = ipa_->init(hwRevision);\n> + int ret = ipa_->init(settings, hwRevision);\n> if (ret < 0) {\n> LOG(RkISP1, Error) << \"IPA initialization failure\";\n> return ret;\n> @@ -965,7 +965,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> isp_->frameStart.connect(data->delayedCtrls_.get(),\n> &DelayedControls::applyControls);\n> \n> - ret = data->loadIPA(media_->hwRevision());\n> + ret = data->loadIPA(IPASettings{ \"\", data->sensor_->model() },\n> + media_->hwRevision());\n\nThis passes the data->sensor_ into a call inside data which already has\nit... (You're giving the sensor info to the camera data, which is where\nyou retrieved the sensor info from....)\n\n\nYou could keep this line the same, and just provide the\nIPASettings during RkISP1CameraData::loadIPA() on the ipa_->init() call\nlike the IPU3 does.\n\n\n\n> if (ret)\n> return ret;\n> \n> -- \n> 2.32.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 61AD8BF415\n\tfor ;\n\tFri, 19 Nov 2021 11:43:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A9B5860378;\n\tFri, 19 Nov 2021 12:43:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D47336033C\n\tfor ;\n\tFri, 19 Nov 2021 12:43:25 +0100 (CET)","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 7BF931959;\n\tFri, 19 Nov 2021 12:43:25 +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=\"dRu9samt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637322205;\n\tbh=d4Ria8aXugespK/anOuIGe7XfNZNQDe8mBXE1K/jaf8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=dRu9samt7K8gyLRDCjoK2fq+BsSTNwkvB+pvH97pq2DPh6JVFBaAztAbD+/q7ekCk\n\tVqsSE95pXfKbRVu0hnDiMOeOHhmnm3pDD+Vb5SQFN6pAzGAQnb31XqzFLR9WauPnqv\n\t0u5oq7MoBsd5VWkz593m7TcQLyqmxfR6UZjF3bPA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211119111654.68445-2-jeanmichel.hautbois@ideasonboard.com>","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-2-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 19 Nov 2021 11:43:23 +0000","Message-ID":"<163732220339.1089182.2201697410917034354@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 1/8] ipa: rkisp1: Pass IPASettings\n\tat init 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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]