[libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Rationalise use of RPi::ConfigParameters
Naushir Patuck
naush at raspberrypi.com
Sat Jan 30 12:16:51 CET 2021
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 at 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
More information about the libcamera-devel
mailing list