[libcamera-devel,1/2] pipeline: ipa: raspberrypi: Rename RPi::ConfigParameters enum values

11101 diff mbox series

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.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/ipa/raspberrypi.h            | 10 +++++-----
 src/ipa/raspberrypi/raspberrypi.cpp            | 18 +++++++++---------
 .../pipeline/raspberrypi/raspberrypi.cpp       | 12 ++++++------
 3 files changed, 20 insertions(+), 20 deletions(-)

Hi Naush,

Thank you for the patch.

On Mon, Feb 01, 2021 at 12:56:32PM +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.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  include/libcamera/ipa/raspberrypi.h            | 10 +++++-----
>  src/ipa/raspberrypi/raspberrypi.cpp            | 18 +++++++++---------
>  .../pipeline/raspberrypi/raspberrypi.cpp       | 12 ++++++------
>  3 files changed, 20 insertions(+), 20 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..48e5943edc1a 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,13 +1213,13 @@ 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.
> @@ -1237,7 +1237,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
>  		}
>  	}
>  
> -	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);
>  	}