[v7,13/15] ipa: mali-c55: agc: Quantise the ISP Digital Gain
diff mbox series

Message ID 20260213-kbingham-quantizers-v7-13-1626b9aaabf1@ideasonboard.com
State Accepted
Headers show
Series
  • libipa: Introduce a Quantized type
Related show

Commit Message

Kieran Bingham Feb. 13, 2026, 4:57 p.m. UTC 
The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8
format, a range of 0.0 to (very nearly) 32.0.

Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will
support the conversion, clamping and quantisation so that the metadata
and debug prints can now report the effective gain applied instead of
the potentially inaccurate float.

As the UQ<5, 8> type already clamps values, remove the explicit
clamping.  This removes the clamping to a minimum of 1.0 gain, so we
rely on calculateNewEv to provide a valid gain.

Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
v5:
 - Use UQ<5, 8> type directly.
 - Use operator<< to report the UQ<5, 8> value.
---
 src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------
 src/ipa/mali-c55/ipa_context.h      |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

Comments

Stefan Klug Feb. 19, 2026, 10:24 a.m. UTC | #1 
Quoting Kieran Bingham (2026-02-13 17:57:52)
> The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8
> format, a range of 0.0 to (very nearly) 32.0.
> 
> Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will
> support the conversion, clamping and quantisation so that the metadata
> and debug prints can now report the effective gain applied instead of
> the potentially inaccurate float.
> 
> As the UQ<5, 8> type already clamps values, remove the explicit
> clamping.  This removes the clamping to a minimum of 1.0 gain, so we
> rely on calculateNewEv to provide a valid gain.
> 
> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Looks good to me.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Cheers,
Stefan

