libcamera: pipeline_handler: Add createIPA() function

IPA proxies are created with a call to IPAManager::createIPA(), which is
a static member function. This requires a complicated dance in the
createIPA() function to retrieve the IPAManager instance through the
camera manager, itself retrieved from the pipeline handler. Simplify the
code by turning IPAManager::createIPA() into a non-static member
function, and providing a wrapper in the PipelineHandler class to keep
instantiation of IPA proxies easy in pipeline handlers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>

Documentation/guides/ipa.rst

@@ -387,20 +387,20 @@ In the pipeline handler, we first need to construct a specialized IPA proxy.
 From the point of view of the pipeline hander, this is the object that is the
 IPA.
 
-To do so, we invoke the IPAManager:
+To do so, we call the PipelineHandler::createIPA() function:
 
 .. code-block:: C++
 
         std::unique_ptr<ipa::rpi::IPAProxyRPi> ipa_ =
-                IPAManager::createIPA<ipa::rpi::IPAProxyRPi>(pipe_, 1, 1);
+                pipe_->createIPA<ipa::rpi::IPAProxyRPi>(1, 1);
 
 The ipa::rpi namespace comes from the namespace that we defined in the mojo
 data definition file, in the "Namespacing" section. The name of the proxy,
 IPAProxyRPi, comes from the name given to the main IPA interface,
 IPARPiInterface, in the "The Main IPA interface" section.
 
-The return value of IPAManager::createIPA shall be error-checked, to confirm
-that the returned pointer is not a nullptr.
+The return value of createIPA() shall be error-checked, to confirm that the
+returned pointer is not a nullptr.
 
 After this, before initializing the IPA, slots should be connected to all of
 the IPA's signals, as defined in the Event IPA interface:

include/libcamera/internal/ipa_manager.h

@@ -17,15 +17,16 @@
 #include <libcamera/ipa/ipa_module_info.h>
 
 #include "libcamera/internal/camera_manager.h"
