[libcamera-devel,v2,3/3] pipeline: ipa: raspberrypi: Pass controls to IPA on start
diff mbox series

Message ID 20201126095126.997055-3-naush@raspberrypi.com
State Superseded
Headers show
Series
  • [libcamera-devel,v2,1/3] libcamera: pipeline: Pass libcamera controls into pipeline_handler::start()
Related show

Commit Message

Naushir Patuck Nov. 26, 2020, 9:51 a.m. UTC 
Forward any controls passed into the pipeline handler to the IPA.
The IPA then sets up the Raspberry Pi controller with these settings
appropriately, and passes back any V4L2 sensor controls that need
to be applied.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/ipa/raspberrypi.h           |  1 +
 src/ipa/raspberrypi/raspberrypi.cpp           | 53 ++++++++++++-------
 .../pipeline/raspberrypi/raspberrypi.cpp      | 14 ++++-
 3 files changed, 47 insertions(+), 21 deletions(-)

Comments

Jacopo Mondi Dec. 4, 2020, 11:35 a.m. UTC | #1 
Hi Naush,

On Thu, Nov 26, 2020 at 09:51:26AM +0000, Naushir Patuck wrote:
> Forward any controls passed into the pipeline handler to the IPA.
> The IPA then sets up the Raspberry Pi controller with these settings
> appropriately, and passes back any V4L2 sensor controls that need
> to be applied.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> Tested-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/ipa/raspberrypi.h           |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           | 53 ++++++++++++-------
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 14 ++++-
>  3 files changed, 47 insertions(+), 21 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index 2b55dbfc..c91a14bd 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -21,6 +21,7 @@ enum ConfigParameters {
>  	IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
>  	IPA_CONFIG_SENSOR = (1 << 2),
>  	IPA_CONFIG_DROP_FRAMES = (1 << 3),
> +	IPA_CONFIG_STARTUP = (1 << 4)
>  };
>
>  enum Operations {
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 7a07b477..c09b3d07 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -77,8 +77,7 @@ public:
>  	}
>
>  	int init(const IPASettings &settings) override;
> -	int start([[maybe_unused]] const IPAOperationData &ipaConfig,
> -		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
> +	int start(const IPAOperationData &ipaConfig, IPAOperationData *result) override;
>  	void stop() override {}
>
>  	void configure(const CameraSensorInfo &sensorInfo,
> @@ -154,6 +153,35 @@ int IPARPi::init(const IPASettings &settings)
>  	return 0;
>  }
>
> +int IPARPi::start(const IPAOperationData &ipaConfig, IPAOperationData *result)
> +{
> +	RPiController::Metadata metadata;
> +
> +	result->operation = 0;

Do you need to assert (result) ?
It might help catch development errors

> +	if (ipaConfig.operation & RPi::IPA_CONFIG_STARTUP) {
> +		/* We have been given some controls to action before start. */
> +		queueRequest(ipaConfig.controls[0]);

This seems to assume controls[0] is always populated...

> +	}
> +
> +	controller_.SwitchMode(mode_, &metadata);
> +
> +	/* SwitchMode may supply updated exposure/gain values to use. */
> +	AgcStatus agcStatus;
> +	agcStatus.shutter_time = 0.0;
> +	agcStatus.analogue_gain = 0.0;
> +
> +	/* SwitchMode may supply updated exposure/gain values to use. */
> +	metadata.Get("agc.status", agcStatus);
> +	if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {
> +		ControlList ctrls(unicamCtrls_);
> +		applyAGC(&agcStatus, ctrls);
> +		result->controls.emplace_back(ctrls);
> +		result->operation |= RPi::IPA_CONFIG_SENSOR;
> +	}
> +
> +	return 0;
> +}
> +
>  void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
>  {
>  	mode_.bitdepth = sensorInfo.bitsPerPixel;
> @@ -229,7 +257,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		result->data.push_back(gainDelay);
>  		result->data.push_back(exposureDelay);
>  		result->data.push_back(sensorMetadata);
> -

Unrelated, but nothing big

>  		result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;
>  	}
>
> @@ -285,11 +312,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  	result->data.push_back(dropFrame);
>  	result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
>
> -	/* These zero values mean not program anything (unless overwritten). */
> -	struct AgcStatus agcStatus;
> -	agcStatus.shutter_time = 0.0;
> -	agcStatus.analogue_gain = 0.0;
> -
>  	if (!controllerInit_) {
>  		/* Load the tuning file for this sensor. */
>  		controller_.Read(tuningFile_.c_str());
> @@ -297,20 +319,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		controllerInit_ = true;
>
>  		/* Supply initial values for gain and exposure. */
> +		ControlList ctrls(unicamCtrls_);
> +		AgcStatus agcStatus;
>  		agcStatus.shutter_time = DefaultExposureTime;
>  		agcStatus.analogue_gain = DefaultAnalogueGain;
> -	}
> -
> -	RPiController::Metadata metadata;
> -	controller_.SwitchMode(mode_, &metadata);

I trust your judgment on this part that moves the mode switch to
start() unconditionally

> -
> -	/* SwitchMode may supply updated exposure/gain values to use. */
> -	metadata.Get("agc.status", agcStatus);
> -	if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {
> -		ControlList ctrls(unicamCtrls_);
>  		applyAGC(&agcStatus, ctrls);
> -		result->controls.push_back(ctrls);
>
> +		result->controls.emplace_back(ctrls);
>  		result->operation |= RPi::IPA_CONFIG_SENSOR;
>  	}
>
> @@ -843,7 +858,7 @@ void IPARPi::processStats(unsigned int bufferId)
>
>  		IPAOperationData op;
>  		op.operation = RPi::IPA_ACTION_V4L2_SET_STAGGERED;
> -		op.controls.push_back(ctrls);
> +		op.controls.emplace_back(ctrls);
>  		queueFrameAction.emit(0, op);
>  	}
>  }
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 29bcef07..3eb8c190 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -748,7 +748,10 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
>
>  	/* Start the IPA. */
>  	IPAOperationData ipaConfig = {}, result = {};
> -	ipaConfig.controls.emplace_back(*controls);
> +	if (controls) {
> +		ipaConfig.operation = RPi::IPA_CONFIG_STARTUP;
> +		ipaConfig.controls.emplace_back(*controls);
> +	}
>  	ret = data->ipa_->start(ipaConfig, &result);
>  	if (ret) {
>  		LOG(RPI, Error)
> @@ -757,6 +760,14 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
>  		return ret;
>  	}
>
> +	/* Apply any gain/exposure settings that the IPA may have passed back. */
> +	ASSERT(data->staggeredCtrl_);
> +	if (result.operation & RPi::IPA_CONFIG_SENSOR) {
> +		const ControlList &ctrls = result.controls[0];
> +		if (!data->staggeredCtrl_.set(ctrls))
> +			LOG(RPI, Error) << "V4L2 staggered set failed";
> +	}
> +
>  	/*
>  	 * IPA configure may have changed the sensor flips - hence the bayer
>  	 * order. Get the sensor format and set the ISP input now.
> @@ -777,7 +788,6 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
>  	 * starting. First check that the staggered ctrl has been initialised
>  	 * by configure().
>  	 */
> -	ASSERT(data->staggeredCtrl_);
>  	data->staggeredCtrl_.reset();
>  	data->staggeredCtrl_.write();
>  	data->expectedSequence_ = 0;
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Naushir Patuck Dec. 4, 2020, 1:20 p.m. UTC | #2 
Hi Jacopo,

