[{"id":37878,"web_url":"https://patchwork.libcamera.org/comment/37878/","msgid":"<176909207202.2317359.1807167609834297548@isaac-ThinkPad-T16-Gen-2>","date":"2026-01-22T14:27:52","subject":"Re: [PATCH v10 4/7] ipa: rkisp1: algorithms: dpf: Refactor prepare()\n\tinto helpers","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Rui,\n\nQuoting Rui Wang (2026-01-20 15:30:54)\n> Split the prepare() function into prepareDisabledMode() and\n> prepareEnabledMode() to improve readability and maintainability.\n> \n> This separates the logic for disabling and enabling the DPF, making\n> the main prepare() function cleaner and easier to follow.\n> \n> Signed-off-by: Rui Wang \n> Reviewed-by: Jacopo Mondi \n> \n> ---\n> changelog since v5/v6: Nothing changed\n> \n> changelog since v8 :\n> - remove [[maybe_unused] for prepareEnable and prepareDisable\n> \n> Reviewed-by tags from v5 are carried over (no code changes).\n> ---\n> src/ipa/rkisp1/algorithms/dpf.cpp | 80 +++++++++++++++++++------------\n> src/ipa/rkisp1/algorithms/dpf.h | 3 ++\n> 2 files changed, 52 insertions(+), 31 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index 21ce8ace..ff2cbc28 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -358,40 +358,58 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame,\n> if (!frameContext.dpf.update && frame > 0)\n> return;\n> \n> + if (!frameContext.dpf.denoise) {\n> + prepareDisabledMode(params);\n> + return;\n> + }\n> +\n> + prepareEnabledMode(context, params);\n> +}\n> +\n> +void Dpf::prepareDisabledMode(RkISP1Params *params)\n> +{\n> + auto dpfConfig = params->block();\n> + dpfConfig.setEnabled(false);\n> + auto dpfStrength = params->block();\n> + dpfStrength.setEnabled(false);\n> +}\n> +\n> +void Dpf::prepareEnabledMode(IPAContext &context, RkISP1Params *params)\n> +{\n> + if (activeMode_ == noiseReductionModes_.end())\n> + return;\n> +\n> + const ModeConfig &modeConfig = *activeMode_;\n> +\n> auto config = params->block();\n> - config.setEnabled(frameContext.dpf.denoise);\n> + config.setEnabled(true);\n> + *config = modeConfig.dpf;\n> +\n> + const auto &awb = context.configuration.awb;\n> + const auto &lsc = context.configuration.lsc;\n> +\n> + auto &mode = config->gain.mode;\n> +\n> + /*\n> + * The DPF needs to take into account the total amount of\n> + * digital gain, which comes from the AWB and LSC modules. The\n> + * DPF hardware can be programmed with a digital gain value\n> + * manually, but can also use the gains supplied by the AWB and\n> + * LSC modules automatically when they are enabled. Use that\n> + * mode of operation as it simplifies control of the DPF.\n> + */\n> + if (awb.enabled && lsc.enabled)\n> + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;\n> + else if (awb.enabled)\n> + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;\n> + else if (lsc.enabled)\n> + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;\n> + else\n> + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;\n\nReviewed-by: Isaac Scott \n\n> \n> auto strengthConfig = params->block();\n> - strengthConfig.setEnabled(frameContext.dpf.denoise);\n> -\n> - if (frameContext.dpf.denoise) {\n> - const ModeConfig &modeConfig = *activeMode_;\n> -\n> - *config = modeConfig.dpf;\n> - *strengthConfig = modeConfig.strength;\n> -\n> - const auto &awb = context.configuration.awb;\n> - const auto &lsc = context.configuration.lsc;\n> -\n> - auto &mode = config->gain.mode;\n> -\n> - /*\n> - * The DPF needs to take into account the total amount of\n> - * digital gain, which comes from the AWB and LSC modules. The\n> - * DPF hardware can be programmed with a digital gain value\n> - * manually, but can also use the gains supplied by the AWB and\n> - * LSC modules automatically when they are enabled. Use that\n> - * mode of operation as it simplifies control of the DPF.\n> - */\n> - if (awb.enabled && lsc.enabled)\n> - mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;\n> - else if (awb.enabled)\n> - mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;\n> - else if (lsc.enabled)\n> - mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;\n> - else\n> - mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;\n> - }\n> + strengthConfig.setEnabled(true);\n> + *strengthConfig = modeConfig.strength;\n> }\n> \n> REGISTER_IPA_ALGORITHM(Dpf, \"Dpf\")\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> index 43effcbe..208d8fe9 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.h\n> +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> @@ -44,6 +44,9 @@ private:\n> \n> bool loadConfig(int32_t mode);\n> \n> + void prepareDisabledMode(RkISP1Params *params);\n> + void prepareEnabledMode(IPAContext &context, RkISP1Params *params);\n> +\n> std::vector noiseReductionModes_;\n> std::vector::const_iterator activeMode_;\n> };\n> -- \n> 2.43.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 8A5B0C3220\n\tfor ;\n\tThu, 22 Jan 2026 14:27:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C872561FC9;\n\tThu, 22 Jan 2026 15:27:56 +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 E05DC61F84\n\tfor ;\n\tThu, 22 Jan 2026 15:27:54 +0100 (CET)","from thinkpad.ideasonboard.com\n\t(cpc90716-aztw32-2-0-cust408.18-1.cable.virginm.net [86.26.101.153])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1A370324;\n\tThu, 22 Jan 2026 15:27:22 +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=\"fTZa9RWW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769092042;\n\tbh=YeVH5c8hlceF803Gl3t8IXfKOEj/FClSdMYZVU7Wnrk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=fTZa9RWW1z2MBS5YJ1NhXi/OqjCmQUltnJr3gGhzOCkK/FjbNPWTQoIOdWGTae4c3\n\t/d8VRUVbpol4IoC3Dqpl+nbo056PoIn7fURceMS5Og8SCKUEUvtnorwH7d9vl4IQra\n\taGV/vE+trJbJ7wegmUktjy0jZAPAQGQyssv2aIwI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260120153057.1703714-5-rui.wang@ideasonboard.com>","References":"<20260120153057.1703714-1-rui.wang@ideasonboard.com>\n\t<20260120153057.1703714-5-rui.wang@ideasonboard.com>","Subject":"Re: [PATCH v10 4/7] ipa: rkisp1: algorithms: dpf: Refactor prepare()\n\tinto helpers","From":"Isaac Scott ","Cc":"Rui Wang ,\n\tJacopo Mondi ","To":"Rui Wang , libcamera-devel@lists.libcamera.org","Date":"Thu, 22 Jan 2026 14:27:52 +0000","Message-ID":"<176909207202.2317359.1807167609834297548@isaac-ThinkPad-T16-Gen-2>","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\" "}}]