[libcamera-devel] [PATCH v2 22/24] ipa: Allow short-circuiting the ipa_context_ops

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Nov 8 21:54:07 CET 2019


When an IPA module is loaded without isolation and implements the
IPAInterface internally, going through ipa_context_ops is a waste of
time. Add an operation to retrieve the IPAInterface, and use it directly
in the IPAContextWrapper.

For debugging purpose, make it possible to forcing usage of the C API by
defining the LIBCAMERA_IPA_FORCE_C_API environment variable.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
---
 include/ipa/ipa_interface.h                 |  1 +
 src/ipa/libipa/ipa_interface_wrapper.cpp    |  8 +++++
 src/ipa/libipa/ipa_interface_wrapper.h      |  1 +
 src/libcamera/include/ipa_context_wrapper.h |  4 +++
 src/libcamera/ipa_context_wrapper.cpp       | 34 +++++++++++++++++++--
 src/libcamera/ipa_interface.cpp             | 18 +++++++++++
 6 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
index 92f1aac50b85..0ea53d120fe1 100644
--- a/include/ipa/ipa_interface.h
+++ b/include/ipa/ipa_interface.h
@@ -62,6 +62,7 @@ struct ipa_callback_ops {
 
 struct ipa_context_ops {
 	void (*destroy)(struct ipa_context *ctx);
+	void *(*get_interface)(struct ipa_context *ctx);
 	void (*init)(struct ipa_context *ctx);
 	void (*register_callbacks)(struct ipa_context *ctx,
 				   const struct ipa_callback_ops *callbacks,
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index 80c5648ffed6..6a389dfa714a 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -72,6 +72,13 @@ void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx)
 	delete ctx;
 }
 
+void *IPAInterfaceWrapper::get_interface(struct ipa_context *_ctx)
+{
+	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
+
+	return ctx->ipa_.get();
+}
+
 void IPAInterfaceWrapper::init(struct ipa_context *_ctx)
 {
 	IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
@@ -228,6 +235,7 @@ void IPAInterfaceWrapper::queueFrameAction(unsigned int frame,
  */
 const struct ipa_context_ops IPAInterfaceWrapper::operations_ = {
 	.destroy = &IPAInterfaceWrapper::destroy,
+	.get_interface = &IPAInterfaceWrapper::get_interface,
 	.init = &IPAInterfaceWrapper::init,
 	.register_callbacks = &IPAInterfaceWrapper::register_callbacks,
 	.configure = &IPAInterfaceWrapper::configure,
diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h
index 17be2062b6c7..3fb7b4476439 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.h
+++ b/src/ipa/libipa/ipa_interface_wrapper.h
@@ -22,6 +22,7 @@ public:
 
 private:
 	static void destroy(struct ipa_context *ctx);
+	static void *get_interface(struct ipa_context *ctx);
 	static void init(struct ipa_context *ctx);
 	static void register_callbacks(struct ipa_context *ctx,
 				       const struct ipa_callback_ops *callbacks,
diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h
index 060888218838..c9e194120de6 100644
--- a/src/libcamera/include/ipa_context_wrapper.h
+++ b/src/libcamera/include/ipa_context_wrapper.h
@@ -33,7 +33,11 @@ private:
 				       struct ipa_operation_data &data);
 	static const struct ipa_callback_ops callbacks_;
 
+	void doQueueFrameAction(unsigned int frame,
+				const IPAOperationData &data);
+
 	struct ipa_context *ctx_;
+	IPAInterface *intf_;
 
 	ControlSerializer serializer_;
 };
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index 66fc59b82373..0efbae0f675f 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -12,6 +12,7 @@
 #include <libcamera/controls.h>
 
 #include "byte_stream_buffer.h"
+#include "utils.h"
 
 /**
  * \file ipa_context_wrapper.h
@@ -43,11 +44,19 @@ namespace libcamera {
  * with it.
  */
 IPAContextWrapper::IPAContextWrapper(struct ipa_context *context)
-	: ctx_(context)
+	: ctx_(context), intf_(nullptr)
 {
 	if (!ctx_)
 		return;
 
+	bool forceCApi = !!utils::secure_getenv("LIBCAMERA_IPA_FORCE_C_API");
+
+	if (!forceCApi && ctx_ && ctx_->ops->get_interface) {
+		intf_ = reinterpret_cast<IPAInterface *>(ctx_->ops->get_interface(ctx_));
+		intf_->queueFrameAction.connect(this, &IPAContextWrapper::doQueueFrameAction);
+		return;
+	}
+
 	ctx_->ops->register_callbacks(ctx_, &IPAContextWrapper::callbacks_,
 				      this);
 }
@@ -60,6 +69,9 @@ IPAContextWrapper::~IPAContextWrapper()
 
 int IPAContextWrapper::init()
 {
+	if (intf_)
+		return intf_->init();
+
 	if (!ctx_)
 		return 0;
 
@@ -71,6 +83,9 @@ int IPAContextWrapper::init()
 void IPAContextWrapper::configure(const std::map<unsigned int, IPAStream> &streamConfig,
 				  const std::map<unsigned int, const ControlInfoMap &> &entityControls)
 {
+	if (intf_)
+		return intf_->configure(streamConfig, entityControls);
+
 	if (!ctx_)
 		return;
 
@@ -121,6 +136,9 @@ void IPAContextWrapper::configure(const std::map<unsigned int, IPAStream> &strea
 
 void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
 {
+	if (intf_)
+		return intf_->mapBuffers(buffers);
+
 	if (!ctx_)
 		return;
 
@@ -146,6 +164,9 @@ void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
 
 void IPAContextWrapper::unmapBuffers(const std::vector<unsigned int> &ids)
 {
+	if (intf_)
+		return intf_->unmapBuffers(ids);
+
 	if (!ctx_)
 		return;
 
@@ -154,6 +175,9 @@ void IPAContextWrapper::unmapBuffers(const std::vector<unsigned int> &ids)
 
 void IPAContextWrapper::processEvent(const IPAOperationData &data)
 {
+	if (intf_)
+		return intf_->processEvent(data);
+
 	if (!ctx_)
 		return;
 
@@ -187,6 +211,12 @@ void IPAContextWrapper::processEvent(const IPAOperationData &data)
 	ctx_->ops->process_event(ctx_, &c_data);
 }
 
+void IPAContextWrapper::doQueueFrameAction(unsigned int frame,
+					   const IPAOperationData &data)
+{
+	IPAInterface::queueFrameAction.emit(frame, data);
+}
+
 void IPAContextWrapper::queue_frame_action(void *ctx, unsigned int frame,
 					   struct ipa_operation_data &data)
 {
@@ -203,7 +233,7 @@ void IPAContextWrapper::queue_frame_action(void *ctx, unsigned int frame,
 		opData.controls.push_back(_this->serializer_.deserialize<ControlList>(b));
 	}
 
-	_this->queueFrameAction.emit(frame, opData);
+	_this->doQueueFrameAction(frame, opData);
 }
 
 #ifndef __DOXYGEN__
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index cb2767a04711..715e7972271b 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -62,6 +62,12 @@
  * handlers to communicate with IPA modules. IPA modules may use the
  * IPAInterface API internally if they want to benefit from the data and helper
  * classes offered by libcamera.
+ *
+ * When an IPA module is loaded directly into the libcamera process and uses
+ * the IPAInterface API internally, short-circuiting the path to the
+ * ipa_context_ops and back to IPAInterface is desirable. To support this, IPA
+ * modules may implement the ipa_context_ops::get_interface function to return a
+ * pointer to their internal IPAInterface.
  */
 
 /**
@@ -209,6 +215,18 @@
  * \param[in] ctx The IPA context
  */
 
+/**
+ * \var ipa_context_ops::get_interface
+ * \brief Retrieve the IPAInterface implemented by the ipa_context (optional)
+ * \param[in] ctx The IPA context
+ *
+ * IPA modules may implement this function to expose their internal
+ * IPAInterface, if any. When implemented, libcamera may at its sole discretion
+ * call it and then bypass the ipa_context_ops API by calling the IPAInterface
+ * methods directly. IPA modules shall still implement and support the full
+ * ipa_context_ops API.
+ */
+
 /**
  * \var ipa_context_ops::init
  * \brief Initialise the IPA context
-- 
Regards,

Laurent Pinchart



More information about the libcamera-devel mailing list