[v7,11/15] ipa: mali-c55: Reduce AWB calculations to float precision
diff mbox series

Message ID 20260213-kbingham-quantizers-v7-11-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 AWB calculations are determined using double precision, and then
will be soon stored in a quantized float.

Use float types for the intermediate types after the sums have been
converted to an average to remove static cast assignments.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
v5:
 - New in v5
---
 src/ipa/mali-c55/algorithms/awb.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Barnabás Pőcze Feb. 16, 2026, 4:40 p.m. UTC | #1 
2026. 02. 13. 17:57 keltezéssel, Kieran Bingham írta:
> The AWB calculations are determined using double precision, and then
> will be soon stored in a quantized float.
> 
> Use float types for the intermediate types after the sums have been
> converted to an average to remove static cast assignments.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> ---
> v5:
>   - New in v5
> ---
>   src/ipa/mali-c55/algorithms/awb.cpp | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
> index 964e810882a9..b179dd7f0c1c 100644
> --- a/src/ipa/mali-c55/algorithms/awb.cpp
> +++ b/src/ipa/mali-c55/algorithms/awb.cpp
> @@ -159,7 +159,7 @@ void Awb::process(IPAContext &context, const uint32_t frame,
>   	 * Sometimes the first frame's statistics have no valid pixels, in which
>   	 * case we'll just assume a grey world until they say otherwise.
>   	 */
> -	double rgAvg, bgAvg;
> +	float rgAvg, bgAvg;
>   	if (!counted_zones) {
>   		rgAvg = 1.0;
>   		bgAvg = 1.0;
> @@ -174,15 +174,15 @@ void Awb::process(IPAContext &context, const uint32_t frame,
>   	 * figure by the gains that were applied when the statistics for this
>   	 * frame were generated.
>   	 */
> -	double rRatio = rgAvg / frameContext.awb.rGain;
> -	double bRatio = bgAvg / frameContext.awb.bGain;
> +	float rRatio = rgAvg / frameContext.awb.rGain;
> +	float bRatio = bgAvg / frameContext.awb.bGain;
>   
>   	/*
>   	 * And then we can simply invert the ratio to find the gain we should
>   	 * apply.
>   	 */
> -	double rGain = 1 / rRatio;
> -	double bGain = 1 / bRatio;
> +	float rGain = 1 / rRatio;
> +	float bGain = 1 / bRatio;
>   
>   	/*
>   	 * Running at full speed, this algorithm results in oscillations in the
> @@ -190,7 +190,7 @@ void Awb::process(IPAContext &context, const uint32_t frame,
>   	 * changes in gain, unless we're in the startup phase in which case we
>   	 * want to fix the miscolouring as quickly as possible.
>   	 */
> -	double speed = frame < kNumStartupFrames ? 1.0 : 0.2;
> +	float speed = frame < kNumStartupFrames ? 1.0 : 0.2;
>   	rGain = speed * rGain + context.activeState.awb.rGain * (1.0 - speed);
>   	bGain = speed * bGain + context.activeState.awb.bGain * (1.0 - speed);

That `1.0 - speed` is `double`. There is `-Wfloat-conversion` to diagnose such cases.
(There are quite few places where that warning fires in the current code base.)

The precision of `float` looks sufficient to me, in any case.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

And I think a review was missed: https://patchwork.libcamera.org/patch/25918/#37912




>   
>
Kieran Bingham Feb. 16, 2026, 7:12 p.m. UTC | #2 
Quoting Barnabás Pőcze (2026-02-16 16:40:50)
> 2026. 02. 13. 17:57 keltezéssel, Kieran Bingham írta:
> > The AWB calculations are determined using double precision, and then
> > will be soon stored in a quantized float.
> > 
> > Use float types for the intermediate types after the sums have been
> > converted to an average to remove static cast assignments.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > ---
> > v5:
> >   - New in v5
> > ---
> >   src/ipa/mali-c55/algorithms/awb.cpp | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
> > index 964e810882a9..b179dd7f0c1c 100644
> > --- a/src/ipa/mali-c55/algorithms/awb.cpp
> > +++ b/src/ipa/mali-c55/algorithms/awb.cpp
> > @@ -159,7 +159,7 @@ void Awb::process(IPAContext &context, const uint32_t frame,
> >        * Sometimes the first frame's statistics have no valid pixels, in which
> >        * case we'll just assume a grey world until they say otherwise.
> >        */
> > -     double rgAvg, bgAvg;
> > +     float rgAvg, bgAvg;
> >       if (!counted_zones) {
> >               rgAvg = 1.0;
> >               bgAvg = 1.0;
> > @@ -174,15 +174,15 @@ void Awb::process(IPAContext &context, const uint32_t frame,
> >        * figure by the gains that were applied when the statistics for this
> >        * frame were generated.
> >        */
> > -     double rRatio = rgAvg / frameContext.awb.rGain;
> > -     double bRatio = bgAvg / frameContext.awb.bGain;
> > +     float rRatio = rgAvg / frameContext.awb.rGain;
> > +     float bRatio = bgAvg / frameContext.awb.bGain;
> >   
> >       /*
> >        * And then we can simply invert the ratio to find the gain we should
> >        * apply.
> >        */
> > -     double rGain = 1 / rRatio;
> > -     double bGain = 1 / bRatio;
> > +     float rGain = 1 / rRatio;
> > +     float bGain = 1 / bRatio;
> >   
> >       /*
> >        * Running at full speed, this algorithm results in oscillations in the
> > @@ -190,7 +190,7 @@ void Awb::process(IPAContext &context, const uint32_t frame,
> >        * changes in gain, unless we're in the startup phase in which case we
> >        * want to fix the miscolouring as quickly as possible.
> >        */
> > -     double speed = frame < kNumStartupFrames ? 1.0 : 0.2;
> > +     float speed = frame < kNumStartupFrames ? 1.0 : 0.2;
> >       rGain = speed * rGain + context.activeState.awb.rGain * (1.0 - speed);
> >       bGain = speed * bGain + context.activeState.awb.bGain * (1.0 - speed);
> 
> That `1.0 - speed` is `double`. There is `-Wfloat-conversion` to diagnose such cases.
> (There are quite few places where that warning fires in the current code base.)
> 
> The precision of `float` looks sufficient to me, in any case.
> 
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> And I think a review was missed: https://patchwork.libcamera.org/patch/25918/#37912

Aha, thanks, - I'll collect that here for patchwork:

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

> 
> 
> 
> 
> >   
> > 
>
Laurent Pinchart Feb. 19, 2026, 11:56 a.m. UTC | #3 
On Mon, Feb 16, 2026 at 05:40:50PM +0100, Barnabás Pőcze wrote:
> 2026. 02. 13. 17:57 keltezéssel, Kieran Bingham írta:
> > The AWB calculations are determined using double precision, and then
> > will be soon stored in a quantized float.
> > 
> > Use float types for the intermediate types after the sums have been
> > converted to an average to remove static cast assignments.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > ---
> > v5:
> >   - New in v5
> > ---
> >   src/ipa/mali-c55/algorithms/awb.cpp | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
> > index 964e810882a9..b179dd7f0c1c 100644
> > --- a/src/ipa/mali-c55/algorithms/awb.cpp
> > +++ b/src/ipa/mali-c55/algorithms/awb.cpp
> > @@ -159,7 +159,7 @@ void Awb::process(IPAContext &context, const uint32_t frame,
> >   	 * Sometimes the first frame's statistics have no valid pixels, in which
> >   	 * case we'll just assume a grey world until they say otherwise.
> >   	 */
> > -	double rgAvg, bgAvg;
> > +	float rgAvg, bgAvg;
> >   	if (!counted_zones) {
> >   		rgAvg = 1.0;
> >   		bgAvg = 1.0;
> > @@ -174,15 +174,15 @@ void Awb::process(IPAContext &context, const uint32_t frame,
> >   	 * figure by the gains that were applied when the statistics for this
> >   	 * frame were generated.
> >   	 */
> > -	double rRatio = rgAvg / frameContext.awb.rGain;
> > -	double bRatio = bgAvg / frameContext.awb.bGain;
> > +	float rRatio = rgAvg / frameContext.awb.rGain;
> > +	float bRatio = bgAvg / frameContext.awb.bGain;
> >   
> >   	/*
> >   	 * And then we can simply invert the ratio to find the gain we should
> >   	 * apply.
> >   	 */
> > -	double rGain = 1 / rRatio;
> > -	double bGain = 1 / bRatio;
> > +	float rGain = 1 / rRatio;
> > +	float bGain = 1 / bRatio;
> >   
> >   	/*
> >   	 * Running at full speed, this algorithm results in oscillations in the
> > @@ -190,7 +190,7 @@ void Awb::process(IPAContext &context, const uint32_t frame,
> >   	 * changes in gain, unless we're in the startup phase in which case we
> >   	 * want to fix the miscolouring as quickly as possible.
> >   	 */
> > -	double speed = frame < kNumStartupFrames ? 1.0 : 0.2;
> > +	float speed = frame < kNumStartupFrames ? 1.0 : 0.2;
> >   	rGain = speed * rGain + context.activeState.awb.rGain * (1.0 - speed);
> >   	bGain = speed * bGain + context.activeState.awb.bGain * (1.0 - speed);
> 
> That `1.0 - speed` is `double`. There is `-Wfloat-conversion` to diagnose such cases.
> (There are quite few places where that warning fires in the current code base.)

It could be a good idea to enable the warning and fix the existing code.

With 1.0f and 0.2f above,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> The precision of `float` looks sufficient to me, in any case.
> 
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> And I think a review was missed: https://patchwork.libcamera.org/patch/25918/#37912

Patch
diff mbox series 

diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
index 964e810882a9..b179dd7f0c1c 100644
--- a/src/ipa/mali-c55/algorithms/awb.cpp
+++ b/src/ipa/mali-c55/algorithms/awb.cpp
@@ -159,7 +159,7 @@  void Awb::process(IPAContext &context, const uint32_t frame,
 	 * Sometimes the first frame's statistics have no valid pixels, in which
 	 * case we'll just assume a grey world until they say otherwise.
 	 */
-	double rgAvg, bgAvg;
+	float rgAvg, bgAvg;
 	if (!counted_zones) {
 		rgAvg = 1.0;
 		bgAvg = 1.0;
@@ -174,15 +174,15 @@  void Awb::process(IPAContext &context, const uint32_t frame,
 	 * figure by the gains that were applied when the statistics for this
 	 * frame were generated.
 	 */
-	double rRatio = rgAvg / frameContext.awb.rGain;
-	double bRatio = bgAvg / frameContext.awb.bGain;
+	float rRatio = rgAvg / frameContext.awb.rGain;
+	float bRatio = bgAvg / frameContext.awb.bGain;
 
 	/*
 	 * And then we can simply invert the ratio to find the gain we should
 	 * apply.
 	 */
-	double rGain = 1 / rRatio;
-	double bGain = 1 / bRatio;
+	float rGain = 1 / rRatio;
+	float bGain = 1 / bRatio;
 
 	/*
 	 * Running at full speed, this algorithm results in oscillations in the
@@ -190,7 +190,7 @@  void Awb::process(IPAContext &context, const uint32_t frame,
 	 * changes in gain, unless we're in the startup phase in which case we
 	 * want to fix the miscolouring as quickly as possible.
 	 */
-	double speed = frame < kNumStartupFrames ? 1.0 : 0.2;
+	float speed = frame < kNumStartupFrames ? 1.0 : 0.2;
 	rGain = speed * rGain + context.activeState.awb.rGain * (1.0 - speed);
 	bGain = speed * bGain + context.activeState.awb.bGain * (1.0 - speed);