[{"id":36597,"web_url":"https://patchwork.libcamera.org/comment/36597/","msgid":"<176202034861.3925733.11218614347991542982@ping.linuxembedded.co.uk>","date":"2025-11-01T18:05:48","subject":"Re: [PATCH v1 11/17] ipa: rkisp1: algorithms: dpf: detect DPF dev\n\toverrides","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Rui Wang (2025-10-28 21:17:44)\n> Introduce checkDevModeOverridesChanged() to detect when developer-mode\n> controls have been modified. This function compares override values against\n> the current hardware configuration for:\n> - Spatial coefficients (green and RB)\n> - RB filter size\n> - NLL coefficients and scale mode\n> \n> Updates queueRequest() to trigger hardware updates whenever dev-mode\n> overrides differ from active settings. This ensures spatial kernels, NLL\n> tables, and filter size changes are applied immediately in manual mode.\n> \n> Signed-off-by: Rui Wang \n> ---\n> src/ipa/rkisp1/algorithms/dpf.cpp | 51 +++++++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/dpf.h | 5 +++\n> 2 files changed, 56 insertions(+)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index 145f10b7..cefa5da5 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -290,6 +290,54 @@ void Dpf::collectManualOverrides(const ControlList &controls)\n> }\n> }\n> \n> +bool Dpf::checkDevModeOverridesChanged()\n> +{\n> + if (!isDevMode())\n> + return false;\n> +\n> + bool changed = false;\n> + if (overrides_.spatialGreen) {\n> + bool coeffsChanged = false;\n> + for (unsigned i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {\n> + if (overrides_.spatialGreen->coeffs[i] != config_.g_flt.spatial_coeff[i]) {\n> + coeffsChanged = true;\n> + break;\n> + }\n> + }\n> + if (coeffsChanged) {\n> + changed = true;\n\nHaving two bools here seems redudnant.\n\n> + }\n\nSingle statements do not need { }.\n\n> + }\n\n\nPlease add blank lines in between conditional blocks to ease/improve\nreadability. \n\n> + if (overrides_.spatialRb) {\n> + bool coeffsChanged = false;\n> + for (unsigned i = 0; i < RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS; ++i) {\n> + if (overrides_.spatialRb->coeffs[i] != config_.rb_flt.spatial_coeff[i]) {\n> + coeffsChanged = true;\n> + break;\n> + }\n> + }\n> + if (coeffsChanged) {\n> + changed = true;\n> + }\n> + }\n> + if (overrides_.rbSize && *overrides_.rbSize != (config_.rb_flt.fltsize == RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 ? 1 : 0)) {\n> + changed = true;\n> + }\n\nWhere necessary we can support long lines, but try to aim for 80 and\nreally try to keep under 120 unless theres something more like a table\nforcing us to go longer.\n\n\tif (overrides_.rbSize && \n\t *overrides_.rbSize != (config_.rb_flt.fltsize == RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9 ? 1 : 0)\n\t \tchanged = true;\n\nBut even that's awkward, there must be somethign better we can do here.\n\nWhy isn't this just something like\n\n\tif (overrides_.rbSize && *overrides_.rbSize != config_.rb_flt.fltsize)\n\t\tchanged = true;\n\n\n\n\n> + if (overrides_.nll) {\n> + bool coeffsChanged = false;\n> + for (unsigned i = 0; i < RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS; ++i) {\n> + if (overrides_.nll->coeffs[i] != config_.nll.coeff[i]) {\n> + coeffsChanged = true;\n> + break;\n> + }\n> + }\n> + if (coeffsChanged || overrides_.nll->scaleMode != (config_.nll.scale_mode == RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC ? 1 : 0)) {\n> + changed = true;\n> + }\n> + }\n> + return changed;\n> +}\n> +\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> */\n> @@ -310,6 +358,9 @@ void Dpf::queueRequest(IPAContext &context,\n> overrides_.strength->b != strengthConfig_.b)) {\n> frameContext.dpf.update = true;\n> }\n> + if (checkDevModeOverridesChanged()) {\n> + frameContext.dpf.update = true;\n> + }\n> }\n> }\n> \n> diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h\n> index 1a33a8c4..b971619b 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.h\n> +++ b/src/ipa/rkisp1/algorithms/dpf.h\n> @@ -80,8 +80,13 @@ private:\n> bool enableDpf_ = true; /* YAML master enable */\n> \n> void handleEnableControl(const ControlList &controls, IPAFrameContext &frameContext, IPAContext &context) override;\n> +\n> void collectManualOverrides(const ControlList &controls) override;\n> +\n> + bool checkDevModeOverridesChanged();\n> +\n> bool parseConfig(const YamlObject &tuningData) override;\n> +\n> bool parseSingleConfig(const YamlObject &config,\n> rkisp1_cif_isp_dpf_config &cfg,\n> rkisp1_cif_isp_dpf_strength_config &strength);\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 B5FF0C3241\n\tfor ;\n\tSat, 1 Nov 2025 18:05:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14E096086F;\n\tSat, 1 Nov 2025 19:05:53 +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 5CD18606D5\n\tfor ;\n\tSat, 1 Nov 2025 19:05:51 +0100 (CET)","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 A6396BB;\n\tSat, 1 Nov 2025 19:03:59 +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=\"JYR/oyHI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762020239;\n\tbh=iVUHg3pCk5UwDGWuo/D8IwL85AOt8TEKxSLgBZ0mcgw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=JYR/oyHISocM/g5iEpkxCWKixl/741jBre8rzmPux8thL4gFtcKy/LhTi2gWObZ0w\n\tZt7Pg973vhqVFPBub6LPxUYIh9O2/Hx3nLMR0Ox9ASdO0LZiv/k7+yhpduo8GkmNjh\n\tRGPTpcQuxJnsOfGX2VBn56MmAOpwlT7YPwN7CMQ4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251028211751.2761420-11-rui.wang@ideasonboard.com>","References":"<20251028211751.2761420-1-rui.wang@ideasonboard.com>\n\t<20251028211751.2761420-11-rui.wang@ideasonboard.com>","Subject":"Re: [PATCH v1 11/17] ipa: rkisp1: algorithms: dpf: detect DPF dev\n\toverrides","From":"Kieran Bingham ","Cc":"Rui Wang ","To":"Rui Wang , libcamera-devel@lists.libcamera.org","Date":"Sat, 01 Nov 2025 18:05:48 +0000","Message-ID":"<176202034861.3925733.11218614347991542982@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\" "}}]