pipeline: ipa: raspberrypi: Open the CamHelper on ipa::init()

Move the opening of the CamHelper from ipa::configure() to ipa::init().
This allows the pipeline handler to get the sensor specific parameters
in pipeline_handler::match() where the ipa is initialised.

Having the sensor parameters available earlier will allow selective
use of the embedded data node in a future change.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

include/libcamera/ipa/raspberrypi.mojom

@@ -15,10 +15,6 @@ enum BufferMask {
 /* Size of the LS grid allocation. */
 const uint32 MaxLsGridSize = 0x8000;
 
-enum ConfigOutputParameters {
-	ConfigSensorParams = 0x01,
-};
-
 struct SensorConfig {
 	uint32 gainDelay;
 	uint32 exposureDelay;
@@ -40,8 +36,6 @@ struct ConfigInput {
 };
 
 struct ConfigOutput {
-	uint32 params;
-	SensorConfig sensorConfig;
 	ControlList controls;
 };
 
@@ -51,7 +45,7 @@ struct StartControls {
 };
 
 interface IPARPiInterface {
-	init(IPASettings settings) => (int32 ret);
+	init(IPASettings settings) => (int32 ret, SensorConfig sensorConfig);
 	start(StartControls controls) => (StartControls result);
 	stop();
 

src/ipa/raspberrypi/raspberrypi.cpp

@@ -79,7 +79,7 @@ public:
 			munmap(lsTable_, ipa::RPi::MaxLsGridSize);
 	}
 
-	int init(const IPASettings &settings) override;
+	int init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig) override;
 	void start(const ipa::RPi::StartControls &data,
 		   ipa::RPi::StartControls *result) override;
 	void stop() override {}
@@ -164,9 +164,35 @@ private:
 	double maxFrameDuration_;
 };
 
-int IPARPi::init(const IPASettings &settings)
+int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)
 {
 	tuningFile_ = settings.configurationFile;
+
+	/*
+	 * Load the "helper" for this sensor. This tells us all the device specific stuff
+	 * that the kernel driver doesn't. We only do this the first time; we don't need
+	 * to re-parse the metadata after a simple mode-switch for no reason.
+	 */
+	helper_ = std::unique_ptr<RPiController::CamHelper>(RPiController::CamHelper::Create(settings.sensorModel));
+	if (!helper_) {
+		LOG(IPARPI, Error) << "Could not create camera helper for "
+				   << settings.sensorModel;
+		return -EINVAL;
+	}
+
+	/*
+	 * Pass out the sensor config to the pipeline handler in order
+	 * to setup the staggered writer class.
+	 */
+	int gainDelay, exposureDelay, vblankDelay, sensorMetadata;
+	helper_->GetDelays(exposureDelay, gainDelay, vblankDelay);
+	sensorMetadata = helper_->SensorEmbeddedDataPresent();
+
+	sensorConfig->gainDelay = gainDelay;
+	sensorConfig->exposureDelay = exposureDelay;
+	sensorConfig->vblankDelay = vblankDelay;
+	sensorConfig->sensorMetadata = sensorMetadata;
+
 	return 0;
 }
 
@@ -301,8 +327,6 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		return -1;
 	}
 
-	result->params = 0;
-
 	sensorCtrls_ = entityControls.at(0);
 	ispCtrls_ = entityControls.at(1);
 
@@ -319,36 +343,6 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo,
 	/* Setup a metadata ControlList to output metadata. */
 	libcameraMetadata_ = ControlList(controls::controls);
 
-	/*
-	 * Load the "helper" for this sensor. This tells us all the device specific stuff
-	 * that the kernel driver doesn't. We only do this the first time; we don't need
-	 * to re-parse the metadata after a simple mode-switch for no reason.
-	 */
-	std::string cameraName(sensorInfo.model);
-	if (!helper_) {
-		helper_ = std::unique_ptr<RPiController::CamHelper>(RPiController::CamHelper::Create(cameraName));
-
-		if (!helper_) {
-			LOG(IPARPI, Error) << "Could not create camera helper for "
-					   << cameraName;
-			return -1;
-		}
-
-		/*
-		 * Pass out the sensor config to the pipeline handler in order
-		 * to setup the staggered writer class.
-		 */
-		int gainDelay, exposureDelay, vblankDelay, sensorMetadata;
-		helper_->GetDelays(exposureDelay, gainDelay, vblankDelay);
-		sensorMetadata = helper_->SensorEmbeddedDataPresent();
-
-		result->params |= ipa::RPi::ConfigSensorParams;
-		result->sensorConfig.gainDelay = gainDelay;
-		result->sensorConfig.exposureDelay = exposureDelay;
-		result->sensorConfig.vblankDelay = vblankDelay;
-		result->sensorConfig.sensorMetadata = sensorMetadata;
-	}
-
 	/* Re-assemble camera mode using the sensor info. */
 	setMode(sensorInfo);
 

src/libcamera/pipeline/raspberrypi/raspberrypi.cpp

@@ -146,7 +146,7 @@ public:
 
 	void frameStarted(uint32_t sequence);
 
-	int loadIPA();
+	int loadIPA(ipa::RPi::SensorConfig *sensorConfig);
 	int configureIPA(const CameraConfiguration *config);
 
 	void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);
@@ -1030,11 +1030,24 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
 	if (data->sensor_->init())
 		return false;
 
-	if (data->loadIPA()) {
+	ipa::RPi::SensorConfig sensorConfig;
+	if (data->loadIPA(&sensorConfig)) {
 		LOG(RPI, Error) << "Failed to load a suitable IPA library";
 		return false;
 	}
 
+	/*
+	 * Setup our delayed control writer with the sensor default
+	 * gain and exposure delays. Mark VBLANK for priority write.
+	 */
+	std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
+		{ V4L2_CID_ANALOGUE_GAIN, { sensorConfig.gainDelay, false } },
+		{ V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false } },
+		{ V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }
+	};
+	data->delayedCtrls_ = std::make_unique<DelayedControls>(data->unicam_[Unicam::Image].dev(), params);
+	data->sensorMetadata_ = sensorConfig.sensorMetadata;
+
 	/* Register the controls that the Raspberry Pi IPA can handle. */
 	data->controlInfo_ = RPi::Controls;
 	/* Initialize the camera properties. */
@@ -1214,7 +1227,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)
 	delayedCtrls_->applyControls(sequence);
 }
 
-int RPiCameraData::loadIPA()
+int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)
 {
 	ipa_ = IPAManager::createIPA<ipa::RPi::IPAProxyRPi>(pipe_, 1, 1);
 
@@ -1230,7 +1243,7 @@ int RPiCameraData::loadIPA()
 	IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"),
 			     sensor_->model());
 
-	return ipa_->init(settings);
+	return ipa_->init(settings, sensorConfig);
 }
 
 int RPiCameraData::configureIPA(const CameraConfiguration *config)
@@ -1293,21 +1306,6 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
 		return -EPIPE;
 	}
 
-	if (result.params & ipa::RPi::ConfigSensorParams) {
-		/*
-		 * Setup our delayed control writer with the sensor default
-		 * gain and exposure delays. Mark VBLANK for priority write.
-		 */
-		std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
-			{ V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },
-			{ V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },
-			{ V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }
-		};
-
-		delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), params);
-		sensorMetadata_ = result.sensorConfig.sensorMetadata;
-	}
-
 	if (!result.controls.empty()) {
 		ControlList &ctrls = result.controls;
 		unicam_[Unicam::Image].dev()->setControls(&ctrls);