[02/11] ipa: mali-c55: awb: Port to use libipa AwbAlgorithm
diff mbox series

Message ID 20260615-libipa-algorithms-v1-2-e949c937422e@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: libipa: Introduce libipa algorithms
Related show

Commit Message

Jacopo Mondi June 15, 2026, 2:05 p.m. UTC 
Port the Mali-C55 Awb algorithm to use the new libipa implementation
of AwbAlgorithm.

The awbAlgo_ class member is initialized with the Q<4, 8> type that
represents the register format and the MaliC55AwbStats type handles
the WB statistics format as produced by the Mali-C55 which already
calculates the mean values as R/G and B/G values.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/mali-c55/algorithms/awb.cpp | 198 ++++++++++++++++++------------------
 src/ipa/mali-c55/algorithms/awb.h   |  30 ++++--
 src/ipa/mali-c55/ipa_context.cpp    |   6 ++
 src/ipa/mali-c55/ipa_context.h      |  17 ++--
 src/ipa/mali-c55/mali-c55.cpp       |  27 +++--
 5 files changed, 152 insertions(+), 126 deletions(-)

Comments

Kieran Bingham June 16, 2026, 11:55 a.m. UTC | #1 
Quoting Jacopo Mondi (2026-06-15 15:05:27)
> Port the Mali-C55 Awb algorithm to use the new libipa implementation
> of AwbAlgorithm.
> 
> The awbAlgo_ class member is initialized with the Q<4, 8> type that
> represents the register format and the MaliC55AwbStats type handles
> the WB statistics format as produced by the Mali-C55 which already
> calculates the mean values as R/G and B/G values.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/ipa/mali-c55/algorithms/awb.cpp | 198 ++++++++++++++++++------------------
>  src/ipa/mali-c55/algorithms/awb.h   |  30 ++++--
>  src/ipa/mali-c55/ipa_context.cpp    |   6 ++
>  src/ipa/mali-c55/ipa_context.h      |  17 ++--
>  src/ipa/mali-c55/mali-c55.cpp       |  27 +++--
>  5 files changed, 152 insertions(+), 126 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
> index 8a671b52be59..97964f395325 100644
> --- a/src/ipa/mali-c55/algorithms/awb.cpp
> +++ b/src/ipa/mali-c55/algorithms/awb.cpp
> @@ -2,7 +2,7 @@
>  /*
>   * Copyright (C) 2024, Ideas On Board Oy
>   *
> - * Mali C55 grey world auto white balance algorithm
> + * Mali C55 auto white balance algorithm
>   */
>  
>  #include "awb.h"
> @@ -14,63 +14,65 @@
>  
>  #include <libcamera/control_ids.h>
>  
> -#include "libipa/fixedpoint.h"
> -
>  namespace libcamera {
>  
>  namespace ipa::mali_c55::algorithms {
>  
>  LOG_DEFINE_CATEGORY(MaliC55Awb)
>  
> -/* Number of frames at which we should run AWB at full speed */
> -static constexpr uint32_t kNumStartupFrames = 4;
> +/* \todo Mali-C55 doesn't support the Lux algorithm. */
> +static constexpr unsigned int kDefaultLux = 500;
> +
> +class MaliC55AwbStats final : public AwbStats
> +{
> +public:
> +       MaliC55AwbStats() = default;
> +       MaliC55AwbStats(const RGB<double> &rgbMeans)
> +               : AwbStats(rgbMeans)
> +       {
> +               /* The Mali-C55 ISP already provides stats as R/G and B/G ratios. */
> +               rg_ = rgbMeans_.r();
> +               bg_ = rgbMeans_.b();
> +       }
>  
> -Awb::Awb()
> +       /* Minimum mean value below which AWB can't operate. */
> +       double minColourValue() const override
> +       {
> +               return 0.2;
> +       }
> +};
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Awb::init(IPAContext &context, const ValueNode &tuningData)
>  {
> +       return awbAlgo_.init(tuningData, context.ctrlMap);
>  }
>  
> -int Awb::configure([[maybe_unused]] IPAContext &context,
> +/**
> + * \copydoc libcamera::ipa::Algorithm::configure
> + */
> +int Awb::configure(IPAContext &context,
>                    [[maybe_unused]] const IPACameraSensorInfo &configInfo)
>  {
> -       /*
> -        * Initially we have no idea what the colour balance will be like, so
> -        * for the first frame we will make no assumptions and leave the R/B
> -        * channels unmodified.
> -        */
> -       context.activeState.awb.rGain = 1.0f;
> -       context.activeState.awb.bGain = 1.0f;
> -
> -       return 0;
> +       return awbAlgo_.configure(context.activeState.awb,
> +                                 context.configuration.awb);
>  }
>  
> -void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
> -                               IPAFrameContext &frameContext)
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Awb::queueRequest(IPAContext &context,
> +                      const uint32_t frame,
> +                      IPAFrameContext &frameContext,
> +                      const ControlList &controls)
>  {
> -       UQ<4, 8> rGain = context.activeState.awb.rGain;
> -       UQ<4, 8> bGain = context.activeState.awb.bGain;
> -
> -       /*
> -        * The gains here map as follows:
> -        *      gain00 = R
> -        *      gain01 = Gr
> -        *      gain10 = Gb
> -        *      gain11 = B
> -        *
> -        * This holds true regardless of the bayer order of the input data, as
> -        * the mapping is done internally in the ISP.
> -        */
> -       auto block = params->block<MaliC55Blocks::AwbGains>();
> -
> -       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;
> +       awbAlgo_.queueRequest(context.activeState.awb, frame, frameContext.awb,
> +                             controls);
>  }
>  
> -void Awb::fillConfigParamBlock(MaliC55Params *params)
> +void Awb::configAwbMeas(MaliC55Params *params)

Is 'Meas' correct ? Perhaps configureAwbMeans()

>  {
>         auto block = params->block<MaliC55Blocks::AwbConfig>();
>  
> @@ -117,56 +119,77 @@ void Awb::fillConfigParamBlock(MaliC55Params *params)
>         block->cb_low = 64;
>  }
>  
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
>  void Awb::prepare(IPAContext &context, const uint32_t frame,
>                   IPAFrameContext &frameContext, MaliC55Params *params)
>  {
> -       fillGainsParamBlock(params, context, frameContext);
> +       awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
> +
> +       /*
> +        * The gains here map as follows:
> +        *      gain00 = R
> +        *      gain01 = Gr
> +        *      gain10 = Gb
> +        *      gain11 = B
> +        *
> +        * This holds true regardless of the bayer order of the input data, as
> +        * the mapping is done internally in the ISP.
> +        */
> +       auto block = params->block<MaliC55Blocks::AwbGains>();
> +       block.setEnabled(true);
> +
> +       block->gain00 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.r()))
> +                               .quantized();

I really wonder if we should just use float in the libipa AWB
implementation. I can't see why we're using double there ...

> +       block->gain01 = UQ<4, 8>(1.0f).quantized();
> +       block->gain10 = UQ<4, 8>(1.0f).quantized();
> +       block->gain11 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.b()))
> +                               .quantized();
>  
>         if (frame > 0)
>                 return;
>  
> -       fillConfigParamBlock(params);
> +       configAwbMeas(params);
>  }
>  
> -void Awb::process(IPAContext &context, const uint32_t frame,
> -                 IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
> -                 [[maybe_unused]] ControlList &metadata)
> +MaliC55AwbStats Awb::calculateRgbMeans(const IPAFrameContext &frameContext,
> +                                      const mali_c55_stats_buffer *stats) const
>  {
> -       const struct mali_c55_awb_average_ratios *awb_ratios = stats->awb_ratios;
> +       const struct mali_c55_awb_average_ratios *awb = stats->awb_ratios;
>  
>         /*
>          * The ISP produces average R:G and B:G ratios for zones. We take the
> -        * average of all the zones with data and simply invert them to provide
> -        * gain figures that we can apply to approximate a grey world.
> +        * average of all the zones with data and calculate the mean values.
>          */
> -       unsigned int counted_zones = 0;
> -       float rgSum = 0, bgSum = 0;
> +       unsigned int active_zones = 0;
> +       double rgSum = 0, bgSum = 0;
>  
>         for (unsigned int i = 0; i < 225; i++) {
> -               if (!awb_ratios[i].num_pixels)
> +               if (!awb[i].num_pixels)
>                         continue;
>  
>                 /*
> -                * The statistics are in Q4.8 format, so we convert to float
> +                * The statistics are in Q4.8 format, so we convert to double
>                  * here.
>                  */
> -               rgSum += UQ<4, 8>(awb_ratios[i].avg_rg_gr).value();
> -               bgSum += UQ<4, 8>(awb_ratios[i].avg_bg_br).value();
> -               counted_zones++;
> +               rgSum += UQ<4, 8>(awb[i].avg_rg_gr).value();
> +               bgSum += UQ<4, 8>(awb[i].avg_bg_br).value();
> +               active_zones++;
>         }
>  
>         /*
>          * Sometimes the first frame's statistics have no valid pixels, in which
>          * case we'll just assume a grey world until they say otherwise.
>          */
> -       float rgAvg, bgAvg;
> -       if (!counted_zones) {
> -               rgAvg = 1.0;
> -               bgAvg = 1.0;
> -       } else {
> -               rgAvg = rgSum / counted_zones;
> -               bgAvg = bgSum / counted_zones;
> -       }
> +       if (!active_zones)
> +               return {};
> +
> +       RGB<double> rgbMeans = { {
> +               rgSum / active_zones,
> +               1.0,
> +               bgSum / active_zones,
> +       } };
>  
>         /*
>          * The statistics are generated _after_ white balancing is performed in
> @@ -174,41 +197,22 @@ 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.value();
> -       float bRatio = bgAvg / frameContext.awb.bGain.value();
> +       rgbMeans /= frameContext.awb.gains.max(0.01);
>  
> -       /*
> -        * And then we can simply invert the ratio to find the gain we should
> -        * apply.
> -        */
> -       float rGain = 1 / rRatio;
> -       float bGain = 1 / bRatio;
> +       return MaliC55AwbStats(rgbMeans);
> +}
>  
> -       /*
> -        * Running at full speed, this algorithm results in oscillations in the
> -        * colour balance. To remove those we dampen the speed at which it makes
> -        * changes in gain, unless we're in the startup phase in which case we
> -        * want to fix the miscolouring as quickly as possible.
> -        */
> -       float speed = frame < kNumStartupFrames ? 1.0f : 0.2f;
> -       rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0f - speed);
> -       bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0f - speed);
> -
> -       context.activeState.awb.rGain = rGain;
> -       context.activeState.awb.bGain = bGain;
> -
> -       metadata.set(controls::ColourGains, {
> -               frameContext.awb.rGain.value(),
> -               frameContext.awb.bGain.value(),
> -       });
> -
> -       LOG(MaliC55Awb, Debug) << "For frame number " << frame << ": "
> -               << "Average R/G Ratio: " << rgAvg
> -               << ", Average B/G Ratio: " << bgAvg
> -               << "\nrGain applied to this frame: " << frameContext.awb.rGain
> -               << ", bGain applied to this frame: " << frameContext.awb.bGain
> -               << "\nrGain to apply: " << context.activeState.awb.rGain
> -               << ", bGain to apply: " << context.activeState.awb.bGain;
> +/**
> + * \copydoc libcamera::ipa::Algorithm::process
> + */
> +void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +                 IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
> +                 ControlList &metadata)
> +{
> +       MaliC55AwbStats awbStats = calculateRgbMeans(frameContext, stats);
> +
> +       awbAlgo_.process(context.activeState.awb, frameContext.awb, awbStats,
> +                        kDefaultLux, metadata);
>  }
>  
>  REGISTER_IPA_ALGORITHM(Awb, "Awb")
> diff --git a/src/ipa/mali-c55/algorithms/awb.h b/src/ipa/mali-c55/algorithms/awb.h
> index 683a62af263a..cc7ef583eafc 100644
> --- a/src/ipa/mali-c55/algorithms/awb.h
> +++ b/src/ipa/mali-c55/algorithms/awb.h
> @@ -2,24 +2,41 @@
>  /*
>   * Copyright (C) 2024, Ideas on Board Oy
>   *
> - * Mali C55 grey world auto white balance algorithm
> + * Mali C55 auto white balance algorithm
>   */
>  
> +#pragma once
> +
> +#include <linux/media/arm/mali-c55-config.h>
> +
> +#include <libcamera/controls.h>
> +
> +#include "libcamera/internal/value_node.h"
> +
> +#include "libipa/awb.h"
> +#include "libipa/fixedpoint.h"
> +
>  #include "algorithm.h"
>  #include "ipa_context.h"
> +#include "params.h"
>  
>  namespace libcamera {
>  
>  namespace ipa::mali_c55::algorithms {
>  
> +class MaliC55AwbStats;
> +
>  class Awb : public Algorithm
>  {
>  public:
> -       Awb();
>         ~Awb() = default;
>  
> +       int init(IPAContext &context, const ValueNode &tuningData) override;
>         int configure(IPAContext &context,
>                       const IPACameraSensorInfo &configInfo) override;
> +       void queueRequest(IPAContext &context, const uint32_t frame,
> +                         IPAFrameContext &frameContext,
> +                         const ControlList &controls) override;
>         void prepare(IPAContext &context, const uint32_t frame,
>                      IPAFrameContext &frameContext,
>                      MaliC55Params *params) override;
> @@ -29,10 +46,11 @@ public:
>                      ControlList &metadata) override;
>  
>  private:
> -       void fillGainsParamBlock(MaliC55Params *params,
> -                                IPAContext &context,
> -                                IPAFrameContext &frameContext);
> -       void fillConfigParamBlock(MaliC55Params *params);
> +       void configAwbMeas(MaliC55Params *params);
> +       MaliC55AwbStats calculateRgbMeans(const IPAFrameContext &frameContext,
> +                                         const mali_c55_stats_buffer *stats) const;
> +
> +       AwbAlgorithm<UQ<4, 8>> awbAlgo_;
>  };
>  
>  } /* namespace ipa::mali_c55::algorithms */
> diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
> index 1b203e2b2605..a1e4c39f3870 100644
> --- a/src/ipa/mali-c55/ipa_context.cpp
> +++ b/src/ipa/mali-c55/ipa_context.cpp
> @@ -96,6 +96,12 @@ namespace libcamera::ipa::mali_c55 {
>   *
>   * \var IPAContext::frameContexts
>   * \brief Ring buffer of per-frame contexts
> + *
> + * \var IPAContext::ctrlMap
> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
> + *
> + * \var IPAContext::camHelper
> + * \brief The camera sensor helper
>   */
>  
>  } /* namespace libcamera::ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> index ac4b83773803..3d884ea17eb8 100644
> --- a/src/ipa/mali-c55/ipa_context.h
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -12,8 +12,10 @@
>  
>  #include "libcamera/internal/bayer_format.h"
>  
> +#include <libipa/camera_sensor_helper.h>
>  #include <libipa/fc_queue.h>
>  
> +#include "libipa/awb.h"
>  #include "libipa/fixedpoint.h"
>  
>  namespace libcamera {
> @@ -29,6 +31,8 @@ struct IPASessionConfiguration {
>                 double maxAnalogueGain;
>         } agc;
>  
> +       ipa::awb::Session awb;
> +
>         struct {
>                 BayerFormat::Order bayerOrder;
>                 utils::Duration lineDuration;
> @@ -54,10 +58,7 @@ struct IPAActiveState {
>                 uint32_t temperatureK;
>         } agc;
>  
> -       struct {
> -               UQ<4, 8> rGain;
> -               UQ<4, 8> bGain;
> -       } awb;
> +       ipa::awb::ActiveState awb;
>  };
>  
>  struct IPAFrameContext : public FrameContext {
> @@ -67,10 +68,7 @@ struct IPAFrameContext : public FrameContext {
>                 UQ<5, 8> ispGain;
>         } agc;
>  
> -       struct {
> -               UQ<4, 8> rGain;
> -               UQ<4, 8> bGain;
> -       } awb;
> +       ipa::awb::FrameContext awb;
>  };
>  
>  struct IPAContext {
> @@ -85,6 +83,9 @@ struct IPAContext {
>         FCQueue<IPAFrameContext> frameContexts;
>  
>         ControlInfoMap::Map ctrlMap;
> +
> +       /* Interface to the Camera Helper */
> +       std::unique_ptr<CameraSensorHelper> camHelper;
>  };
>  
>  } /* namespace ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
> index 1d3af0627fdb..c35cc10bea01 100644
> --- a/src/ipa/mali-c55/mali-c55.cpp
> +++ b/src/ipa/mali-c55/mali-c55.cpp
> @@ -77,9 +77,6 @@ private:
>  
>         ControlInfoMap sensorControls_;
>  
> -       /* Interface to the Camera Helper */
> -       std::unique_ptr<CameraSensorHelper> camHelper_;
> -
>         /* Local parameter storage */
>         struct IPAContext context_;
>  };
> @@ -101,8 +98,8 @@ std::string IPAMaliC55::logPrefix() const
>  int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
>                      ControlInfoMap *ipaControls)
>  {
> -       camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> -       if (!camHelper_) {
> +       context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> +       if (!context_.camHelper) {

I had this split out during:
 - https://patchwork.libcamera.org/project/libcamera/list/?series=5486&state=*

 - https://patchwork.libcamera.org/patch/24590/

Which already had your review by tag too ;-)
But I guess it's fine in here.


>                 LOG(IPAMaliC55, Error)
>                         << "Failed to create camera sensor helper for "
>                         << settings.sensorModel;
> @@ -145,10 +142,10 @@ void IPAMaliC55::setControls()
>  
>         if (activeState.agc.autoEnabled) {
>                 exposure = activeState.agc.automatic.exposure;
> -               gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
> +               gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
>         } else {
>                 exposure = activeState.agc.manual.exposure;
> -               gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
> +               gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
>         }
>  
>         ControlList ctrls(sensorControls_);
> @@ -194,17 +191,17 @@ void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
>         context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
>         context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
>         context_.configuration.agc.defaultExposure = defExposure;
> -       context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> -       context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> +       context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
> +       context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
>  
> -       if (camHelper_->blackLevel().has_value()) {
> +       if (context_.camHelper->blackLevel().has_value()) {

I guess we should rebase https://patchwork.libcamera.org/patch/24591/
too.

But that can be on top of this series now.


My only thoughts are really that we are doing a lot to convert doubles
and floats and it all feels a bit unnecessary to work on a value that
will be clamped to a UQ4,8 (12 bit preceision) while float has an
equivalent of 24 bit precision, and double gives 53.

And perhaps the patch itself could be split up - but I can look the
otherway on either case so:


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

>                 /*
>                  * The black level from CameraSensorHelper is a 16-bit value.
>                  * The Mali-C55 ISP expects 20-bit settings, so we shift it to
>                  * the appropriate width
>                  */
>                 context_.configuration.sensor.blackLevel =
> -                       camHelper_->blackLevel().value() << 4;
> +                       context_.camHelper->blackLevel().value() << 4;
>         }
>  }
>  
> @@ -255,9 +252,9 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
>  
>         /* Compute the analogue gain limits. */
>         const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
> -       float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
> -       float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
> -       float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
> +       float minGain = context_.camHelper->gain(v4l2Gain.min().get<int32_t>());
> +       float maxGain = context_.camHelper->gain(v4l2Gain.max().get<int32_t>());
> +       float defGain = context_.camHelper->gain(v4l2Gain.def().get<int32_t>());
>         ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
>  
>         /*
> @@ -355,7 +352,7 @@ void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
>         frameContext.agc.exposure =
>                 sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>         frameContext.agc.sensorGain =
> -               camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +               context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>  
>         ControlList metadata(controls::controls);
>  
> 
> -- 
> 2.54.0
>
Jacopo Mondi June 16, 2026, 12:29 p.m. UTC | #2 
Hi Kieran

On Tue, Jun 16, 2026 at 12:55:32PM +0100, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2026-06-15 15:05:27)
> > Port the Mali-C55 Awb algorithm to use the new libipa implementation
> > of AwbAlgorithm.
> >
> > The awbAlgo_ class member is initialized with the Q<4, 8> type that
> > represents the register format and the MaliC55AwbStats type handles
> > the WB statistics format as produced by the Mali-C55 which already
> > calculates the mean values as R/G and B/G values.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  src/ipa/mali-c55/algorithms/awb.cpp | 198 ++++++++++++++++++------------------
> >  src/ipa/mali-c55/algorithms/awb.h   |  30 ++++--
> >  src/ipa/mali-c55/ipa_context.cpp    |   6 ++
> >  src/ipa/mali-c55/ipa_context.h      |  17 ++--
> >  src/ipa/mali-c55/mali-c55.cpp       |  27 +++--
> >  5 files changed, 152 insertions(+), 126 deletions(-)
> >
> > diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
> > index 8a671b52be59..97964f395325 100644
> > --- a/src/ipa/mali-c55/algorithms/awb.cpp
> > +++ b/src/ipa/mali-c55/algorithms/awb.cpp
> > @@ -2,7 +2,7 @@
> >  /*
> >   * Copyright (C) 2024, Ideas On Board Oy
> >   *
> > - * Mali C55 grey world auto white balance algorithm
> > + * Mali C55 auto white balance algorithm
> >   */
> >
> >  #include "awb.h"
> > @@ -14,63 +14,65 @@
> >
> >  #include <libcamera/control_ids.h>
> >
> > -#include "libipa/fixedpoint.h"
> > -
> >  namespace libcamera {
> >
> >  namespace ipa::mali_c55::algorithms {
> >
> >  LOG_DEFINE_CATEGORY(MaliC55Awb)
> >
> > -/* Number of frames at which we should run AWB at full speed */
> > -static constexpr uint32_t kNumStartupFrames = 4;
> > +/* \todo Mali-C55 doesn't support the Lux algorithm. */
> > +static constexpr unsigned int kDefaultLux = 500;
> > +
> > +class MaliC55AwbStats final : public AwbStats
> > +{
> > +public:
> > +       MaliC55AwbStats() = default;
> > +       MaliC55AwbStats(const RGB<double> &rgbMeans)
> > +               : AwbStats(rgbMeans)
> > +       {
> > +               /* The Mali-C55 ISP already provides stats as R/G and B/G ratios. */
> > +               rg_ = rgbMeans_.r();
> > +               bg_ = rgbMeans_.b();
> > +       }
> >
> > -Awb::Awb()
> > +       /* Minimum mean value below which AWB can't operate. */
> > +       double minColourValue() const override
> > +       {
> > +               return 0.2;
> > +       }
> > +};
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::init
> > + */
> > +int Awb::init(IPAContext &context, const ValueNode &tuningData)
> >  {
> > +       return awbAlgo_.init(tuningData, context.ctrlMap);
> >  }
> >
> > -int Awb::configure([[maybe_unused]] IPAContext &context,
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::configure
> > + */
> > +int Awb::configure(IPAContext &context,
> >                    [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> >  {
> > -       /*
> > -        * Initially we have no idea what the colour balance will be like, so
> > -        * for the first frame we will make no assumptions and leave the R/B
> > -        * channels unmodified.
> > -        */
> > -       context.activeState.awb.rGain = 1.0f;
> > -       context.activeState.awb.bGain = 1.0f;
> > -
> > -       return 0;
> > +       return awbAlgo_.configure(context.activeState.awb,
> > +                                 context.configuration.awb);
> >  }
> >
> > -void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
> > -                               IPAFrameContext &frameContext)
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::queueRequest
> > + */
> > +void Awb::queueRequest(IPAContext &context,
> > +                      const uint32_t frame,
> > +                      IPAFrameContext &frameContext,
> > +                      const ControlList &controls)
> >  {
> > -       UQ<4, 8> rGain = context.activeState.awb.rGain;
> > -       UQ<4, 8> bGain = context.activeState.awb.bGain;
> > -
> > -       /*
> > -        * The gains here map as follows:
> > -        *      gain00 = R
> > -        *      gain01 = Gr
> > -        *      gain10 = Gb
> > -        *      gain11 = B
> > -        *
> > -        * This holds true regardless of the bayer order of the input data, as
> > -        * the mapping is done internally in the ISP.
> > -        */
> > -       auto block = params->block<MaliC55Blocks::AwbGains>();
> > -
> > -       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;
> > +       awbAlgo_.queueRequest(context.activeState.awb, frame, frameContext.awb,
> > +                             controls);
> >  }
> >
> > -void Awb::fillConfigParamBlock(MaliC55Params *params)
> > +void Awb::configAwbMeas(MaliC55Params *params)
>
> Is 'Meas' correct ? Perhaps configureAwbMeans()
>

It's intentional for "Measurement". Should I change it ?


> >  {
> >         auto block = params->block<MaliC55Blocks::AwbConfig>();
> >
> > @@ -117,56 +119,77 @@ void Awb::fillConfigParamBlock(MaliC55Params *params)
> >         block->cb_low = 64;
> >  }
> >
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::prepare
> > + */
> >  void Awb::prepare(IPAContext &context, const uint32_t frame,
> >                   IPAFrameContext &frameContext, MaliC55Params *params)
> >  {
> > -       fillGainsParamBlock(params, context, frameContext);
> > +       awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
> > +
> > +       /*
> > +        * The gains here map as follows:
> > +        *      gain00 = R
> > +        *      gain01 = Gr
> > +        *      gain10 = Gb
> > +        *      gain11 = B
> > +        *
> > +        * This holds true regardless of the bayer order of the input data, as
> > +        * the mapping is done internally in the ISP.
> > +        */
> > +       auto block = params->block<MaliC55Blocks::AwbGains>();
> > +       block.setEnabled(true);
> > +
> > +       block->gain00 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.r()))
> > +                               .quantized();
>
> I really wonder if we should just use float in the libipa AWB
> implementation. I can't see why we're using double there ...
>

I don't know, I think that's what we had already, didn't we ?

> > +       block->gain01 = UQ<4, 8>(1.0f).quantized();
> > +       block->gain10 = UQ<4, 8>(1.0f).quantized();
> > +       block->gain11 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.b()))
> > +                               .quantized();
> >
> >         if (frame > 0)
> >                 return;
> >
> > -       fillConfigParamBlock(params);
> > +       configAwbMeas(params);
> >  }
> >
> > -void Awb::process(IPAContext &context, const uint32_t frame,
> > -                 IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
> > -                 [[maybe_unused]] ControlList &metadata)
> > +MaliC55AwbStats Awb::calculateRgbMeans(const IPAFrameContext &frameContext,
> > +                                      const mali_c55_stats_buffer *stats) const
> >  {
> > -       const struct mali_c55_awb_average_ratios *awb_ratios = stats->awb_ratios;
> > +       const struct mali_c55_awb_average_ratios *awb = stats->awb_ratios;
> >
> >         /*
> >          * The ISP produces average R:G and B:G ratios for zones. We take the
> > -        * average of all the zones with data and simply invert them to provide
> > -        * gain figures that we can apply to approximate a grey world.
> > +        * average of all the zones with data and calculate the mean values.
> >          */
> > -       unsigned int counted_zones = 0;
> > -       float rgSum = 0, bgSum = 0;
> > +       unsigned int active_zones = 0;
> > +       double rgSum = 0, bgSum = 0;
> >
> >         for (unsigned int i = 0; i < 225; i++) {
> > -               if (!awb_ratios[i].num_pixels)
> > +               if (!awb[i].num_pixels)
> >                         continue;
> >
> >                 /*
> > -                * The statistics are in Q4.8 format, so we convert to float
> > +                * The statistics are in Q4.8 format, so we convert to double
> >                  * here.
> >                  */
> > -               rgSum += UQ<4, 8>(awb_ratios[i].avg_rg_gr).value();
> > -               bgSum += UQ<4, 8>(awb_ratios[i].avg_bg_br).value();
> > -               counted_zones++;
> > +               rgSum += UQ<4, 8>(awb[i].avg_rg_gr).value();
> > +               bgSum += UQ<4, 8>(awb[i].avg_bg_br).value();
> > +               active_zones++;
> >         }
> >
> >         /*
> >          * Sometimes the first frame's statistics have no valid pixels, in which
> >          * case we'll just assume a grey world until they say otherwise.
> >          */
> > -       float rgAvg, bgAvg;
> > -       if (!counted_zones) {
> > -               rgAvg = 1.0;
> > -               bgAvg = 1.0;
> > -       } else {
> > -               rgAvg = rgSum / counted_zones;
> > -               bgAvg = bgSum / counted_zones;
> > -       }
> > +       if (!active_zones)
> > +               return {};
> > +
> > +       RGB<double> rgbMeans = { {
> > +               rgSum / active_zones,
> > +               1.0,
> > +               bgSum / active_zones,
> > +       } };
> >
> >         /*
> >          * The statistics are generated _after_ white balancing is performed in
> > @@ -174,41 +197,22 @@ 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.value();
> > -       float bRatio = bgAvg / frameContext.awb.bGain.value();
> > +       rgbMeans /= frameContext.awb.gains.max(0.01);
> >
> > -       /*
> > -        * And then we can simply invert the ratio to find the gain we should
> > -        * apply.
> > -        */
> > -       float rGain = 1 / rRatio;
> > -       float bGain = 1 / bRatio;
> > +       return MaliC55AwbStats(rgbMeans);
> > +}
> >
> > -       /*
> > -        * Running at full speed, this algorithm results in oscillations in the
> > -        * colour balance. To remove those we dampen the speed at which it makes
> > -        * changes in gain, unless we're in the startup phase in which case we
> > -        * want to fix the miscolouring as quickly as possible.
> > -        */
> > -       float speed = frame < kNumStartupFrames ? 1.0f : 0.2f;
> > -       rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0f - speed);
> > -       bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0f - speed);
> > -
> > -       context.activeState.awb.rGain = rGain;
> > -       context.activeState.awb.bGain = bGain;
> > -
> > -       metadata.set(controls::ColourGains, {
> > -               frameContext.awb.rGain.value(),
> > -               frameContext.awb.bGain.value(),
> > -       });
> > -
> > -       LOG(MaliC55Awb, Debug) << "For frame number " << frame << ": "
> > -               << "Average R/G Ratio: " << rgAvg
> > -               << ", Average B/G Ratio: " << bgAvg
> > -               << "\nrGain applied to this frame: " << frameContext.awb.rGain
> > -               << ", bGain applied to this frame: " << frameContext.awb.bGain
> > -               << "\nrGain to apply: " << context.activeState.awb.rGain
> > -               << ", bGain to apply: " << context.activeState.awb.bGain;
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::process
> > + */
> > +void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > +                 IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
> > +                 ControlList &metadata)
> > +{
> > +       MaliC55AwbStats awbStats = calculateRgbMeans(frameContext, stats);
> > +
> > +       awbAlgo_.process(context.activeState.awb, frameContext.awb, awbStats,
> > +                        kDefaultLux, metadata);
> >  }
> >
> >  REGISTER_IPA_ALGORITHM(Awb, "Awb")
> > diff --git a/src/ipa/mali-c55/algorithms/awb.h b/src/ipa/mali-c55/algorithms/awb.h
> > index 683a62af263a..cc7ef583eafc 100644
> > --- a/src/ipa/mali-c55/algorithms/awb.h
> > +++ b/src/ipa/mali-c55/algorithms/awb.h
> > @@ -2,24 +2,41 @@
> >  /*
> >   * Copyright (C) 2024, Ideas on Board Oy
> >   *
> > - * Mali C55 grey world auto white balance algorithm
> > + * Mali C55 auto white balance algorithm
> >   */
> >
> > +#pragma once
> > +
> > +#include <linux/media/arm/mali-c55-config.h>
> > +
> > +#include <libcamera/controls.h>
> > +
> > +#include "libcamera/internal/value_node.h"
> > +
> > +#include "libipa/awb.h"
> > +#include "libipa/fixedpoint.h"
> > +
> >  #include "algorithm.h"
> >  #include "ipa_context.h"
> > +#include "params.h"
> >
> >  namespace libcamera {
> >
> >  namespace ipa::mali_c55::algorithms {
> >
> > +class MaliC55AwbStats;
> > +
> >  class Awb : public Algorithm
> >  {
> >  public:
> > -       Awb();
> >         ~Awb() = default;
> >
> > +       int init(IPAContext &context, const ValueNode &tuningData) override;
> >         int configure(IPAContext &context,
> >                       const IPACameraSensorInfo &configInfo) override;
> > +       void queueRequest(IPAContext &context, const uint32_t frame,
> > +                         IPAFrameContext &frameContext,
> > +                         const ControlList &controls) override;
> >         void prepare(IPAContext &context, const uint32_t frame,
> >                      IPAFrameContext &frameContext,
> >                      MaliC55Params *params) override;
> > @@ -29,10 +46,11 @@ public:
> >                      ControlList &metadata) override;
> >
> >  private:
> > -       void fillGainsParamBlock(MaliC55Params *params,
> > -                                IPAContext &context,
> > -                                IPAFrameContext &frameContext);
> > -       void fillConfigParamBlock(MaliC55Params *params);
> > +       void configAwbMeas(MaliC55Params *params);
> > +       MaliC55AwbStats calculateRgbMeans(const IPAFrameContext &frameContext,
> > +                                         const mali_c55_stats_buffer *stats) const;
> > +
> > +       AwbAlgorithm<UQ<4, 8>> awbAlgo_;
> >  };
> >
> >  } /* namespace ipa::mali_c55::algorithms */
> > diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
> > index 1b203e2b2605..a1e4c39f3870 100644
> > --- a/src/ipa/mali-c55/ipa_context.cpp
> > +++ b/src/ipa/mali-c55/ipa_context.cpp
> > @@ -96,6 +96,12 @@ namespace libcamera::ipa::mali_c55 {
> >   *
> >   * \var IPAContext::frameContexts
> >   * \brief Ring buffer of per-frame contexts
> > + *
> > + * \var IPAContext::ctrlMap
> > + * \brief A ControlInfoMap::Map of controls populated by the algorithms
> > + *
> > + * \var IPAContext::camHelper
> > + * \brief The camera sensor helper
> >   */
> >
> >  } /* namespace libcamera::ipa::mali_c55 */
> > diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> > index ac4b83773803..3d884ea17eb8 100644
> > --- a/src/ipa/mali-c55/ipa_context.h
> > +++ b/src/ipa/mali-c55/ipa_context.h
> > @@ -12,8 +12,10 @@
> >
> >  #include "libcamera/internal/bayer_format.h"
> >
> > +#include <libipa/camera_sensor_helper.h>
> >  #include <libipa/fc_queue.h>
> >
> > +#include "libipa/awb.h"
> >  #include "libipa/fixedpoint.h"
> >
> >  namespace libcamera {
> > @@ -29,6 +31,8 @@ struct IPASessionConfiguration {
> >                 double maxAnalogueGain;
> >         } agc;
> >
> > +       ipa::awb::Session awb;
> > +
> >         struct {
> >                 BayerFormat::Order bayerOrder;
> >                 utils::Duration lineDuration;
> > @@ -54,10 +58,7 @@ struct IPAActiveState {
> >                 uint32_t temperatureK;
> >         } agc;
> >
> > -       struct {
> > -               UQ<4, 8> rGain;
> > -               UQ<4, 8> bGain;
> > -       } awb;
> > +       ipa::awb::ActiveState awb;
> >  };
> >
> >  struct IPAFrameContext : public FrameContext {
> > @@ -67,10 +68,7 @@ struct IPAFrameContext : public FrameContext {
> >                 UQ<5, 8> ispGain;
> >         } agc;
> >
> > -       struct {
> > -               UQ<4, 8> rGain;
> > -               UQ<4, 8> bGain;
> > -       } awb;
> > +       ipa::awb::FrameContext awb;
> >  };
> >
> >  struct IPAContext {
> > @@ -85,6 +83,9 @@ struct IPAContext {
> >         FCQueue<IPAFrameContext> frameContexts;
> >
> >         ControlInfoMap::Map ctrlMap;
> > +
> > +       /* Interface to the Camera Helper */
> > +       std::unique_ptr<CameraSensorHelper> camHelper;
> >  };
> >
> >  } /* namespace ipa::mali_c55 */
> > diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
> > index 1d3af0627fdb..c35cc10bea01 100644
> > --- a/src/ipa/mali-c55/mali-c55.cpp
> > +++ b/src/ipa/mali-c55/mali-c55.cpp
> > @@ -77,9 +77,6 @@ private:
> >
> >         ControlInfoMap sensorControls_;
> >
> > -       /* Interface to the Camera Helper */
> > -       std::unique_ptr<CameraSensorHelper> camHelper_;
> > -
> >         /* Local parameter storage */
> >         struct IPAContext context_;
> >  };
> > @@ -101,8 +98,8 @@ std::string IPAMaliC55::logPrefix() const
> >  int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
> >                      ControlInfoMap *ipaControls)
> >  {
> > -       camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> > -       if (!camHelper_) {
> > +       context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> > +       if (!context_.camHelper) {
>
> I had this split out during:
>  - https://patchwork.libcamera.org/project/libcamera/list/?series=5486&state=*
>
>  - https://patchwork.libcamera.org/patch/24590/
>
> Which already had your review by tag too ;-)
> But I guess it's fine in here.
>

I might have just squashed that patch in ?
To be honest I don't remember..

>
> >                 LOG(IPAMaliC55, Error)
> >                         << "Failed to create camera sensor helper for "
> >                         << settings.sensorModel;
> > @@ -145,10 +142,10 @@ void IPAMaliC55::setControls()
> >
> >         if (activeState.agc.autoEnabled) {
> >                 exposure = activeState.agc.automatic.exposure;
> > -               gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
> > +               gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
> >         } else {
> >                 exposure = activeState.agc.manual.exposure;
> > -               gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
> > +               gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
> >         }
> >
> >         ControlList ctrls(sensorControls_);
> > @@ -194,17 +191,17 @@ void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
> >         context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
> >         context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
> >         context_.configuration.agc.defaultExposure = defExposure;
> > -       context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> > -       context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> > +       context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
> > +       context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
> >
> > -       if (camHelper_->blackLevel().has_value()) {
> > +       if (context_.camHelper->blackLevel().has_value()) {
>
> I guess we should rebase https://patchwork.libcamera.org/patch/24591/
> too.
>
> But that can be on top of this series now.
>
>
> My only thoughts are really that we are doing a lot to convert doubles
> and floats and it all feels a bit unnecessary to work on a value that
> will be clamped to a UQ4,8 (12 bit preceision) while float has an
> equivalent of 24 bit precision, and double gives 53.

Yes, it feels wrong. However I thought that if double were used there
might have been a reason, but maybe there was not ?

>
> And perhaps the patch itself could be split up - but I can look the
> otherway on either case so:

To split out the moving of the camHelper_ ? Is it worth it ??

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

Thanks
  j

> >                 /*
> >                  * The black level from CameraSensorHelper is a 16-bit value.
> >                  * The Mali-C55 ISP expects 20-bit settings, so we shift it to
> >                  * the appropriate width
> >                  */
> >                 context_.configuration.sensor.blackLevel =
> > -                       camHelper_->blackLevel().value() << 4;
> > +                       context_.camHelper->blackLevel().value() << 4;
> >         }
> >  }
> >
> > @@ -255,9 +252,9 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
> >
> >         /* Compute the analogue gain limits. */
> >         const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
> > -       float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
> > -       float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
> > -       float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
> > +       float minGain = context_.camHelper->gain(v4l2Gain.min().get<int32_t>());
> > +       float maxGain = context_.camHelper->gain(v4l2Gain.max().get<int32_t>());
> > +       float defGain = context_.camHelper->gain(v4l2Gain.def().get<int32_t>());
> >         ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
> >
> >         /*
> > @@ -355,7 +352,7 @@ void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
> >         frameContext.agc.exposure =
> >                 sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> >         frameContext.agc.sensorGain =
> > -               camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> > +               context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> >
> >         ControlList metadata(controls::controls);
> >
> >
> > --
> > 2.54.0
> >
Kieran Bingham June 22, 2026, 6:03 p.m. UTC | #3 
Quoting Jacopo Mondi (2026-06-16 13:29:51)
> Hi Kieran
> 
> On Tue, Jun 16, 2026 at 12:55:32PM +0100, Kieran Bingham wrote:
> > Quoting Jacopo Mondi (2026-06-15 15:05:27)
> > > Port the Mali-C55 Awb algorithm to use the new libipa implementation
> > > of AwbAlgorithm.
> > >
> > > The awbAlgo_ class member is initialized with the Q<4, 8> type that
> > > represents the register format and the MaliC55AwbStats type handles
> > > the WB statistics format as produced by the Mali-C55 which already
> > > calculates the mean values as R/G and B/G values.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---

<snip>

> > >
> > > -void Awb::fillConfigParamBlock(MaliC55Params *params)
> > > +void Awb::configAwbMeas(MaliC55Params *params)
> >
> > Is 'Meas' correct ? Perhaps configureAwbMeans()
> >
> 
> It's intentional for "Measurement". Should I change it ?

It feels a bit unclear.

Does it even need to change from fillConfigParamBlock?

 
> > >  {
> > >         auto block = params->block<MaliC55Blocks::AwbConfig>();
> > >
> > > @@ -117,56 +119,77 @@ void Awb::fillConfigParamBlock(MaliC55Params *params)
> > >         block->cb_low = 64;
> > >  }
> > >
> > > +/**
> > > + * \copydoc libcamera::ipa::Algorithm::prepare
> > > + */
> > >  void Awb::prepare(IPAContext &context, const uint32_t frame,
> > >                   IPAFrameContext &frameContext, MaliC55Params *params)
> > >  {
> > > -       fillGainsParamBlock(params, context, frameContext);
> > > +       awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
> > > +
> > > +       /*
> > > +        * The gains here map as follows:
> > > +        *      gain00 = R
> > > +        *      gain01 = Gr
> > > +        *      gain10 = Gb
> > > +        *      gain11 = B
> > > +        *
> > > +        * This holds true regardless of the bayer order of the input data, as
> > > +        * the mapping is done internally in the ISP.
> > > +        */
> > > +       auto block = params->block<MaliC55Blocks::AwbGains>();
> > > +       block.setEnabled(true);
> > > +
> > > +       block->gain00 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.r()))
> > > +                               .quantized();
> >
> > I really wonder if we should just use float in the libipa AWB
> > implementation. I can't see why we're using double there ...
> >
> 
> I don't know, I think that's what we had already, didn't we ?

Well maybe that's a cleanup on top to reduce some casting after we get a
fresh baseline.

<snip>

> > > @@ -101,8 +98,8 @@ std::string IPAMaliC55::logPrefix() const
> > >  int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
> > >                      ControlInfoMap *ipaControls)
> > >  {
> > > -       camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> > > -       if (!camHelper_) {
> > > +       context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> > > +       if (!context_.camHelper) {
> >
> > I had this split out during:
> >  - https://patchwork.libcamera.org/project/libcamera/list/?series=5486&state=*
> >
> >  - https://patchwork.libcamera.org/patch/24590/
> >
> > Which already had your review by tag too ;-)
> > But I guess it's fine in here.
> >
> 
> I might have just squashed that patch in ?
> To be honest I don't remember..
> 
> >
> > >                 LOG(IPAMaliC55, Error)
> > >                         << "Failed to create camera sensor helper for "
> > >                         << settings.sensorModel;
> > > @@ -145,10 +142,10 @@ void IPAMaliC55::setControls()
> > >
> > >         if (activeState.agc.autoEnabled) {
> > >                 exposure = activeState.agc.automatic.exposure;
> > > -               gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
> > > +               gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
> > >         } else {
> > >                 exposure = activeState.agc.manual.exposure;
> > > -               gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
> > > +               gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
> > >         }
> > >
> > >         ControlList ctrls(sensorControls_);
> > > @@ -194,17 +191,17 @@ void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
> > >         context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
> > >         context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
> > >         context_.configuration.agc.defaultExposure = defExposure;
> > > -       context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> > > -       context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> > > +       context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
> > > +       context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
> > >
> > > -       if (camHelper_->blackLevel().has_value()) {
> > > +       if (context_.camHelper->blackLevel().has_value()) {
> >
> > I guess we should rebase https://patchwork.libcamera.org/patch/24591/
> > too.
> >
> > But that can be on top of this series now.
> >
> >
> > My only thoughts are really that we are doing a lot to convert doubles
> > and floats and it all feels a bit unnecessary to work on a value that
> > will be clamped to a UQ4,8 (12 bit preceision) while float has an
> > equivalent of 24 bit precision, and double gives 53.
> 
> Yes, it feels wrong. However I thought that if double were used there
> might have been a reason, but maybe there was not ?
> 
> >
> > And perhaps the patch itself could be split up - but I can look the
> > otherway on either case so:
> 
> To split out the moving of the camHelper_ ? Is it worth it ??

No you don't have to for me - it was just confusing when I read through
this first.


> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >
> 
> Thanks
>   j
>
Dan Scally June 22, 2026, 8:25 p.m. UTC | #4 
Hi Jacopo

On 15/06/2026 15:05, Jacopo Mondi wrote:
> Port the Mali-C55 Awb algorithm to use the new libipa implementation
> of AwbAlgorithm.
> 
> The awbAlgo_ class member is initialized with the Q<4, 8> type that
> represents the register format and the MaliC55AwbStats type handles
> the WB statistics format as produced by the Mali-C55 which already
> calculates the mean values as R/G and B/G values.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---

I have basically the exact same comments that Kieran already said, so I won't bother repeating them:

Reviewed-and-Tested-by: Daniel Scally <dan.scally@ideasonboard.com>

>   src/ipa/mali-c55/algorithms/awb.cpp | 198 ++++++++++++++++++------------------
>   src/ipa/mali-c55/algorithms/awb.h   |  30 ++++--
>   src/ipa/mali-c55/ipa_context.cpp    |   6 ++
>   src/ipa/mali-c55/ipa_context.h      |  17 ++--
>   src/ipa/mali-c55/mali-c55.cpp       |  27 +++--
>   5 files changed, 152 insertions(+), 126 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
> index 8a671b52be59..97964f395325 100644
> --- a/src/ipa/mali-c55/algorithms/awb.cpp
> +++ b/src/ipa/mali-c55/algorithms/awb.cpp
> @@ -2,7 +2,7 @@
>   /*
>    * Copyright (C) 2024, Ideas On Board Oy
>    *
> - * Mali C55 grey world auto white balance algorithm
> + * Mali C55 auto white balance algorithm
>    */
>   
>   #include "awb.h"
> @@ -14,63 +14,65 @@
>   
>   #include <libcamera/control_ids.h>
>   
> -#include "libipa/fixedpoint.h"
> -
>   namespace libcamera {
>   
>   namespace ipa::mali_c55::algorithms {
>   
>   LOG_DEFINE_CATEGORY(MaliC55Awb)
>   
> -/* Number of frames at which we should run AWB at full speed */
> -static constexpr uint32_t kNumStartupFrames = 4;
> +/* \todo Mali-C55 doesn't support the Lux algorithm. */
> +static constexpr unsigned int kDefaultLux = 500;
> +
> +class MaliC55AwbStats final : public AwbStats
> +{
> +public:
> +	MaliC55AwbStats() = default;
> +	MaliC55AwbStats(const RGB<double> &rgbMeans)
> +		: AwbStats(rgbMeans)
> +	{
> +		/* The Mali-C55 ISP already provides stats as R/G and B/G ratios. */
> +		rg_ = rgbMeans_.r();
> +		bg_ = rgbMeans_.b();
> +	}
>   
> -Awb::Awb()
> +	/* Minimum mean value below which AWB can't operate. */
> +	double minColourValue() const override
> +	{
> +		return 0.2;
> +	}
> +};
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Awb::init(IPAContext &context, const ValueNode &tuningData)
>   {
> +	return awbAlgo_.init(tuningData, context.ctrlMap);
>   }
>   
> -int Awb::configure([[maybe_unused]] IPAContext &context,
> +/**
> + * \copydoc libcamera::ipa::Algorithm::configure
> + */
> +int Awb::configure(IPAContext &context,
>   		   [[maybe_unused]] const IPACameraSensorInfo &configInfo)
>   {
> -	/*
> -	 * Initially we have no idea what the colour balance will be like, so
> -	 * for the first frame we will make no assumptions and leave the R/B
> -	 * channels unmodified.
> -	 */
> -	context.activeState.awb.rGain = 1.0f;
> -	context.activeState.awb.bGain = 1.0f;
> -
> -	return 0;
> +	return awbAlgo_.configure(context.activeState.awb,
> +				  context.configuration.awb);
>   }
>   
> -void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
> -				IPAFrameContext &frameContext)
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Awb::queueRequest(IPAContext &context,
> +		       const uint32_t frame,
> +		       IPAFrameContext &frameContext,
> +		       const ControlList &controls)
>   {
> -	UQ<4, 8> rGain = context.activeState.awb.rGain;
> -	UQ<4, 8> bGain = context.activeState.awb.bGain;
> -
> -	/*
> -	 * The gains here map as follows:
> -	 *	gain00 = R
> -	 *	gain01 = Gr
> -	 *	gain10 = Gb
> -	 *	gain11 = B
> -	 *
> -	 * This holds true regardless of the bayer order of the input data, as
> -	 * the mapping is done internally in the ISP.
> -	 */
> -	auto block = params->block<MaliC55Blocks::AwbGains>();
> -
> -	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;
> +	awbAlgo_.queueRequest(context.activeState.awb, frame, frameContext.awb,
> +			      controls);
>   }
>   
> -void Awb::fillConfigParamBlock(MaliC55Params *params)
> +void Awb::configAwbMeas(MaliC55Params *params)
>   {
>   	auto block = params->block<MaliC55Blocks::AwbConfig>();
>   
> @@ -117,56 +119,77 @@ void Awb::fillConfigParamBlock(MaliC55Params *params)
>   	block->cb_low = 64;
>   }
>   
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
>   void Awb::prepare(IPAContext &context, const uint32_t frame,
>   		  IPAFrameContext &frameContext, MaliC55Params *params)
>   {
> -	fillGainsParamBlock(params, context, frameContext);
> +	awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
> +
> +	/*
> +	 * The gains here map as follows:
> +	 *	gain00 = R
> +	 *	gain01 = Gr
> +	 *	gain10 = Gb
> +	 *	gain11 = B
> +	 *
> +	 * This holds true regardless of the bayer order of the input data, as
> +	 * the mapping is done internally in the ISP.
> +	 */
> +	auto block = params->block<MaliC55Blocks::AwbGains>();
> +	block.setEnabled(true);
> +
> +	block->gain00 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.r()))
> +				.quantized();
> +	block->gain01 = UQ<4, 8>(1.0f).quantized();
> +	block->gain10 = UQ<4, 8>(1.0f).quantized();
> +	block->gain11 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.b()))
> +				.quantized();
>   
>   	if (frame > 0)
>   		return;
>   
> -	fillConfigParamBlock(params);
> +	configAwbMeas(params);
>   }
>   
> -void Awb::process(IPAContext &context, const uint32_t frame,
> -		  IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
> -		  [[maybe_unused]] ControlList &metadata)
> +MaliC55AwbStats Awb::calculateRgbMeans(const IPAFrameContext &frameContext,
> +				       const mali_c55_stats_buffer *stats) const
>   {
> -	const struct mali_c55_awb_average_ratios *awb_ratios = stats->awb_ratios;
> +	const struct mali_c55_awb_average_ratios *awb = stats->awb_ratios;
>   
>   	/*
>   	 * The ISP produces average R:G and B:G ratios for zones. We take the
> -	 * average of all the zones with data and simply invert them to provide
> -	 * gain figures that we can apply to approximate a grey world.
> +	 * average of all the zones with data and calculate the mean values.
>   	 */
> -	unsigned int counted_zones = 0;
> -	float rgSum = 0, bgSum = 0;
> +	unsigned int active_zones = 0;
> +	double rgSum = 0, bgSum = 0;
>   
>   	for (unsigned int i = 0; i < 225; i++) {
> -		if (!awb_ratios[i].num_pixels)
> +		if (!awb[i].num_pixels)
>   			continue;
>   
>   		/*
> -		 * The statistics are in Q4.8 format, so we convert to float
> +		 * The statistics are in Q4.8 format, so we convert to double
>   		 * here.
>   		 */
> -		rgSum += UQ<4, 8>(awb_ratios[i].avg_rg_gr).value();
> -		bgSum += UQ<4, 8>(awb_ratios[i].avg_bg_br).value();
> -		counted_zones++;
> +		rgSum += UQ<4, 8>(awb[i].avg_rg_gr).value();
> +		bgSum += UQ<4, 8>(awb[i].avg_bg_br).value();
> +		active_zones++;
>   	}
>   
>   	/*
>   	 * Sometimes the first frame's statistics have no valid pixels, in which
>   	 * case we'll just assume a grey world until they say otherwise.
>   	 */
> -	float rgAvg, bgAvg;
> -	if (!counted_zones) {
> -		rgAvg = 1.0;
> -		bgAvg = 1.0;
> -	} else {
> -		rgAvg = rgSum / counted_zones;
> -		bgAvg = bgSum / counted_zones;
> -	}
> +	if (!active_zones)
> +		return {};
> +
> +	RGB<double> rgbMeans = { {
> +		rgSum / active_zones,
> +		1.0,
> +		bgSum / active_zones,
> +	} };
>   
>   	/*
>   	 * The statistics are generated _after_ white balancing is performed in
> @@ -174,41 +197,22 @@ 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.value();
> -	float bRatio = bgAvg / frameContext.awb.bGain.value();
> +	rgbMeans /= frameContext.awb.gains.max(0.01);
>   
> -	/*
> -	 * And then we can simply invert the ratio to find the gain we should
> -	 * apply.
> -	 */
> -	float rGain = 1 / rRatio;
> -	float bGain = 1 / bRatio;
> +	return MaliC55AwbStats(rgbMeans);
> +}
>   
> -	/*
> -	 * Running at full speed, this algorithm results in oscillations in the
> -	 * colour balance. To remove those we dampen the speed at which it makes
> -	 * changes in gain, unless we're in the startup phase in which case we
> -	 * want to fix the miscolouring as quickly as possible.
> -	 */
> -	float speed = frame < kNumStartupFrames ? 1.0f : 0.2f;
> -	rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0f - speed);
> -	bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0f - speed);
> -
> -	context.activeState.awb.rGain = rGain;
> -	context.activeState.awb.bGain = bGain;
> -
> -	metadata.set(controls::ColourGains, {
> -		frameContext.awb.rGain.value(),
> -		frameContext.awb.bGain.value(),
> -	});
> -
> -	LOG(MaliC55Awb, Debug) << "For frame number " << frame << ": "
> -		<< "Average R/G Ratio: " << rgAvg
> -		<< ", Average B/G Ratio: " << bgAvg
> -		<< "\nrGain applied to this frame: " << frameContext.awb.rGain
> -		<< ", bGain applied to this frame: " << frameContext.awb.bGain
> -		<< "\nrGain to apply: " << context.activeState.awb.rGain
> -		<< ", bGain to apply: " << context.activeState.awb.bGain;
> +/**
> + * \copydoc libcamera::ipa::Algorithm::process
> + */
> +void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +		  IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
> +		  ControlList &metadata)
> +{
> +	MaliC55AwbStats awbStats = calculateRgbMeans(frameContext, stats);
> +
> +	awbAlgo_.process(context.activeState.awb, frameContext.awb, awbStats,
> +			 kDefaultLux, metadata);
>   }
>   
>   REGISTER_IPA_ALGORITHM(Awb, "Awb")
> diff --git a/src/ipa/mali-c55/algorithms/awb.h b/src/ipa/mali-c55/algorithms/awb.h
> index 683a62af263a..cc7ef583eafc 100644
> --- a/src/ipa/mali-c55/algorithms/awb.h
> +++ b/src/ipa/mali-c55/algorithms/awb.h
> @@ -2,24 +2,41 @@
>   /*
>    * Copyright (C) 2024, Ideas on Board Oy
>    *
> - * Mali C55 grey world auto white balance algorithm
> + * Mali C55 auto white balance algorithm
>    */
>   
> +#pragma once
> +
> +#include <linux/media/arm/mali-c55-config.h>
> +
> +#include <libcamera/controls.h>
> +
> +#include "libcamera/internal/value_node.h"
> +
> +#include "libipa/awb.h"
> +#include "libipa/fixedpoint.h"
> +
>   #include "algorithm.h"
>   #include "ipa_context.h"
> +#include "params.h"
>   
>   namespace libcamera {
>   
>   namespace ipa::mali_c55::algorithms {
>   
> +class MaliC55AwbStats;
> +
>   class Awb : public Algorithm
>   {
>   public:
> -	Awb();
>   	~Awb() = default;
>   
> +	int init(IPAContext &context, const ValueNode &tuningData) override;
>   	int configure(IPAContext &context,
>   		      const IPACameraSensorInfo &configInfo) override;
> +	void queueRequest(IPAContext &context, const uint32_t frame,
> +			  IPAFrameContext &frameContext,
> +			  const ControlList &controls) override;
>   	void prepare(IPAContext &context, const uint32_t frame,
>   		     IPAFrameContext &frameContext,
>   		     MaliC55Params *params) override;
> @@ -29,10 +46,11 @@ public:
>   		     ControlList &metadata) override;
>   
>   private:
> -	void fillGainsParamBlock(MaliC55Params *params,
> -				 IPAContext &context,
> -				 IPAFrameContext &frameContext);
> -	void fillConfigParamBlock(MaliC55Params *params);
> +	void configAwbMeas(MaliC55Params *params);
> +	MaliC55AwbStats calculateRgbMeans(const IPAFrameContext &frameContext,
> +					  const mali_c55_stats_buffer *stats) const;
> +
> +	AwbAlgorithm<UQ<4, 8>> awbAlgo_;
>   };
>   
>   } /* namespace ipa::mali_c55::algorithms */
> diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
> index 1b203e2b2605..a1e4c39f3870 100644
> --- a/src/ipa/mali-c55/ipa_context.cpp
> +++ b/src/ipa/mali-c55/ipa_context.cpp
> @@ -96,6 +96,12 @@ namespace libcamera::ipa::mali_c55 {
>    *
>    * \var IPAContext::frameContexts
>    * \brief Ring buffer of per-frame contexts
> + *
> + * \var IPAContext::ctrlMap
> + * \brief A ControlInfoMap::Map of controls populated by the algorithms
> + *
> + * \var IPAContext::camHelper
> + * \brief The camera sensor helper
>    */
>   
>   } /* namespace libcamera::ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> index ac4b83773803..3d884ea17eb8 100644
> --- a/src/ipa/mali-c55/ipa_context.h
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -12,8 +12,10 @@
>   
>   #include "libcamera/internal/bayer_format.h"
>   
> +#include <libipa/camera_sensor_helper.h>
>   #include <libipa/fc_queue.h>
>   
> +#include "libipa/awb.h"
>   #include "libipa/fixedpoint.h"
>   
>   namespace libcamera {
> @@ -29,6 +31,8 @@ struct IPASessionConfiguration {
>   		double maxAnalogueGain;
>   	} agc;
>   
> +	ipa::awb::Session awb;
> +
>   	struct {
>   		BayerFormat::Order bayerOrder;
>   		utils::Duration lineDuration;
> @@ -54,10 +58,7 @@ struct IPAActiveState {
>   		uint32_t temperatureK;
>   	} agc;
>   
> -	struct {
> -		UQ<4, 8> rGain;
> -		UQ<4, 8> bGain;
> -	} awb;
> +	ipa::awb::ActiveState awb;
>   };
>   
>   struct IPAFrameContext : public FrameContext {
> @@ -67,10 +68,7 @@ struct IPAFrameContext : public FrameContext {
>   		UQ<5, 8> ispGain;
>   	} agc;
>   
> -	struct {
> -		UQ<4, 8> rGain;
> -		UQ<4, 8> bGain;
> -	} awb;
> +	ipa::awb::FrameContext awb;
>   };
>   
>   struct IPAContext {
> @@ -85,6 +83,9 @@ struct IPAContext {
>   	FCQueue<IPAFrameContext> frameContexts;
>   
>   	ControlInfoMap::Map ctrlMap;
> +
> +	/* Interface to the Camera Helper */
> +	std::unique_ptr<CameraSensorHelper> camHelper;
>   };
>   
>   } /* namespace ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
> index 1d3af0627fdb..c35cc10bea01 100644
> --- a/src/ipa/mali-c55/mali-c55.cpp
> +++ b/src/ipa/mali-c55/mali-c55.cpp
> @@ -77,9 +77,6 @@ private:
>   
>   	ControlInfoMap sensorControls_;
>   
> -	/* Interface to the Camera Helper */
> -	std::unique_ptr<CameraSensorHelper> camHelper_;
> -
>   	/* Local parameter storage */
>   	struct IPAContext context_;
>   };
> @@ -101,8 +98,8 @@ std::string IPAMaliC55::logPrefix() const
>   int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
>   		     ControlInfoMap *ipaControls)
>   {
> -	camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> -	if (!camHelper_) {
> +	context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> +	if (!context_.camHelper) {
>   		LOG(IPAMaliC55, Error)
>   			<< "Failed to create camera sensor helper for "
>   			<< settings.sensorModel;
> @@ -145,10 +142,10 @@ void IPAMaliC55::setControls()
>   
>   	if (activeState.agc.autoEnabled) {
>   		exposure = activeState.agc.automatic.exposure;
> -		gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
> +		gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
>   	} else {
>   		exposure = activeState.agc.manual.exposure;
> -		gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
> +		gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
>   	}
>   
>   	ControlList ctrls(sensorControls_);
> @@ -194,17 +191,17 @@ void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
>   	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
>   	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
>   	context_.configuration.agc.defaultExposure = defExposure;
> -	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> -	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> +	context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
> +	context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
>   
> -	if (camHelper_->blackLevel().has_value()) {
> +	if (context_.camHelper->blackLevel().has_value()) {
>   		/*
>   		 * The black level from CameraSensorHelper is a 16-bit value.
>   		 * The Mali-C55 ISP expects 20-bit settings, so we shift it to
>   		 * the appropriate width
>   		 */
>   		context_.configuration.sensor.blackLevel =
> -			camHelper_->blackLevel().value() << 4;
> +			context_.camHelper->blackLevel().value() << 4;
>   	}
>   }
>   
> @@ -255,9 +252,9 @@ void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
>   
>   	/* Compute the analogue gain limits. */
>   	const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
> -	float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
> -	float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
> -	float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
> +	float minGain = context_.camHelper->gain(v4l2Gain.min().get<int32_t>());
> +	float maxGain = context_.camHelper->gain(v4l2Gain.max().get<int32_t>());
> +	float defGain = context_.camHelper->gain(v4l2Gain.def().get<int32_t>());
>   	ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
>   
>   	/*
> @@ -355,7 +352,7 @@ void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
>   	frameContext.agc.exposure =
>   		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>   	frameContext.agc.sensorGain =
> -		camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +		context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>   
>   	ControlList metadata(controls::controls);
>   
>
Jacopo Mondi June 23, 2026, 6:36 a.m. UTC | #5 
Hi Kieran

On Mon, Jun 22, 2026 at 07:03:07PM +0100, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2026-06-16 13:29:51)
> > Hi Kieran
> >
> > On Tue, Jun 16, 2026 at 12:55:32PM +0100, Kieran Bingham wrote:
> > > Quoting Jacopo Mondi (2026-06-15 15:05:27)
> > > > Port the Mali-C55 Awb algorithm to use the new libipa implementation
> > > > of AwbAlgorithm.
> > > >
> > > > The awbAlgo_ class member is initialized with the Q<4, 8> type that
> > > > represents the register format and the MaliC55AwbStats type handles
> > > > the WB statistics format as produced by the Mali-C55 which already
> > > > calculates the mean values as R/G and B/G values.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > > ---
>
> <snip>
>
> > > >
> > > > -void Awb::fillConfigParamBlock(MaliC55Params *params)
> > > > +void Awb::configAwbMeas(MaliC55Params *params)
> > >
> > > Is 'Meas' correct ? Perhaps configureAwbMeans()
> > >
> >
> > It's intentional for "Measurement". Should I change it ?
>
> It feels a bit unclear.
>
> Does it even need to change from fillConfigParamBlock?
>

As per lsc, I found the existing name unnecessarily long and not
clear.

However, it's arbitrary, so I can backtrack


>
> > > >  {
> > > >         auto block = params->block<MaliC55Blocks::AwbConfig>();
> > > >
> > > > @@ -117,56 +119,77 @@ void Awb::fillConfigParamBlock(MaliC55Params *params)
> > > >         block->cb_low = 64;
> > > >  }
> > > >
> > > > +/**
> > > > + * \copydoc libcamera::ipa::Algorithm::prepare
> > > > + */
> > > >  void Awb::prepare(IPAContext &context, const uint32_t frame,
> > > >                   IPAFrameContext &frameContext, MaliC55Params *params)
> > > >  {
> > > > -       fillGainsParamBlock(params, context, frameContext);
> > > > +       awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
> > > > +
> > > > +       /*
> > > > +        * The gains here map as follows:
> > > > +        *      gain00 = R
> > > > +        *      gain01 = Gr
> > > > +        *      gain10 = Gb
> > > > +        *      gain11 = B
> > > > +        *
> > > > +        * This holds true regardless of the bayer order of the input data, as
> > > > +        * the mapping is done internally in the ISP.
> > > > +        */
> > > > +       auto block = params->block<MaliC55Blocks::AwbGains>();
> > > > +       block.setEnabled(true);
> > > > +
> > > > +       block->gain00 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.r()))
> > > > +                               .quantized();
> > >
> > > I really wonder if we should just use float in the libipa AWB
> > > implementation. I can't see why we're using double there ...
> > >
> >
> > I don't know, I think that's what we had already, didn't we ?
>
> Well maybe that's a cleanup on top to reduce some casting after we get a
> fresh baseline.
>
> <snip>
>
> > > > @@ -101,8 +98,8 @@ std::string IPAMaliC55::logPrefix() const
> > > >  int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
> > > >                      ControlInfoMap *ipaControls)
> > > >  {
> > > > -       camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> > > > -       if (!camHelper_) {
> > > > +       context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> > > > +       if (!context_.camHelper) {
> > >
> > > I had this split out during:
> > >  - https://patchwork.libcamera.org/project/libcamera/list/?series=5486&state=*
> > >
> > >  - https://patchwork.libcamera.org/patch/24590/
> > >
> > > Which already had your review by tag too ;-)
> > > But I guess it's fine in here.
> > >
> >
> > I might have just squashed that patch in ?
> > To be honest I don't remember..
> >
> > >
> > > >                 LOG(IPAMaliC55, Error)
> > > >                         << "Failed to create camera sensor helper for "
> > > >                         << settings.sensorModel;
> > > > @@ -145,10 +142,10 @@ void IPAMaliC55::setControls()
> > > >
> > > >         if (activeState.agc.autoEnabled) {
> > > >                 exposure = activeState.agc.automatic.exposure;
> > > > -               gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
> > > > +               gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
> > > >         } else {
> > > >                 exposure = activeState.agc.manual.exposure;
> > > > -               gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
> > > > +               gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
> > > >         }
> > > >
> > > >         ControlList ctrls(sensorControls_);
> > > > @@ -194,17 +191,17 @@ void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
> > > >         context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
> > > >         context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
> > > >         context_.configuration.agc.defaultExposure = defExposure;
> > > > -       context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
> > > > -       context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> > > > +       context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
> > > > +       context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
> > > >
> > > > -       if (camHelper_->blackLevel().has_value()) {
> > > > +       if (context_.camHelper->blackLevel().has_value()) {
> > >
> > > I guess we should rebase https://patchwork.libcamera.org/patch/24591/
> > > too.
> > >
> > > But that can be on top of this series now.
> > >
> > >
> > > My only thoughts are really that we are doing a lot to convert doubles
> > > and floats and it all feels a bit unnecessary to work on a value that
> > > will be clamped to a UQ4,8 (12 bit preceision) while float has an
> > > equivalent of 24 bit precision, and double gives 53.
> >
> > Yes, it feels wrong. However I thought that if double were used there
> > might have been a reason, but maybe there was not ?
> >
> > >
> > > And perhaps the patch itself could be split up - but I can look the
> > > otherway on either case so:
> >
> > To split out the moving of the camHelper_ ? Is it worth it ??
>
> No you don't have to for me - it was just confusing when I read through
> this first.
>
>
> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > >
> >
> > Thanks
> >   j
> >

Patch
diff mbox series 

diff --git a/src/ipa/mali-c55/algorithms/awb.cpp b/src/ipa/mali-c55/algorithms/awb.cpp
index 8a671b52be59..97964f395325 100644
--- a/src/ipa/mali-c55/algorithms/awb.cpp
+++ b/src/ipa/mali-c55/algorithms/awb.cpp
@@ -2,7 +2,7 @@ 
 /*
  * Copyright (C) 2024, Ideas On Board Oy
  *
- * Mali C55 grey world auto white balance algorithm
+ * Mali C55 auto white balance algorithm
  */
 
 #include "awb.h"
@@ -14,63 +14,65 @@ 
 
 #include <libcamera/control_ids.h>
 
-#include "libipa/fixedpoint.h"
-
 namespace libcamera {
 
 namespace ipa::mali_c55::algorithms {
 
 LOG_DEFINE_CATEGORY(MaliC55Awb)
 
-/* Number of frames at which we should run AWB at full speed */
-static constexpr uint32_t kNumStartupFrames = 4;
+/* \todo Mali-C55 doesn't support the Lux algorithm. */
+static constexpr unsigned int kDefaultLux = 500;
+
+class MaliC55AwbStats final : public AwbStats
+{
+public:
+	MaliC55AwbStats() = default;
+	MaliC55AwbStats(const RGB<double> &rgbMeans)
+		: AwbStats(rgbMeans)
+	{
+		/* The Mali-C55 ISP already provides stats as R/G and B/G ratios. */
+		rg_ = rgbMeans_.r();
+		bg_ = rgbMeans_.b();
+	}
 
-Awb::Awb()
+	/* Minimum mean value below which AWB can't operate. */
+	double minColourValue() const override
+	{
+		return 0.2;
+	}
+};
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::init
+ */
+int Awb::init(IPAContext &context, const ValueNode &tuningData)
 {
+	return awbAlgo_.init(tuningData, context.ctrlMap);
 }
 
-int Awb::configure([[maybe_unused]] IPAContext &context,
+/**
+ * \copydoc libcamera::ipa::Algorithm::configure
+ */
+int Awb::configure(IPAContext &context,
 		   [[maybe_unused]] const IPACameraSensorInfo &configInfo)
 {
-	/*
-	 * Initially we have no idea what the colour balance will be like, so
-	 * for the first frame we will make no assumptions and leave the R/B
-	 * channels unmodified.
-	 */
-	context.activeState.awb.rGain = 1.0f;
-	context.activeState.awb.bGain = 1.0f;
-
-	return 0;
+	return awbAlgo_.configure(context.activeState.awb,
+				  context.configuration.awb);
 }
 
-void Awb::fillGainsParamBlock(MaliC55Params *params, IPAContext &context,
-				IPAFrameContext &frameContext)
+/**
+ * \copydoc libcamera::ipa::Algorithm::queueRequest
+ */
+void Awb::queueRequest(IPAContext &context,
+		       const uint32_t frame,
+		       IPAFrameContext &frameContext,
+		       const ControlList &controls)
 {
-	UQ<4, 8> rGain = context.activeState.awb.rGain;
-	UQ<4, 8> bGain = context.activeState.awb.bGain;
-
-	/*
-	 * The gains here map as follows:
-	 *	gain00 = R
-	 *	gain01 = Gr
-	 *	gain10 = Gb
-	 *	gain11 = B
-	 *
-	 * This holds true regardless of the bayer order of the input data, as
-	 * the mapping is done internally in the ISP.
-	 */
-	auto block = params->block<MaliC55Blocks::AwbGains>();
-
-	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;
+	awbAlgo_.queueRequest(context.activeState.awb, frame, frameContext.awb,
+			      controls);
 }
 
-void Awb::fillConfigParamBlock(MaliC55Params *params)
+void Awb::configAwbMeas(MaliC55Params *params)
 {
 	auto block = params->block<MaliC55Blocks::AwbConfig>();
 
@@ -117,56 +119,77 @@  void Awb::fillConfigParamBlock(MaliC55Params *params)
 	block->cb_low = 64;
 }
 
+/**
+ * \copydoc libcamera::ipa::Algorithm::prepare
+ */
 void Awb::prepare(IPAContext &context, const uint32_t frame,
 		  IPAFrameContext &frameContext, MaliC55Params *params)
 {
-	fillGainsParamBlock(params, context, frameContext);
+	awbAlgo_.prepare(context.activeState.awb, frameContext.awb);
+
+	/*
+	 * The gains here map as follows:
+	 *	gain00 = R
+	 *	gain01 = Gr
+	 *	gain10 = Gb
+	 *	gain11 = B
+	 *
+	 * This holds true regardless of the bayer order of the input data, as
+	 * the mapping is done internally in the ISP.
+	 */
+	auto block = params->block<MaliC55Blocks::AwbGains>();
+	block.setEnabled(true);
+
+	block->gain00 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.r()))
+				.quantized();
+	block->gain01 = UQ<4, 8>(1.0f).quantized();
+	block->gain10 = UQ<4, 8>(1.0f).quantized();
+	block->gain11 = UQ<4, 8>(static_cast<float>(frameContext.awb.gains.b()))
+				.quantized();
 
 	if (frame > 0)
 		return;
 
-	fillConfigParamBlock(params);
+	configAwbMeas(params);
 }
 
-void Awb::process(IPAContext &context, const uint32_t frame,
-		  IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
-		  [[maybe_unused]] ControlList &metadata)
+MaliC55AwbStats Awb::calculateRgbMeans(const IPAFrameContext &frameContext,
+				       const mali_c55_stats_buffer *stats) const
 {
-	const struct mali_c55_awb_average_ratios *awb_ratios = stats->awb_ratios;
+	const struct mali_c55_awb_average_ratios *awb = stats->awb_ratios;
 
 	/*
 	 * The ISP produces average R:G and B:G ratios for zones. We take the
-	 * average of all the zones with data and simply invert them to provide
-	 * gain figures that we can apply to approximate a grey world.
+	 * average of all the zones with data and calculate the mean values.
 	 */
-	unsigned int counted_zones = 0;
-	float rgSum = 0, bgSum = 0;
+	unsigned int active_zones = 0;
+	double rgSum = 0, bgSum = 0;
 
 	for (unsigned int i = 0; i < 225; i++) {
-		if (!awb_ratios[i].num_pixels)
+		if (!awb[i].num_pixels)
 			continue;
 
 		/*
-		 * The statistics are in Q4.8 format, so we convert to float
+		 * The statistics are in Q4.8 format, so we convert to double
 		 * here.
 		 */
-		rgSum += UQ<4, 8>(awb_ratios[i].avg_rg_gr).value();
-		bgSum += UQ<4, 8>(awb_ratios[i].avg_bg_br).value();
-		counted_zones++;
+		rgSum += UQ<4, 8>(awb[i].avg_rg_gr).value();
+		bgSum += UQ<4, 8>(awb[i].avg_bg_br).value();
+		active_zones++;
 	}
 
 	/*
 	 * Sometimes the first frame's statistics have no valid pixels, in which
 	 * case we'll just assume a grey world until they say otherwise.
 	 */
-	float rgAvg, bgAvg;
-	if (!counted_zones) {
-		rgAvg = 1.0;
-		bgAvg = 1.0;
-	} else {
-		rgAvg = rgSum / counted_zones;
-		bgAvg = bgSum / counted_zones;
-	}
+	if (!active_zones)
+		return {};
+
+	RGB<double> rgbMeans = { {
+		rgSum / active_zones,
+		1.0,
+		bgSum / active_zones,
+	} };
 
 	/*
 	 * The statistics are generated _after_ white balancing is performed in
@@ -174,41 +197,22 @@  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.value();
-	float bRatio = bgAvg / frameContext.awb.bGain.value();
+	rgbMeans /= frameContext.awb.gains.max(0.01);
 
-	/*
-	 * And then we can simply invert the ratio to find the gain we should
-	 * apply.
-	 */
-	float rGain = 1 / rRatio;
-	float bGain = 1 / bRatio;
+	return MaliC55AwbStats(rgbMeans);
+}
 
-	/*
-	 * Running at full speed, this algorithm results in oscillations in the
-	 * colour balance. To remove those we dampen the speed at which it makes
-	 * changes in gain, unless we're in the startup phase in which case we
-	 * want to fix the miscolouring as quickly as possible.
-	 */
-	float speed = frame < kNumStartupFrames ? 1.0f : 0.2f;
-	rGain = speed * rGain + context.activeState.awb.rGain.value() * (1.0f - speed);
-	bGain = speed * bGain + context.activeState.awb.bGain.value() * (1.0f - speed);
-
-	context.activeState.awb.rGain = rGain;
-	context.activeState.awb.bGain = bGain;
-
-	metadata.set(controls::ColourGains, {
-		frameContext.awb.rGain.value(),
-		frameContext.awb.bGain.value(),
-	});
-
-	LOG(MaliC55Awb, Debug) << "For frame number " << frame << ": "
-		<< "Average R/G Ratio: " << rgAvg
-		<< ", Average B/G Ratio: " << bgAvg
-		<< "\nrGain applied to this frame: " << frameContext.awb.rGain
-		<< ", bGain applied to this frame: " << frameContext.awb.bGain
-		<< "\nrGain to apply: " << context.activeState.awb.rGain
-		<< ", bGain to apply: " << context.activeState.awb.bGain;
+/**
+ * \copydoc libcamera::ipa::Algorithm::process
+ */
+void Awb::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
+		  IPAFrameContext &frameContext, const mali_c55_stats_buffer *stats,
+		  ControlList &metadata)
+{
+	MaliC55AwbStats awbStats = calculateRgbMeans(frameContext, stats);
+
+	awbAlgo_.process(context.activeState.awb, frameContext.awb, awbStats,
+			 kDefaultLux, metadata);
 }
 
 REGISTER_IPA_ALGORITHM(Awb, "Awb")
diff --git a/src/ipa/mali-c55/algorithms/awb.h b/src/ipa/mali-c55/algorithms/awb.h
index 683a62af263a..cc7ef583eafc 100644
--- a/src/ipa/mali-c55/algorithms/awb.h
+++ b/src/ipa/mali-c55/algorithms/awb.h
@@ -2,24 +2,41 @@ 
 /*
  * Copyright (C) 2024, Ideas on Board Oy
  *
- * Mali C55 grey world auto white balance algorithm
+ * Mali C55 auto white balance algorithm
  */
 
+#pragma once
+
+#include <linux/media/arm/mali-c55-config.h>
+
+#include <libcamera/controls.h>
+
+#include "libcamera/internal/value_node.h"
+
+#include "libipa/awb.h"
+#include "libipa/fixedpoint.h"
+
 #include "algorithm.h"
 #include "ipa_context.h"
+#include "params.h"
 
 namespace libcamera {
 
 namespace ipa::mali_c55::algorithms {
 
+class MaliC55AwbStats;
+
 class Awb : public Algorithm
 {
 public:
-	Awb();
 	~Awb() = default;
 
+	int init(IPAContext &context, const ValueNode &tuningData) override;
 	int configure(IPAContext &context,
 		      const IPACameraSensorInfo &configInfo) override;
+	void queueRequest(IPAContext &context, const uint32_t frame,
+			  IPAFrameContext &frameContext,
+			  const ControlList &controls) override;
 	void prepare(IPAContext &context, const uint32_t frame,
 		     IPAFrameContext &frameContext,
 		     MaliC55Params *params) override;
@@ -29,10 +46,11 @@  public:
 		     ControlList &metadata) override;
 
 private:
-	void fillGainsParamBlock(MaliC55Params *params,
-				 IPAContext &context,
-				 IPAFrameContext &frameContext);
-	void fillConfigParamBlock(MaliC55Params *params);
+	void configAwbMeas(MaliC55Params *params);
+	MaliC55AwbStats calculateRgbMeans(const IPAFrameContext &frameContext,
+					  const mali_c55_stats_buffer *stats) const;
+
+	AwbAlgorithm<UQ<4, 8>> awbAlgo_;
 };
 
 } /* namespace ipa::mali_c55::algorithms */
diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
index 1b203e2b2605..a1e4c39f3870 100644
--- a/src/ipa/mali-c55/ipa_context.cpp
+++ b/src/ipa/mali-c55/ipa_context.cpp
@@ -96,6 +96,12 @@  namespace libcamera::ipa::mali_c55 {
  *
  * \var IPAContext::frameContexts
  * \brief Ring buffer of per-frame contexts
+ *
+ * \var IPAContext::ctrlMap
+ * \brief A ControlInfoMap::Map of controls populated by the algorithms
+ *
+ * \var IPAContext::camHelper
+ * \brief The camera sensor helper
  */
 
 } /* namespace libcamera::ipa::mali_c55 */
diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
index ac4b83773803..3d884ea17eb8 100644
--- a/src/ipa/mali-c55/ipa_context.h
+++ b/src/ipa/mali-c55/ipa_context.h
@@ -12,8 +12,10 @@ 
 
 #include "libcamera/internal/bayer_format.h"
 
+#include <libipa/camera_sensor_helper.h>
 #include <libipa/fc_queue.h>
 
+#include "libipa/awb.h"
 #include "libipa/fixedpoint.h"
 
 namespace libcamera {
@@ -29,6 +31,8 @@  struct IPASessionConfiguration {
 		double maxAnalogueGain;
 	} agc;
 
+	ipa::awb::Session awb;
+
 	struct {
 		BayerFormat::Order bayerOrder;
 		utils::Duration lineDuration;
@@ -54,10 +58,7 @@  struct IPAActiveState {
 		uint32_t temperatureK;
 	} agc;
 
-	struct {
-		UQ<4, 8> rGain;
-		UQ<4, 8> bGain;
-	} awb;
+	ipa::awb::ActiveState awb;
 };
 
 struct IPAFrameContext : public FrameContext {
@@ -67,10 +68,7 @@  struct IPAFrameContext : public FrameContext {
 		UQ<5, 8> ispGain;
 	} agc;
 
-	struct {
-		UQ<4, 8> rGain;
-		UQ<4, 8> bGain;
-	} awb;
+	ipa::awb::FrameContext awb;
 };
 
 struct IPAContext {
@@ -85,6 +83,9 @@  struct IPAContext {
 	FCQueue<IPAFrameContext> frameContexts;
 
 	ControlInfoMap::Map ctrlMap;
+
+	/* Interface to the Camera Helper */
+	std::unique_ptr<CameraSensorHelper> camHelper;
 };
 
 } /* namespace ipa::mali_c55 */
diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
index 1d3af0627fdb..c35cc10bea01 100644
--- a/src/ipa/mali-c55/mali-c55.cpp
+++ b/src/ipa/mali-c55/mali-c55.cpp
@@ -77,9 +77,6 @@  private:
 
 	ControlInfoMap sensorControls_;
 
-	/* Interface to the Camera Helper */
-	std::unique_ptr<CameraSensorHelper> camHelper_;
-
 	/* Local parameter storage */
 	struct IPAContext context_;
 };
@@ -101,8 +98,8 @@  std::string IPAMaliC55::logPrefix() const
 int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
 		     ControlInfoMap *ipaControls)
 {
-	camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
-	if (!camHelper_) {
+	context_.camHelper = CameraSensorHelperFactoryBase::create(settings.sensorModel);
+	if (!context_.camHelper) {
 		LOG(IPAMaliC55, Error)
 			<< "Failed to create camera sensor helper for "
 			<< settings.sensorModel;
@@ -145,10 +142,10 @@  void IPAMaliC55::setControls()
 
 	if (activeState.agc.autoEnabled) {
 		exposure = activeState.agc.automatic.exposure;
-		gain = camHelper_->gainCode(activeState.agc.automatic.sensorGain);
+		gain = context_.camHelper->gainCode(activeState.agc.automatic.sensorGain);
 	} else {
 		exposure = activeState.agc.manual.exposure;
-		gain = camHelper_->gainCode(activeState.agc.manual.sensorGain);
+		gain = context_.camHelper->gainCode(activeState.agc.manual.sensorGain);
 	}
 
 	ControlList ctrls(sensorControls_);
@@ -194,17 +191,17 @@  void IPAMaliC55::updateSessionConfiguration(const IPACameraSensorInfo &info,
 	context_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;
 	context_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;
 	context_.configuration.agc.defaultExposure = defExposure;
-	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
-	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
+	context_.configuration.agc.minAnalogueGain = context_.camHelper->gain(minGain);
+	context_.configuration.agc.maxAnalogueGain = context_.camHelper->gain(maxGain);
 
-	if (camHelper_->blackLevel().has_value()) {
+	if (context_.camHelper->blackLevel().has_value()) {
 		/*
 		 * The black level from CameraSensorHelper is a 16-bit value.
 		 * The Mali-C55 ISP expects 20-bit settings, so we shift it to
 		 * the appropriate width
 		 */
 		context_.configuration.sensor.blackLevel =
-			camHelper_->blackLevel().value() << 4;
+			context_.camHelper->blackLevel().value() << 4;
 	}
 }
 
@@ -255,9 +252,9 @@  void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
 
 	/* Compute the analogue gain limits. */
 	const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
-	float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
-	float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
-	float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
+	float minGain = context_.camHelper->gain(v4l2Gain.min().get<int32_t>());
+	float maxGain = context_.camHelper->gain(v4l2Gain.max().get<int32_t>());
+	float defGain = context_.camHelper->gain(v4l2Gain.def().get<int32_t>());
 	ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
 
 	/*
@@ -355,7 +352,7 @@  void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
 	frameContext.agc.exposure =
 		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
 	frameContext.agc.sensorGain =
-		camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
+		context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
 
 	ControlList metadata(controls::controls);