[libcamera-devel] pipeline: ipa: raspberrypi: Rationalise use of RPi::ConfigParameters
diff mbox series

Message ID 20210130111651.1073981-1-naush@raspberrypi.com
State Superseded
Headers show
Series
  • [libcamera-devel] pipeline: ipa: raspberrypi: Rationalise use of RPi::ConfigParameters
Related show

Commit Message

Naushir Patuck Jan. 30, 2021, 11:16 a.m. UTC 
Rename the enum values to indicate pipeline handler -> IPA actions
(IPA_CONFIG_*) and IPA -> pipeline handler return results (IPA_RESULT_*).
Additionally, provide more descriptive names for these values.

Fixup logic when handling IPA_RESULT_SENSOR_PARAMS where we must
overwrite the parameters if provided by IPA. In the current codebase,
this only happens once on startup, so there is no effective functional
difference. But this change allows the option for the IPA to request new
sensor parameters per-mode if required.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 include/libcamera/ipa/raspberrypi.h           | 10 +++---
 src/ipa/raspberrypi/raspberrypi.cpp           | 18 +++++------
 .../pipeline/raspberrypi/raspberrypi.cpp      | 31 +++++++++----------
 3 files changed, 28 insertions(+), 31 deletions(-)

Comments

Jacopo Mondi Feb. 1, 2021, 10:53 a.m. UTC | #1 
Hi Naush

On Sat, Jan 30, 2021 at 11:16:51AM +0000, Naushir Patuck wrote:
> Rename the enum values to indicate pipeline handler -> IPA actions
> (IPA_CONFIG_*) and IPA -> pipeline handler return results (IPA_RESULT_*).
> Additionally, provide more descriptive names for these values.

This part is ok, make naming more clear
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

>
> Fixup logic when handling IPA_RESULT_SENSOR_PARAMS where we must
> overwrite the parameters if provided by IPA. In the current codebase,
> this only happens once on startup, so there is no effective functional
> difference. But this change allows the option for the IPA to request new
> sensor parameters per-mode if required.

This seems it's worth a separate patch, as it does not depends on
renaming but it's rather a change in behaviour which it would probably
better be separate.

If I got this right you refer to 'parameters' in the commit message
here to indicate the controls delay could be overwritten at each
configureIPA() am I right ? Out of curiosity, how does the sensor mode
change impact on the control delay ?

Thanks
  j


