[{"id":38238,"web_url":"https://patchwork.libcamera.org/comment/38238/","msgid":"<177149668258.3673516.10720206566693925129@localhost>","date":"2026-02-19T10:24:42","subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Quoting Kieran Bingham (2026-02-13 17:57:52)\n> The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8\n> format, a range of 0.0 to (very nearly) 32.0.\n> \n> Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will\n> support the conversion, clamping and quantisation so that the metadata\n> and debug prints can now report the effective gain applied instead of\n> the potentially inaccurate float.\n> \n> As the UQ<5, 8> type already clamps values, remove the explicit\n> clamping. This removes the clamping to a minimum of 1.0 gain, so we\n> rely on calculateNewEv to provide a valid gain.\n> \n> Reviewed-by: Isaac Scott \n> Signed-off-by: Kieran Bingham \n\nLooks good to me.\n\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\n> \n> ---\n> v5:\n> - Use UQ<5, 8> type directly.\n> - Use operator<< to report the UQ<5, 8> value.\n> ---\n> src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------\n> src/ipa/mali-c55/ipa_context.h | 6 +++---\n> 2 files changed, 12 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp\n> index 014fd12452ac..075717f44ea6 100644\n> --- a/src/ipa/mali-c55/algorithms/agc.cpp\n> +++ b/src/ipa/mali-c55/algorithms/agc.cpp\n> @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;\n> * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual\n> * max value which is 8191 * 2^-8.\n> */\n> -static constexpr double kMinDigitalGain = 1.0;\n> -static constexpr double kMaxDigitalGain = 31.99609375;\n> +static constexpr float kMinDigitalGain = 1.0;\n> +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;\n> \n> uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)\n> {\n> @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> MaliC55Params *params)\n> {\n> IPAActiveState &activeState = context.activeState;\n> - double gain;\n> + UQ<5, 8> gain;\n> \n> if (activeState.agc.autoEnabled)\n> gain = activeState.agc.automatic.ispGain;\n> @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> gain = activeState.agc.manual.ispGain;\n> \n> auto block = params->block();\n> - block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);\n> + block->gain = gain.quantized();\n> \n> frameContext.agc.ispGain = gain;\n> }\n> @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,\n> */\n> uint32_t exposure = frameContext.agc.exposure;\n> double analogueGain = frameContext.agc.sensorGain;\n> - double digitalGain = frameContext.agc.ispGain;\n> + double digitalGain = frameContext.agc.ispGain.value();\n> double totalGain = analogueGain * digitalGain;\n> utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> utils::Duration effectiveExposureValue = currentShutter * totalGain;\n> @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,\n> activeState.agc.exposureMode, statistics_.yHist,\n> effectiveExposureValue);\n> \n> - dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);\n> + UQ<5, 8> dGainQ = static_cast(dGain);\n> \n> LOG(MaliC55Agc, Debug)\n> << \"Divided up shutter, analogue gain and digital gain are \"\n> - << shutterTime << \", \" << aGain << \" and \" << dGain;\n> + << shutterTime << \", \" << aGain << \" and \" << dGainQ;\n> \n> activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;\n> activeState.agc.automatic.sensorGain = aGain;\n> - activeState.agc.automatic.ispGain = dGain;\n> + activeState.agc.automatic.ispGain = dGainQ;\n> \n> metadata.set(controls::ExposureTime, currentShutter.get());\n> metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);\n> - metadata.set(controls::DigitalGain, frameContext.agc.ispGain);\n> + metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());\n> metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);\n> }\n> \n> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> index 08f78e4f74ce..ac4b83773803 100644\n> --- a/src/ipa/mali-c55/ipa_context.h\n> +++ b/src/ipa/mali-c55/ipa_context.h\n> @@ -41,12 +41,12 @@ struct IPAActiveState {\n> struct {\n> uint32_t exposure;\n> double sensorGain;\n> - double ispGain;\n> + UQ<5, 8> ispGain;\n> } automatic;\n> struct {\n> uint32_t exposure;\n> double sensorGain;\n> - double ispGain;\n> + UQ<5, 8> ispGain;\n> } manual;\n> bool autoEnabled;\n> uint32_t constraintMode;\n> @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {\n> struct {\n> uint32_t exposure;\n> double sensorGain;\n> - double ispGain;\n> + UQ<5, 8> ispGain;\n> } agc;\n> \n> struct {\n> \n> -- \n> 2.52.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 99D13C31E9\n\tfor ;\n\tThu, 19 Feb 2026 10:24:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9395362243;\n\tThu, 19 Feb 2026 11:24:47 +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 1CCFD62010\n\tfor ;\n\tThu, 19 Feb 2026 11:24:46 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:6ed3:b29f:c3e2:dd5f])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id DCEEA4D3;\n\tThu, 19 Feb 2026 11:23:52 +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=\"bOFMncvf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1771496633;\n\tbh=KwtOXEhEqX2MnsGeexKi0qwu8dl+qiI7HyVhcaJIqG0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=bOFMncvf4Edjxa/QU+xUDAqyBy5dG+u5Z3T2AUzbDZjhYN1V08+YZOGWCBo3jS9AM\n\t0zZU59VcHrwAvQf4zmYy4vXz4HkXXFKudwQoOGeaxLmGuRfywiSoz/+LNBD1CmvmFe\n\tlgfyxORQ7eKL/gKCccD6KoO16Vpu1jsEbF8IX6aM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260213-kbingham-quantizers-v7-13-1626b9aaabf1@ideasonboard.com>","References":"<20260213-kbingham-quantizers-v7-0-1626b9aaabf1@ideasonboard.com>\n\t<20260213-kbingham-quantizers-v7-13-1626b9aaabf1@ideasonboard.com>","Subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","From":"Stefan Klug ","Cc":"Kieran Bingham ,\n\tIsaac Scott ","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 19 Feb 2026 11:24:42 +0100","Message-ID":"<177149668258.3673516.10720206566693925129@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":38245,"web_url":"https://patchwork.libcamera.org/comment/38245/","msgid":"<20260219120800.GF688396@killaraus.ideasonboard.com>","date":"2026-02-19T12:08:00","subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Feb 13, 2026 at 04:57:52PM +0000, Kieran Bingham wrote:\n> The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8\n> format, a range of 0.0 to (very nearly) 32.0.\n> \n> Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will\n> support the conversion, clamping and quantisation so that the metadata\n> and debug prints can now report the effective gain applied instead of\n> the potentially inaccurate float.\n> \n> As the UQ<5, 8> type already clamps values, remove the explicit\n> clamping. This removes the clamping to a minimum of 1.0 gain, so we\n> rely on calculateNewEv to provide a valid gain.\n\nLooks like the review discussion from v4 as not been taken into account.\n\n> Reviewed-by: Isaac Scott \n> Signed-off-by: Kieran Bingham \n> \n> ---\n> v5:\n> - Use UQ<5, 8> type directly.\n> - Use operator<< to report the UQ<5, 8> value.\n> ---\n> src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------\n> src/ipa/mali-c55/ipa_context.h | 6 +++---\n> 2 files changed, 12 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp\n> index 014fd12452ac..075717f44ea6 100644\n> --- a/src/ipa/mali-c55/algorithms/agc.cpp\n> +++ b/src/ipa/mali-c55/algorithms/agc.cpp\n> @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;\n> * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual\n> * max value which is 8191 * 2^-8.\n> */\n> -static constexpr double kMinDigitalGain = 1.0;\n> -static constexpr double kMaxDigitalGain = 31.99609375;\n> +static constexpr float kMinDigitalGain = 1.0;\n> +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;\n> \n> uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)\n> {\n> @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> \t\t\t MaliC55Params *params)\n> {\n> \tIPAActiveState &activeState = context.activeState;\n> -\tdouble gain;\n> +\tUQ<5, 8> gain;\n> \n> \tif (activeState.agc.autoEnabled)\n> \t\tgain = activeState.agc.automatic.ispGain;\n> @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> \t\tgain = activeState.agc.manual.ispGain;\n> \n> \tauto block = params->block();\n> -\tblock->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);\n> +\tblock->gain = gain.quantized();\n> \n> \tframeContext.agc.ispGain = gain;\n> }\n> @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,\n> \t */\n> \tuint32_t exposure = frameContext.agc.exposure;\n> \tdouble analogueGain = frameContext.agc.sensorGain;\n> -\tdouble digitalGain = frameContext.agc.ispGain;\n> +\tdouble digitalGain = frameContext.agc.ispGain.value();\n> \tdouble totalGain = analogueGain * digitalGain;\n> \tutils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> \tutils::Duration effectiveExposureValue = currentShutter * totalGain;\n> @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,\n> \t\t\t activeState.agc.exposureMode, statistics_.yHist,\n> \t\t\t effectiveExposureValue);\n> \n> -\tdGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);\n> +\tUQ<5, 8> dGainQ = static_cast(dGain);\n> \n> \tLOG(MaliC55Agc, Debug)\n> \t\t<< \"Divided up shutter, analogue gain and digital gain are \"\n> -\t\t<< shutterTime << \", \" << aGain << \" and \" << dGain;\n> +\t\t<< shutterTime << \", \" << aGain << \" and \" << dGainQ;\n> \n> \tactiveState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;\n> \tactiveState.agc.automatic.sensorGain = aGain;\n> -\tactiveState.agc.automatic.ispGain = dGain;\n> +\tactiveState.agc.automatic.ispGain = dGainQ;\n> \n> \tmetadata.set(controls::ExposureTime, currentShutter.get());\n> \tmetadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);\n> -\tmetadata.set(controls::DigitalGain, frameContext.agc.ispGain);\n> +\tmetadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());\n> \tmetadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);\n> }\n> \n> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> index 08f78e4f74ce..ac4b83773803 100644\n> --- a/src/ipa/mali-c55/ipa_context.h\n> +++ b/src/ipa/mali-c55/ipa_context.h\n> @@ -41,12 +41,12 @@ struct IPAActiveState {\n> \t\tstruct {\n> \t\t\tuint32_t exposure;\n> \t\t\tdouble sensorGain;\n> -\t\t\tdouble ispGain;\n> +\t\t\tUQ<5, 8> ispGain;\n> \t\t} automatic;\n> \t\tstruct {\n> \t\t\tuint32_t exposure;\n> \t\t\tdouble sensorGain;\n> -\t\t\tdouble ispGain;\n> +\t\t\tUQ<5, 8> ispGain;\n> \t\t} manual;\n> \t\tbool autoEnabled;\n> \t\tuint32_t constraintMode;\n> @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {\n> \tstruct {\n> \t\tuint32_t exposure;\n> \t\tdouble sensorGain;\n> -\t\tdouble ispGain;\n> +\t\tUQ<5, 8> ispGain;\n> \t} agc;\n> \n> \tstruct {","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 8DEB5C0DA4\n\tfor ;\n\tThu, 19 Feb 2026 12:08:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB0576224A;\n\tThu, 19 Feb 2026 13:08:05 +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 75BB6620C9\n\tfor ;\n\tThu, 19 Feb 2026 13:08:04 +0100 (CET)","from killaraus.ideasonboard.com (unknown [83.245.237.175])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id C03654D3;\n\tThu, 19 Feb 2026 13:07:10 +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=\"Y80iqS/H\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1771502831;\n\tbh=s3KmaBs9PcbkQxDOhp436sgKH4kFis21s3tRFA5TxKU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Y80iqS/Hs5DVc2n9qWcv1X8/Ygy9rGjgTT3Jku4/jNjFufw90+kLFpMEhR7BQJbjx\n\tGt5JvrL4UCRAKIoGOv3D83pR/CDd+FGrQAitKqHCm+JKVHbU0Ld2o8EbHL+LZg68AZ\n\tS3alLLfEeECzmnNPNg9TXao9745Pe0a/Jg6biwJM=","Date":"Thu, 19 Feb 2026 13:08:00 +0100","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tIsaac Scott ","Subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","Message-ID":"<20260219120800.GF688396@killaraus.ideasonboard.com>","References":"<20260213-kbingham-quantizers-v7-0-1626b9aaabf1@ideasonboard.com>\n\t<20260213-kbingham-quantizers-v7-13-1626b9aaabf1@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260213-kbingham-quantizers-v7-13-1626b9aaabf1@ideasonboard.com>","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":38250,"web_url":"https://patchwork.libcamera.org/comment/38250/","msgid":"<177150798983.2767776.12478092785249470363@ping.linuxembedded.co.uk>","date":"2026-02-19T13:33:09","subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2026-02-19 12:08:00)\n> On Fri, Feb 13, 2026 at 04:57:52PM +0000, Kieran Bingham wrote:\n> > The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8\n> > format, a range of 0.0 to (very nearly) 32.0.\n> > \n> > Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will\n> > support the conversion, clamping and quantisation so that the metadata\n> > and debug prints can now report the effective gain applied instead of\n> > the potentially inaccurate float.\n> > \n> > As the UQ<5, 8> type already clamps values, remove the explicit\n> > clamping. This removes the clamping to a minimum of 1.0 gain, so we\n> > rely on calculateNewEv to provide a valid gain.\n> \n> Looks like the review discussion from v4 as not been taken into account.\n\nYeah, I really hate how discussions get split across series!\n\nI wish there was a way to keep all relevant discussion to a specific\nchange in one place.\n\nIt looks like your last statement was:\n\n│I agree with Kieran that duplicating clamping is not ideal. This being\n│said, we do duplicate code between AGC implementations already, and I\n│think Jacopo is working on fixing that by factoring out code to a common\n│helper in libipa. We can clamp in calculateNewEv(), or somewhere else,\n│depending on how we define calculateNewEv(). As long as things are well\n│designed (as in giving calculateNewEv() a clear and meaningful purpose)\n│and clearly documented, I don't mind much.\n\nI understood that to mean that we need clamping in the calculateNewEv\nwhich would be on top of this series.\n\ndo you want something changed /in this patch/ ?\n\n> \n> > Reviewed-by: Isaac Scott \n> > Signed-off-by: Kieran Bingham \n> > \n> > ---\n> > v5:\n> > - Use UQ<5, 8> type directly.\n> > - Use operator<< to report the UQ<5, 8> value.\n> > ---\n> > src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------\n> > src/ipa/mali-c55/ipa_context.h | 6 +++---\n> > 2 files changed, 12 insertions(+), 12 deletions(-)\n> > \n> > diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp\n> > index 014fd12452ac..075717f44ea6 100644\n> > --- a/src/ipa/mali-c55/algorithms/agc.cpp\n> > +++ b/src/ipa/mali-c55/algorithms/agc.cpp\n> > @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;\n> > * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual\n> > * max value which is 8191 * 2^-8.\n> > */\n> > -static constexpr double kMinDigitalGain = 1.0;\n> > -static constexpr double kMaxDigitalGain = 31.99609375;\n> > +static constexpr float kMinDigitalGain = 1.0;\n> > +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;\n> > \n> > uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)\n> > {\n> > @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> > MaliC55Params *params)\n> > {\n> > IPAActiveState &activeState = context.activeState;\n> > - double gain;\n> > + UQ<5, 8> gain;\n> > \n> > if (activeState.agc.autoEnabled)\n> > gain = activeState.agc.automatic.ispGain;\n> > @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> > gain = activeState.agc.manual.ispGain;\n> > \n> > auto block = params->block();\n> > - block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);\n> > + block->gain = gain.quantized();\n> > \n> > frameContext.agc.ispGain = gain;\n> > }\n> > @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,\n> > */\n> > uint32_t exposure = frameContext.agc.exposure;\n> > double analogueGain = frameContext.agc.sensorGain;\n> > - double digitalGain = frameContext.agc.ispGain;\n> > + double digitalGain = frameContext.agc.ispGain.value();\n> > double totalGain = analogueGain * digitalGain;\n> > utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> > utils::Duration effectiveExposureValue = currentShutter * totalGain;\n> > @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,\n> > activeState.agc.exposureMode, statistics_.yHist,\n> > effectiveExposureValue);\n> > \n> > - dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);\n> > + UQ<5, 8> dGainQ = static_cast(dGain);\n> > \n> > LOG(MaliC55Agc, Debug)\n> > << \"Divided up shutter, analogue gain and digital gain are \"\n> > - << shutterTime << \", \" << aGain << \" and \" << dGain;\n> > + << shutterTime << \", \" << aGain << \" and \" << dGainQ;\n> > \n> > activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;\n> > activeState.agc.automatic.sensorGain = aGain;\n> > - activeState.agc.automatic.ispGain = dGain;\n> > + activeState.agc.automatic.ispGain = dGainQ;\n> > \n> > metadata.set(controls::ExposureTime, currentShutter.get());\n> > metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);\n> > - metadata.set(controls::DigitalGain, frameContext.agc.ispGain);\n> > + metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());\n> > metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);\n> > }\n> > \n> > diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> > index 08f78e4f74ce..ac4b83773803 100644\n> > --- a/src/ipa/mali-c55/ipa_context.h\n> > +++ b/src/ipa/mali-c55/ipa_context.h\n> > @@ -41,12 +41,12 @@ struct IPAActiveState {\n> > struct {\n> > uint32_t exposure;\n> > double sensorGain;\n> > - double ispGain;\n> > + UQ<5, 8> ispGain;\n> > } automatic;\n> > struct {\n> > uint32_t exposure;\n> > double sensorGain;\n> > - double ispGain;\n> > + UQ<5, 8> ispGain;\n> > } manual;\n> > bool autoEnabled;\n> > uint32_t constraintMode;\n> > @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {\n> > struct {\n> > uint32_t exposure;\n> > double sensorGain;\n> > - double ispGain;\n> > + UQ<5, 8> ispGain;\n> > } agc;\n> > \n> > struct {\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 AF795C0DA4\n\tfor ;\n\tThu, 19 Feb 2026 13:33:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B830B6224A;\n\tThu, 19 Feb 2026 14:33:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA72C62243\n\tfor ;\n\tThu, 19 Feb 2026 14:33:12 +0100 (CET)","from monstersaurus.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 98A0C673;\n\tThu, 19 Feb 2026 14:32:19 +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=\"Nz1NCpa5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1771507939;\n\tbh=glIppcZvzZHibnP7FW+ToVNnriSpKpEdbFwul+LBmoA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Nz1NCpa5akZ0X1zygNQ+ihOAkU2YrHbmRlTu6i005Hzuc0HUZNXC8PNMkSefzR0BE\n\tacVYZTTdsbU+qWHyCcs65mqWjIsEbzTBvjAy1UXaP/f11vJp5S32id1X1/m2KvorTC\n\tvueyY+KpNpFEc2oSGO/7SRSmExYe1173l3ScnpPo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260219120800.GF688396@killaraus.ideasonboard.com>","References":"<20260213-kbingham-quantizers-v7-0-1626b9aaabf1@ideasonboard.com>\n\t<20260213-kbingham-quantizers-v7-13-1626b9aaabf1@ideasonboard.com>\n\t<20260219120800.GF688396@killaraus.ideasonboard.com>","Subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tIsaac Scott ","To":"Laurent Pinchart ","Date":"Thu, 19 Feb 2026 13:33:09 +0000","Message-ID":"<177150798983.2767776.12478092785249470363@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":38251,"web_url":"https://patchwork.libcamera.org/comment/38251/","msgid":"<20260219135337.GN520738@killaraus.ideasonboard.com>","date":"2026-02-19T13:53:37","subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Feb 19, 2026 at 01:33:09PM +0000, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2026-02-19 12:08:00)\n> > On Fri, Feb 13, 2026 at 04:57:52PM +0000, Kieran Bingham wrote:\n> > > The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8\n> > > format, a range of 0.0 to (very nearly) 32.0.\n> > > \n> > > Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will\n> > > support the conversion, clamping and quantisation so that the metadata\n> > > and debug prints can now report the effective gain applied instead of\n> > > the potentially inaccurate float.\n> > > \n> > > As the UQ<5, 8> type already clamps values, remove the explicit\n> > > clamping. This removes the clamping to a minimum of 1.0 gain, so we\n> > > rely on calculateNewEv to provide a valid gain.\n> > \n> > Looks like the review discussion from v4 as not been taken into account.\n> \n> Yeah, I really hate how discussions get split across series!\n> \n> I wish there was a way to keep all relevant discussion to a specific\n> change in one place.\n> \n> It looks like your last statement was:\n> \n> │I agree with Kieran that duplicating clamping is not ideal. This being\n> │said, we do duplicate code between AGC implementations already, and I\n> │think Jacopo is working on fixing that by factoring out code to a common\n> │helper in libipa. We can clamp in calculateNewEv(), or somewhere else,\n> │depending on how we define calculateNewEv(). As long as things are well\n> │designed (as in giving calculateNewEv() a clear and meaningful purpose)\n> │and clearly documented, I don't mind much.\n> \n> I understood that to mean that we need clamping in the calculateNewEv\n> which would be on top of this series.\n\nEither here or in calculateNewEv(), up to you. If it's in\ncalculateNewEv(), then the function documentation has to be updated to\nclearly explain what the function's purpose is.\n\n> do you want something changed /in this patch/ ?\n\nThis patch drops clamping, and relies on an undocumented assumption\nabout calculateNewEv() that may not always hold true. If you include a\npatch for calculateNewEv() in the series then there's nothing to change\nhere. Otherwise I'll keep the clamp in this patch for now.\n\n> > > Reviewed-by: Isaac Scott \n> > > Signed-off-by: Kieran Bingham \n> > > \n> > > ---\n> > > v5:\n> > > - Use UQ<5, 8> type directly.\n> > > - Use operator<< to report the UQ<5, 8> value.\n> > > ---\n> > > src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------\n> > > src/ipa/mali-c55/ipa_context.h | 6 +++---\n> > > 2 files changed, 12 insertions(+), 12 deletions(-)\n> > > \n> > > diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp\n> > > index 014fd12452ac..075717f44ea6 100644\n> > > --- a/src/ipa/mali-c55/algorithms/agc.cpp\n> > > +++ b/src/ipa/mali-c55/algorithms/agc.cpp\n> > > @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;\n> > > * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual\n> > > * max value which is 8191 * 2^-8.\n> > > */\n> > > -static constexpr double kMinDigitalGain = 1.0;\n> > > -static constexpr double kMaxDigitalGain = 31.99609375;\n> > > +static constexpr float kMinDigitalGain = 1.0;\n> > > +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;\n> > > \n> > > uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)\n> > > {\n> > > @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> > > MaliC55Params *params)\n> > > {\n> > > IPAActiveState &activeState = context.activeState;\n> > > - double gain;\n> > > + UQ<5, 8> gain;\n> > > \n> > > if (activeState.agc.autoEnabled)\n> > > gain = activeState.agc.automatic.ispGain;\n> > > @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,\n> > > gain = activeState.agc.manual.ispGain;\n> > > \n> > > auto block = params->block();\n> > > - block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);\n> > > + block->gain = gain.quantized();\n> > > \n> > > frameContext.agc.ispGain = gain;\n> > > }\n> > > @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,\n> > > */\n> > > uint32_t exposure = frameContext.agc.exposure;\n> > > double analogueGain = frameContext.agc.sensorGain;\n> > > - double digitalGain = frameContext.agc.ispGain;\n> > > + double digitalGain = frameContext.agc.ispGain.value();\n> > > double totalGain = analogueGain * digitalGain;\n> > > utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n> > > utils::Duration effectiveExposureValue = currentShutter * totalGain;\n> > > @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,\n> > > activeState.agc.exposureMode, statistics_.yHist,\n> > > effectiveExposureValue);\n> > > \n> > > - dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);\n> > > + UQ<5, 8> dGainQ = static_cast(dGain);\n> > > \n> > > LOG(MaliC55Agc, Debug)\n> > > << \"Divided up shutter, analogue gain and digital gain are \"\n> > > - << shutterTime << \", \" << aGain << \" and \" << dGain;\n> > > + << shutterTime << \", \" << aGain << \" and \" << dGainQ;\n> > > \n> > > activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;\n> > > activeState.agc.automatic.sensorGain = aGain;\n> > > - activeState.agc.automatic.ispGain = dGain;\n> > > + activeState.agc.automatic.ispGain = dGainQ;\n> > > \n> > > metadata.set(controls::ExposureTime, currentShutter.get());\n> > > metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);\n> > > - metadata.set(controls::DigitalGain, frameContext.agc.ispGain);\n> > > + metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());\n> > > metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);\n> > > }\n> > > \n> > > diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> > > index 08f78e4f74ce..ac4b83773803 100644\n> > > --- a/src/ipa/mali-c55/ipa_context.h\n> > > +++ b/src/ipa/mali-c55/ipa_context.h\n> > > @@ -41,12 +41,12 @@ struct IPAActiveState {\n> > > struct {\n> > > uint32_t exposure;\n> > > double sensorGain;\n> > > - double ispGain;\n> > > + UQ<5, 8> ispGain;\n> > > } automatic;\n> > > struct {\n> > > uint32_t exposure;\n> > > double sensorGain;\n> > > - double ispGain;\n> > > + UQ<5, 8> ispGain;\n> > > } manual;\n> > > bool autoEnabled;\n> > > uint32_t constraintMode;\n> > > @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {\n> > > struct {\n> > > uint32_t exposure;\n> > > double sensorGain;\n> > > - double ispGain;\n> > > + UQ<5, 8> ispGain;\n> > > } agc;\n> > > \n> > > struct {","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 CE3D4C31E9\n\tfor ;\n\tThu, 19 Feb 2026 13:53:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F2FB962248;\n\tThu, 19 Feb 2026 14:53:41 +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 147B0620C9\n\tfor ;\n\tThu, 19 Feb 2026 14:53:41 +0100 (CET)","from killaraus.ideasonboard.com (unknown [83.245.237.175])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 77AC73A4;\n\tThu, 19 Feb 2026 14:52:47 +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=\"gZbNeYHi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1771509167;\n\tbh=0GBEb7Hs93E6ojvzo1gSOUNbXAUIyMLmC3bsykPOS2E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gZbNeYHipZ+1sF/pJHnC9r20EspoERZM0YFB8kpKC2BlK0Z7j/i4PB0PLR3f5+yku\n\t9hEUcP8FLzogYpRyOyd3NFkFRMxMDf4/C3H1+F4hSViWlukSORuBFZBdgeqB+dNfOm\n\tREMgjUIR/5RyGy/z+0efJ/2fVWv0UrBp2/JRoGmo=","Date":"Thu, 19 Feb 2026 14:53:37 +0100","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tIsaac Scott ","Subject":"Re: [PATCH v7 13/15] ipa: mali-c55: agc: Quantise the ISP Digital\n\tGain","Message-ID":"<20260219135337.GN520738@killaraus.ideasonboard.com>","References":"<20260213-kbingham-quantizers-v7-0-1626b9aaabf1@ideasonboard.com>\n\t<20260213-kbingham-quantizers-v7-13-1626b9aaabf1@ideasonboard.com>\n\t<20260219120800.GF688396@killaraus.ideasonboard.com>\n\t<177150798983.2767776.12478092785249470363@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<177150798983.2767776.12478092785249470363@ping.linuxembedded.co.uk>","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\" "}}]