[libcamera-devel,RFC,2/2] ipu3: Translate IPA protocol to new mojo interface
diff mbox series

Message ID 20210208234733.2637061-3-niklas.soderlund@ragnatech.se
State Superseded
Delegated to: Paul Elder
Headers show
Series
  • ipu3: Translate IPA protocol to new mojo interface
Related show

Commit Message

Niklas Söderlund Feb. 8, 2021, 11:47 p.m. UTC
Translate the IPA interface to the out-of-tree protocol based on mojo.
Please borrow, modify and squash anything in this patch to expedite the
integration of the new and _much_ nicer IPA IPC interface!

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/ipa/ipu3.h         | 23 -------
 include/libcamera/ipa/ipu3.mojom     | 43 +++++++++++++
 include/libcamera/ipa/meson.build    |  1 +
 src/ipa/ipu3/ipu3.cpp                | 74 ++++++++-------------
 src/ipa/ipu3/meson.build             |  6 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp | 96 +++++++++++-----------------
 6 files changed, 113 insertions(+), 130 deletions(-)
 delete mode 100644 include/libcamera/ipa/ipu3.h
 create mode 100644 include/libcamera/ipa/ipu3.mojom

Patch
diff mbox series

diff --git a/include/libcamera/ipa/ipu3.h b/include/libcamera/ipa/ipu3.h
deleted file mode 100644
index cbaaef04417b701b..0000000000000000
--- a/include/libcamera/ipa/ipu3.h
+++ /dev/null
@@ -1,23 +0,0 @@ 
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2020, Google Inc.
- *
- * ipu3.h - Image Processing Algorithm interface for IPU3
- */
-#ifndef __LIBCAMERA_IPA_INTERFACE_IPU3_H__
-#define __LIBCAMERA_IPA_INTERFACE_IPU3_H__
-
-#ifndef __DOXYGEN__
-
-enum IPU3Operations {
-	IPU3_IPA_ACTION_SET_SENSOR_CONTROLS = 1,
-	IPU3_IPA_ACTION_PARAM_FILLED = 2,
-	IPU3_IPA_ACTION_METADATA_READY = 3,
-	IPU3_IPA_EVENT_PROCESS_CONTROLS = 4,
-	IPU3_IPA_EVENT_STAT_READY = 5,
-	IPU3_IPA_EVENT_FILL_PARAMS = 6,
-};
-
-#endif /* __DOXYGEN__ */
-
-#endif /* __LIBCAMERA_IPA_INTERFACE_IPU3_H__ */
diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
new file mode 100644
index 0000000000000000..6ee1133392d0787c
--- /dev/null
+++ b/include/libcamera/ipa/ipu3.mojom
@@ -0,0 +1,43 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+module ipa.ipu3;
+
+import "include/libcamera/ipa/core.mojom";
+
+enum IPU3Operations {
+	ActionSetSensorControls = 1,
+	ActionParamFilled = 2,
+	ActionMetadataReady = 3,
+	EventProcessControls = 4,
+	EventStatReady = 5,
+	EventFillParams = 6,
+};
+
+struct IPU3Event {
+	IPU3Operations op;
+	uint32 frame;
+	uint32 bufferId;
+	ControlList controls;
+};
+
+struct IPU3Action {
+	IPU3Operations op;
+	ControlList controls;
+};
+
+interface IPAIPU3Interface {
+	init(IPASettings settings) => (int32 ret);
+	start() => (int32 ret);
+	stop();
+
+	configure(map<uint32, ControlInfoMap> entityControls) => ();
+
+	mapBuffers(array<IPABuffer> buffers);
+	unmapBuffers(array<uint32> ids);
+
+	[async] processEvent(IPU3Event ev);
+};
+
+interface IPAIPU3EventInterface {
+	queueFrameAction(uint32 frame, IPU3Action action);
+};
diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
index d701bccc7d7c3065..fe8aa65b7d0bfd6d 100644
--- a/include/libcamera/ipa/meson.build
+++ b/include/libcamera/ipa/meson.build
@@ -55,6 +55,7 @@  libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
                   ])
 
 ipa_mojom_files = [
+    'ipu3.mojom',
     'raspberrypi.mojom',
     'rkisp1.mojom',
     'vimc.mojom',
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index b11b03efa6ceb666..fcd8889cbd24fc8c 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -5,8 +5,6 @@ 
  * ipu3.cpp - IPU3 Image Processing Algorithms
  */
 
-#include <libcamera/ipa/ipu3.h>
-
 #include <stdint.h>
 #include <sys/mman.h>
 
@@ -17,10 +15,9 @@ 
 #include <libcamera/control_ids.h>
 #include <libcamera/ipa/ipa_interface.h>
 #include <libcamera/ipa/ipa_module_info.h>
+#include <libcamera/ipa/ipu3_ipa_interface.h>
 #include <libcamera/request.h>
 
-#include <libipa/ipa_interface_wrapper.h>
-
 #include "libcamera/internal/buffer.h"
 #include "libcamera/internal/log.h"
 
@@ -28,25 +25,21 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(IPAIPU3)
 
-class IPAIPU3 : public IPAInterface
+class IPAIPU3 : public ipa::ipu3::IPAIPU3Interface
 {
 public:
 	int init([[maybe_unused]] const IPASettings &settings) override
 	{
 		return 0;
 	}
-	int start([[maybe_unused]] const IPAOperationData &data,
-		  [[maybe_unused]] IPAOperationData *result) override { return 0; }
+	int start() override { return 0; }
 	void stop() override {}
 
-	void configure(const CameraSensorInfo &info,
-		       const std::map<unsigned int, IPAStream> &streamConfig,
-		       const std::map<unsigned int, const ControlInfoMap &> &entityControls,
-		       const IPAOperationData &ipaConfig,
-		       IPAOperationData *response) override;
+	void configure(const std::map<uint32_t, ControlInfoMap> &entityControls) override;
+
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
-	void processEvent(const IPAOperationData &event) override;
+	void processEvent(const ipa::ipu3::IPU3Event &event) override;
 
 private:
 	void processControls(unsigned int frame, const ControlList &controls);
@@ -69,11 +62,7 @@  private:
 	uint32_t maxGain_;
 };
 
-void IPAIPU3::configure([[maybe_unused]] const CameraSensorInfo &info,
-			[[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
-			const std::map<unsigned int, const ControlInfoMap &> &entityControls,
-			[[maybe_unused]] const IPAOperationData &ipaConfig,
-			[[maybe_unused]] IPAOperationData *result)
+void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls)
 {
 	if (entityControls.empty())
 		return;
@@ -123,19 +112,15 @@  void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
 	}
 }
 
-void IPAIPU3::processEvent(const IPAOperationData &event)
+void IPAIPU3::processEvent(const ipa::ipu3::IPU3Event &event)
 {
-	switch (event.operation) {
-	case IPU3_IPA_EVENT_PROCESS_CONTROLS: {
-		unsigned int frame = event.data[0];
-		processControls(frame, event.controls[0]);
+	switch (event.op) {
+	case ipa::ipu3::EventProcessControls: {
+		processControls(event.frame, event.controls);
 		break;
 	}
-	case IPU3_IPA_EVENT_STAT_READY: {
-		unsigned int frame = event.data[0];
-		unsigned int bufferId = event.data[1];
-
-		auto it = buffers_.find(bufferId);
+	case ipa::ipu3::EventStatReady: {
+		auto it = buffers_.find(event.bufferId);
 		if (it == buffers_.end()) {
 			LOG(IPAIPU3, Error) << "Could not find stats buffer!";
 			return;
@@ -145,14 +130,11 @@  void IPAIPU3::processEvent(const IPAOperationData &event)
 		const ipu3_uapi_stats_3a *stats =
 			reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
 
-		parseStatistics(frame, stats);
+		parseStatistics(event.frame, stats);
 		break;
 	}
-	case IPU3_IPA_EVENT_FILL_PARAMS: {
-		unsigned int frame = event.data[0];
-		unsigned int bufferId = event.data[1];
-
-		auto it = buffers_.find(bufferId);
+	case ipa::ipu3::EventFillParams: {
+		auto it = buffers_.find(event.bufferId);
 		if (it == buffers_.end()) {
 			LOG(IPAIPU3, Error) << "Could not find param buffer!";
 			return;
@@ -162,11 +144,11 @@  void IPAIPU3::processEvent(const IPAOperationData &event)
 		ipu3_uapi_params *params =
 			reinterpret_cast<ipu3_uapi_params *>(mem.data());
 
-		fillParams(frame, params);
+		fillParams(event.frame, params);
 		break;
 	}
 	default:
-		LOG(IPAIPU3, Error) << "Unknown event " << event.operation;
+		LOG(IPAIPU3, Error) << "Unknown event " << event.op;
 		break;
 	}
 }
@@ -184,8 +166,8 @@  void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
 
 	/* \todo Fill in parameters buffer. */
 
-	IPAOperationData op;
-	op.operation = IPU3_IPA_ACTION_PARAM_FILLED;
+	ipa::ipu3::IPU3Action op;
+	op.op = ipa::ipu3::ActionParamFilled;
 
 	queueFrameAction.emit(frame, op);
 
@@ -201,22 +183,22 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 	/* \todo React to statistics and update internal state machine. */
 	/* \todo Add meta-data information to ctrls. */
 
-	IPAOperationData op;
-	op.operation = IPU3_IPA_ACTION_METADATA_READY;
-	op.controls.push_back(ctrls);
+	ipa::ipu3::IPU3Action op;
+	op.op = ipa::ipu3::ActionMetadataReady;
+	op.controls = ctrls;
 
 	queueFrameAction.emit(frame, op);
 }
 
 void IPAIPU3::setControls(unsigned int frame)
 {
-	IPAOperationData op;
-	op.operation = IPU3_IPA_ACTION_SET_SENSOR_CONTROLS;
+	ipa::ipu3::IPU3Action op;
+	op.op = ipa::ipu3::ActionSetSensorControls;
 
 	ControlList ctrls(ctrls_);
 	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
 	ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));
-	op.controls.push_back(ctrls);
+	op.controls = ctrls;
 
 	queueFrameAction.emit(frame, op);
 }
@@ -233,9 +215,9 @@  const struct IPAModuleInfo ipaModuleInfo = {
 	"ipu3",
 };
 
-struct ipa_context *ipaCreate()
+IPAInterface *ipaCreate()
 {
-	return new IPAInterfaceWrapper(std::make_unique<IPAIPU3>());
+	return new IPAIPU3();
 }
 }
 
diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build
index 444c82453eac42ff..1ced00ae2ce4bf92 100644
--- a/src/ipa/ipu3/meson.build
+++ b/src/ipa/ipu3/meson.build
@@ -3,10 +3,10 @@ 
 ipa_name = 'ipa_ipu3'
 
 mod = shared_module(ipa_name,
-                    'ipu3.cpp',
+                    ['ipu3.cpp', libcamera_generated_ipa_headers],
                     name_prefix : '',
-                    include_directories : [ ipa_includes, libipa_includes ],
-                    dependencies : [ libatomic, libcamera_dep ],
+                    include_directories : [ipa_includes, libipa_includes],
+                    dependencies : libcamera_dep,
                     link_with : libipa,
                     install : true,
                     install_dir : ipa_install_dir)
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 61f7bf43ea08699a..c21bfa46be1ba85a 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -14,7 +14,9 @@ 
 #include <libcamera/camera.h>
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
-#include <libcamera/ipa/ipu3.h>
+#include <libcamera/ipa/core_ipa_interface.h>
+#include <libcamera/ipa/ipu3_ipa_interface.h>
+#include <libcamera/ipa/ipu3_ipa_proxy.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
@@ -77,8 +79,11 @@  public:
 	std::unique_ptr<DelayedControls> delayedCtrls_;
 	IPU3Frames frameInfos_;
 
+	std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
+
 private:
-	void queueFrameAction(unsigned int id, const IPAOperationData &op);
+	void queueFrameAction(unsigned int id,
+			      const ipa::ipu3::IPU3Action &action);
 };
 
 class IPU3CameraConfiguration : public CameraConfiguration
@@ -609,18 +614,14 @@  int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
 
 	for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {
 		buffer->setCookie(ipaBufferId++);
-		ipaBuffers_.push_back({
-			.id = buffer->cookie(),
-			.planes = buffer->planes()
-		});
+		ipaBuffers_.push_back(IPABuffer(buffer->cookie(),
+						buffer->planes()));
 	}
 
 	for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {
 		buffer->setCookie(ipaBufferId++);
-		ipaBuffers_.push_back({
-			.id = buffer->cookie(),
-			.planes = buffer->planes()
-		});
+		ipaBuffers_.push_back(IPABuffer(buffer->cookie(),
+						buffer->planes()));
 	}
 
 	data->ipa_->mapBuffers(ipaBuffers_);
@@ -650,16 +651,10 @@  int PipelineHandlerIPU3::freeBuffers(Camera *camera)
 
 int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *controls)
 {
+	std::map<uint32_t, ControlInfoMap> entityControls;
 	IPU3CameraData *data = cameraData(camera);
 	CIO2Device *cio2 = &data->cio2_;
 	ImgUDevice *imgu = data->imgu_;
-
-	CameraSensorInfo sensorInfo = {};
-	std::map<unsigned int, IPAStream> streamConfig;
-	std::map<unsigned int, const ControlInfoMap &> entityControls;
-	IPAOperationData ipaConfig;
-	IPAOperationData result = {};
-
 	int ret;
 
 	/* Allocate buffers for internal pipeline usage. */
@@ -667,8 +662,7 @@  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con
 	if (ret)
 		return ret;
 
-	IPAOperationData ipaData = {};
-	ret = data->ipa_->start(ipaData, nullptr);
+	ret = data->ipa_->start();
 	if (ret)
 		goto error;
 
@@ -684,24 +678,8 @@  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con
 	if (ret)
 		goto error;
 
-	/* Inform IPA of stream configuration and sensor controls. */
-	ret = data->cio2_.sensor()->sensorInfo(&sensorInfo);
-	if (ret)
-		goto error;
-
-	streamConfig[0] = {
-		.pixelFormat = data->outStream_.configuration().pixelFormat,
-		.size = data->outStream_.configuration().size,
-	};
-	streamConfig[1] = {
-		.pixelFormat = data->vfStream_.configuration().pixelFormat,
-		.size = data->vfStream_.configuration().size,
-	};
-
 	entityControls.emplace(0, data->cio2_.sensor()->controls());
-
-	data->ipa_->configure(sensorInfo, streamConfig, entityControls,
-			      ipaConfig, &result);
+	data->ipa_->configure(entityControls);
 
 	return 0;
 
@@ -751,11 +729,11 @@  int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
 
 	info->rawBuffer = rawBuffer;
 
-	IPAOperationData op;
-	op.operation = IPU3_IPA_EVENT_PROCESS_CONTROLS;
-	op.data = { info->id };
-	op.controls = { request->controls() };
-	data->ipa_->processEvent(op);
+	ipa::ipu3::IPU3Event ev;
+	ev.op = ipa::ipu3::EventProcessControls;
+	ev.frame = info->id;
+	ev.controls = request->controls();
+	data->ipa_->processEvent(ev);
 
 	return 0;
 }
@@ -1048,7 +1026,7 @@  int PipelineHandlerIPU3::registerCameras()
 
 int IPU3CameraData::loadIPA()
 {
-	ipa_ = IPAManager::createIPA(pipe_, 1, 1);
+	ipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe_, 1, 1);
 	if (!ipa_)
 		return -ENOENT;
 
@@ -1060,15 +1038,15 @@  int IPU3CameraData::loadIPA()
 }
 
 void IPU3CameraData::queueFrameAction(unsigned int id,
-				      const IPAOperationData &action)
+				      const ipa::ipu3::IPU3Action &action)
 {
-	switch (action.operation) {
-	case IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {
-		const ControlList &controls = action.controls[0];
+	switch (action.op) {
+	case ipa::ipu3::ActionSetSensorControls: {
+		const ControlList &controls = action.controls;
 		delayedCtrls_->push(controls);
 		break;
 	}
-	case IPU3_IPA_ACTION_PARAM_FILLED: {
+	case ipa::ipu3::ActionParamFilled: {
 		IPU3Frames::Info *info = frameInfos_.find(id);
 		if (!info)
 			break;
@@ -1090,13 +1068,13 @@  void IPU3CameraData::queueFrameAction(unsigned int id,
 
 		break;
 	}
-	case IPU3_IPA_ACTION_METADATA_READY: {
+	case ipa::ipu3::ActionMetadataReady: {
 		IPU3Frames::Info *info = frameInfos_.find(id);
 		if (!info)
 			break;
 
 		Request *request = info->request;
-		request->metadata() = action.controls[0];
+		request->metadata() = action.controls;
 		info->metadataProcessed = true;
 		if (frameInfos_.tryComplete(info))
 			pipe_->completeRequest(request);
@@ -1104,7 +1082,7 @@  void IPU3CameraData::queueFrameAction(unsigned int id,
 		break;
 	}
 	default:
-		LOG(IPU3, Error) << "Unknown action " << action.operation;
+		LOG(IPU3, Error) << "Unknown action " << action.op;
 		break;
 	}
 }
@@ -1172,10 +1150,11 @@  void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
 	if (request->findBuffer(&rawStream_))
 		pipe_->completeBuffer(request, buffer);
 
-	IPAOperationData op;
-	op.operation = IPU3_IPA_EVENT_FILL_PARAMS;
-	op.data = { info->id, info->paramBuffer->cookie() };
-	ipa_->processEvent(op);
+	ipa::ipu3::IPU3Event ev;
+	ev.op = ipa::ipu3::EventFillParams;
+	ev.frame = info->id;
+	ev.bufferId = info->paramBuffer->cookie();
+	ipa_->processEvent(ev);
 }
 
 void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
@@ -1202,10 +1181,11 @@  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
 		return;
 	}
 
-	IPAOperationData op;
-	op.operation = IPU3_IPA_EVENT_STAT_READY;
-	op.data = { info->id, info->statBuffer->cookie() };
-	ipa_->processEvent(op);
+	ipa::ipu3::IPU3Event ev;
+	ev.op = ipa::ipu3::EventStatReady;
+	ev.frame = info->id;
+	ev.bufferId = info->statBuffer->cookie();
+	ipa_->processEvent(ev);
 }
 
 REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)