> 
> ---
> v5:
>  - Use UQ<5, 8> type directly.
>  - Use operator<< to report the UQ<5, 8> value.
> ---
>  src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------
>  src/ipa/mali-c55/ipa_context.h      |  6 +++---
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> index 014fd12452ac..075717f44ea6 100644
> --- a/src/ipa/mali-c55/algorithms/agc.cpp
> +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;
>   * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual
>   * max value which is 8191 * 2^-8.
>   */
> -static constexpr double kMinDigitalGain = 1.0;
> -static constexpr double kMaxDigitalGain = 31.99609375;
> +static constexpr float kMinDigitalGain = 1.0;
> +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;
>  
>  uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)
>  {
> @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
>                              MaliC55Params *params)
>  {
>         IPAActiveState &activeState = context.activeState;
> -       double gain;
> +       UQ<5, 8> gain;
>  
>         if (activeState.agc.autoEnabled)
>                 gain = activeState.agc.automatic.ispGain;
> @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
>                 gain = activeState.agc.manual.ispGain;
>  
>         auto block = params->block<MaliC55Blocks::Dgain>();
> -       block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);
> +       block->gain = gain.quantized();
>  
>         frameContext.agc.ispGain = gain;
>  }
> @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,
>          */
>         uint32_t exposure = frameContext.agc.exposure;
>         double analogueGain = frameContext.agc.sensorGain;
> -       double digitalGain = frameContext.agc.ispGain;
> +       double digitalGain = frameContext.agc.ispGain.value();
>         double totalGain = analogueGain * digitalGain;
>         utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
>         utils::Duration effectiveExposureValue = currentShutter * totalGain;
> @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,
>                                activeState.agc.exposureMode, statistics_.yHist,
>                                effectiveExposureValue);
>  
> -       dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);
> +       UQ<5, 8> dGainQ = static_cast<float>(dGain);
>  
>         LOG(MaliC55Agc, Debug)
>                 << "Divided up shutter, analogue gain and digital gain are "
> -               << shutterTime << ", " << aGain << " and " << dGain;
> +               << shutterTime << ", " << aGain << " and " << dGainQ;
>  
>         activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
>         activeState.agc.automatic.sensorGain = aGain;
> -       activeState.agc.automatic.ispGain = dGain;
> +       activeState.agc.automatic.ispGain = dGainQ;
>  
>         metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
>         metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
> -       metadata.set(controls::DigitalGain, frameContext.agc.ispGain);
> +       metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());
>         metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);
>  }
>  
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> index 08f78e4f74ce..ac4b83773803 100644
> --- a/src/ipa/mali-c55/ipa_context.h
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -41,12 +41,12 @@ struct IPAActiveState {
>                 struct {
>                         uint32_t exposure;
>                         double sensorGain;
> -                       double ispGain;
> +                       UQ<5, 8> ispGain;
>                 } automatic;
>                 struct {
>                         uint32_t exposure;
>                         double sensorGain;
> -                       double ispGain;
> +                       UQ<5, 8> ispGain;
>                 } manual;
>                 bool autoEnabled;
>                 uint32_t constraintMode;
> @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {
>         struct {
>                 uint32_t exposure;
>                 double sensorGain;
> -               double ispGain;
> +               UQ<5, 8> ispGain;
>         } agc;
>  
>         struct {
> 
> -- 
> 2.52.0
>
Laurent Pinchart Feb. 19, 2026, 12:08 p.m. UTC | #2 
On Fri, Feb 13, 2026 at 04:57:52PM +0000, Kieran Bingham wrote:
> The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8
> format, a range of 0.0 to (very nearly) 32.0.
> 
> Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will
> support the conversion, clamping and quantisation so that the metadata
> and debug prints can now report the effective gain applied instead of
> the potentially inaccurate float.
> 
> As the UQ<5, 8> type already clamps values, remove the explicit
> clamping.  This removes the clamping to a minimum of 1.0 gain, so we
> rely on calculateNewEv to provide a valid gain.

Looks like the review discussion from v4 as not been taken into account.

> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> ---
> v5:
>  - Use UQ<5, 8> type directly.
>  - Use operator<< to report the UQ<5, 8> value.
> ---
>  src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------
>  src/ipa/mali-c55/ipa_context.h      |  6 +++---
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> index 014fd12452ac..075717f44ea6 100644
> --- a/src/ipa/mali-c55/algorithms/agc.cpp
> +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;
>   * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual
>   * max value which is 8191 * 2^-8.
>   */
> -static constexpr double kMinDigitalGain = 1.0;
> -static constexpr double kMaxDigitalGain = 31.99609375;
> +static constexpr float kMinDigitalGain = 1.0;
> +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;
>  
>  uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)
>  {
> @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
>  			     MaliC55Params *params)
>  {
>  	IPAActiveState &activeState = context.activeState;
> -	double gain;
> +	UQ<5, 8> gain;
>  
>  	if (activeState.agc.autoEnabled)
>  		gain = activeState.agc.automatic.ispGain;
> @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
>  		gain = activeState.agc.manual.ispGain;
>  
>  	auto block = params->block<MaliC55Blocks::Dgain>();
> -	block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);
> +	block->gain = gain.quantized();
>  
>  	frameContext.agc.ispGain = gain;
>  }
> @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,
>  	 */
>  	uint32_t exposure = frameContext.agc.exposure;
>  	double analogueGain = frameContext.agc.sensorGain;
> -	double digitalGain = frameContext.agc.ispGain;
> +	double digitalGain = frameContext.agc.ispGain.value();
>  	double totalGain = analogueGain * digitalGain;
>  	utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
>  	utils::Duration effectiveExposureValue = currentShutter * totalGain;
> @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,
>  			       activeState.agc.exposureMode, statistics_.yHist,
>  			       effectiveExposureValue);
>  
> -	dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);
> +	UQ<5, 8> dGainQ = static_cast<float>(dGain);
>  
>  	LOG(MaliC55Agc, Debug)
>  		<< "Divided up shutter, analogue gain and digital gain are "
> -		<< shutterTime << ", " << aGain << " and " << dGain;
> +		<< shutterTime << ", " << aGain << " and " << dGainQ;
>  
>  	activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
>  	activeState.agc.automatic.sensorGain = aGain;
> -	activeState.agc.automatic.ispGain = dGain;
> +	activeState.agc.automatic.ispGain = dGainQ;
>  
>  	metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
>  	metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
> -	metadata.set(controls::DigitalGain, frameContext.agc.ispGain);
> +	metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());
>  	metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);
>  }
>  
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> index 08f78e4f74ce..ac4b83773803 100644
> --- a/src/ipa/mali-c55/ipa_context.h
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -41,12 +41,12 @@ struct IPAActiveState {
>  		struct {
>  			uint32_t exposure;
>  			double sensorGain;
> -			double ispGain;
> +			UQ<5, 8> ispGain;
>  		} automatic;
>  		struct {
>  			uint32_t exposure;
>  			double sensorGain;
> -			double ispGain;
> +			UQ<5, 8> ispGain;
>  		} manual;
>  		bool autoEnabled;
>  		uint32_t constraintMode;
> @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {
>  	struct {
>  		uint32_t exposure;
>  		double sensorGain;
> -		double ispGain;
> +		UQ<5, 8> ispGain;
>  	} agc;
>  
>  	struct {
Kieran Bingham Feb. 19, 2026, 1:33 p.m. UTC | #3 
Quoting Laurent Pinchart (2026-02-19 12:08:00)
> On Fri, Feb 13, 2026 at 04:57:52PM +0000, Kieran Bingham wrote:
> > The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8
> > format, a range of 0.0 to (very nearly) 32.0.
> > 
> > Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will
> > support the conversion, clamping and quantisation so that the metadata
> > and debug prints can now report the effective gain applied instead of
> > the potentially inaccurate float.
> > 
> > As the UQ<5, 8> type already clamps values, remove the explicit
> > clamping.  This removes the clamping to a minimum of 1.0 gain, so we
> > rely on calculateNewEv to provide a valid gain.
> 
> Looks like the review discussion from v4 as not been taken into account.

Yeah, I really hate how discussions get split across series!

I wish there was a way to keep all relevant discussion to a specific
change in one place.

It looks like your last statement was:

│I agree with Kieran that duplicating clamping is not ideal. This being
│said, we do duplicate code between AGC implementations already, and I
│think Jacopo is working on fixing that by factoring out code to a common
│helper in libipa. We can clamp in calculateNewEv(), or somewhere else,
│depending on how we define calculateNewEv(). As long as things are well
│designed (as in giving calculateNewEv() a clear and meaningful purpose)
│and clearly documented, I don't mind much.

I understood that to mean that we need clamping in the calculateNewEv
which would be on top of this series.

do you want something changed /in this patch/ ?

> 
> > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > ---
> > v5:
> >  - Use UQ<5, 8> type directly.
> >  - Use operator<< to report the UQ<5, 8> value.
> > ---
> >  src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------
> >  src/ipa/mali-c55/ipa_context.h      |  6 +++---
> >  2 files changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> > index 014fd12452ac..075717f44ea6 100644
> > --- a/src/ipa/mali-c55/algorithms/agc.cpp
> > +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> > @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;
> >   * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual
> >   * max value which is 8191 * 2^-8.
> >   */
> > -static constexpr double kMinDigitalGain = 1.0;
> > -static constexpr double kMaxDigitalGain = 31.99609375;
> > +static constexpr float kMinDigitalGain = 1.0;
> > +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;
> >  
> >  uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)
> >  {
> > @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
> >                            MaliC55Params *params)
> >  {
> >       IPAActiveState &activeState = context.activeState;
> > -     double gain;
> > +     UQ<5, 8> gain;
> >  
> >       if (activeState.agc.autoEnabled)
> >               gain = activeState.agc.automatic.ispGain;
> > @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
> >               gain = activeState.agc.manual.ispGain;
> >  
> >       auto block = params->block<MaliC55Blocks::Dgain>();
> > -     block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);
> > +     block->gain = gain.quantized();
> >  
> >       frameContext.agc.ispGain = gain;
> >  }
> > @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,
> >        */
> >       uint32_t exposure = frameContext.agc.exposure;
> >       double analogueGain = frameContext.agc.sensorGain;
> > -     double digitalGain = frameContext.agc.ispGain;
> > +     double digitalGain = frameContext.agc.ispGain.value();
> >       double totalGain = analogueGain * digitalGain;
> >       utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
> >       utils::Duration effectiveExposureValue = currentShutter * totalGain;
> > @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,
> >                              activeState.agc.exposureMode, statistics_.yHist,
> >                              effectiveExposureValue);
> >  
> > -     dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);
> > +     UQ<5, 8> dGainQ = static_cast<float>(dGain);
> >  
> >       LOG(MaliC55Agc, Debug)
> >               << "Divided up shutter, analogue gain and digital gain are "
> > -             << shutterTime << ", " << aGain << " and " << dGain;
> > +             << shutterTime << ", " << aGain << " and " << dGainQ;
> >  
> >       activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
> >       activeState.agc.automatic.sensorGain = aGain;
> > -     activeState.agc.automatic.ispGain = dGain;
> > +     activeState.agc.automatic.ispGain = dGainQ;
> >  
> >       metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
> >       metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
> > -     metadata.set(controls::DigitalGain, frameContext.agc.ispGain);
> > +     metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());
> >       metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);
> >  }
> >  
> > diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> > index 08f78e4f74ce..ac4b83773803 100644
> > --- a/src/ipa/mali-c55/ipa_context.h
> > +++ b/src/ipa/mali-c55/ipa_context.h
> > @@ -41,12 +41,12 @@ struct IPAActiveState {
> >               struct {
> >                       uint32_t exposure;
> >                       double sensorGain;
> > -                     double ispGain;
> > +                     UQ<5, 8> ispGain;
> >               } automatic;
> >               struct {
> >                       uint32_t exposure;
> >                       double sensorGain;
> > -                     double ispGain;
> > +                     UQ<5, 8> ispGain;
> >               } manual;
> >               bool autoEnabled;
> >               uint32_t constraintMode;
> > @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {
> >       struct {
> >               uint32_t exposure;
> >               double sensorGain;
> > -             double ispGain;
> > +             UQ<5, 8> ispGain;
> >       } agc;
> >  
> >       struct {
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Feb. 19, 2026, 1:53 p.m. UTC | #4 
On Thu, Feb 19, 2026 at 01:33:09PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2026-02-19 12:08:00)
> > On Fri, Feb 13, 2026 at 04:57:52PM +0000, Kieran Bingham wrote:
> > > The Mali-C55 ISP has a digital gain block which allows setting gain in Q5.8
> > > format, a range of 0.0 to (very nearly) 32.0.
> > > 
> > > Convert usage to the new UQ<5, 8> FixedPoint Quantised type which will
> > > support the conversion, clamping and quantisation so that the metadata
> > > and debug prints can now report the effective gain applied instead of
> > > the potentially inaccurate float.
> > > 
> > > As the UQ<5, 8> type already clamps values, remove the explicit
> > > clamping.  This removes the clamping to a minimum of 1.0 gain, so we
> > > rely on calculateNewEv to provide a valid gain.
> > 
> > Looks like the review discussion from v4 as not been taken into account.
> 
> Yeah, I really hate how discussions get split across series!
> 
> I wish there was a way to keep all relevant discussion to a specific
> change in one place.
> 
> It looks like your last statement was:
> 
> │I agree with Kieran that duplicating clamping is not ideal. This being
> │said, we do duplicate code between AGC implementations already, and I
> │think Jacopo is working on fixing that by factoring out code to a common
> │helper in libipa. We can clamp in calculateNewEv(), or somewhere else,
> │depending on how we define calculateNewEv(). As long as things are well
> │designed (as in giving calculateNewEv() a clear and meaningful purpose)
> │and clearly documented, I don't mind much.
> 
> I understood that to mean that we need clamping in the calculateNewEv
> which would be on top of this series.

Either here or in calculateNewEv(), up to you. If it's in
calculateNewEv(), then the function documentation has to be updated to
clearly explain what the function's purpose is.

> do you want something changed /in this patch/ ?

This patch drops clamping, and relies on an undocumented assumption
about calculateNewEv() that may not always hold true. If you include a
patch for calculateNewEv() in the series then there's nothing to change
here. Otherwise I'll keep the clamp in this patch for now.

> > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > 
> > > ---
> > > v5:
> > >  - Use UQ<5, 8> type directly.
> > >  - Use operator<< to report the UQ<5, 8> value.
> > > ---
> > >  src/ipa/mali-c55/algorithms/agc.cpp | 18 +++++++++---------
> > >  src/ipa/mali-c55/ipa_context.h      |  6 +++---
> > >  2 files changed, 12 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> > > index 014fd12452ac..075717f44ea6 100644
> > > --- a/src/ipa/mali-c55/algorithms/agc.cpp
> > > +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> > > @@ -38,8 +38,8 @@ static constexpr unsigned int kNumHistogramBins = 256;
> > >   * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual
> > >   * max value which is 8191 * 2^-8.
> > >   */
> > > -static constexpr double kMinDigitalGain = 1.0;
> > > -static constexpr double kMaxDigitalGain = 31.99609375;
> > > +static constexpr float kMinDigitalGain = 1.0;
> > > +static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;
> > >  
> > >  uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)
> > >  {
> > > @@ -245,7 +245,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
> > >                            MaliC55Params *params)
> > >  {
> > >       IPAActiveState &activeState = context.activeState;
> > > -     double gain;
> > > +     UQ<5, 8> gain;
> > >  
> > >       if (activeState.agc.autoEnabled)
> > >               gain = activeState.agc.automatic.ispGain;
> > > @@ -253,7 +253,7 @@ void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
> > >               gain = activeState.agc.manual.ispGain;
> > >  
> > >       auto block = params->block<MaliC55Blocks::Dgain>();
> > > -     block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);
> > > +     block->gain = gain.quantized();
> > >  
> > >       frameContext.agc.ispGain = gain;
> > >  }
> > > @@ -358,7 +358,7 @@ void Agc::process(IPAContext &context,
> > >        */
> > >       uint32_t exposure = frameContext.agc.exposure;
> > >       double analogueGain = frameContext.agc.sensorGain;
> > > -     double digitalGain = frameContext.agc.ispGain;
> > > +     double digitalGain = frameContext.agc.ispGain.value();
> > >       double totalGain = analogueGain * digitalGain;
> > >       utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
> > >       utils::Duration effectiveExposureValue = currentShutter * totalGain;
> > > @@ -370,19 +370,19 @@ void Agc::process(IPAContext &context,
> > >                              activeState.agc.exposureMode, statistics_.yHist,
> > >                              effectiveExposureValue);
> > >  
> > > -     dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);
> > > +     UQ<5, 8> dGainQ = static_cast<float>(dGain);
> > >  
> > >       LOG(MaliC55Agc, Debug)
> > >               << "Divided up shutter, analogue gain and digital gain are "
> > > -             << shutterTime << ", " << aGain << " and " << dGain;
> > > +             << shutterTime << ", " << aGain << " and " << dGainQ;
> > >  
> > >       activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
> > >       activeState.agc.automatic.sensorGain = aGain;
> > > -     activeState.agc.automatic.ispGain = dGain;
> > > +     activeState.agc.automatic.ispGain = dGainQ;
> > >  
> > >       metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
> > >       metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
> > > -     metadata.set(controls::DigitalGain, frameContext.agc.ispGain);
> > > +     metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());
> > >       metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);
> > >  }
> > >  
> > > diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> > > index 08f78e4f74ce..ac4b83773803 100644
> > > --- a/src/ipa/mali-c55/ipa_context.h
> > > +++ b/src/ipa/mali-c55/ipa_context.h
> > > @@ -41,12 +41,12 @@ struct IPAActiveState {
> > >               struct {
> > >                       uint32_t exposure;
> > >                       double sensorGain;
> > > -                     double ispGain;
> > > +                     UQ<5, 8> ispGain;
> > >               } automatic;
> > >               struct {
> > >                       uint32_t exposure;
> > >                       double sensorGain;
> > > -                     double ispGain;
> > > +                     UQ<5, 8> ispGain;
> > >               } manual;
> > >               bool autoEnabled;
> > >               uint32_t constraintMode;
> > > @@ -64,7 +64,7 @@ struct IPAFrameContext : public FrameContext {
> > >       struct {
> > >               uint32_t exposure;
> > >               double sensorGain;
> > > -             double ispGain;
> > > +             UQ<5, 8> ispGain;
> > >       } agc;
> > >  
> > >       struct {

Patch
diff mbox series 

diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
index 014fd12452ac..075717f44ea6 100644
--- a/src/ipa/mali-c55/algorithms/agc.cpp
+++ b/src/ipa/mali-c55/algorithms/agc.cpp
@@ -38,8 +38,8 @@  static constexpr unsigned int kNumHistogramBins = 256;
  * format, a range of 0.0 to (very nearly) 32.0. We clamp from 1.0 to the actual
  * max value which is 8191 * 2^-8.
  */
-static constexpr double kMinDigitalGain = 1.0;
-static constexpr double kMaxDigitalGain = 31.99609375;
+static constexpr float kMinDigitalGain = 1.0;
+static constexpr float kMaxDigitalGain = UQ<5, 8>::TraitsType::max;
 
 uint32_t AgcStatistics::decodeBinValue(uint16_t binVal)
 {
@@ -245,7 +245,7 @@  void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
 			     MaliC55Params *params)
 {
 	IPAActiveState &activeState = context.activeState;
-	double gain;
+	UQ<5, 8> gain;
 
 	if (activeState.agc.autoEnabled)
 		gain = activeState.agc.automatic.ispGain;
@@ -253,7 +253,7 @@  void Agc::fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext,
 		gain = activeState.agc.manual.ispGain;
 
 	auto block = params->block<MaliC55Blocks::Dgain>();
-	block->gain = floatingToFixedPoint<5, 8, uint16_t, double>(gain);
+	block->gain = gain.quantized();
 
 	frameContext.agc.ispGain = gain;
 }
@@ -358,7 +358,7 @@  void Agc::process(IPAContext &context,
 	 */
 	uint32_t exposure = frameContext.agc.exposure;
 	double analogueGain = frameContext.agc.sensorGain;
-	double digitalGain = frameContext.agc.ispGain;
+	double digitalGain = frameContext.agc.ispGain.value();
 	double totalGain = analogueGain * digitalGain;
 	utils::Duration currentShutter = exposure * configuration.sensor.lineDuration;
 	utils::Duration effectiveExposureValue = currentShutter * totalGain;
@@ -370,19 +370,19 @@  void Agc::process(IPAContext &context,
 			       activeState.agc.exposureMode, statistics_.yHist,
 			       effectiveExposureValue);
 
-	dGain = std::clamp(dGain, kMinDigitalGain, kMaxDigitalGain);
+	UQ<5, 8> dGainQ = static_cast<float>(dGain);
 
 	LOG(MaliC55Agc, Debug)
 		<< "Divided up shutter, analogue gain and digital gain are "
-		<< shutterTime << ", " << aGain << " and " << dGain;
+		<< shutterTime << ", " << aGain << " and " << dGainQ;
 
 	activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration;
 	activeState.agc.automatic.sensorGain = aGain;
-	activeState.agc.automatic.ispGain = dGain;
+	activeState.agc.automatic.ispGain = dGainQ;
 
 	metadata.set(controls::ExposureTime, currentShutter.get<std::micro>());
 	metadata.set(controls::AnalogueGain, frameContext.agc.sensorGain);
-	metadata.set(controls::DigitalGain, frameContext.agc.ispGain);
+	metadata.set(controls::DigitalGain, frameContext.agc.ispGain.value());
 	metadata.set(controls::ColourTemperature, context.activeState.agc.temperatureK);
 }
 
diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
index 08f78e4f74ce..ac4b83773803 100644
--- a/src/ipa/mali-c55/ipa_context.h
+++ b/src/ipa/mali-c55/ipa_context.h
@@ -41,12 +41,12 @@  struct IPAActiveState {
 		struct {
 			uint32_t exposure;
 			double sensorGain;
-			double ispGain;
+			UQ<5, 8> ispGain;
 		} automatic;
 		struct {
 			uint32_t exposure;
 			double sensorGain;
-			double ispGain;
+			UQ<5, 8> ispGain;
 		} manual;
 		bool autoEnabled;
 		uint32_t constraintMode;
@@ -64,7 +64,7 @@  struct IPAFrameContext : public FrameContext {
 	struct {
 		uint32_t exposure;
 		double sensorGain;
-		double ispGain;
+		UQ<5, 8> ispGain;
 	} agc;
 
 	struct {