[{"id":37914,"web_url":"https://patchwork.libcamera.org/comment/37914/","msgid":"<176917876739.302817.338338391294991965@localhost>","date":"2026-01-23T14:32:47","subject":"Re: [PATCH v6 13/16] 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":"Hi Kieran,\n\nQuoting Kieran Bingham (2026-01-21 18:37:32)\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> \n> ---\n> v5:\n> - Use UQ<5, 8> type directly.\n> - Use operator<< to report the UQ<5, 8> value.\n> \n> Signed-off-by: Kieran Bingham \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\nI think this could benefit from a \"using DigitalGainQ=UQ<5, 8>;\"\ndeclaration in ipa_context.h.\n\nOtherwise looks good to me.\n\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\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> 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 69220C3220\n\tfor ;\n\tFri, 23 Jan 2026 14:32:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 64D9061FC9;\n\tFri, 23 Jan 2026 15:32:51 +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 A9CFC615B2\n\tfor ;\n\tFri, 23 Jan 2026 15:32:50 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:1d92:9f27:5dd1:dc89])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id D8A79103D; \n\tFri, 23 Jan 2026 15:32:16 +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=\"iJ0Uc8wk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769178737;\n\tbh=PpYGiGLUHe0xt0aYJo/s+aIQH/8Cjm7/w3oeKyNV4Dw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=iJ0Uc8wkJpVXSNZrvuq7sF+LG+eI1dnrdnJMK8ndP5ETlNBfANdKHOA7c4B0zZkjw\n\tYDBDXfBlUzR3T+xeJM4AyaEUN5iPiAMfzWd7kdRZBrFepWA/FWBNvT9a0kWWu+L3Rd\n\tBhGj9GyOz3wshPahAIQHTnXVwWEwuq+fsi0/hrsg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260121173737.376113-14-kieran.bingham@ideasonboard.com>","References":"<20260121173737.376113-1-kieran.bingham@ideasonboard.com>\n\t<20260121173737.376113-14-kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v6 13/16] 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 ","Date":"Fri, 23 Jan 2026 15:32:47 +0100","Message-ID":"<176917876739.302817.338338391294991965@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\" "}}]