[{"id":30077,"web_url":"https://patchwork.libcamera.org/comment/30077/","msgid":"<171934842555.739659.3265751443829855953@ping.linuxembedded.co.uk>","date":"2024-06-25T20:47:05","subject":"Re: [PATCH v3 1/4] libcamera: rkisp1: Prepare for additional camera\n\tcontrols","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2024-06-25 07:23:24)\n> Currently the rkisp1 pipeline handler only registers controls that are\n> related to the IPA. This patch prepares the rkisp1 pipeline-handler to\n> register camera controls which might not related to the IPA.\n\n'might not be' ... but I would write \"which are not\" here.\n\n> \n> Hence, introduce a additional ControlInfoMap for IPA controls. These\n> controls will be merged together with the controls in the pipeline\n> handler (introduced subsequently) as part of updateControls() and\n> together will be registered during the registeration of the camera.\n\ns/registration/\n\n> This is similar to what IPU3 pipeline handler handles its controls.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 28 +++++++++++++++++++++---\n> 1 file changed, 25 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 4cbf105d..bfc44239 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -109,6 +109,8 @@ public:\n> \n> std::unique_ptr ipa_;\n> \n> + ControlInfoMap ipaControls_;\n> +\n> private:\n> void paramFilled(unsigned int frame);\n> void setSensorControls(unsigned int frame,\n> @@ -184,6 +186,8 @@ private:\n> int allocateBuffers(Camera *camera);\n> int freeBuffers(Camera *camera);\n> \n> + int updateControls(RkISP1CameraData *data);\n> +\n> MediaDevice *media_;\n> std::unique_ptr isp_;\n> std::unique_ptr param_;\n> @@ -370,7 +374,7 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> }\n> \n> ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> - sensorInfo, sensor_->controls(), &controlInfo_);\n> + sensorInfo, sensor_->controls(), &ipaControls_);\n> if (ret < 0) {\n> LOG(RkISP1, Error) << \"IPA initialization failure\";\n> return ret;\n> @@ -820,12 +824,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \n> ipaConfig.sensorControls = data->sensor_->controls();\n> \n> - ret = data->ipa_->configure(ipaConfig, streamConfig, &data->controlInfo_);\n> + ret = data->ipa_->configure(ipaConfig, streamConfig, &data->ipaControls_);\n> if (ret) {\n> LOG(RkISP1, Error) << \"failed configuring IPA (\" << ret << \")\";\n> return ret;\n> }\n> - return 0;\n> +\n> + return updateControls(data);\n> }\n> \n> int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,\n> @@ -1101,8 +1106,23 @@ int PipelineHandlerRkISP1::initLinks(Camera *camera,\n> return 0;\n> }\n> \n> +int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> +{\n> + ControlInfoMap::Map rkisp1Controls;\n> +\n> + /* Add the IPA registered controls to list of camera controls. */\n> + for (const auto &ipaControl : data->ipaControls_)\n> + rkisp1Controls[ipaControl.first] = ipaControl.second;\n\nI wonder if we should have helpers to directly merge ControlInfoMaps\nlike we do for controls.\n\nThis patch looks like all it ever does is copy the ipa controls into the\ncontrolInfo that gets registered for the camera. Looks like a redundant\ncopy at the moment, so I'll come back to this after the next patches in\nthe series\n\n> +\n> + data->controlInfo_ = ControlInfoMap(std::move(rkisp1Controls),\n> + controls::controls);\n> +\n> + return 0;\n> +}\n> +\n> int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> {\n> + ControlInfoMap::Map rkisp1Controls;\n> int ret;\n> \n> std::unique_ptr data =\n> @@ -1137,6 +1157,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> if (ret)\n> return ret;\n> \n> + updateControls(data.get());\n> +\n> std::set streams{\n> &data->mainPathStream_,\n> &data->selfPathStream_,\n> -- \n> 2.44.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 23C1FBDB1D\n\tfor ;\n\tTue, 25 Jun 2024 20:47:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3B5EE654A9;\n\tTue, 25 Jun 2024 22:47:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9486D654A1\n\tfor ;\n\tTue, 25 Jun 2024 22:47:08 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C6E437E0;\n\tTue, 25 Jun 2024 22:46:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FjxtSfg0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719348405;\n\tbh=LqPnygc9LOdg3Rd1XAi0iLZdU93afCAU7gV4PbMPKsw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=FjxtSfg07CK/ovvEHgTMwZL1j+Zycd0fdXwDGKCzdEwl0kxbeo6bT+sw68sRlV1EK\n\t0iEusqBYdC1ZJFPF/UueOngFW5wvr2WnNbfHJHW8J3vIZuw5oV7TsDBOrYbeS07/PI\n\tN7QrWfbCqLVGYuxepvIbVEpEOVG2jMMhgQIWJh3c=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240625062327.50940-2-umang.jain@ideasonboard.com>","References":"<20240625062327.50940-1-umang.jain@ideasonboard.com>\n\t<20240625062327.50940-2-umang.jain@ideasonboard.com>","Subject":"Re: [PATCH v3 1/4] libcamera: rkisp1: Prepare for additional camera\n\tcontrols","From":"Kieran Bingham ","Cc":"Umang Jain ","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 25 Jun 2024 21:47:05 +0100","Message-ID":"<171934842555.739659.3265751443829855953@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}}]