[v6,12/16] ipa: mali-c55: Convert AWB to UQ<4, 8> usage
diff mbox series

Message ID 20260121173737.376113-13-kieran.bingham@ideasonboard.com
State New
Headers show
Series
  • libipa: Introduce a Quantized type
Related show

Commit Message

Kieran Bingham Jan. 21, 2026, 5:37 p.m. UTC 
Utilise the new FixedPoint type to explicitly calculate gains for AWB in Q4.8
format. This ensures that reporting of gains in metadata reflect the true
AWB gains applied.

Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---
v5:
- Use UQ<4, 8> directly
- Change debug prints to use operator<<
- Now based on top of float types instead of doubles

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/mali-c55/algorithms/awb.cpp | 36 ++++++++++++++---------------
 src/ipa/mali-c55/ipa_context.h      | 10 ++++----
 2 files changed, 24 insertions(+), 22 deletions(-)

Comments

Stefan Klug Jan. 23, 2026, 2:24 p.m. UTC | #1 
Hi Kieran,

Quoting Kieran Bingham (2026-01-21 18:37:31)
> Utilise the new FixedPoint type to explicitly calculate gains for AWB in Q4.8
> format. This ensures that reporting of gains in metadata reflect the true
> AWB gains applied.
> 
> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> ---
> v5:
> - Use UQ<4, 8> directly
> - Change debug prints to use operator<<
> - Now based on top of float types instead of doubles
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/ipa/mali-c55/algorithms/awb.cpp | 36 ++++++++++++++---------------
>  src/ipa/mali-c55/ipa_context.h      | 10 ++++----
>  2 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
> index b179dd7f0c1c..a5150bbf3d97 100644
> --- a/src/ipa/mali-c55/algorithms/awb.cpp
> +++ b/src/ipa/mali-c55/algorithms/awb.cpp
> @@ -37,8 +37,8 @@ int Awb::configure([[maybe_unused]] IPAContext &context,
>          * for the first frame we will make no assumptions and leave the R/B
>          * channels unmodified.
>          */
> -       context.activeState.awb.rGain = 1.0;
> -       context.activeState.awb.bGain = 1.0;
> +       context.activeState.awb.rGain = 1.0f;
> +       context.activeState.awb.bGain = 1.0f;
>  
>         return 0;
>  }
> @@ -46,8 +46,8 @@ int Awb::configure([[maybe_unused]] IPAContext &context,
>  void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
>                                 IPAFrameContext &frameContext)
>  {
> -       double rGain = context.activeState.awb.rGain;
> -       double bGain = context.activeState.awb.bGain;
> +       UQ<4, 8> rGain = context.activeState.awb.rGain;
> +       UQ<4, 8> bGain = context.activeState.awb.bGain;
>  
>         /*
>          * The gains here map as follows:
> @@ -61,10 +61,10 @@ void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
>          */
>         auto block = params->block<MaliC55Blocks::AwbGains>();
>  
> -       block->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain);
> -       block->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);
> -       block->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);
> -       block->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain);
> +       block->gain00 = rGain.quantized();
> +       block->gain01 = UQ<4, 8>(1.0f).quantized();
> +       block->gain10 = UQ<4, 8>(1.0f).quantized();
> +       block->gain11 = bGain.quantized();
>  
>         frameContext.awb.rGain = rGain;
>         frameContext.awb.bGain = bGain;
> @@ -140,18 +140,18 @@ void Awb::process(IPAContext &context, const uint32_t frame,
>          * gain figures that we can apply to approximate a grey world.
>          */
>         unsigned int counted_zones = 0;
> -       double rgSum = 0, bgSum = 0;
> +       float rgSum = 0, bgSum = 0;
>  
>         for (unsigned int i = 0; i < 225; i++) {
>                 if (!awb_ratios[i].num_pixels)
>                         continue;
>  
>                 /*
> -                * The statistics are in Q4.8 format, so we convert to double
> +                * The statistics are in Q4.8 format, so we convert to float
>                  * here.
>                  */
> -               rgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_rg_gr);
> -               bgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_bg_br);
> +               rgSum += UQ<4, 8>(awb_ratios[i].avg_rg_gr).value();
> +               bgSum += UQ<4, 8>(awb_ratios[i].avg_bg_br).value();
>                 counted_zones++;
>         }
>  
> @@ -174,8 +174,8 @@ void Awb::process(IPAContext &context, const uint32_t frame,
>          * figure by the gains that were applied when the statistics for this
>          * frame were generated.
>          */
> -       float rRatio = rgAvg / frameContext.awb.rGain;
> -       float bRatio = bgAvg / frameContext.awb.bGain;
> +       float rRatio = rgAvg / frameContext.awb.rGain.value();
> +       float bRatio = bgAvg / frameContext.awb.bGain.value();
>  
>         /*
>          * And then we can simply invert the ratio to find the gain we should
> @@ -191,15 +191,15 @@ void Awb::process(IPAContext &context, const uint32_t frame,
>          * want to fix the miscolouring as quickly as possible.
>          */
>         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);
> +       rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0 - speed);
> +       bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0 - speed);
>  
>         context.activeState.awb.rGain = rGain;
>         context.activeState.awb.bGain = bGain;
>  
>         metadata.set(controls::ColourGains, {
> -               static_cast<float>(frameContext.awb.rGain),
> -               static_cast<float>(frameContext.awb.bGain),
> +               frameContext.awb.rGain.value(),
> +               frameContext.awb.bGain.value(),
>         });
>  
>         LOG(MaliC55Awb, Debug) << "For frame number " << frame << ": "
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> index 13885eb83b5c..08f78e4f74ce 100644
> --- a/src/ipa/mali-c55/ipa_context.h
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -14,6 +14,8 @@
>  
>  #include <libipa/fc_queue.h>
>  
> +#include "libipa/fixedpoint.h"
> +
>  namespace libcamera {
>  
>  namespace ipa::mali_c55 {
> @@ -53,8 +55,8 @@ struct IPAActiveState {
>         } agc;
>  
>         struct {
> -               double rGain;
> -               double bGain;
> +               UQ<4, 8> rGain;
> +               UQ<4, 8> bGain;
>         } awb;
>  };
>  
> @@ -66,8 +68,8 @@ struct IPAFrameContext : public FrameContext {
>         } agc;
>  
>         struct {
> -               double rGain;
> -               double bGain;
> +               UQ<4, 8> rGain;
> +               UQ<4, 8> bGain;

This is the only part I don't really like. We encode hardware specific
sizes in the context structure which I think are better kept in the
algorithm (this is the same in the RkISP1). But as you already planned
to pull in the context types from the corresponding algorithms I guess
this is temporary.

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

Cheers,
Stefan

>         } awb;
>  };
>  
> -- 
> 2.52.0
>