-#include "libcamera/internal/global_configuration.h"
-#include "libcamera/internal/ipa_module.h"
-#include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/pub_key.h"
 
 namespace libcamera {
 
 LOG_DECLARE_CATEGORY(IPAManager)
 
+class GlobalConfiguration;
+class IPAModule;
+class PipelineHandler;
+
 class IPAManager
 {
 public:
@@ -33,23 +34,18 @@ public:
 	~IPAManager();
 
 	template<typename T>
-	static std::unique_ptr<T> createIPA(PipelineHandler *pipe,
-					    uint32_t minVersion,
-					    uint32_t maxVersion)
+	std::unique_ptr<T> createIPA(PipelineHandler *pipe, uint32_t minVersion,
+				     uint32_t maxVersion)
 	{
-		CameraManager *cm = pipe->cameraManager();
-		IPAManager *self = cm->_d()->ipaManager();
-		IPAModule *m = self->module(pipe, minVersion, maxVersion);
+		IPAModule *m = module(pipe, minVersion, maxVersion);
 		if (!m)
 			return nullptr;
 
-		const GlobalConfiguration &configuration = cm->_d()->configuration();
-
 		auto proxy = [&]() -> std::unique_ptr<T> {
-			if (self->isSignatureValid(m))
-				return std::make_unique<typename T::Threaded>(m, configuration);
+			if (isSignatureValid(m))
+				return std::make_unique<typename T::Threaded>(m, configuration_);
 			else
-				return std::make_unique<typename T::Isolated>(m, configuration);
+				return std::make_unique<typename T::Isolated>(m, configuration_);
 		}();
 
 		if (!proxy->isValid()) {
@@ -77,6 +73,7 @@ private:
 
 	bool isSignatureValid(IPAModule *ipa) const;
 
+	const GlobalConfiguration &configuration_;
 	std::vector<std::unique_ptr<IPAModule>> modules_;
 
 #if HAVE_IPA_PUBKEY

include/libcamera/internal/pipeline_handler.h

@@ -17,11 +17,13 @@
 #include <libcamera/controls.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/camera_manager.h"
+#include "libcamera/internal/ipa_manager.h"
+
 namespace libcamera {
 
 class Camera;
 class CameraConfiguration;
-class CameraManager;
 class DeviceEnumerator;
 class DeviceMatch;
 class FrameBuffer;
@@ -70,6 +72,13 @@ public:
 
 	CameraManager *cameraManager() const { return manager_; }
 
+	template<typename T>
+	std::unique_ptr<T> createIPA(uint32_t minVersion, uint32_t maxVersion)
+	{
+		IPAManager *ipaManager = manager_->_d()->ipaManager();
+		return ipaManager->createIPA<T>(this, minVersion, maxVersion);
+	}
+
 protected:
 	void registerCamera(std::shared_ptr<Camera> camera);
 	void hotplugMediaDevice(std::shared_ptr<MediaDevice> media);

src/libcamera/ipa_manager.cpp

@@ -105,6 +105,7 @@ LOG_DEFINE_CATEGORY(IPAManager)
  * CameraManager.
  */
 IPAManager::IPAManager(const GlobalConfiguration &configuration)
+	: configuration_(configuration)
 {
 #if HAVE_IPA_PUBKEY
 	if (!pubKey_.isValid())

src/libcamera/pipeline/ipu3/ipu3.cpp

@@ -31,7 +31,6 @@
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/framebuffer.h"
-#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/request.h"
@@ -1151,7 +1150,7 @@ int PipelineHandlerIPU3::registerCameras()
 
 int IPU3CameraData::loadIPA()
 {
-	ipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe(), 1, 1);
+	ipa_ = pipe()->createIPA<ipa::ipu3::IPAProxyIPU3>(1, 1);
 	if (!ipa_)
 		return -ENOENT;
 

src/libcamera/pipeline/mali-c55/mali-c55.cpp

@@ -35,7 +35,6 @@
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/framebuffer.h"
-#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/request.h"
@@ -382,7 +381,7 @@ int MaliC55CameraData::loadIPA()
 	if (!sensor_)
 		return 0;
 
-	ipa_ = IPAManager::createIPA<ipa::mali_c55::IPAProxyMaliC55>(pipe(), 1, 1);
+	ipa_ = pipe()->createIPA<ipa::mali_c55::IPAProxyMaliC55>(1, 1);
 	if (!ipa_)
 		return -ENOENT;
 

src/libcamera/pipeline/rkisp1/rkisp1.cpp

@@ -40,7 +40,6 @@
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/framebuffer.h"
-#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/media_pipeline.h"
 #include "libcamera/internal/pipeline_handler.h"
@@ -395,7 +394,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const
 
 int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks)
 {
-	ipa_ = IPAManager::createIPA<ipa::rkisp1::IPAProxyRkISP1>(pipe(), 1, 1);
+	ipa_ = pipe()->createIPA<ipa::rkisp1::IPAProxyRkISP1>(1, 1);
 	if (!ipa_)
 		return -ENOENT;
 

src/libcamera/pipeline/rpi/common/pipeline_base.cpp

@@ -20,7 +20,6 @@
 #include <libcamera/property_ids.h>
 
 #include "libcamera/internal/camera_lens.h"
-#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/v4l2_subdevice.h"
 
 using namespace std::chrono_literals;
@@ -1161,7 +1160,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)
 {
 	int ret;
 
-	ipa_ = IPAManager::createIPA<ipa::RPi::IPAProxyRPi>(pipe(), 1, 1);
+	ipa_ = pipe()->createIPA<ipa::RPi::IPAProxyRPi>(1, 1);
 
 	if (!ipa_)
 		return -ENOENT;

src/libcamera/pipeline/vimc/vimc.cpp

@@ -35,7 +35,6 @@
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/framebuffer.h"
-#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/request.h"
@@ -488,7 +487,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
 	if (data->init())
 		return false;
 
-	data->ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(this, 0, 0);
+	data->ipa_ = createIPA<ipa::vimc::IPAProxyVimc>(0, 0);
 	if (!data->ipa_) {
 		LOG(VIMC, Error) << "no matching IPA found";
 		return false;

src/libcamera/pipeline_handler.cpp

@@ -835,6 +835,16 @@ void PipelineHandler::disconnect()
  * \return The CameraManager for this pipeline handler
  */
 
+/**
+ * \fn PipelineHandler::createIPA()
+ * \brief Create an IPA proxy that matches this pipeline handler
+ * \param[in] minVersion Minimum acceptable version of IPA module
+ * \param[in] maxVersion Maximum acceptable version of IPA module
+ *
+ * \return A newly created IPA proxy, or nullptr if no matching IPA module is
+ * found or if the IPA proxy fails to initialize
+ */
+
 /**
  * \class PipelineHandlerFactoryBase
  * \brief Base class for pipeline handler factories

src/libcamera/software_isp/software_isp.cpp

@@ -23,7 +23,6 @@
 
 #include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/framebuffer.h"
-#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/software_isp/debayer_params.h"
 
 #include "debayer_cpu.h"
@@ -127,7 +126,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 	debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);
 	debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);
 
-	ipa_ = IPAManager::createIPA<ipa::soft::IPAProxySoft>(pipe, 0, 0);
+	ipa_ = pipe->createIPA<ipa::soft::IPAProxySoft>(0, 0);
 	if (!ipa_) {
 		LOG(SoftwareIsp, Error)
 			<< "Creating IPA for software ISP failed";

test/ipa/ipa_interface_test.cpp

@@ -22,7 +22,6 @@
 
 #include "libcamera/internal/camera_manager.h"
 #include "libcamera/internal/device_enumerator.h"
-#include "libcamera/internal/ipa_manager.h"
 #include "libcamera/internal/ipa_module.h"
 #include "libcamera/internal/pipeline_handler.h"
 
@@ -99,7 +98,7 @@ protected:
 		EventDispatcher *dispatcher = thread()->eventDispatcher();
 		Timer timer;
 
-		ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(pipe_.get(), 0, 0);
+		ipa_ = pipe_->createIPA<ipa::vimc::IPAProxyVimc>(0, 0);
 		if (!ipa_) {
 			cerr << "Failed to create VIMC IPA interface" << endl;
 			return TestFail;