>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  include/libcamera/ipa/raspberrypi.h           | 10 +++---
>  src/ipa/raspberrypi/raspberrypi.cpp           | 18 +++++------
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 31 +++++++++----------
>  3 files changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index 5a9433825d5a..970b9e931188 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -20,11 +20,11 @@ namespace RPi {
>
>  enum ConfigParameters {
>  	IPA_CONFIG_LS_TABLE = (1 << 0),
> -	IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> -	IPA_CONFIG_SENSOR = (1 << 2),
> -	IPA_CONFIG_DROP_FRAMES = (1 << 3),
> -	IPA_CONFIG_FAILED = (1 << 4),
> -	IPA_CONFIG_STARTUP = (1 << 5),
> +	IPA_CONFIG_STARTUP_CTRLS = (1 << 1),
> +	IPA_RESULT_CONFIG_FAILED = (1 << 2),
> +	IPA_RESULT_SENSOR_PARAMS = (1 << 3),
> +	IPA_RESULT_SENSOR_CTRLS = (1 << 4),
> +	IPA_RESULT_DROP_FRAMES = (1 << 5),
>  };
>
>  enum Operations {
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 681ab9211b7c..fea1ea3957bb 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -170,7 +170,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)
>
>  	ASSERT(result);
>  	result->operation = 0;
> -	if (data.operation & RPi::IPA_CONFIG_STARTUP) {
> +	if (data.operation & RPi::IPA_CONFIG_STARTUP_CTRLS) {
>  		/* We have been given some controls to action before start. */
>  		queueRequest(data.controls[0]);
>  	}
> @@ -188,7 +188,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)
>  		ControlList ctrls(sensorCtrls_);
>  		applyAGC(&agcStatus, ctrls);
>  		result->controls.emplace_back(ctrls);
> -		result->operation |= RPi::IPA_CONFIG_SENSOR;
> +		result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;
>  	}
>
>  	/*
> @@ -236,7 +236,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)
>  	}
>
>  	result->data.push_back(dropFrame);
> -	result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
> +	result->operation |= RPi::IPA_RESULT_DROP_FRAMES;
>
>  	firstStart_ = false;
>
> @@ -289,7 +289,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  {
>  	if (entityControls.size() != 2) {
>  		LOG(IPARPI, Error) << "No ISP or sensor controls found.";
> -		result->operation = RPi::IPA_CONFIG_FAILED;
> +		result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
>  		return;
>  	}
>
> @@ -300,13 +300,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>
>  	if (!validateSensorControls()) {
>  		LOG(IPARPI, Error) << "Sensor control validation failed.";
> -		result->operation = RPi::IPA_CONFIG_FAILED;
> +		result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
>  		return;
>  	}
>
>  	if (!validateIspControls()) {
>  		LOG(IPARPI, Error) << "ISP control validation failed.";
> -		result->operation = RPi::IPA_CONFIG_FAILED;
> +		result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
>  		return;
>  	}
>
> @@ -325,7 +325,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		if (!helper_) {
>  			LOG(IPARPI, Error) << "Could not create camera helper for "
>  					   << cameraName;
> -			result->operation = RPi::IPA_CONFIG_FAILED;
> +			result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
>  			return;
>  		}
>
> @@ -342,7 +342,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		result->data.push_back(exposureDelay); /* For VBLANK ctrl */
>  		result->data.push_back(sensorMetadata);
>
> -		result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;
> +		result->operation |= RPi::IPA_RESULT_SENSOR_PARAMS;
>  	}
>
>  	/* Re-assemble camera mode using the sensor info. */
> @@ -395,7 +395,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		applyAGC(&agcStatus, ctrls);
>
>  		result->controls.emplace_back(ctrls);
> -		result->operation |= RPi::IPA_CONFIG_SENSOR;
> +		result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;
>  	}
>
>  	lastMode_ = mode_;
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 5ad12d99638f..c44cb437a596 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -753,7 +753,7 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
>  	IPAOperationData ipaData = {};
>  	IPAOperationData result = {};
>  	if (controls) {
> -		ipaData.operation = RPi::IPA_CONFIG_STARTUP;
> +		ipaData.operation = RPi::IPA_CONFIG_STARTUP_CTRLS;
>  		ipaData.controls.emplace_back(*controls);
>  	}
>  	ret = data->ipa_->start(ipaData, &result);
> @@ -765,12 +765,12 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
>  	}
>
>  	/* Apply any gain/exposure settings that the IPA may have passed back. */
> -	if (result.operation & RPi::IPA_CONFIG_SENSOR) {
> +	if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {
>  		ControlList &ctrls = result.controls[0];
>  		data->unicam_[Unicam::Image].dev()->setControls(&ctrls);
>  	}
>
> -	if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) {
> +	if (result.operation & RPi::IPA_RESULT_DROP_FRAMES) {
>  		/* Configure the number of dropped frames required on startup. */
>  		data->dropFrameCount_ = result.data[0];
>  	}
> @@ -1213,31 +1213,28 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
>  	ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
>  			&result);
>
> -	if (result.operation & RPi::IPA_CONFIG_FAILED) {
> +	if (result.operation & RPi::IPA_RESULT_CONFIG_FAILED) {
>  		LOG(RPI, Error) << "IPA configuration failed!";
>  		return -EPIPE;
>  	}
>
>  	unsigned int resultIdx = 0;
> -	if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {
> +	if (result.operation & RPi::IPA_RESULT_SENSOR_PARAMS) {
>  		/*
>  		 * Setup our delayed control writer with the sensor default
>  		 * gain and exposure delays.
>  		 */
> -		if (!delayedCtrls_) {
> -			std::unordered_map<uint32_t, unsigned int> delays = {
> -				{ V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
> -				{ V4L2_CID_EXPOSURE, result.data[resultIdx++] },
> -				{ V4L2_CID_VBLANK, result.data[resultIdx++] }
> -			};
> -
> -			delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);
> -
> -			sensorMetadata_ = result.data[resultIdx++];
> -		}
> +		std::unordered_map<uint32_t, unsigned int> delays = {
> +			{ V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
> +			{ V4L2_CID_EXPOSURE, result.data[resultIdx++] },
> +			{ V4L2_CID_VBLANK, result.data[resultIdx++] }
> +		};
> +
> +		delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);
> +		sensorMetadata_ = result.data[resultIdx++];
>  	}
>
> -	if (result.operation & RPi::IPA_CONFIG_SENSOR) {
> +	if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {
>  		ControlList &ctrls = result.controls[0];
>  		unicam_[Unicam::Image].dev()->setControls(&ctrls);
>  	}
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Naushir Patuck Feb. 1, 2021, 12:41 p.m. UTC | #2 
Hi Jacopo,

Thank you for your review feedback.

On Mon, 1 Feb 2021 at 10:53, Jacopo Mondi <jacopo@jmondi.org> wrote:

> Hi Naush
>
> On Sat, Jan 30, 2021 at 11:16:51AM +0000, Naushir Patuck wrote:
> > Rename the enum values to indicate pipeline handler -> IPA actions
> > (IPA_CONFIG_*) and IPA -> pipeline handler return results (IPA_RESULT_*).
> > Additionally, provide more descriptive names for these values.
>
> This part is ok, make naming more clear
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> >
> > Fixup logic when handling IPA_RESULT_SENSOR_PARAMS where we must
> > overwrite the parameters if provided by IPA. In the current codebase,
> > this only happens once on startup, so there is no effective functional
> > difference. But this change allows the option for the IPA to request new
> > sensor parameters per-mode if required.
>
> This seems it's worth a separate patch, as it does not depends on
> renaming but it's rather a change in behaviour which it would probably
> better be separate.
>

Ack. I can separate these out into 2 patches.  Will post an update shortly.


>
> If I got this right you refer to 'parameters' in the commit message
> here to indicate the controls delay could be overwritten at each
> configureIPA() am I right ? Out of curiosity, how does the sensor mode
> change impact on the control delay ?
>

In almost all cases it won't.  However, I have sensors that will disable
embedded data in certain modes, but again, this is very rare.

Regards,
Naush



>
> Thanks
>   j
>
>
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  include/libcamera/ipa/raspberrypi.h           | 10 +++---
> >  src/ipa/raspberrypi/raspberrypi.cpp           | 18 +++++------
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 31 +++++++++----------
> >  3 files changed, 28 insertions(+), 31 deletions(-)
> >
> > diff --git a/include/libcamera/ipa/raspberrypi.h
> b/include/libcamera/ipa/raspberrypi.h
> > index 5a9433825d5a..970b9e931188 100644
> > --- a/include/libcamera/ipa/raspberrypi.h
> > +++ b/include/libcamera/ipa/raspberrypi.h
> > @@ -20,11 +20,11 @@ namespace RPi {
> >
> >  enum ConfigParameters {
> >       IPA_CONFIG_LS_TABLE = (1 << 0),
> > -     IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> > -     IPA_CONFIG_SENSOR = (1 << 2),
> > -     IPA_CONFIG_DROP_FRAMES = (1 << 3),
> > -     IPA_CONFIG_FAILED = (1 << 4),
> > -     IPA_CONFIG_STARTUP = (1 << 5),
> > +     IPA_CONFIG_STARTUP_CTRLS = (1 << 1),
> > +     IPA_RESULT_CONFIG_FAILED = (1 << 2),
> > +     IPA_RESULT_SENSOR_PARAMS = (1 << 3),
> > +     IPA_RESULT_SENSOR_CTRLS = (1 << 4),
> > +     IPA_RESULT_DROP_FRAMES = (1 << 5),
> >  };
> >
> >  enum Operations {
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 681ab9211b7c..fea1ea3957bb 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -170,7 +170,7 @@ int IPARPi::start(const IPAOperationData &data,
> IPAOperationData *result)
> >
> >       ASSERT(result);
> >       result->operation = 0;
> > -     if (data.operation & RPi::IPA_CONFIG_STARTUP) {
> > +     if (data.operation & RPi::IPA_CONFIG_STARTUP_CTRLS) {
> >               /* We have been given some controls to action before
> start. */
> >               queueRequest(data.controls[0]);
> >       }
> > @@ -188,7 +188,7 @@ int IPARPi::start(const IPAOperationData &data,
> IPAOperationData *result)
> >               ControlList ctrls(sensorCtrls_);
> >               applyAGC(&agcStatus, ctrls);
> >               result->controls.emplace_back(ctrls);
> > -             result->operation |= RPi::IPA_CONFIG_SENSOR;
> > +             result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;
> >       }
> >
> >       /*
> > @@ -236,7 +236,7 @@ int IPARPi::start(const IPAOperationData &data,
> IPAOperationData *result)
> >       }
> >
> >       result->data.push_back(dropFrame);
> > -     result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
> > +     result->operation |= RPi::IPA_RESULT_DROP_FRAMES;
> >
> >       firstStart_ = false;
> >
> > @@ -289,7 +289,7 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >  {
> >       if (entityControls.size() != 2) {
> >               LOG(IPARPI, Error) << "No ISP or sensor controls found.";
> > -             result->operation = RPi::IPA_CONFIG_FAILED;
> > +             result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
> >               return;
> >       }
> >
> > @@ -300,13 +300,13 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >
> >       if (!validateSensorControls()) {
> >               LOG(IPARPI, Error) << "Sensor control validation failed.";
> > -             result->operation = RPi::IPA_CONFIG_FAILED;
> > +             result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
> >               return;
> >       }
> >
> >       if (!validateIspControls()) {
> >               LOG(IPARPI, Error) << "ISP control validation failed.";
> > -             result->operation = RPi::IPA_CONFIG_FAILED;
> > +             result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
> >               return;
> >       }
> >
> > @@ -325,7 +325,7 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >               if (!helper_) {
> >                       LOG(IPARPI, Error) << "Could not create camera
> helper for "
> >                                          << cameraName;
> > -                     result->operation = RPi::IPA_CONFIG_FAILED;
> > +                     result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
> >                       return;
> >               }
> >
> > @@ -342,7 +342,7 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >               result->data.push_back(exposureDelay); /* For VBLANK ctrl
> */
> >               result->data.push_back(sensorMetadata);
> >
> > -             result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;
> > +             result->operation |= RPi::IPA_RESULT_SENSOR_PARAMS;
> >       }
> >
> >       /* Re-assemble camera mode using the sensor info. */
> > @@ -395,7 +395,7 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >               applyAGC(&agcStatus, ctrls);
> >
> >               result->controls.emplace_back(ctrls);
> > -             result->operation |= RPi::IPA_CONFIG_SENSOR;
> > +             result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;
> >       }
> >
> >       lastMode_ = mode_;
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 5ad12d99638f..c44cb437a596 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -753,7 +753,7 @@ int PipelineHandlerRPi::start(Camera *camera,
> [[maybe_unused]] ControlList *cont
> >       IPAOperationData ipaData = {};
> >       IPAOperationData result = {};
> >       if (controls) {
> > -             ipaData.operation = RPi::IPA_CONFIG_STARTUP;
> > +             ipaData.operation = RPi::IPA_CONFIG_STARTUP_CTRLS;
> >               ipaData.controls.emplace_back(*controls);
> >       }
> >       ret = data->ipa_->start(ipaData, &result);
> > @@ -765,12 +765,12 @@ int PipelineHandlerRPi::start(Camera *camera,
> [[maybe_unused]] ControlList *cont
> >       }
> >
> >       /* Apply any gain/exposure settings that the IPA may have passed
> back. */
> > -     if (result.operation & RPi::IPA_CONFIG_SENSOR) {
> > +     if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {
> >               ControlList &ctrls = result.controls[0];
> >               data->unicam_[Unicam::Image].dev()->setControls(&ctrls);
> >       }
> >
> > -     if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) {
> > +     if (result.operation & RPi::IPA_RESULT_DROP_FRAMES) {
> >               /* Configure the number of dropped frames required on
> startup. */
> >               data->dropFrameCount_ = result.data[0];
> >       }
> > @@ -1213,31 +1213,28 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
> >       ipa_->configure(sensorInfo_, streamConfig, entityControls,
> ipaConfig,
> >                       &result);
> >
> > -     if (result.operation & RPi::IPA_CONFIG_FAILED) {
> > +     if (result.operation & RPi::IPA_RESULT_CONFIG_FAILED) {
> >               LOG(RPI, Error) << "IPA configuration failed!";
> >               return -EPIPE;
> >       }
> >
> >       unsigned int resultIdx = 0;
> > -     if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {
> > +     if (result.operation & RPi::IPA_RESULT_SENSOR_PARAMS) {
> >               /*
> >                * Setup our delayed control writer with the sensor default
> >                * gain and exposure delays.
> >                */
> > -             if (!delayedCtrls_) {
> > -                     std::unordered_map<uint32_t, unsigned int> delays
> = {
> > -                             { V4L2_CID_ANALOGUE_GAIN,
> result.data[resultIdx++] },
> > -                             { V4L2_CID_EXPOSURE,
> result.data[resultIdx++] },
> > -                             { V4L2_CID_VBLANK,
> result.data[resultIdx++] }
> > -                     };
> > -
> > -                     delayedCtrls_ =
> std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);
> > -
> > -                     sensorMetadata_ = result.data[resultIdx++];
> > -             }
> > +             std::unordered_map<uint32_t, unsigned int> delays = {
> > +                     { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++]
> },
> > +                     { V4L2_CID_EXPOSURE, result.data[resultIdx++] },
> > +                     { V4L2_CID_VBLANK, result.data[resultIdx++] }
> > +             };
> > +
> > +             delayedCtrls_ =
> std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);
> > +             sensorMetadata_ = result.data[resultIdx++];
> >       }
> >
> > -     if (result.operation & RPi::IPA_CONFIG_SENSOR) {
> > +     if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {
> >               ControlList &ctrls = result.controls[0];
> >               unicam_[Unicam::Image].dev()->setControls(&ctrls);
> >       }
> > --
> > 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 5a9433825d5a..970b9e931188 100644
--- a/include/libcamera/ipa/raspberrypi.h
+++ b/include/libcamera/ipa/raspberrypi.h
@@ -20,11 +20,11 @@  namespace RPi {
 
 enum ConfigParameters {
 	IPA_CONFIG_LS_TABLE = (1 << 0),
-	IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
-	IPA_CONFIG_SENSOR = (1 << 2),
-	IPA_CONFIG_DROP_FRAMES = (1 << 3),
-	IPA_CONFIG_FAILED = (1 << 4),
-	IPA_CONFIG_STARTUP = (1 << 5),
+	IPA_CONFIG_STARTUP_CTRLS = (1 << 1),
+	IPA_RESULT_CONFIG_FAILED = (1 << 2),
+	IPA_RESULT_SENSOR_PARAMS = (1 << 3),
+	IPA_RESULT_SENSOR_CTRLS = (1 << 4),
+	IPA_RESULT_DROP_FRAMES = (1 << 5),
 };
 
 enum Operations {
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 681ab9211b7c..fea1ea3957bb 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -170,7 +170,7 @@  int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)
 
 	ASSERT(result);
 	result->operation = 0;
-	if (data.operation & RPi::IPA_CONFIG_STARTUP) {
+	if (data.operation & RPi::IPA_CONFIG_STARTUP_CTRLS) {
 		/* We have been given some controls to action before start. */
 		queueRequest(data.controls[0]);
 	}
@@ -188,7 +188,7 @@  int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)
 		ControlList ctrls(sensorCtrls_);
 		applyAGC(&agcStatus, ctrls);
 		result->controls.emplace_back(ctrls);
-		result->operation |= RPi::IPA_CONFIG_SENSOR;
+		result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;
 	}
 
 	/*
@@ -236,7 +236,7 @@  int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)
 	}
 
 	result->data.push_back(dropFrame);
-	result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
+	result->operation |= RPi::IPA_RESULT_DROP_FRAMES;
 
 	firstStart_ = false;
 
@@ -289,7 +289,7 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 {
 	if (entityControls.size() != 2) {
 		LOG(IPARPI, Error) << "No ISP or sensor controls found.";
-		result->operation = RPi::IPA_CONFIG_FAILED;
+		result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
 		return;
 	}
 
@@ -300,13 +300,13 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 
 	if (!validateSensorControls()) {
 		LOG(IPARPI, Error) << "Sensor control validation failed.";
-		result->operation = RPi::IPA_CONFIG_FAILED;
+		result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
 		return;
 	}
 
 	if (!validateIspControls()) {
 		LOG(IPARPI, Error) << "ISP control validation failed.";
-		result->operation = RPi::IPA_CONFIG_FAILED;
+		result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
 		return;
 	}
 
@@ -325,7 +325,7 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		if (!helper_) {
 			LOG(IPARPI, Error) << "Could not create camera helper for "
 					   << cameraName;
-			result->operation = RPi::IPA_CONFIG_FAILED;
+			result->operation = RPi::IPA_RESULT_CONFIG_FAILED;
 			return;
 		}
 
@@ -342,7 +342,7 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		result->data.push_back(exposureDelay); /* For VBLANK ctrl */
 		result->data.push_back(sensorMetadata);
 
-		result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;
+		result->operation |= RPi::IPA_RESULT_SENSOR_PARAMS;
 	}
 
 	/* Re-assemble camera mode using the sensor info. */
@@ -395,7 +395,7 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		applyAGC(&agcStatus, ctrls);
 
 		result->controls.emplace_back(ctrls);
-		result->operation |= RPi::IPA_CONFIG_SENSOR;
+		result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;
 	}
 
 	lastMode_ = mode_;
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 5ad12d99638f..c44cb437a596 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -753,7 +753,7 @@  int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
 	IPAOperationData ipaData = {};
 	IPAOperationData result = {};
 	if (controls) {
-		ipaData.operation = RPi::IPA_CONFIG_STARTUP;
+		ipaData.operation = RPi::IPA_CONFIG_STARTUP_CTRLS;
 		ipaData.controls.emplace_back(*controls);
 	}
 	ret = data->ipa_->start(ipaData, &result);
@@ -765,12 +765,12 @@  int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
 	}
 
 	/* Apply any gain/exposure settings that the IPA may have passed back. */
-	if (result.operation & RPi::IPA_CONFIG_SENSOR) {
+	if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {
 		ControlList &ctrls = result.controls[0];
 		data->unicam_[Unicam::Image].dev()->setControls(&ctrls);
 	}
 
-	if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) {
+	if (result.operation & RPi::IPA_RESULT_DROP_FRAMES) {
 		/* Configure the number of dropped frames required on startup. */
 		data->dropFrameCount_ = result.data[0];
 	}
@@ -1213,31 +1213,28 @@  int RPiCameraData::configureIPA(const CameraConfiguration *config)
 	ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
 			&result);
 
-	if (result.operation & RPi::IPA_CONFIG_FAILED) {
+	if (result.operation & RPi::IPA_RESULT_CONFIG_FAILED) {
 		LOG(RPI, Error) << "IPA configuration failed!";
 		return -EPIPE;
 	}
 
 	unsigned int resultIdx = 0;
-	if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {
+	if (result.operation & RPi::IPA_RESULT_SENSOR_PARAMS) {
 		/*
 		 * Setup our delayed control writer with the sensor default
 		 * gain and exposure delays.
 		 */
-		if (!delayedCtrls_) {
-			std::unordered_map<uint32_t, unsigned int> delays = {
-				{ V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
-				{ V4L2_CID_EXPOSURE, result.data[resultIdx++] },
-				{ V4L2_CID_VBLANK, result.data[resultIdx++] }
-			};
-
-			delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);
-
-			sensorMetadata_ = result.data[resultIdx++];
-		}
+		std::unordered_map<uint32_t, unsigned int> delays = {
+			{ V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
+			{ V4L2_CID_EXPOSURE, result.data[resultIdx++] },
+			{ V4L2_CID_VBLANK, result.data[resultIdx++] }
+		};
+
+		delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);
+		sensorMetadata_ = result.data[resultIdx++];
 	}
 
-	if (result.operation & RPi::IPA_CONFIG_SENSOR) {
+	if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {
 		ControlList &ctrls = result.controls[0];
 		unicam_[Unicam::Image].dev()->setControls(&ctrls);
 	}