Patch
diff mbox series 

diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
index b179dd7f0c1c..a5150bbf3d97 100644
--- a/src/ipa/mali-c55/algorithms/awb.cpp
+++ b/src/ipa/mali-c55/algorithms/awb.cpp
@@ -37,8 +37,8 @@  int Awb::configure([[maybe_unused]] IPAContext &context,
 	 * for the first frame we will make no assumptions and leave the R/B
 	 * channels unmodified.
 	 */
-	context.activeState.awb.rGain = 1.0;
-	context.activeState.awb.bGain = 1.0;
+	context.activeState.awb.rGain = 1.0f;
+	context.activeState.awb.bGain = 1.0f;
 
 	return 0;
 }
@@ -46,8 +46,8 @@  int Awb::configure([[maybe_unused]] IPAContext &context,
 void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
 				IPAFrameContext &frameContext)
 {
-	double rGain = context.activeState.awb.rGain;
-	double bGain = context.activeState.awb.bGain;
+	UQ<4, 8> rGain = context.activeState.awb.rGain;
+	UQ<4, 8> bGain = context.activeState.awb.bGain;
 
 	/*
 	 * The gains here map as follows:
@@ -61,10 +61,10 @@  void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
 	 */
 	auto block = params->block<MaliC55Blocks::AwbGains>();
 
-	block->gain00 = floatingToFixedPoint<4, 8, uint16_t, double>(rGain);
-	block->gain01 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);
-	block->gain10 = floatingToFixedPoint<4, 8, uint16_t, double>(1.0);
-	block->gain11 = floatingToFixedPoint<4, 8, uint16_t, double>(bGain);
+	block->gain00 = rGain.quantized();
+	block->gain01 = UQ<4, 8>(1.0f).quantized();
+	block->gain10 = UQ<4, 8>(1.0f).quantized();
+	block->gain11 = bGain.quantized();
 
 	frameContext.awb.rGain = rGain;
 	frameContext.awb.bGain = bGain;
