[v3,09/16] ipa: rkisp1: Refactor automatic/manual structure in IPAActiveState
diff mbox series

Message ID 20250403154925.382973-10-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • Some rkisp1 awb improvements
Related show

Commit Message

Stefan Klug April 3, 2025, 3:49 p.m. UTC 
Swap gains and automatic/manual in the IPAActiveState structure. This is
in preparation to adding another member, which is easier in the new
structure. The patch contains no functional changes.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---

Changes in v2:
- Use one named struct instead of two anonymous ones

Changes in v3:
- Collected tags
- Updated documentation
---
 src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------
 src/ipa/rkisp1/ipa_context.cpp    | 13 ++++++++-----
 src/ipa/rkisp1/ipa_context.h      | 10 ++++++----
 3 files changed, 26 insertions(+), 21 deletions(-)

Comments

Paul Elder May 7, 2025, 4:51 p.m. UTC | #1 
On Thu, Apr 03, 2025 at 05:49:14PM +0200, Stefan Klug wrote:
> Swap gains and automatic/manual in the IPAActiveState structure. This is
> in preparation to adding another member, which is easier in the new
> structure. The patch contains no functional changes.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Use one named struct instead of two anonymous ones
> 
> Changes in v3:
> - Collected tags
> - Updated documentation
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------
>  src/ipa/rkisp1/ipa_context.cpp    | 13 ++++++++-----
>  src/ipa/rkisp1/ipa_context.h      | 10 ++++++----
>  3 files changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index eafe93081bb1..a9759e53f593 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -124,8 +124,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)
>  int Awb::configure(IPAContext &context,
>  		   const IPACameraSensorInfo &configInfo)
>  {
> -	context.activeState.awb.gains.manual = RGB<double>{ 1.0 };
> -	context.activeState.awb.gains.automatic =
> +	context.activeState.awb.manual.gains = RGB<double>{ 1.0 };
> +	context.activeState.awb.automatic.gains =
>  		awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);
>  	context.activeState.awb.autoEnabled = true;
>  	context.activeState.awb.temperatureK = kDefaultColourTemperature;
> @@ -173,8 +173,8 @@ void Awb::queueRequest(IPAContext &context,
>  	const auto &colourTemperature = controls.get(controls::ColourTemperature);
>  	bool update = false;
>  	if (colourGains) {
> -		awb.gains.manual.r() = (*colourGains)[0];
> -		awb.gains.manual.b() = (*colourGains)[1];
> +		awb.manual.gains.r() = (*colourGains)[0];
> +		awb.manual.gains.b() = (*colourGains)[1];
>  		/*
>  		 * \todo Colour temperature reported in metadata is now
>  		 * incorrect, as we can't deduce the temperature from the gains.
> @@ -183,17 +183,17 @@ void Awb::queueRequest(IPAContext &context,
>  		update = true;
>  	} else if (colourTemperature) {
>  		const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);
> -		awb.gains.manual.r() = gains.r();
> -		awb.gains.manual.b() = gains.b();
> +		awb.manual.gains.r() = gains.r();
> +		awb.manual.gains.b() = gains.b();
>  		awb.temperatureK = *colourTemperature;
>  		update = true;
>  	}
>  
>  	if (update)
>  		LOG(RkISP1Awb, Debug)
> -			<< "Set colour gains to " << awb.gains.manual;
> +			<< "Set colour gains to " << awb.manual.gains;
>  
> -	frameContext.awb.gains = awb.gains.manual;
> +	frameContext.awb.gains = awb.manual.gains;
>  	frameContext.awb.temperatureK = awb.temperatureK;
>  }
>  
> @@ -208,7 +208,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,
>  	 * most up-to-date automatic values we can read.
>  	 */
>  	if (frameContext.awb.autoEnabled) {
> -		frameContext.awb.gains = context.activeState.awb.gains.automatic;
> +		frameContext.awb.gains = context.activeState.awb.automatic.gains;
>  		frameContext.awb.temperatureK = context.activeState.awb.temperatureK;
>  	}
>  
> @@ -325,14 +325,14 @@ void Awb::process(IPAContext &context,
>  	/* Filter the values to avoid oscillations. */
>  	double speed = 0.2;
>  	awbResult.gains = awbResult.gains * speed +
> -			  activeState.awb.gains.automatic * (1 - speed);
> +			  activeState.awb.automatic.gains * (1 - speed);
>  
> -	activeState.awb.gains.automatic = awbResult.gains;
> +	activeState.awb.automatic.gains = awbResult.gains;
>  
>  	LOG(RkISP1Awb, Debug)
>  		<< std::showpoint
>  		<< "Means " << rgbMeans << ", gains "
> -		<< activeState.awb.gains.automatic << ", temp "
> +		<< activeState.awb.automatic.gains << ", temp "
>  		<< activeState.awb.temperatureK << "K";
>  }
>  
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 99611bd5b390..39b97d143e95 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -191,14 +191,17 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAActiveState::awb
>   * \brief State for the Automatic White Balance algorithm
>   *
> - * \struct IPAActiveState::awb.gains
> + * \var IPAActiveState::awb::AwbState

Shouldn't this be \struct ?

> + * \brief Struct for the AWB regulation state
> + *
> + * \struct IPAActiveState::awb::AwbState.gains

And this \var ?


Paul

>   * \brief White balance gains
>   *
> - * \var IPAActiveState::awb.gains.manual
> - * \brief Manual white balance gains (set through requests)
> + * \var IPAActiveState::awb.manual
> + * \brief Manual regulation state (set through requests)
>   *
> - * \var IPAActiveState::awb.gains.automatic
> - * \brief Automatic white balance gains (computed by the algorithm)
> + * \var IPAActiveState::awb.automatic
> + * \brief Automatic regulation state (computed by the algorithm)
>   *
>   * \var IPAActiveState::awb.temperatureK
>   * \brief Estimated color temperature
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 474f7036f003..6bc922a82971 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -89,10 +89,12 @@ struct IPAActiveState {
>  	} agc;
>  
>  	struct {
> -		struct {
> -			RGB<double> manual;
> -			RGB<double> automatic;
> -		} gains;
> +		struct AwbState {
> +			RGB<double> gains;
> +		};
> +
> +		AwbState manual;
> +		AwbState automatic;
>  
>  		unsigned int temperatureK;
>  		bool autoEnabled;
> -- 
> 2.43.0
>
Stefan Klug May 20, 2025, 8 a.m. UTC | #2 
Hi Paul,

Thank you for the review.

Quoting Paul Elder (2025-05-07 18:51:17)
> On Thu, Apr 03, 2025 at 05:49:14PM +0200, Stefan Klug wrote:
> > Swap gains and automatic/manual in the IPAActiveState structure. This is
> > in preparation to adding another member, which is easier in the new
> > structure. The patch contains no functional changes.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > ---
> > 
> > Changes in v2:
> > - Use one named struct instead of two anonymous ones
> > 
> > Changes in v3:
> > - Collected tags
> > - Updated documentation
> > ---
> >  src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------
> >  src/ipa/rkisp1/ipa_context.cpp    | 13 ++++++++-----
> >  src/ipa/rkisp1/ipa_context.h      | 10 ++++++----
> >  3 files changed, 26 insertions(+), 21 deletions(-)
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > index eafe93081bb1..a9759e53f593 100644
> > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > @@ -124,8 +124,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)
> >  int Awb::configure(IPAContext &context,
> >                  const IPACameraSensorInfo &configInfo)
> >  {
> > -     context.activeState.awb.gains.manual = RGB<double>{ 1.0 };
> > -     context.activeState.awb.gains.automatic =
> > +     context.activeState.awb.manual.gains = RGB<double>{ 1.0 };
> > +     context.activeState.awb.automatic.gains =
> >               awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);
> >       context.activeState.awb.autoEnabled = true;
> >       context.activeState.awb.temperatureK = kDefaultColourTemperature;
> > @@ -173,8 +173,8 @@ void Awb::queueRequest(IPAContext &context,
> >       const auto &colourTemperature = controls.get(controls::ColourTemperature);
> >       bool update = false;
> >       if (colourGains) {
> > -             awb.gains.manual.r() = (*colourGains)[0];
> > -             awb.gains.manual.b() = (*colourGains)[1];
> > +             awb.manual.gains.r() = (*colourGains)[0];
> > +             awb.manual.gains.b() = (*colourGains)[1];
> >               /*
> >                * \todo Colour temperature reported in metadata is now
> >                * incorrect, as we can't deduce the temperature from the gains.
> > @@ -183,17 +183,17 @@ void Awb::queueRequest(IPAContext &context,
> >               update = true;
> >       } else if (colourTemperature) {
> >               const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);
> > -             awb.gains.manual.r() = gains.r();
> > -             awb.gains.manual.b() = gains.b();
> > +             awb.manual.gains.r() = gains.r();
> > +             awb.manual.gains.b() = gains.b();
> >               awb.temperatureK = *colourTemperature;
> >               update = true;
> >       }
> >  
> >       if (update)
> >               LOG(RkISP1Awb, Debug)
> > -                     << "Set colour gains to " << awb.gains.manual;
> > +                     << "Set colour gains to " << awb.manual.gains;
> >  
> > -     frameContext.awb.gains = awb.gains.manual;
> > +     frameContext.awb.gains = awb.manual.gains;
> >       frameContext.awb.temperatureK = awb.temperatureK;
> >  }
> >  
> > @@ -208,7 +208,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,
> >        * most up-to-date automatic values we can read.
> >        */
> >       if (frameContext.awb.autoEnabled) {
> > -             frameContext.awb.gains = context.activeState.awb.gains.automatic;
> > +             frameContext.awb.gains = context.activeState.awb.automatic.gains;
> >               frameContext.awb.temperatureK = context.activeState.awb.temperatureK;
> >       }
> >  
> > @@ -325,14 +325,14 @@ void Awb::process(IPAContext &context,
> >       /* Filter the values to avoid oscillations. */
> >       double speed = 0.2;
> >       awbResult.gains = awbResult.gains * speed +
> > -                       activeState.awb.gains.automatic * (1 - speed);
> > +                       activeState.awb.automatic.gains * (1 - speed);
> >  
> > -     activeState.awb.gains.automatic = awbResult.gains;
> > +     activeState.awb.automatic.gains = awbResult.gains;
> >  
> >       LOG(RkISP1Awb, Debug)
> >               << std::showpoint
> >               << "Means " << rgbMeans << ", gains "
> > -             << activeState.awb.gains.automatic << ", temp "
> > +             << activeState.awb.automatic.gains << ", temp "
> >               << activeState.awb.temperatureK << "K";
> >  }
> >  
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 99611bd5b390..39b97d143e95 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -191,14 +191,17 @@ namespace libcamera::ipa::rkisp1 {
> >   * \var IPAActiveState::awb
> >   * \brief State for the Automatic White Balance algorithm
> >   *
> > - * \struct IPAActiveState::awb.gains
> > + * \var IPAActiveState::awb::AwbState
> 
> Shouldn't this be \struct ?
> 
> > + * \brief Struct for the AWB regulation state
> > + *
> > + * \struct IPAActiveState::awb::AwbState.gains
> 
> And this \var ?

I think you are right on both cases. As the rkisp1 is currently excluded
from doxygen generation, these errors don't pop up. We should enable
doxygen for that, but that is for another series :-). I'll change that
while applying.

Cheers,
Stefan

> 
> 
> Paul
> 
> >   * \brief White balance gains
> >   *
> > - * \var IPAActiveState::awb.gains.manual
> > - * \brief Manual white balance gains (set through requests)
> > + * \var IPAActiveState::awb.manual
> > + * \brief Manual regulation state (set through requests)
> >   *
> > - * \var IPAActiveState::awb.gains.automatic
> > - * \brief Automatic white balance gains (computed by the algorithm)
> > + * \var IPAActiveState::awb.automatic
> > + * \brief Automatic regulation state (computed by the algorithm)
> >   *
> >   * \var IPAActiveState::awb.temperatureK
> >   * \brief Estimated color temperature
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 474f7036f003..6bc922a82971 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -89,10 +89,12 @@ struct IPAActiveState {
> >       } agc;
> >  
> >       struct {
> > -             struct {
> > -                     RGB<double> manual;
> > -                     RGB<double> automatic;
> > -             } gains;
> > +             struct AwbState {
> > +                     RGB<double> gains;
> > +             };
> > +
> > +             AwbState manual;
> > +             AwbState automatic;
> >  
> >               unsigned int temperatureK;
> >               bool autoEnabled;
> > -- 
> > 2.43.0
> >
Paul Elder May 21, 2025, 5:24 p.m. UTC | #3 
Quoting Stefan Klug (2025-05-20 10:00:08)
> Hi Paul,
> 
> Thank you for the review.
> 
> Quoting Paul Elder (2025-05-07 18:51:17)
> > On Thu, Apr 03, 2025 at 05:49:14PM +0200, Stefan Klug wrote:
> > > Swap gains and automatic/manual in the IPAActiveState structure. This is
> > > in preparation to adding another member, which is easier in the new
> > > structure. The patch contains no functional changes.
> > > 
> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > ---
> > > 
> > > Changes in v2:
> > > - Use one named struct instead of two anonymous ones
> > > 
> > > Changes in v3:
> > > - Collected tags
> > > - Updated documentation
> > > ---
> > >  src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------
> > >  src/ipa/rkisp1/ipa_context.cpp    | 13 ++++++++-----
> > >  src/ipa/rkisp1/ipa_context.h      | 10 ++++++----
> > >  3 files changed, 26 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> > > index eafe93081bb1..a9759e53f593 100644
> > > --- a/src/ipa/rkisp1/algorithms/awb.cpp
> > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> > > @@ -124,8 +124,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)
> > >  int Awb::configure(IPAContext &context,
> > >                  const IPACameraSensorInfo &configInfo)
> > >  {
> > > -     context.activeState.awb.gains.manual = RGB<double>{ 1.0 };
> > > -     context.activeState.awb.gains.automatic =
> > > +     context.activeState.awb.manual.gains = RGB<double>{ 1.0 };
> > > +     context.activeState.awb.automatic.gains =
> > >               awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);
> > >       context.activeState.awb.autoEnabled = true;
> > >       context.activeState.awb.temperatureK = kDefaultColourTemperature;
> > > @@ -173,8 +173,8 @@ void Awb::queueRequest(IPAContext &context,
> > >       const auto &colourTemperature = controls.get(controls::ColourTemperature);
> > >       bool update = false;
> > >       if (colourGains) {
> > > -             awb.gains.manual.r() = (*colourGains)[0];
> > > -             awb.gains.manual.b() = (*colourGains)[1];
> > > +             awb.manual.gains.r() = (*colourGains)[0];
> > > +             awb.manual.gains.b() = (*colourGains)[1];
> > >               /*
> > >                * \todo Colour temperature reported in metadata is now
> > >                * incorrect, as we can't deduce the temperature from the gains.
> > > @@ -183,17 +183,17 @@ void Awb::queueRequest(IPAContext &context,
> > >               update = true;
> > >       } else if (colourTemperature) {
> > >               const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);
> > > -             awb.gains.manual.r() = gains.r();
> > > -             awb.gains.manual.b() = gains.b();
> > > +             awb.manual.gains.r() = gains.r();
> > > +             awb.manual.gains.b() = gains.b();
> > >               awb.temperatureK = *colourTemperature;
> > >               update = true;
> > >       }
> > >  
> > >       if (update)
> > >               LOG(RkISP1Awb, Debug)
> > > -                     << "Set colour gains to " << awb.gains.manual;
> > > +                     << "Set colour gains to " << awb.manual.gains;
> > >  
> > > -     frameContext.awb.gains = awb.gains.manual;
> > > +     frameContext.awb.gains = awb.manual.gains;
> > >       frameContext.awb.temperatureK = awb.temperatureK;
> > >  }
> > >  
> > > @@ -208,7 +208,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,
> > >        * most up-to-date automatic values we can read.
> > >        */
> > >       if (frameContext.awb.autoEnabled) {
> > > -             frameContext.awb.gains = context.activeState.awb.gains.automatic;
> > > +             frameContext.awb.gains = context.activeState.awb.automatic.gains;
> > >               frameContext.awb.temperatureK = context.activeState.awb.temperatureK;
> > >       }
> > >  
> > > @@ -325,14 +325,14 @@ void Awb::process(IPAContext &context,
> > >       /* Filter the values to avoid oscillations. */
> > >       double speed = 0.2;
> > >       awbResult.gains = awbResult.gains * speed +
> > > -                       activeState.awb.gains.automatic * (1 - speed);
> > > +                       activeState.awb.automatic.gains * (1 - speed);
> > >  
> > > -     activeState.awb.gains.automatic = awbResult.gains;
> > > +     activeState.awb.automatic.gains = awbResult.gains;
> > >  
> > >       LOG(RkISP1Awb, Debug)
> > >               << std::showpoint
> > >               << "Means " << rgbMeans << ", gains "
> > > -             << activeState.awb.gains.automatic << ", temp "
> > > +             << activeState.awb.automatic.gains << ", temp "
> > >               << activeState.awb.temperatureK << "K";
> > >  }
> > >  
> > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > > index 99611bd5b390..39b97d143e95 100644
> > > --- a/src/ipa/rkisp1/ipa_context.cpp
> > > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > > @@ -191,14 +191,17 @@ namespace libcamera::ipa::rkisp1 {
> > >   * \var IPAActiveState::awb
> > >   * \brief State for the Automatic White Balance algorithm
> > >   *
> > > - * \struct IPAActiveState::awb.gains
> > > + * \var IPAActiveState::awb::AwbState
> > 
> > Shouldn't this be \struct ?
> > 
> > > + * \brief Struct for the AWB regulation state
> > > + *
> > > + * \struct IPAActiveState::awb::AwbState.gains
> > 
> > And this \var ?
> 
> I think you are right on both cases. As the rkisp1 is currently excluded
> from doxygen generation, these errors don't pop up. We should enable
> doxygen for that, but that is for another series :-). I'll change that

Yeah, we can do that on top.

> while applying.

Cool :) with that fixed,

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> 
> Cheers,
> Stefan
> 
> > 
> > 
> > Paul
> > 
> > >   * \brief White balance gains
> > >   *
> > > - * \var IPAActiveState::awb.gains.manual
> > > - * \brief Manual white balance gains (set through requests)
> > > + * \var IPAActiveState::awb.manual
> > > + * \brief Manual regulation state (set through requests)
> > >   *
> > > - * \var IPAActiveState::awb.gains.automatic
> > > - * \brief Automatic white balance gains (computed by the algorithm)
> > > + * \var IPAActiveState::awb.automatic
> > > + * \brief Automatic regulation state (computed by the algorithm)
> > >   *
> > >   * \var IPAActiveState::awb.temperatureK
> > >   * \brief Estimated color temperature
> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > > index 474f7036f003..6bc922a82971 100644
> > > --- a/src/ipa/rkisp1/ipa_context.h
> > > +++ b/src/ipa/rkisp1/ipa_context.h
> > > @@ -89,10 +89,12 @@ struct IPAActiveState {
> > >       } agc;
> > >  
> > >       struct {
> > > -             struct {
> > > -                     RGB<double> manual;
> > > -                     RGB<double> automatic;
> > > -             } gains;
> > > +             struct AwbState {
> > > +                     RGB<double> gains;
> > > +             };
> > > +
> > > +             AwbState manual;
> > > +             AwbState automatic;
> > >  
> > >               unsigned int temperatureK;
> > >               bool autoEnabled;
> > > -- 
> > > 2.43.0
> > >

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
index eafe93081bb1..a9759e53f593 100644
--- a/src/ipa/rkisp1/algorithms/awb.cpp
+++ b/src/ipa/rkisp1/algorithms/awb.cpp
@@ -124,8 +124,8 @@  int Awb::init(IPAContext &context, const YamlObject &tuningData)
 int Awb::configure(IPAContext &context,
 		   const IPACameraSensorInfo &configInfo)
 {
-	context.activeState.awb.gains.manual = RGB<double>{ 1.0 };
-	context.activeState.awb.gains.automatic =
+	context.activeState.awb.manual.gains = RGB<double>{ 1.0 };
+	context.activeState.awb.automatic.gains =
 		awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);
 	context.activeState.awb.autoEnabled = true;
 	context.activeState.awb.temperatureK = kDefaultColourTemperature;
@@ -173,8 +173,8 @@  void Awb::queueRequest(IPAContext &context,
 	const auto &colourTemperature = controls.get(controls::ColourTemperature);
 	bool update = false;
 	if (colourGains) {
-		awb.gains.manual.r() = (*colourGains)[0];
-		awb.gains.manual.b() = (*colourGains)[1];
+		awb.manual.gains.r() = (*colourGains)[0];
+		awb.manual.gains.b() = (*colourGains)[1];
 		/*
 		 * \todo Colour temperature reported in metadata is now
 		 * incorrect, as we can't deduce the temperature from the gains.
@@ -183,17 +183,17 @@  void Awb::queueRequest(IPAContext &context,
 		update = true;
 	} else if (colourTemperature) {
 		const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);
-		awb.gains.manual.r() = gains.r();
-		awb.gains.manual.b() = gains.b();
+		awb.manual.gains.r() = gains.r();
+		awb.manual.gains.b() = gains.b();
 		awb.temperatureK = *colourTemperature;
 		update = true;
 	}
 
 	if (update)
 		LOG(RkISP1Awb, Debug)
-			<< "Set colour gains to " << awb.gains.manual;
+			<< "Set colour gains to " << awb.manual.gains;
 
-	frameContext.awb.gains = awb.gains.manual;
+	frameContext.awb.gains = awb.manual.gains;
 	frameContext.awb.temperatureK = awb.temperatureK;
 }
 
@@ -208,7 +208,7 @@  void Awb::prepare(IPAContext &context, const uint32_t frame,
 	 * most up-to-date automatic values we can read.
 	 */
 	if (frameContext.awb.autoEnabled) {
-		frameContext.awb.gains = context.activeState.awb.gains.automatic;
+		frameContext.awb.gains = context.activeState.awb.automatic.gains;
 		frameContext.awb.temperatureK = context.activeState.awb.temperatureK;
 	}
 
@@ -325,14 +325,14 @@  void Awb::process(IPAContext &context,
 	/* Filter the values to avoid oscillations. */
 	double speed = 0.2;
 	awbResult.gains = awbResult.gains * speed +
-			  activeState.awb.gains.automatic * (1 - speed);
+			  activeState.awb.automatic.gains * (1 - speed);
 
-	activeState.awb.gains.automatic = awbResult.gains;
+	activeState.awb.automatic.gains = awbResult.gains;
 
 	LOG(RkISP1Awb, Debug)
 		<< std::showpoint
 		<< "Means " << rgbMeans << ", gains "
-		<< activeState.awb.gains.automatic << ", temp "
+		<< activeState.awb.automatic.gains << ", temp "
 		<< activeState.awb.temperatureK << "K";
 }
 
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index 99611bd5b390..39b97d143e95 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -191,14 +191,17 @@  namespace libcamera::ipa::rkisp1 {
  * \var IPAActiveState::awb
  * \brief State for the Automatic White Balance algorithm
  *
- * \struct IPAActiveState::awb.gains
+ * \var IPAActiveState::awb::AwbState
+ * \brief Struct for the AWB regulation state
+ *
+ * \struct IPAActiveState::awb::AwbState.gains
  * \brief White balance gains
  *
- * \var IPAActiveState::awb.gains.manual
- * \brief Manual white balance gains (set through requests)
+ * \var IPAActiveState::awb.manual
+ * \brief Manual regulation state (set through requests)
  *
- * \var IPAActiveState::awb.gains.automatic
- * \brief Automatic white balance gains (computed by the algorithm)
+ * \var IPAActiveState::awb.automatic
+ * \brief Automatic regulation state (computed by the algorithm)
  *
  * \var IPAActiveState::awb.temperatureK
  * \brief Estimated color temperature
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 474f7036f003..6bc922a82971 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -89,10 +89,12 @@  struct IPAActiveState {
 	} agc;
 
 	struct {
-		struct {
-			RGB<double> manual;
-			RGB<double> automatic;
-		} gains;
+		struct AwbState {
+			RGB<double> gains;
+		};
+
+		AwbState manual;
+		AwbState automatic;
 
 		unsigned int temperatureK;
 		bool autoEnabled;