[{"id":35433,"web_url":"https://patchwork.libcamera.org/comment/35433/","msgid":"","date":"2025-08-15T10:44:07","subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\nOn 15/08/2025 11:29, Stefan Klug wrote:\n> Query the params device for RKISP1_CID_SUPPORTED_PARAMS_BLOCKS and\n> inject the information into the IPA hardware context for use by the\n> algorithms.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Kieran Bingham \n> \nReviewed-by: Daniel Scally \n> ---\n> \n> Changes in v3:\n> - Removed unnecessary log statement\n> - Collected tags\n> - Added case for getControls() failing internally\n> ---\n> include/libcamera/ipa/rkisp1.mojom | 2 +-\n> src/ipa/rkisp1/ipa_context.h | 1 +\n> src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++-----\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 29 ++++++++++++++++++++----\n> 4 files changed, 41 insertions(+), 11 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index 043ad27ea199..068e898848c4 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -16,7 +16,7 @@ struct IPAConfigInfo {\n> \n> interface IPARkISP1Interface {\n> \tinit(libcamera.IPASettings settings,\n> -\t uint32 hwRevision,\n> +\t uint32 hwRevision, uint32 supportedBlocks,\n> \t libcamera.IPACameraSensorInfo sensorInfo,\n> \t libcamera.ControlInfoMap sensorControls)\n> \t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 362ab2fda5fe..60cfab228edf 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -36,6 +36,7 @@ struct IPAHwSettings {\n> \tunsigned int numHistogramBins;\n> \tunsigned int numHistogramWeights;\n> \tunsigned int numGammaOutSamples;\n> +\tuint32_t supportedBlocks;\n> \tbool compand;\n> };\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index cf66d5553dcd..9ba0f0e6eb9d 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -52,6 +52,7 @@ public:\n> \tIPARkISP1();\n> \n> \tint init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t uint32_t supportedBlocks,\n> \t\t const IPACameraSensorInfo &sensorInfo,\n> \t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls) override;\n> @@ -89,27 +90,30 @@ private:\n> \n> namespace {\n> \n> -const IPAHwSettings ipaHwSettingsV10{\n> +IPAHwSettings ipaHwSettingsV10{\n> \tRKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> +\t0,\n> \tfalse,\n> };\n> \n> -const IPAHwSettings ipaHwSettingsIMX8MP{\n> +IPAHwSettings ipaHwSettingsIMX8MP{\n> \tRKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> +\t0,\n> \ttrue,\n> };\n> \n> -const IPAHwSettings ipaHwSettingsV12{\n> +IPAHwSettings ipaHwSettingsV12{\n> \tRKISP1_CIF_ISP_AE_MEAN_MAX_V12,\n> \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n> \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n> \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n> +\t0,\n> \tfalse,\n> };\n> \n> @@ -132,20 +136,22 @@ std::string IPARkISP1::logPrefix() const\n> }\n> \n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t uint32_t supportedBlocks,\n> \t\t const IPACameraSensorInfo &sensorInfo,\n> \t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls)\n> {\n> +\tIPAHwSettings *hw = nullptr;\n> \t/* \\todo Add support for other revisions */\n> \tswitch (hwRevision) {\n> \tcase RKISP1_V10:\n> -\t\tcontext_.hw = &ipaHwSettingsV10;\n> +\t\thw = &ipaHwSettingsV10;\n> \t\tbreak;\n> \tcase RKISP1_V_IMX8MP:\n> -\t\tcontext_.hw = &ipaHwSettingsIMX8MP;\n> +\t\thw = &ipaHwSettingsIMX8MP;\n> \t\tbreak;\n> \tcase RKISP1_V12:\n> -\t\tcontext_.hw = &ipaHwSettingsV12;\n> +\t\thw = &ipaHwSettingsV12;\n> \t\tbreak;\n> \tdefault:\n> \t\tLOG(IPARkISP1, Error)\n> @@ -153,6 +159,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \t\t\t<< \" is currently not supported\";\n> \t\treturn -ENODEV;\n> \t}\n> +\thw->supportedBlocks = supportedBlocks;\n> +\tcontext_.hw = hw;\n> \n> \tLOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index c4d4fdc6aa73..199f53d9e631 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -100,7 +100,7 @@ public:\n> \n> \tPipelineHandlerRkISP1 *pipe();\n> \tconst PipelineHandlerRkISP1 *pipe() const;\n> -\tint loadIPA(unsigned int hwRevision);\n> +\tint loadIPA(unsigned int hwRevision, uint32_t supportedBlocks);\n> \n> \tStream mainPathStream_;\n> \tStream selfPathStream_;\n> @@ -383,7 +383,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const\n> \treturn static_cast(Camera::Private::pipe());\n> }\n> \n> -int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> +int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks)\n> {\n> \tipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> \tif (!ipa_)\n> @@ -405,7 +405,8 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \t}\n> \n> \tret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> -\t\t\t sensorInfo, sensor_->controls(), &ipaControls_);\n> +\t\t\t supportedBlocks, sensorInfo, sensor_->controls(),\n> +\t\t\t &ipaControls_);\n> \tif (ret < 0) {\n> \t\tLOG(RkISP1, Error) << \"IPA initialization failure\";\n> \t\treturn ret;\n> @@ -1311,6 +1312,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> \treturn 0;\n> }\n> \n> +/*\n> + * By default we assume all the blocks that were included in the first\n> + * extensible parameters series are available. That is the lower 20bits.\n> + */\n> +const uint32_t kDefaultExtParamsBlocks = 0xfffff;\n> +\n> int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> {\n> \tint ret;\n> @@ -1348,7 +1355,21 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \tisp_->frameStart.connect(data->delayedCtrls_.get(),\n> \t\t\t\t &DelayedControls::applyControls);\n> \n> -\tret = data->loadIPA(media_->hwRevision());\n> +\tuint32_t supportedBlocks = kDefaultExtParamsBlocks;\n> +\n> +\tauto &controls = param_->controls();\n> +\tif (controls.find(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS) != controls.end()) {\n> +\t\tauto list = param_->getControls({ { RKISP1_CID_SUPPORTED_PARAMS_BLOCKS } });\n> +\t\tif (!list.empty())\n> +\t\t\tsupportedBlocks = static_cast(\n> +\t\t\t\tlist.get(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS)\n> +\t\t\t\t\t.get());\n> +\t} else {\n> +\t\tLOG(RkISP1, Error)\n> +\t\t\t<< \"Failed to query supported params blocks. Falling back to defaults.\";\n> +\t}\n> +\n> +\tret = data->loadIPA(media_->hwRevision(), supportedBlocks);\n> \tif (ret)\n> \t\treturn ret;\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 D34E7BDCC1\n\tfor ;\n\tFri, 15 Aug 2025 10:44:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EAA9D6925A;\n\tFri, 15 Aug 2025 12:44:11 +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 AFF356924E\n\tfor ;\n\tFri, 15 Aug 2025 12:44:10 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BCCC156D\n\tfor ;\n\tFri, 15 Aug 2025 12:43:15 +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=\"tgInnzGt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755254595;\n\tbh=/QnkI1wIPjJczEcx6DmCMVfsRU/TBFl743K8eBo3aX0=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=tgInnzGtulWXBhYlug0DY3lFHkrUG2FTxvf4XL8fTCYQjLuo73akM44M2rVdK3Y+3\n\tE4CtDDxdoHLebzT8djuVqu6aq5l7g9NQrAiXTEZ/XSV+T++SpYrp518Z8t/BkXmz+Q\n\toGTgq0Z7nzP400IHkFtocbLIG6q+Gb6u9VhjoSaQ=","Message-ID":"","Date":"Fri, 15 Aug 2025 11:44:07 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","To":"libcamera-devel@lists.libcamera.org","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-15-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","In-Reply-To":"<20250815102945.1602071-15-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}},{"id":35451,"web_url":"https://patchwork.libcamera.org/comment/35451/","msgid":"<31001a76-a40b-4aba-84cc-2c2dddafdc0d@ideasonboard.com>","date":"2025-08-15T14:52:53","subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 08. 15. 12:29 keltezéssel, Stefan Klug írta:\n> Query the params device for RKISP1_CID_SUPPORTED_PARAMS_BLOCKS and\n> inject the information into the IPA hardware context for use by the\n> algorithms.\n> \n> Signed-off-by: Stefan Klug \n> Reviewed-by: Kieran Bingham \n> \n> ---\n> \n> Changes in v3:\n> - Removed unnecessary log statement\n> - Collected tags\n> - Added case for getControls() failing internally\n> ---\n> include/libcamera/ipa/rkisp1.mojom | 2 +-\n> src/ipa/rkisp1/ipa_context.h | 1 +\n> src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++-----\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 29 ++++++++++++++++++++----\n> 4 files changed, 41 insertions(+), 11 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index 043ad27ea199..068e898848c4 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -16,7 +16,7 @@ struct IPAConfigInfo {\n> \n> interface IPARkISP1Interface {\n> \tinit(libcamera.IPASettings settings,\n> -\t uint32 hwRevision,\n> +\t uint32 hwRevision, uint32 supportedBlocks,\n> \t libcamera.IPACameraSensorInfo sensorInfo,\n> \t libcamera.ControlInfoMap sensorControls)\n> \t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 362ab2fda5fe..60cfab228edf 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -36,6 +36,7 @@ struct IPAHwSettings {\n> \tunsigned int numHistogramBins;\n> \tunsigned int numHistogramWeights;\n> \tunsigned int numGammaOutSamples;\n> +\tuint32_t supportedBlocks;\n> \tbool compand;\n> };\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index cf66d5553dcd..9ba0f0e6eb9d 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -52,6 +52,7 @@ public:\n> \tIPARkISP1();\n> \n> \tint init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t uint32_t supportedBlocks,\n> \t\t const IPACameraSensorInfo &sensorInfo,\n> \t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls) override;\n> @@ -89,27 +90,30 @@ private:\n> \n> namespace {\n> \n> -const IPAHwSettings ipaHwSettingsV10{\n> +IPAHwSettings ipaHwSettingsV10{\n> \tRKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> +\t0,\n> \tfalse,\n> };\n> \n> -const IPAHwSettings ipaHwSettingsIMX8MP{\n> +IPAHwSettings ipaHwSettingsIMX8MP{\n> \tRKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> +\t0,\n> \ttrue,\n> };\n> \n> -const IPAHwSettings ipaHwSettingsV12{\n> +IPAHwSettings ipaHwSettingsV12{\n> \tRKISP1_CIF_ISP_AE_MEAN_MAX_V12,\n> \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n> \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n> \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n> +\t0,\n> \tfalse,\n> };\n> \n> @@ -132,20 +136,22 @@ std::string IPARkISP1::logPrefix() const\n> }\n> \n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t uint32_t supportedBlocks,\n> \t\t const IPACameraSensorInfo &sensorInfo,\n> \t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls)\n> {\n> +\tIPAHwSettings *hw = nullptr;\n> \t/* \\todo Add support for other revisions */\n> \tswitch (hwRevision) {\n> \tcase RKISP1_V10:\n> -\t\tcontext_.hw = &ipaHwSettingsV10;\n> +\t\thw = &ipaHwSettingsV10;\n> \t\tbreak;\n> \tcase RKISP1_V_IMX8MP:\n> -\t\tcontext_.hw = &ipaHwSettingsIMX8MP;\n> +\t\thw = &ipaHwSettingsIMX8MP;\n> \t\tbreak;\n> \tcase RKISP1_V12:\n> -\t\tcontext_.hw = &ipaHwSettingsV12;\n> +\t\thw = &ipaHwSettingsV12;\n> \t\tbreak;\n> \tdefault:\n> \t\tLOG(IPARkISP1, Error)\n> @@ -153,6 +159,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \t\t\t<< \" is currently not supported\";\n> \t\treturn -ENODEV;\n> \t}\n> +\thw->supportedBlocks = supportedBlocks;\n\nThis seems to modify the global data. Is that going to be fine in the long run?\n\n\nRegards,\nBarnabás Pőcze\n\n\n> +\tcontext_.hw = hw;\n> \n> \tLOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index c4d4fdc6aa73..199f53d9e631 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -100,7 +100,7 @@ public:\n> \n> \tPipelineHandlerRkISP1 *pipe();\n> \tconst PipelineHandlerRkISP1 *pipe() const;\n> -\tint loadIPA(unsigned int hwRevision);\n> +\tint loadIPA(unsigned int hwRevision, uint32_t supportedBlocks);\n> \n> \tStream mainPathStream_;\n> \tStream selfPathStream_;\n> @@ -383,7 +383,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const\n> \treturn static_cast(Camera::Private::pipe());\n> }\n> \n> -int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> +int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks)\n> {\n> \tipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> \tif (!ipa_)\n> @@ -405,7 +405,8 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \t}\n> \n> \tret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> -\t\t\t sensorInfo, sensor_->controls(), &ipaControls_);\n> +\t\t\t supportedBlocks, sensorInfo, sensor_->controls(),\n> +\t\t\t &ipaControls_);\n> \tif (ret < 0) {\n> \t\tLOG(RkISP1, Error) << \"IPA initialization failure\";\n> \t\treturn ret;\n> @@ -1311,6 +1312,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> \treturn 0;\n> }\n> \n> +/*\n> + * By default we assume all the blocks that were included in the first\n> + * extensible parameters series are available. That is the lower 20bits.\n> + */\n> +const uint32_t kDefaultExtParamsBlocks = 0xfffff;\n> +\n> int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> {\n> \tint ret;\n> @@ -1348,7 +1355,21 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \tisp_->frameStart.connect(data->delayedCtrls_.get(),\n> \t\t\t\t &DelayedControls::applyControls);\n> \n> -\tret = data->loadIPA(media_->hwRevision());\n> +\tuint32_t supportedBlocks = kDefaultExtParamsBlocks;\n> +\n> +\tauto &controls = param_->controls();\n> +\tif (controls.find(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS) != controls.end()) {\n> +\t\tauto list = param_->getControls({ { RKISP1_CID_SUPPORTED_PARAMS_BLOCKS } });\n> +\t\tif (!list.empty())\n> +\t\t\tsupportedBlocks = static_cast(\n> +\t\t\t\tlist.get(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS)\n> +\t\t\t\t\t.get());\n> +\t} else {\n> +\t\tLOG(RkISP1, Error)\n> +\t\t\t<< \"Failed to query supported params blocks. Falling back to defaults.\";\n> +\t}\n> +\n> +\tret = data->loadIPA(media_->hwRevision(), supportedBlocks);\n> \tif (ret)\n> \t\treturn ret;\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 BF950BEFBE\n\tfor ;\n\tFri, 15 Aug 2025 14:52:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6CB269257;\n\tFri, 15 Aug 2025 16:52:58 +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 7356361443\n\tfor ;\n\tFri, 15 Aug 2025 16:52:57 +0200 (CEST)","from [192.168.33.21] (185.221.141.188.nat.pool.zt.hu\n\t[185.221.141.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 239AA63B;\n\tFri, 15 Aug 2025 16:52:02 +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=\"TMdoTOXE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755269522;\n\tbh=RHAcexFX3eBRvEbbmO4hrfIQ8VlcVoq8AFfyi+kn4TU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=TMdoTOXEsQgBazJBOWhZO4SkHTfO79i8OrNWV8FJPM/84rE5ibXuP32ifhUzAAO1s\n\tWgS25cZrKzOOwLWaeozA2tY7ev7rtMXqxAbZJdx1OLoj1JcAe0vsIDBFMRrBP0LpuP\n\tPkgy8bEt4lIz9vfh0mtSpnWXr+HCr0/K+XibrnIk=","Message-ID":"<31001a76-a40b-4aba-84cc-2c2dddafdc0d@ideasonboard.com>","Date":"Fri, 15 Aug 2025 16:52:53 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-15-stefan.klug@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250815102945.1602071-15-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}},{"id":35465,"web_url":"https://patchwork.libcamera.org/comment/35465/","msgid":"<175550507337.1970129.15514147873475603938@localhost>","date":"2025-08-18T08:17:53","subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Barnabás,\n\nThanks for the comment.\n\nQuoting Barnabás Pőcze (2025-08-15 16:52:53)\n> Hi\n> \n> 2025. 08. 15. 12:29 keltezéssel, Stefan Klug írta:\n> > Query the params device for RKISP1_CID_SUPPORTED_PARAMS_BLOCKS and\n> > inject the information into the IPA hardware context for use by the\n> > algorithms.\n> > \n> > Signed-off-by: Stefan Klug \n> > Reviewed-by: Kieran Bingham \n> > \n> > ---\n> > \n> > Changes in v3:\n> > - Removed unnecessary log statement\n> > - Collected tags\n> > - Added case for getControls() failing internally\n> > ---\n> > include/libcamera/ipa/rkisp1.mojom | 2 +-\n> > src/ipa/rkisp1/ipa_context.h | 1 +\n> > src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++-----\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 29 ++++++++++++++++++++----\n> > 4 files changed, 41 insertions(+), 11 deletions(-)\n> > \n> > diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> > index 043ad27ea199..068e898848c4 100644\n> > --- a/include/libcamera/ipa/rkisp1.mojom\n> > +++ b/include/libcamera/ipa/rkisp1.mojom\n> > @@ -16,7 +16,7 @@ struct IPAConfigInfo {\n> > \n> > interface IPARkISP1Interface {\n> > init(libcamera.IPASettings settings,\n> > - uint32 hwRevision,\n> > + uint32 hwRevision, uint32 supportedBlocks,\n> > libcamera.IPACameraSensorInfo sensorInfo,\n> > libcamera.ControlInfoMap sensorControls)\n> > => (int32 ret, libcamera.ControlInfoMap ipaControls);\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 362ab2fda5fe..60cfab228edf 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -36,6 +36,7 @@ struct IPAHwSettings {\n> > unsigned int numHistogramBins;\n> > unsigned int numHistogramWeights;\n> > unsigned int numGammaOutSamples;\n> > + uint32_t supportedBlocks;\n> > bool compand;\n> > };\n> > \n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index cf66d5553dcd..9ba0f0e6eb9d 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -52,6 +52,7 @@ public:\n> > IPARkISP1();\n> > \n> > int init(const IPASettings &settings, unsigned int hwRevision,\n> > + uint32_t supportedBlocks,\n> > const IPACameraSensorInfo &sensorInfo,\n> > const ControlInfoMap &sensorControls,\n> > ControlInfoMap *ipaControls) override;\n> > @@ -89,27 +90,30 @@ private:\n> > \n> > namespace {\n> > \n> > -const IPAHwSettings ipaHwSettingsV10{\n> > +IPAHwSettings ipaHwSettingsV10{\n> > RKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> > + 0,\n> > false,\n> > };\n> > \n> > -const IPAHwSettings ipaHwSettingsIMX8MP{\n> > +IPAHwSettings ipaHwSettingsIMX8MP{\n> > RKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> > + 0,\n> > true,\n> > };\n> > \n> > -const IPAHwSettings ipaHwSettingsV12{\n> > +IPAHwSettings ipaHwSettingsV12{\n> > RKISP1_CIF_ISP_AE_MEAN_MAX_V12,\n> > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n> > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n> > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n> > + 0,\n> > false,\n> > };\n> > \n> > @@ -132,20 +136,22 @@ std::string IPARkISP1::logPrefix() const\n> > }\n> > \n> > int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > + uint32_t supportedBlocks,\n> > const IPACameraSensorInfo &sensorInfo,\n> > const ControlInfoMap &sensorControls,\n> > ControlInfoMap *ipaControls)\n> > {\n> > + IPAHwSettings *hw = nullptr;\n> > /* \\todo Add support for other revisions */\n> > switch (hwRevision) {\n> > case RKISP1_V10:\n> > - context_.hw = &ipaHwSettingsV10;\n> > + hw = &ipaHwSettingsV10;\n> > break;\n> > case RKISP1_V_IMX8MP:\n> > - context_.hw = &ipaHwSettingsIMX8MP;\n> > + hw = &ipaHwSettingsIMX8MP;\n> > break;\n> > case RKISP1_V12:\n> > - context_.hw = &ipaHwSettingsV12;\n> > + hw = &ipaHwSettingsV12;\n> > break;\n> > default:\n> > LOG(IPARkISP1, Error)\n> > @@ -153,6 +159,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > << \" is currently not supported\";\n> > return -ENODEV;\n> > }\n> > + hw->supportedBlocks = supportedBlocks;\n> \n> This seems to modify the global data. Is that going to be fine in the long run?\n\nThat depends on how many hardware variation we will see :-). But you are\nright, this is not the typical way to solve it. I tried to do a minimal\ninvasive solution were the algorithms don't need any modification.\n\nI now tried it the other way around and I think it is better. THe\nrelevant diff is then:\n\ndiff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex 618b6d9049e2..c333c095632f 100644\n--- a/src/ipa/rkisp1/algorithms/agc.cpp\n+++ b/src/ipa/rkisp1/algorithms/agc.cpp\n@@ -56,7 +56,7 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData)\n\n \t\tstd::vector weights =\n \t\t\tvalue.getList().value_or(std::vector{});\n-\t\tif (weights.size() != context.hw->numHistogramWeights) {\n+\t\tif (weights.size() != context.hw.numHistogramWeights) {\n \t\t\tLOG(RkISP1Agc, Warning)\n \t\t\t\t<< \"Failed to read metering mode'\" << key << \"'\";\n \t\t\tcontinue;\n@@ -68,7 +68,7 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData)\n \tif (meteringModes_.empty()) {\n \t\tLOG(RkISP1Agc, Warning)\n \t\t\t<< \"No metering modes read from tuning file; defaulting to matrix\";\n-\t\tstd::vector weights(context.hw->numHistogramWeights, 1);\n+\t\tstd::vector weights(context.hw.numHistogramWeights, 1);\n\n \t\tmeteringModes_[controls::MeteringMatrix] = weights;\n \t}\n@@ -417,7 +417,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n\n \tSpan weights{\n \t\thstConfig->hist_weight,\n-\t\tcontext.hw->numHistogramWeights\n+\t\tcontext.hw.numHistogramWeights\n \t};\n \tstd::vector &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);\n \tstd::copy(modeWeights.begin(), modeWeights.end(), weights.begin());\n@@ -555,9 +555,9 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \tconst rkisp1_cif_isp_stat *params = &stats->params;\n\n \t/* The lower 4 bits are fractional and meant to be discarded. */\n-\tHistogram hist({ params->hist.hist_bins, context.hw->numHistogramBins },\n+\tHistogram hist({ params->hist.hist_bins, context.hw.numHistogramBins },\n \t\t [](uint32_t x) { return x >> 4; });\n-\texpMeans_ = { params->ae.exp_mean, context.hw->numAeCells };\n+\texpMeans_ = { params->ae.exp_mean, context.hw.numAeCells };\n \tstd::vector &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);\n \tweights_ = { modeWeights.data(), modeWeights.size() };\n\ndiff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\nindex 98cb7145e164..32fc44ffff92 100644\n--- a/src/ipa/rkisp1/algorithms/blc.cpp\n+++ b/src/ipa/rkisp1/algorithms/blc.cpp\n@@ -114,7 +114,7 @@ int BlackLevelCorrection::configure(IPAContext &context,\n \t * of the extensible parameters format.\n \t */\n \tsupported_ = context.configuration.paramFormat == V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n-\t\t !context.hw->compand;\n+\t\t !context.hw.compand;\n\n \tif (!supported_)\n \t\tLOG(RkISP1Blc, Warning)\n@@ -140,7 +140,7 @@ void BlackLevelCorrection::prepare(IPAContext &context,\n \tif (!supported_)\n \t\treturn;\n\n-\tif (context.hw->compand) {\n+\tif (context.hw.compand) {\n \t\tauto config = params->block();\n \t\tconfig.setEnabled(true);\n\ndiff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp\nindex e076727de4f1..84ecef577e9c 100644\n--- a/src/ipa/rkisp1/algorithms/compress.cpp\n+++ b/src/ipa/rkisp1/algorithms/compress.cpp\n@@ -53,7 +53,7 @@ int Compress::configure(IPAContext &context,\n \t\t\t[[maybe_unused]] const IPACameraSensorInfo &configInfo)\n {\n \tif (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n-\t !context.hw->compand) {\n+\t !context.hw.compand) {\n \t\tLOG(RkISP1Compress, Warning)\n \t\t\t<< \"Compression is not supported by the hardware or kernel.\";\n \t\treturn 0;\ndiff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp\nindex a9493678dba7..a0e7030fe5db 100644\n--- a/src/ipa/rkisp1/algorithms/goc.cpp\n+++ b/src/ipa/rkisp1/algorithms/goc.cpp\n@@ -50,7 +50,7 @@ const float kDefaultGamma = 2.2f;\n */\n int GammaOutCorrection::init(IPAContext &context, const YamlObject &tuningData)\n {\n-\tif (context.hw->numGammaOutSamples !=\n+\tif (context.hw.numGammaOutSamples !=\n \t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) {\n \t\tLOG(RkISP1Gamma, Error)\n \t\t\t<< \"Gamma is not implemented for RkISP1 V12\";\n@@ -101,7 +101,7 @@ void GammaOutCorrection::prepare(IPAContext &context,\n \t\t\t\t IPAFrameContext &frameContext,\n \t\t\t\t RkISP1Params *params)\n {\n-\tASSERT(context.hw->numGammaOutSamples ==\n+\tASSERT(context.hw.numGammaOutSamples ==\n \t RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10);\n\n \tif (!frameContext.goc.update)\ndiff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\nindex dd05f18d5e94..e8da69810008 100644\n--- a/src/ipa/rkisp1/algorithms/lux.cpp\n+++ b/src/ipa/rkisp1/algorithms/lux.cpp\n@@ -61,7 +61,7 @@ void Lux::process(IPAContext &context,\n\n \t/* \\todo Deduplicate the histogram calculation from AGC */\n \tconst rkisp1_cif_isp_stat *params = &stats->params;\n-\tHistogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },\n+\tHistogram yHist({ params->hist.hist_bins, context.hw.numHistogramBins },\n \t\t\t[](uint32_t x) { return x >> 4; });\n\n \tdouble lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);\ndiff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\nindex 362ab2fda5fe..57decbd3e2e6 100644\n--- a/src/ipa/rkisp1/ipa_context.h\n+++ b/src/ipa/rkisp1/ipa_context.h\n@@ -36,6 +36,7 @@ struct IPAHwSettings {\n \tunsigned int numHistogramBins;\n \tunsigned int numHistogramWeights;\n \tunsigned int numGammaOutSamples;\n+\tuint32_t supportedBlocks;\n \tbool compand;\n };\n\n@@ -201,11 +202,11 @@ struct IPAFrameContext : public FrameContext {\n\n struct IPAContext {\n \tIPAContext(unsigned int frameContextSize)\n-\t\t: hw(nullptr), frameContexts(frameContextSize)\n+\t\t: frameContexts(frameContextSize)\n \t{\n \t}\n\n-\tconst IPAHwSettings *hw;\n+\tIPAHwSettings hw;\n \tIPACameraSensorInfo sensorInfo;\n \tIPASessionConfiguration configuration;\n \tIPAActiveState activeState;\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex cf66d5553dcd..fa22bfc34904 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -52,6 +52,7 @@ public:\n \tIPARkISP1();\n\n \tint init(const IPASettings &settings, unsigned int hwRevision,\n+\t\t uint32_t supportedBlocks,\n \t\t const IPACameraSensorInfo &sensorInfo,\n \t\t const ControlInfoMap &sensorControls,\n \t\t ControlInfoMap *ipaControls) override;\n@@ -94,6 +95,7 @@ const IPAHwSettings ipaHwSettingsV10{\n \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n+\t0,\n \tfalse,\n };\n\n@@ -102,6 +104,7 @@ const IPAHwSettings ipaHwSettingsIMX8MP{\n \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n+\t0,\n \ttrue,\n };\n\n@@ -110,6 +113,7 @@ const IPAHwSettings ipaHwSettingsV12{\n \tRKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n \tRKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n \tRKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n+\t0,\n \tfalse,\n };\n\n@@ -132,6 +136,7 @@ std::string IPARkISP1::logPrefix() const\n }\n\n int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n+\t\t uint32_t supportedBlocks,\n \t\t const IPACameraSensorInfo &sensorInfo,\n \t\t const ControlInfoMap &sensorControls,\n \t\t ControlInfoMap *ipaControls)\n@@ -139,13 +144,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n \t/* \\todo Add support for other revisions */\n \tswitch (hwRevision) {\n \tcase RKISP1_V10:\n-\t\tcontext_.hw = &ipaHwSettingsV10;\n+\t\tcontext_.hw = ipaHwSettingsV10;\n \t\tbreak;\n \tcase RKISP1_V_IMX8MP:\n-\t\tcontext_.hw = &ipaHwSettingsIMX8MP;\n+\t\tcontext_.hw = ipaHwSettingsIMX8MP;\n \t\tbreak;\n \tcase RKISP1_V12:\n-\t\tcontext_.hw = &ipaHwSettingsV12;\n+\t\tcontext_.hw = ipaHwSettingsV12;\n \t\tbreak;\n \tdefault:\n \t\tLOG(IPARkISP1, Error)\n@@ -153,6 +158,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n \t\t\t<< \" is currently not supported\";\n \t\treturn -ENODEV;\n \t}\n+\tcontext_.hw.supportedBlocks = supportedBlocks;\n\n \tLOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n\n\nWhich version do you like better?\n\nBest regards,\nStefan\n\n\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> > + context_.hw = hw;\n> > \n> > LOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index c4d4fdc6aa73..199f53d9e631 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -100,7 +100,7 @@ public:\n> > \n> > PipelineHandlerRkISP1 *pipe();\n> > const PipelineHandlerRkISP1 *pipe() const;\n> > - int loadIPA(unsigned int hwRevision);\n> > + int loadIPA(unsigned int hwRevision, uint32_t supportedBlocks);\n> > \n> > Stream mainPathStream_;\n> > Stream selfPathStream_;\n> > @@ -383,7 +383,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const\n> > return static_cast(Camera::Private::pipe());\n> > }\n> > \n> > -int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > +int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks)\n> > {\n> > ipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> > if (!ipa_)\n> > @@ -405,7 +405,8 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > }\n> > \n> > ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> > - sensorInfo, sensor_->controls(), &ipaControls_);\n> > + supportedBlocks, sensorInfo, sensor_->controls(),\n> > + &ipaControls_);\n> > if (ret < 0) {\n> > LOG(RkISP1, Error) << \"IPA initialization failure\";\n> > return ret;\n> > @@ -1311,6 +1312,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> > return 0;\n> > }\n> > \n> > +/*\n> > + * By default we assume all the blocks that were included in the first\n> > + * extensible parameters series are available. That is the lower 20bits.\n> > + */\n> > +const uint32_t kDefaultExtParamsBlocks = 0xfffff;\n> > +\n> > int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > {\n> > int ret;\n> > @@ -1348,7 +1355,21 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > isp_->frameStart.connect(data->delayedCtrls_.get(),\n> > &DelayedControls::applyControls);\n> > \n> > - ret = data->loadIPA(media_->hwRevision());\n> > + uint32_t supportedBlocks = kDefaultExtParamsBlocks;\n> > +\n> > + auto &controls = param_->controls();\n> > + if (controls.find(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS) != controls.end()) {\n> > + auto list = param_->getControls({ { RKISP1_CID_SUPPORTED_PARAMS_BLOCKS } });\n> > + if (!list.empty())\n> > + supportedBlocks = static_cast(\n> > + list.get(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS)\n> > + .get());\n> > + } else {\n> > + LOG(RkISP1, Error)\n> > + << \"Failed to query supported params blocks. Falling back to defaults.\";\n> > + }\n> > +\n> > + ret = data->loadIPA(media_->hwRevision(), supportedBlocks);\n> > if (ret)\n> > return ret;\n> > \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 D0E45BEFBE\n\tfor ;\n\tMon, 18 Aug 2025 08:17:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D23B6925B;\n\tMon, 18 Aug 2025 10:17:57 +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 2F0526924E\n\tfor ;\n\tMon, 18 Aug 2025 10:17:56 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:766d:a405:f64e:fe3a])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 14B2F2394; \n\tMon, 18 Aug 2025 10:16:59 +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=\"DwhL8OLO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755505019;\n\tbh=yHtmSawHy6kKD7iEq4gRLe16Wzl4k7rJJCvSUC4nJ4A=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=DwhL8OLOxJjzNYLLrbJixA1JrmwIAGBLDbI+EF3NtoB4IPKTSGM2KAAxnLtQa/2B0\n\tY9pwRPdG9MDOCJjq9zwptJtQ0KBr5wvmFM7X0cJBXrQ4gTW4T3Bxiv/JDAw1eQXcHc\n\tU5mgqybsjXhZ/mhYVRYIDVa7uhZicbLOCCf1pW2Q=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<31001a76-a40b-4aba-84cc-2c2dddafdc0d@ideasonboard.com>","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-15-stefan.klug@ideasonboard.com>\n\t<31001a76-a40b-4aba-84cc-2c2dddafdc0d@ideasonboard.com>","Subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","From":"Stefan Klug ","Cc":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 18 Aug 2025 10:17:53 +0200","Message-ID":"<175550507337.1970129.15514147873475603938@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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\" "}},{"id":35470,"web_url":"https://patchwork.libcamera.org/comment/35470/","msgid":"<175550944369.1721288.1366960452708060611@ping.linuxembedded.co.uk>","date":"2025-08-18T09:30:43","subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-08-18 09:17:53)\n> Hi Barnabás,\n> \n> Thanks for the comment.\n> \n> Quoting Barnabás Pőcze (2025-08-15 16:52:53)\n> > Hi\n> > \n> > 2025. 08. 15. 12:29 keltezéssel, Stefan Klug írta:\n> > > Query the params device for RKISP1_CID_SUPPORTED_PARAMS_BLOCKS and\n> > > inject the information into the IPA hardware context for use by the\n> > > algorithms.\n> > > \n> > > Signed-off-by: Stefan Klug \n> > > Reviewed-by: Kieran Bingham \n> > > \n> > > ---\n> > > \n> > > Changes in v3:\n> > > - Removed unnecessary log statement\n> > > - Collected tags\n> > > - Added case for getControls() failing internally\n> > > ---\n> > > include/libcamera/ipa/rkisp1.mojom | 2 +-\n> > > src/ipa/rkisp1/ipa_context.h | 1 +\n> > > src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++-----\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 29 ++++++++++++++++++++----\n> > > 4 files changed, 41 insertions(+), 11 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> > > index 043ad27ea199..068e898848c4 100644\n> > > --- a/include/libcamera/ipa/rkisp1.mojom\n> > > +++ b/include/libcamera/ipa/rkisp1.mojom\n> > > @@ -16,7 +16,7 @@ struct IPAConfigInfo {\n> > > \n> > > interface IPARkISP1Interface {\n> > > init(libcamera.IPASettings settings,\n> > > - uint32 hwRevision,\n> > > + uint32 hwRevision, uint32 supportedBlocks,\n> > > libcamera.IPACameraSensorInfo sensorInfo,\n> > > libcamera.ControlInfoMap sensorControls)\n> > > => (int32 ret, libcamera.ControlInfoMap ipaControls);\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index 362ab2fda5fe..60cfab228edf 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -36,6 +36,7 @@ struct IPAHwSettings {\n> > > unsigned int numHistogramBins;\n> > > unsigned int numHistogramWeights;\n> > > unsigned int numGammaOutSamples;\n> > > + uint32_t supportedBlocks;\n> > > bool compand;\n> > > };\n> > > \n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index cf66d5553dcd..9ba0f0e6eb9d 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -52,6 +52,7 @@ public:\n> > > IPARkISP1();\n> > > \n> > > int init(const IPASettings &settings, unsigned int hwRevision,\n> > > + uint32_t supportedBlocks,\n> > > const IPACameraSensorInfo &sensorInfo,\n> > > const ControlInfoMap &sensorControls,\n> > > ControlInfoMap *ipaControls) override;\n> > > @@ -89,27 +90,30 @@ private:\n> > > \n> > > namespace {\n> > > \n> > > -const IPAHwSettings ipaHwSettingsV10{\n> > > +IPAHwSettings ipaHwSettingsV10{\n> > > RKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> > > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> > > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> > > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> > > + 0,\n> > > false,\n> > > };\n> > > \n> > > -const IPAHwSettings ipaHwSettingsIMX8MP{\n> > > +IPAHwSettings ipaHwSettingsIMX8MP{\n> > > RKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> > > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> > > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> > > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> > > + 0,\n> > > true,\n> > > };\n> > > \n> > > -const IPAHwSettings ipaHwSettingsV12{\n> > > +IPAHwSettings ipaHwSettingsV12{\n> > > RKISP1_CIF_ISP_AE_MEAN_MAX_V12,\n> > > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n> > > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n> > > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n> > > + 0,\n> > > false,\n> > > };\n> > > \n> > > @@ -132,20 +136,22 @@ std::string IPARkISP1::logPrefix() const\n> > > }\n> > > \n> > > int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > > + uint32_t supportedBlocks,\n> > > const IPACameraSensorInfo &sensorInfo,\n> > > const ControlInfoMap &sensorControls,\n> > > ControlInfoMap *ipaControls)\n> > > {\n> > > + IPAHwSettings *hw = nullptr;\n> > > /* \\todo Add support for other revisions */\n> > > switch (hwRevision) {\n> > > case RKISP1_V10:\n> > > - context_.hw = &ipaHwSettingsV10;\n> > > + hw = &ipaHwSettingsV10;\n> > > break;\n> > > case RKISP1_V_IMX8MP:\n> > > - context_.hw = &ipaHwSettingsIMX8MP;\n> > > + hw = &ipaHwSettingsIMX8MP;\n> > > break;\n> > > case RKISP1_V12:\n> > > - context_.hw = &ipaHwSettingsV12;\n> > > + hw = &ipaHwSettingsV12;\n> > > break;\n> > > default:\n> > > LOG(IPARkISP1, Error)\n> > > @@ -153,6 +159,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > > << \" is currently not supported\";\n> > > return -ENODEV;\n> > > }\n> > > + hw->supportedBlocks = supportedBlocks;\n> > \n> > This seems to modify the global data. Is that going to be fine in the long run?\n> \n> That depends on how many hardware variation we will see :-). But you are\n> right, this is not the typical way to solve it. I tried to do a minimal\n> invasive solution were the algorithms don't need any modification.\n> \n> I now tried it the other way around and I think it is better. THe\n> relevant diff is then:\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 618b6d9049e2..c333c095632f 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -56,7 +56,7 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData)\n> \n> std::vector weights =\n> value.getList().value_or(std::vector{});\n> - if (weights.size() != context.hw->numHistogramWeights) {\n> + if (weights.size() != context.hw.numHistogramWeights) {\n> LOG(RkISP1Agc, Warning)\n> << \"Failed to read metering mode'\" << key << \"'\";\n> continue;\n> @@ -68,7 +68,7 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData)\n> if (meteringModes_.empty()) {\n> LOG(RkISP1Agc, Warning)\n> << \"No metering modes read from tuning file; defaulting to matrix\";\n> - std::vector weights(context.hw->numHistogramWeights, 1);\n> + std::vector weights(context.hw.numHistogramWeights, 1);\n> \n> meteringModes_[controls::MeteringMatrix] = weights;\n> }\n> @@ -417,7 +417,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n> \n> Span weights{\n> hstConfig->hist_weight,\n> - context.hw->numHistogramWeights\n> + context.hw.numHistogramWeights\n> };\n> std::vector &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);\n> std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());\n> @@ -555,9 +555,9 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> const rkisp1_cif_isp_stat *params = &stats->params;\n> \n> /* The lower 4 bits are fractional and meant to be discarded. */\n> - Histogram hist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> + Histogram hist({ params->hist.hist_bins, context.hw.numHistogramBins },\n> [](uint32_t x) { return x >> 4; });\n> - expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };\n> + expMeans_ = { params->ae.exp_mean, context.hw.numAeCells };\n> std::vector &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);\n> weights_ = { modeWeights.data(), modeWeights.size() };\n> \n> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> index 98cb7145e164..32fc44ffff92 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -114,7 +114,7 @@ int BlackLevelCorrection::configure(IPAContext &context,\n> * of the extensible parameters format.\n> */\n> supported_ = context.configuration.paramFormat == V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n> - !context.hw->compand;\n> + !context.hw.compand;\n> \n> if (!supported_)\n> LOG(RkISP1Blc, Warning)\n> @@ -140,7 +140,7 @@ void BlackLevelCorrection::prepare(IPAContext &context,\n> if (!supported_)\n> return;\n> \n> - if (context.hw->compand) {\n> + if (context.hw.compand) {\n> auto config = params->block();\n> config.setEnabled(true);\n> \n> diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp\n> index e076727de4f1..84ecef577e9c 100644\n> --- a/src/ipa/rkisp1/algorithms/compress.cpp\n> +++ b/src/ipa/rkisp1/algorithms/compress.cpp\n> @@ -53,7 +53,7 @@ int Compress::configure(IPAContext &context,\n> [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> {\n> if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n> - !context.hw->compand) {\n> + !context.hw.compand) {\n> LOG(RkISP1Compress, Warning)\n> << \"Compression is not supported by the hardware or kernel.\";\n> return 0;\n> diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp\n> index a9493678dba7..a0e7030fe5db 100644\n> --- a/src/ipa/rkisp1/algorithms/goc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/goc.cpp\n> @@ -50,7 +50,7 @@ const float kDefaultGamma = 2.2f;\n> */\n> int GammaOutCorrection::init(IPAContext &context, const YamlObject &tuningData)\n> {\n> - if (context.hw->numGammaOutSamples !=\n> + if (context.hw.numGammaOutSamples !=\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) {\n> LOG(RkISP1Gamma, Error)\n> << \"Gamma is not implemented for RkISP1 V12\";\n> @@ -101,7 +101,7 @@ void GammaOutCorrection::prepare(IPAContext &context,\n> IPAFrameContext &frameContext,\n> RkISP1Params *params)\n> {\n> - ASSERT(context.hw->numGammaOutSamples ==\n> + ASSERT(context.hw.numGammaOutSamples ==\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10);\n> \n> if (!frameContext.goc.update)\n> diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n> index dd05f18d5e94..e8da69810008 100644\n> --- a/src/ipa/rkisp1/algorithms/lux.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n> @@ -61,7 +61,7 @@ void Lux::process(IPAContext &context,\n> \n> /* \\todo Deduplicate the histogram calculation from AGC */\n> const rkisp1_cif_isp_stat *params = &stats->params;\n> - Histogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> + Histogram yHist({ params->hist.hist_bins, context.hw.numHistogramBins },\n> [](uint32_t x) { return x >> 4; });\n> \n> double lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 362ab2fda5fe..57decbd3e2e6 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -36,6 +36,7 @@ struct IPAHwSettings {\n> unsigned int numHistogramBins;\n> unsigned int numHistogramWeights;\n> unsigned int numGammaOutSamples;\n> + uint32_t supportedBlocks;\n> bool compand;\n> };\n> \n> @@ -201,11 +202,11 @@ struct IPAFrameContext : public FrameContext {\n> \n> struct IPAContext {\n> IPAContext(unsigned int frameContextSize)\n> - : hw(nullptr), frameContexts(frameContextSize)\n> + : frameContexts(frameContextSize)\n> {\n> }\n> \n> - const IPAHwSettings *hw;\n> + IPAHwSettings hw;\n> IPACameraSensorInfo sensorInfo;\n> IPASessionConfiguration configuration;\n> IPAActiveState activeState;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index cf66d5553dcd..fa22bfc34904 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -52,6 +52,7 @@ public:\n> IPARkISP1();\n> \n> int init(const IPASettings &settings, unsigned int hwRevision,\n> + uint32_t supportedBlocks,\n> const IPACameraSensorInfo &sensorInfo,\n> const ControlInfoMap &sensorControls,\n> ControlInfoMap *ipaControls) override;\n> @@ -94,6 +95,7 @@ const IPAHwSettings ipaHwSettingsV10{\n> RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> + 0,\n> false,\n> };\n> \n> @@ -102,6 +104,7 @@ const IPAHwSettings ipaHwSettingsIMX8MP{\n> RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> + 0,\n> true,\n> };\n> \n> @@ -110,6 +113,7 @@ const IPAHwSettings ipaHwSettingsV12{\n> RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n> RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n> + 0,\n> false,\n> };\n> \n> @@ -132,6 +136,7 @@ std::string IPARkISP1::logPrefix() const\n> }\n> \n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> + uint32_t supportedBlocks,\n> const IPACameraSensorInfo &sensorInfo,\n> const ControlInfoMap &sensorControls,\n> ControlInfoMap *ipaControls)\n> @@ -139,13 +144,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> /* \\todo Add support for other revisions */\n> switch (hwRevision) {\n> case RKISP1_V10:\n> - context_.hw = &ipaHwSettingsV10;\n> + context_.hw = ipaHwSettingsV10;\n> break;\n> case RKISP1_V_IMX8MP:\n> - context_.hw = &ipaHwSettingsIMX8MP;\n> + context_.hw = ipaHwSettingsIMX8MP;\n> break;\n> case RKISP1_V12:\n> - context_.hw = &ipaHwSettingsV12;\n> + context_.hw = ipaHwSettingsV12;\n> break;\n> default:\n> LOG(IPARkISP1, Error)\n> @@ -153,6 +158,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> << \" is currently not supported\";\n> return -ENODEV;\n> }\n> + context_.hw.supportedBlocks = supportedBlocks;\n> \n> LOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n> \n> \n> Which version do you like better?\n\nI think I like this new version where the context_.hw is initialised\nfrom the static data - but doesn't modify the global.\n\nRemembering to put the static initialisation data back to const of\ncourse ;-)\n\n--\nKieran\n\n> \n> Best regards,\n> Stefan\n> \n> \n> > \n> > \n> > Regards,\n> > Barnabás Pőcze\n> > \n> > \n> > > + context_.hw = hw;\n> > > \n> > > LOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n> > > \n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index c4d4fdc6aa73..199f53d9e631 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -100,7 +100,7 @@ public:\n> > > \n> > > PipelineHandlerRkISP1 *pipe();\n> > > const PipelineHandlerRkISP1 *pipe() const;\n> > > - int loadIPA(unsigned int hwRevision);\n> > > + int loadIPA(unsigned int hwRevision, uint32_t supportedBlocks);\n> > > \n> > > Stream mainPathStream_;\n> > > Stream selfPathStream_;\n> > > @@ -383,7 +383,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const\n> > > return static_cast(Camera::Private::pipe());\n> > > }\n> > > \n> > > -int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > > +int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks)\n> > > {\n> > > ipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> > > if (!ipa_)\n> > > @@ -405,7 +405,8 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > > }\n> > > \n> > > ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> > > - sensorInfo, sensor_->controls(), &ipaControls_);\n> > > + supportedBlocks, sensorInfo, sensor_->controls(),\n> > > + &ipaControls_);\n> > > if (ret < 0) {\n> > > LOG(RkISP1, Error) << \"IPA initialization failure\";\n> > > return ret;\n> > > @@ -1311,6 +1312,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> > > return 0;\n> > > }\n> > > \n> > > +/*\n> > > + * By default we assume all the blocks that were included in the first\n> > > + * extensible parameters series are available. That is the lower 20bits.\n> > > + */\n> > > +const uint32_t kDefaultExtParamsBlocks = 0xfffff;\n> > > +\n> > > int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > > {\n> > > int ret;\n> > > @@ -1348,7 +1355,21 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > > isp_->frameStart.connect(data->delayedCtrls_.get(),\n> > > &DelayedControls::applyControls);\n> > > \n> > > - ret = data->loadIPA(media_->hwRevision());\n> > > + uint32_t supportedBlocks = kDefaultExtParamsBlocks;\n> > > +\n> > > + auto &controls = param_->controls();\n> > > + if (controls.find(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS) != controls.end()) {\n> > > + auto list = param_->getControls({ { RKISP1_CID_SUPPORTED_PARAMS_BLOCKS } });\n> > > + if (!list.empty())\n> > > + supportedBlocks = static_cast(\n> > > + list.get(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS)\n> > > + .get());\n> > > + } else {\n> > > + LOG(RkISP1, Error)\n> > > + << \"Failed to query supported params blocks. Falling back to defaults.\";\n> > > + }\n> > > +\n> > > + ret = data->loadIPA(media_->hwRevision(), supportedBlocks);\n> > > if (ret)\n> > > return ret;\n> > > \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 53E57BDCC1\n\tfor ;\n\tMon, 18 Aug 2025 09:30:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 00E786925A;\n\tMon, 18 Aug 2025 11:30:48 +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 4FFA061427\n\tfor ;\n\tMon, 18 Aug 2025 11:30:46 +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 3BC3B17D1;\n\tMon, 18 Aug 2025 11:29:49 +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=\"WQ8D/2/2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755509389;\n\tbh=YygIoraJt2htyTJ0rZFQfnz5M+oGV505z2JEHXbb6GQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=WQ8D/2/2C6NLrlfRxKvKcEd1/LKN01PHxf3l5CFtB5YEPsL6ELOSHFJsy8sNsvAH8\n\tmqwgOZL8UpkC7YdkZzyU7eYdl+7sKcdhGmnBVKpuGQ2PLmgJU5bdnopLW7tryXH8to\n\tg1/LH26g185GuFaYwgfMGuA8D/JAWdD9zCH0stD8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175550507337.1970129.15514147873475603938@localhost>","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-15-stefan.klug@ideasonboard.com>\n\t<31001a76-a40b-4aba-84cc-2c2dddafdc0d@ideasonboard.com>\n\t<175550507337.1970129.15514147873475603938@localhost>","Subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","From":"Kieran Bingham ","Cc":"","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tStefan Klug , \n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 18 Aug 2025 10:30:43 +0100","Message-ID":"<175550944369.1721288.1366960452708060611@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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\" "}},{"id":35705,"web_url":"https://patchwork.libcamera.org/comment/35705/","msgid":"<175706897556.1787083.5731519852834021849@neptunite.rasen.tech>","date":"2025-09-05T10:42:55","subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-08-18 17:17:53)\n> Hi Barnabás,\n> \n> Thanks for the comment.\n> \n> Quoting Barnabás Pőcze (2025-08-15 16:52:53)\n> > Hi\n> > \n> > 2025. 08. 15. 12:29 keltezéssel, Stefan Klug írta:\n> > > Query the params device for RKISP1_CID_SUPPORTED_PARAMS_BLOCKS and\n> > > inject the information into the IPA hardware context for use by the\n> > > algorithms.\n> > > \n> > > Signed-off-by: Stefan Klug \n> > > Reviewed-by: Kieran Bingham \n> > > \n> > > ---\n> > > \n> > > Changes in v3:\n> > > - Removed unnecessary log statement\n> > > - Collected tags\n> > > - Added case for getControls() failing internally\n> > > ---\n> > > include/libcamera/ipa/rkisp1.mojom | 2 +-\n> > > src/ipa/rkisp1/ipa_context.h | 1 +\n> > > src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++-----\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 29 ++++++++++++++++++++----\n> > > 4 files changed, 41 insertions(+), 11 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> > > index 043ad27ea199..068e898848c4 100644\n> > > --- a/include/libcamera/ipa/rkisp1.mojom\n> > > +++ b/include/libcamera/ipa/rkisp1.mojom\n> > > @@ -16,7 +16,7 @@ struct IPAConfigInfo {\n> > > \n> > > interface IPARkISP1Interface {\n> > > init(libcamera.IPASettings settings,\n> > > - uint32 hwRevision,\n> > > + uint32 hwRevision, uint32 supportedBlocks,\n> > > libcamera.IPACameraSensorInfo sensorInfo,\n> > > libcamera.ControlInfoMap sensorControls)\n> > > => (int32 ret, libcamera.ControlInfoMap ipaControls);\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index 362ab2fda5fe..60cfab228edf 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -36,6 +36,7 @@ struct IPAHwSettings {\n> > > unsigned int numHistogramBins;\n> > > unsigned int numHistogramWeights;\n> > > unsigned int numGammaOutSamples;\n> > > + uint32_t supportedBlocks;\n> > > bool compand;\n> > > };\n> > > \n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index cf66d5553dcd..9ba0f0e6eb9d 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -52,6 +52,7 @@ public:\n> > > IPARkISP1();\n> > > \n> > > int init(const IPASettings &settings, unsigned int hwRevision,\n> > > + uint32_t supportedBlocks,\n> > > const IPACameraSensorInfo &sensorInfo,\n> > > const ControlInfoMap &sensorControls,\n> > > ControlInfoMap *ipaControls) override;\n> > > @@ -89,27 +90,30 @@ private:\n> > > \n> > > namespace {\n> > > \n> > > -const IPAHwSettings ipaHwSettingsV10{\n> > > +IPAHwSettings ipaHwSettingsV10{\n> > > RKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> > > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> > > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> > > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> > > + 0,\n> > > false,\n> > > };\n> > > \n> > > -const IPAHwSettings ipaHwSettingsIMX8MP{\n> > > +IPAHwSettings ipaHwSettingsIMX8MP{\n> > > RKISP1_CIF_ISP_AE_MEAN_MAX_V10,\n> > > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> > > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> > > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> > > + 0,\n> > > true,\n> > > };\n> > > \n> > > -const IPAHwSettings ipaHwSettingsV12{\n> > > +IPAHwSettings ipaHwSettingsV12{\n> > > RKISP1_CIF_ISP_AE_MEAN_MAX_V12,\n> > > RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n> > > RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n> > > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n> > > + 0,\n> > > false,\n> > > };\n> > > \n> > > @@ -132,20 +136,22 @@ std::string IPARkISP1::logPrefix() const\n> > > }\n> > > \n> > > int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > > + uint32_t supportedBlocks,\n> > > const IPACameraSensorInfo &sensorInfo,\n> > > const ControlInfoMap &sensorControls,\n> > > ControlInfoMap *ipaControls)\n> > > {\n> > > + IPAHwSettings *hw = nullptr;\n> > > /* \\todo Add support for other revisions */\n> > > switch (hwRevision) {\n> > > case RKISP1_V10:\n> > > - context_.hw = &ipaHwSettingsV10;\n> > > + hw = &ipaHwSettingsV10;\n> > > break;\n> > > case RKISP1_V_IMX8MP:\n> > > - context_.hw = &ipaHwSettingsIMX8MP;\n> > > + hw = &ipaHwSettingsIMX8MP;\n> > > break;\n> > > case RKISP1_V12:\n> > > - context_.hw = &ipaHwSettingsV12;\n> > > + hw = &ipaHwSettingsV12;\n> > > break;\n> > > default:\n> > > LOG(IPARkISP1, Error)\n> > > @@ -153,6 +159,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > > << \" is currently not supported\";\n> > > return -ENODEV;\n> > > }\n> > > + hw->supportedBlocks = supportedBlocks;\n> > \n> > This seems to modify the global data. Is that going to be fine in the long run?\n> \n> That depends on how many hardware variation we will see :-). But you are\n> right, this is not the typical way to solve it. I tried to do a minimal\n> invasive solution were the algorithms don't need any modification.\n> \n> I now tried it the other way around and I think it is better. THe\n> relevant diff is then:\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 618b6d9049e2..c333c095632f 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -56,7 +56,7 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData)\n> \n> std::vector weights =\n> value.getList().value_or(std::vector{});\n> - if (weights.size() != context.hw->numHistogramWeights) {\n> + if (weights.size() != context.hw.numHistogramWeights) {\n> LOG(RkISP1Agc, Warning)\n> << \"Failed to read metering mode'\" << key << \"'\";\n> continue;\n> @@ -68,7 +68,7 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData)\n> if (meteringModes_.empty()) {\n> LOG(RkISP1Agc, Warning)\n> << \"No metering modes read from tuning file; defaulting to matrix\";\n> - std::vector weights(context.hw->numHistogramWeights, 1);\n> + std::vector weights(context.hw.numHistogramWeights, 1);\n> \n> meteringModes_[controls::MeteringMatrix] = weights;\n> }\n> @@ -417,7 +417,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n> \n> Span weights{\n> hstConfig->hist_weight,\n> - context.hw->numHistogramWeights\n> + context.hw.numHistogramWeights\n> };\n> std::vector &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);\n> std::copy(modeWeights.begin(), modeWeights.end(), weights.begin());\n> @@ -555,9 +555,9 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> const rkisp1_cif_isp_stat *params = &stats->params;\n> \n> /* The lower 4 bits are fractional and meant to be discarded. */\n> - Histogram hist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> + Histogram hist({ params->hist.hist_bins, context.hw.numHistogramBins },\n> [](uint32_t x) { return x >> 4; });\n> - expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };\n> + expMeans_ = { params->ae.exp_mean, context.hw.numAeCells };\n> std::vector &modeWeights = meteringModes_.at(frameContext.agc.meteringMode);\n> weights_ = { modeWeights.data(), modeWeights.size() };\n> \n> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> index 98cb7145e164..32fc44ffff92 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -114,7 +114,7 @@ int BlackLevelCorrection::configure(IPAContext &context,\n> * of the extensible parameters format.\n> */\n> supported_ = context.configuration.paramFormat == V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n> - !context.hw->compand;\n> + !context.hw.compand;\n> \n> if (!supported_)\n> LOG(RkISP1Blc, Warning)\n> @@ -140,7 +140,7 @@ void BlackLevelCorrection::prepare(IPAContext &context,\n> if (!supported_)\n> return;\n> \n> - if (context.hw->compand) {\n> + if (context.hw.compand) {\n> auto config = params->block();\n> config.setEnabled(true);\n> \n> diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp\n> index e076727de4f1..84ecef577e9c 100644\n> --- a/src/ipa/rkisp1/algorithms/compress.cpp\n> +++ b/src/ipa/rkisp1/algorithms/compress.cpp\n> @@ -53,7 +53,7 @@ int Compress::configure(IPAContext &context,\n> [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> {\n> if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n> - !context.hw->compand) {\n> + !context.hw.compand) {\n> LOG(RkISP1Compress, Warning)\n> << \"Compression is not supported by the hardware or kernel.\";\n> return 0;\n> diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp\n> index a9493678dba7..a0e7030fe5db 100644\n> --- a/src/ipa/rkisp1/algorithms/goc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/goc.cpp\n> @@ -50,7 +50,7 @@ const float kDefaultGamma = 2.2f;\n> */\n> int GammaOutCorrection::init(IPAContext &context, const YamlObject &tuningData)\n> {\n> - if (context.hw->numGammaOutSamples !=\n> + if (context.hw.numGammaOutSamples !=\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10) {\n> LOG(RkISP1Gamma, Error)\n> << \"Gamma is not implemented for RkISP1 V12\";\n> @@ -101,7 +101,7 @@ void GammaOutCorrection::prepare(IPAContext &context,\n> IPAFrameContext &frameContext,\n> RkISP1Params *params)\n> {\n> - ASSERT(context.hw->numGammaOutSamples ==\n> + ASSERT(context.hw.numGammaOutSamples ==\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10);\n> \n> if (!frameContext.goc.update)\n> diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n> index dd05f18d5e94..e8da69810008 100644\n> --- a/src/ipa/rkisp1/algorithms/lux.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n> @@ -61,7 +61,7 @@ void Lux::process(IPAContext &context,\n> \n> /* \\todo Deduplicate the histogram calculation from AGC */\n> const rkisp1_cif_isp_stat *params = &stats->params;\n> - Histogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> + Histogram yHist({ params->hist.hist_bins, context.hw.numHistogramBins },\n> [](uint32_t x) { return x >> 4; });\n> \n> double lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 362ab2fda5fe..57decbd3e2e6 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -36,6 +36,7 @@ struct IPAHwSettings {\n> unsigned int numHistogramBins;\n> unsigned int numHistogramWeights;\n> unsigned int numGammaOutSamples;\n> + uint32_t supportedBlocks;\n> bool compand;\n> };\n> \n> @@ -201,11 +202,11 @@ struct IPAFrameContext : public FrameContext {\n> \n> struct IPAContext {\n> IPAContext(unsigned int frameContextSize)\n> - : hw(nullptr), frameContexts(frameContextSize)\n> + : frameContexts(frameContextSize)\n> {\n> }\n> \n> - const IPAHwSettings *hw;\n> + IPAHwSettings hw;\n> IPACameraSensorInfo sensorInfo;\n> IPASessionConfiguration configuration;\n> IPAActiveState activeState;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index cf66d5553dcd..fa22bfc34904 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -52,6 +52,7 @@ public:\n> IPARkISP1();\n> \n> int init(const IPASettings &settings, unsigned int hwRevision,\n> + uint32_t supportedBlocks,\n> const IPACameraSensorInfo &sensorInfo,\n> const ControlInfoMap &sensorControls,\n> ControlInfoMap *ipaControls) override;\n> @@ -94,6 +95,7 @@ const IPAHwSettings ipaHwSettingsV10{\n> RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> + 0,\n> false,\n> };\n> \n> @@ -102,6 +104,7 @@ const IPAHwSettings ipaHwSettingsIMX8MP{\n> RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10,\n> RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10,\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10,\n> + 0,\n> true,\n> };\n> \n> @@ -110,6 +113,7 @@ const IPAHwSettings ipaHwSettingsV12{\n> RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12,\n> RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12,\n> RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12,\n> + 0,\n> false,\n> };\n> \n> @@ -132,6 +136,7 @@ std::string IPARkISP1::logPrefix() const\n> }\n> \n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> + uint32_t supportedBlocks,\n> const IPACameraSensorInfo &sensorInfo,\n> const ControlInfoMap &sensorControls,\n> ControlInfoMap *ipaControls)\n> @@ -139,13 +144,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> /* \\todo Add support for other revisions */\n> switch (hwRevision) {\n> case RKISP1_V10:\n> - context_.hw = &ipaHwSettingsV10;\n> + context_.hw = ipaHwSettingsV10;\n> break;\n> case RKISP1_V_IMX8MP:\n> - context_.hw = &ipaHwSettingsIMX8MP;\n> + context_.hw = ipaHwSettingsIMX8MP;\n> break;\n> case RKISP1_V12:\n> - context_.hw = &ipaHwSettingsV12;\n> + context_.hw = ipaHwSettingsV12;\n> break;\n> default:\n> LOG(IPARkISP1, Error)\n> @@ -153,6 +158,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> << \" is currently not supported\";\n> return -ENODEV;\n> }\n> + context_.hw.supportedBlocks = supportedBlocks;\n> \n> LOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n> \n> \n> Which version do you like better?\n\nI like this version more too :)\n\nReviewed-by: Paul Elder \n\n> \n> Best regards,\n> Stefan\n> \n> \n> > \n> > \n> > Regards,\n> > Barnabás Pőcze\n> > \n> > \n> > > + context_.hw = hw;\n> > > \n> > > LOG(IPARkISP1, Debug) << \"Hardware revision is \" << hwRevision;\n> > > \n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index c4d4fdc6aa73..199f53d9e631 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -100,7 +100,7 @@ public:\n> > > \n> > > PipelineHandlerRkISP1 *pipe();\n> > > const PipelineHandlerRkISP1 *pipe() const;\n> > > - int loadIPA(unsigned int hwRevision);\n> > > + int loadIPA(unsigned int hwRevision, uint32_t supportedBlocks);\n> > > \n> > > Stream mainPathStream_;\n> > > Stream selfPathStream_;\n> > > @@ -383,7 +383,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const\n> > > return static_cast(Camera::Private::pipe());\n> > > }\n> > > \n> > > -int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > > +int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks)\n> > > {\n> > > ipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> > > if (!ipa_)\n> > > @@ -405,7 +405,8 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > > }\n> > > \n> > > ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> > > - sensorInfo, sensor_->controls(), &ipaControls_);\n> > > + supportedBlocks, sensorInfo, sensor_->controls(),\n> > > + &ipaControls_);\n> > > if (ret < 0) {\n> > > LOG(RkISP1, Error) << \"IPA initialization failure\";\n> > > return ret;\n> > > @@ -1311,6 +1312,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> > > return 0;\n> > > }\n> > > \n> > > +/*\n> > > + * By default we assume all the blocks that were included in the first\n> > > + * extensible parameters series are available. That is the lower 20bits.\n> > > + */\n> > > +const uint32_t kDefaultExtParamsBlocks = 0xfffff;\n> > > +\n> > > int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > > {\n> > > int ret;\n> > > @@ -1348,7 +1355,21 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > > isp_->frameStart.connect(data->delayedCtrls_.get(),\n> > > &DelayedControls::applyControls);\n> > > \n> > > - ret = data->loadIPA(media_->hwRevision());\n> > > + uint32_t supportedBlocks = kDefaultExtParamsBlocks;\n> > > +\n> > > + auto &controls = param_->controls();\n> > > + if (controls.find(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS) != controls.end()) {\n> > > + auto list = param_->getControls({ { RKISP1_CID_SUPPORTED_PARAMS_BLOCKS } });\n> > > + if (!list.empty())\n> > > + supportedBlocks = static_cast(\n> > > + list.get(RKISP1_CID_SUPPORTED_PARAMS_BLOCKS)\n> > > + .get());\n> > > + } else {\n> > > + LOG(RkISP1, Error)\n> > > + << \"Failed to query supported params blocks. Falling back to defaults.\";\n> > > + }\n> > > +\n> > > + ret = data->loadIPA(media_->hwRevision(), supportedBlocks);\n> > > if (ret)\n> > > return ret;\n> > > \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 86A61C332A\n\tfor ;\n\tFri, 5 Sep 2025 10:43:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA9BD69369;\n\tFri, 5 Sep 2025 12:43:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 087BD69337\n\tfor ;\n\tFri, 5 Sep 2025 12:43:04 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:dbfb:387c:7405:52bc])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id A7C971306; \n\tFri, 5 Sep 2025 12:41:52 +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=\"gAWytt2u\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757068913;\n\tbh=9iBWy4zxeZ7QiwkOXQV/wzYq7sd8+kwG8Se7vxdIV1k=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=gAWytt2uZI+zTZf2URYaaw5I7ZNpgjux9wAZVFrG1qH5f1vpcno3NknMLcekG3L2D\n\tCCUissM08Tz1njNWUHNQ67HarhVogkuCoL2NcHg1PVn+G8fWwdvSGswT6VHpFTW4aa\n\t5ROPVjfrlnLJKIF+JuwFG6AEIWJhUTfAZOsBhxNc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175550507337.1970129.15514147873475603938@localhost>","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-15-stefan.klug@ideasonboard.com>\n\t<31001a76-a40b-4aba-84cc-2c2dddafdc0d@ideasonboard.com>\n\t<175550507337.1970129.15514147873475603938@localhost>","Subject":"Re: [PATCH v3 14/19] pipeline: rkisp1: Query kernel for available\n\tparams blocks","From":"Paul Elder ","Cc":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tStefan Klug , \n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 05 Sep 2025 19:42:55 +0900","Message-ID":"<175706897556.1787083.5731519852834021849@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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\" "}}]