@@ -140,18 +140,18 @@  void Awb::process(IPAContext &context, const uint32_t frame,
 	 * gain figures that we can apply to approximate a grey world.
 	 */
 	unsigned int counted_zones = 0;
-	double rgSum = 0, bgSum = 0;
+	float rgSum = 0, bgSum = 0;
 
 	for (unsigned int i = 0; i < 225; i++) {
 		if (!awb_ratios[i].num_pixels)
 			continue;
 
 		/*
-		 * The statistics are in Q4.8 format, so we convert to double
+		 * The statistics are in Q4.8 format, so we convert to float
 		 * here.
 		 */
-		rgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_rg_gr);
-		bgSum += fixedToFloatingPoint<4, 8, double, uint16_t>(awb_ratios[i].avg_bg_br);
+		rgSum += UQ<4, 8>(awb_ratios[i].avg_rg_gr).value();
+		bgSum += UQ<4, 8>(awb_ratios[i].avg_bg_br).value();
 		counted_zones++;
 	}
 
@@ -174,8 +174,8 @@  void Awb::process(IPAContext &context, const uint32_t frame,
 	 * figure by the gains that were applied when the statistics for this
 	 * frame were generated.
 	 */
-	float rRatio = rgAvg / frameContext.awb.rGain;
-	float bRatio = bgAvg / frameContext.awb.bGain;
+	float rRatio = rgAvg / frameContext.awb.rGain.value();
+	float bRatio = bgAvg / frameContext.awb.bGain.value();
 
 	/*
 	 * And then we can simply invert the ratio to find the gain we should
@@ -191,15 +191,15 @@  void Awb::process(IPAContext &context, const uint32_t frame,
 	 * want to fix the miscolouring as quickly as possible.
 	 */
 	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);
+	rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0 - speed);
+	bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0 - speed);
 
 	context.activeState.awb.rGain = rGain;
 	context.activeState.awb.bGain = bGain;
 
 	metadata.set(controls::ColourGains, {
-		static_cast<float>(frameContext.awb.rGain),
-		static_cast<float>(frameContext.awb.bGain),
+		frameContext.awb.rGain.value(),
+		frameContext.awb.bGain.value(),
 	});
 
 	LOG(MaliC55Awb, Debug) << "For frame number " << frame << ": "
diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
index 13885eb83b5c..08f78e4f74ce 100644
--- a/src/ipa/mali-c55/ipa_context.h
+++ b/src/ipa/mali-c55/ipa_context.h
@@ -14,6 +14,8 @@ 
 
 #include <libipa/fc_queue.h>
 
+#include "libipa/fixedpoint.h"
+
 namespace libcamera {
 
 namespace ipa::mali_c55 {
@@ -53,8 +55,8 @@  struct IPAActiveState {
 	} agc;
 
 	struct {
-		double rGain;
-		double bGain;
+		UQ<4, 8> rGain;
+		UQ<4, 8> bGain;
 	} awb;
 };
 
@@ -66,8 +68,8 @@  struct IPAFrameContext : public FrameContext {
 	} agc;
 
 	struct {
-		double rGain;
-		double bGain;
+		UQ<4, 8> rGain;
+		UQ<4, 8> bGain;
 	} awb;
 };