Thank you for your review comments.

On Fri, 4 Dec 2020 at 11:35, Jacopo Mondi <jacopo@jmondi.org> wrote:

> Hi Naush,
>
> On Thu, Nov 26, 2020 at 09:51:26AM +0000, Naushir Patuck wrote:
> > Forward any controls passed into the pipeline handler to the IPA.
> > The IPA then sets up the Raspberry Pi controller with these settings
> > appropriately, and passes back any V4L2 sensor controls that need
> > to be applied.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> > Tested-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  include/libcamera/ipa/raspberrypi.h           |  1 +
> >  src/ipa/raspberrypi/raspberrypi.cpp           | 53 ++++++++++++-------
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 14 ++++-
> >  3 files changed, 47 insertions(+), 21 deletions(-)
> >
> > diff --git a/include/libcamera/ipa/raspberrypi.h
> b/include/libcamera/ipa/raspberrypi.h
> > index 2b55dbfc..c91a14bd 100644
> > --- a/include/libcamera/ipa/raspberrypi.h
> > +++ b/include/libcamera/ipa/raspberrypi.h
> > @@ -21,6 +21,7 @@ enum ConfigParameters {
> >       IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> >       IPA_CONFIG_SENSOR = (1 << 2),
> >       IPA_CONFIG_DROP_FRAMES = (1 << 3),
> > +     IPA_CONFIG_STARTUP = (1 << 4)
> >  };
> >
> >  enum Operations {
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 7a07b477..c09b3d07 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -77,8 +77,7 @@ public:
> >       }
> >
> >       int init(const IPASettings &settings) override;
> > -     int start([[maybe_unused]] const IPAOperationData &ipaConfig,
> > -               [[maybe_unused]] IPAOperationData *result) override {
> return 0; }
> > +     int start(const IPAOperationData &ipaConfig, IPAOperationData
> *result) override;
> >       void stop() override {}
> >
> >       void configure(const CameraSensorInfo &sensorInfo,
> > @@ -154,6 +153,35 @@ int IPARPi::init(const IPASettings &settings)
> >       return 0;
> >  }
> >
> > +int IPARPi::start(const IPAOperationData &ipaConfig, IPAOperationData
> *result)
> > +{
> > +     RPiController::Metadata metadata;
> > +
> > +     result->operation = 0;
>
> Do you need to assert (result) ?
> It might help catch development errors
>

Given the only caller of this is the Raspberry Pi pipeline handler, result
will always be valid.  However, always good to be extra safe, so I will add
an assertion as suggested.


>
> > +     if (ipaConfig.operation & RPi::IPA_CONFIG_STARTUP) {
> > +             /* We have been given some controls to action before
> start. */
> > +             queueRequest(ipaConfig.controls[0]);
>
> This seems to assume controls[0] is always populated...
>

The pipeline handler will only set RPi::IPA_CONFIG_STARTUP when a list of
startup controls is available.  Thus, ipaConfig.controls[0] is guaranteed
to be populated within the if() clause.


>
> > +     }
> > +
> > +     controller_.SwitchMode(mode_, &metadata);
> > +
> > +     /* SwitchMode may supply updated exposure/gain values to use. */
> > +     AgcStatus agcStatus;
> > +     agcStatus.shutter_time = 0.0;
> > +     agcStatus.analogue_gain = 0.0;
> > +
> > +     /* SwitchMode may supply updated exposure/gain values to use. */
> > +     metadata.Get("agc.status", agcStatus);
> > +     if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain !=
> 0.0) {
> > +             ControlList ctrls(unicamCtrls_);
> > +             applyAGC(&agcStatus, ctrls);
> > +             result->controls.emplace_back(ctrls);
> > +             result->operation |= RPi::IPA_CONFIG_SENSOR;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
> >  {
> >       mode_.bitdepth = sensorInfo.bitsPerPixel;
> > @@ -229,7 +257,6 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >               result->data.push_back(gainDelay);
> >               result->data.push_back(exposureDelay);
> >               result->data.push_back(sensorMetadata);
> > -
>
> Unrelated, but nothing big
>
> >               result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;
> >       }
> >
> > @@ -285,11 +312,6 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >       result->data.push_back(dropFrame);
> >       result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
> >
> > -     /* These zero values mean not program anything (unless
> overwritten). */
> > -     struct AgcStatus agcStatus;
> > -     agcStatus.shutter_time = 0.0;
> > -     agcStatus.analogue_gain = 0.0;
> > -
> >       if (!controllerInit_) {
> >               /* Load the tuning file for this sensor. */
> >               controller_.Read(tuningFile_.c_str());
> > @@ -297,20 +319,13 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >               controllerInit_ = true;
> >
> >               /* Supply initial values for gain and exposure. */
> > +             ControlList ctrls(unicamCtrls_);
> > +             AgcStatus agcStatus;
> >               agcStatus.shutter_time = DefaultExposureTime;
> >               agcStatus.analogue_gain = DefaultAnalogueGain;
> > -     }
> > -
> > -     RPiController::Metadata metadata;
> > -     controller_.SwitchMode(mode_, &metadata);
>
> I trust your judgment on this part that moves the mode switch to
> start() unconditionally
>

Yes, this is correct.  The rationale is that our controller_.SwitchMode()
will return out the ISP and sensor configuration to apply before streaming,
so must happen on every start.

Regards,
Naush



>
> > -
> > -     /* SwitchMode may supply updated exposure/gain values to use. */
> > -     metadata.Get("agc.status", agcStatus);
> > -     if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain !=
> 0.0) {
> > -             ControlList ctrls(unicamCtrls_);
> >               applyAGC(&agcStatus, ctrls);
> > -             result->controls.push_back(ctrls);
> >
> > +             result->controls.emplace_back(ctrls);
> >               result->operation |= RPi::IPA_CONFIG_SENSOR;
> >       }
> >
> > @@ -843,7 +858,7 @@ void IPARPi::processStats(unsigned int bufferId)
> >
> >               IPAOperationData op;
> >               op.operation = RPi::IPA_ACTION_V4L2_SET_STAGGERED;
> > -             op.controls.push_back(ctrls);
> > +             op.controls.emplace_back(ctrls);
> >               queueFrameAction.emit(0, op);
> >       }
> >  }
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 29bcef07..3eb8c190 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -748,7 +748,10 @@ int PipelineHandlerRPi::start(Camera *camera,
> [[maybe_unused]] ControlList *cont
> >
> >       /* Start the IPA. */
> >       IPAOperationData ipaConfig = {}, result = {};
> > -     ipaConfig.controls.emplace_back(*controls);
> > +     if (controls) {
> > +             ipaConfig.operation = RPi::IPA_CONFIG_STARTUP;
> > +             ipaConfig.controls.emplace_back(*controls);
> > +     }
> >       ret = data->ipa_->start(ipaConfig, &result);
> >       if (ret) {
> >               LOG(RPI, Error)
> > @@ -757,6 +760,14 @@ int PipelineHandlerRPi::start(Camera *camera,
> [[maybe_unused]] ControlList *cont
> >               return ret;
> >       }
> >
> > +     /* Apply any gain/exposure settings that the IPA may have passed
> back. */
> > +     ASSERT(data->staggeredCtrl_);
> > +     if (result.operation & RPi::IPA_CONFIG_SENSOR) {
> > +             const ControlList &ctrls = result.controls[0];
> > +             if (!data->staggeredCtrl_.set(ctrls))
> > +                     LOG(RPI, Error) << "V4L2 staggered set failed";
> > +     }
> > +
> >       /*
> >        * IPA configure may have changed the sensor flips - hence the
> bayer
> >        * order. Get the sensor format and set the ISP input now.
> > @@ -777,7 +788,6 @@ int PipelineHandlerRPi::start(Camera *camera,
> [[maybe_unused]] ControlList *cont
> >        * starting. First check that the staggered ctrl has been
> initialised
> >        * by configure().
> >        */
> > -     ASSERT(data->staggeredCtrl_);
> >       data->staggeredCtrl_.reset();
> >       data->staggeredCtrl_.write();
> >       data->expectedSequence_ = 0;
> > --
> > 2.25.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
index 2b55dbfc..c91a14bd 100644
--- a/include/libcamera/ipa/raspberrypi.h
+++ b/include/libcamera/ipa/raspberrypi.h
@@ -21,6 +21,7 @@  enum ConfigParameters {
 	IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
 	IPA_CONFIG_SENSOR = (1 << 2),
 	IPA_CONFIG_DROP_FRAMES = (1 << 3),
+	IPA_CONFIG_STARTUP = (1 << 4)
 };
 
 enum Operations {
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 7a07b477..c09b3d07 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -77,8 +77,7 @@  public:
 	}
 
 	int init(const IPASettings &settings) override;
-	int start([[maybe_unused]] const IPAOperationData &ipaConfig,
-		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
+	int start(const IPAOperationData &ipaConfig, IPAOperationData *result) override;
 	void stop() override {}
 
 	void configure(const CameraSensorInfo &sensorInfo,
@@ -154,6 +153,35 @@  int IPARPi::init(const IPASettings &settings)
 	return 0;
 }
 
+int IPARPi::start(const IPAOperationData &ipaConfig, IPAOperationData *result)
+{
+	RPiController::Metadata metadata;
+
+	result->operation = 0;
+	if (ipaConfig.operation & RPi::IPA_CONFIG_STARTUP) {
+		/* We have been given some controls to action before start. */
+		queueRequest(ipaConfig.controls[0]);
+	}
+
+	controller_.SwitchMode(mode_, &metadata);
+
+	/* SwitchMode may supply updated exposure/gain values to use. */
+	AgcStatus agcStatus;
+	agcStatus.shutter_time = 0.0;
+	agcStatus.analogue_gain = 0.0;
+
+	/* SwitchMode may supply updated exposure/gain values to use. */
+	metadata.Get("agc.status", agcStatus);
+	if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {
+		ControlList ctrls(unicamCtrls_);
+		applyAGC(&agcStatus, ctrls);
+		result->controls.emplace_back(ctrls);
+		result->operation |= RPi::IPA_CONFIG_SENSOR;
+	}
+
+	return 0;
+}
+
 void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
 {
 	mode_.bitdepth = sensorInfo.bitsPerPixel;
@@ -229,7 +257,6 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		result->data.push_back(gainDelay);
 		result->data.push_back(exposureDelay);
 		result->data.push_back(sensorMetadata);
-
 		result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;
 	}
 
@@ -285,11 +312,6 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 	result->data.push_back(dropFrame);
 	result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
 
-	/* These zero values mean not program anything (unless overwritten). */
-	struct AgcStatus agcStatus;
-	agcStatus.shutter_time = 0.0;
-	agcStatus.analogue_gain = 0.0;
-
 	if (!controllerInit_) {
 		/* Load the tuning file for this sensor. */
 		controller_.Read(tuningFile_.c_str());
@@ -297,20 +319,13 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		controllerInit_ = true;
 
 		/* Supply initial values for gain and exposure. */
+		ControlList ctrls(unicamCtrls_);
+		AgcStatus agcStatus;
 		agcStatus.shutter_time = DefaultExposureTime;
 		agcStatus.analogue_gain = DefaultAnalogueGain;
-	}
-
-	RPiController::Metadata metadata;
-	controller_.SwitchMode(mode_, &metadata);
-
-	/* SwitchMode may supply updated exposure/gain values to use. */
-	metadata.Get("agc.status", agcStatus);
-	if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {
-		ControlList ctrls(unicamCtrls_);
 		applyAGC(&agcStatus, ctrls);
-		result->controls.push_back(ctrls);
 
+		result->controls.emplace_back(ctrls);
 		result->operation |= RPi::IPA_CONFIG_SENSOR;
 	}
 
@@ -843,7 +858,7 @@  void IPARPi::processStats(unsigned int bufferId)
 
 		IPAOperationData op;
 		op.operation = RPi::IPA_ACTION_V4L2_SET_STAGGERED;
-		op.controls.push_back(ctrls);
+		op.controls.emplace_back(ctrls);
 		queueFrameAction.emit(0, op);
 	}
 }
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 29bcef07..3eb8c190 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -748,7 +748,10 @@  int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
 
 	/* Start the IPA. */
 	IPAOperationData ipaConfig = {}, result = {};
-	ipaConfig.controls.emplace_back(*controls);
+	if (controls) {
+		ipaConfig.operation = RPi::IPA_CONFIG_STARTUP;
+		ipaConfig.controls.emplace_back(*controls);
+	}
 	ret = data->ipa_->start(ipaConfig, &result);
 	if (ret) {
 		LOG(RPI, Error)
@@ -757,6 +760,14 @@  int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
 		return ret;
 	}
 
+	/* Apply any gain/exposure settings that the IPA may have passed back. */
+	ASSERT(data->staggeredCtrl_);
+	if (result.operation & RPi::IPA_CONFIG_SENSOR) {
+		const ControlList &ctrls = result.controls[0];
+		if (!data->staggeredCtrl_.set(ctrls))
+			LOG(RPI, Error) << "V4L2 staggered set failed";
+	}
+
 	/*
 	 * IPA configure may have changed the sensor flips - hence the bayer
 	 * order. Get the sensor format and set the ISP input now.
@@ -777,7 +788,6 @@  int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
 	 * starting. First check that the staggered ctrl has been initialised
 	 * by configure().
 	 */
-	ASSERT(data->staggeredCtrl_);
 	data->staggeredCtrl_.reset();
 	data->staggeredCtrl_.write();
 	data->expectedSequence_ = 0;