[libcamera-devel] [PATCH v7 11/12] libcamera: pipeline, ipa: rkisp1: Support the new IPC mechanism

Paul Elder paul.elder at ideasonboard.com
Thu Feb 11 08:18:45 CET 2021


Add support to the rkisp1 pipeline handler and IPA for the new IPC
mechanism.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---
No change in v7

Changes in v6:
- move the enum and const from the header to the mojom file
- remove the per-pipeline ControlInfoMap
- since rkisp1.h has nothing in it anymore, remove it
- use the namespace in the pipeline handler and IPA

Changes in v5:
- import the generated header with the new file name

Changes in v4:
- rename Controls to controls
- rename IPARkISP1CallbackInterface to IPARkISP1EventInterface
- rename libcamera_generated_headers to libcamera_generated_ipa_headers
  in meson
- use the new header name, rkisp1_ipa_interface.h

Changes in v3:
- change namespacing of base ControlInfoMap structure

New in v2
---
 include/libcamera/ipa/meson.build        |  1 +
 include/libcamera/ipa/rkisp1.h           | 22 -------
 include/libcamera/ipa/rkisp1.mojom       | 44 +++++++++++++
 src/ipa/rkisp1/meson.build               |  2 +-
 src/ipa/rkisp1/rkisp1.cpp                | 61 +++++++++---------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 79 ++++++++++++------------
 6 files changed, 115 insertions(+), 94 deletions(-)
 delete mode 100644 include/libcamera/ipa/rkisp1.h
 create mode 100644 include/libcamera/ipa/rkisp1.mojom

diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
index 67e3f049..d701bccc 100644
--- a/include/libcamera/ipa/meson.build
+++ b/include/libcamera/ipa/meson.build
@@ -56,6 +56,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
 
 ipa_mojom_files = [
     'raspberrypi.mojom',
+    'rkisp1.mojom',
     'vimc.mojom',
 ]
 
diff --git a/include/libcamera/ipa/rkisp1.h b/include/libcamera/ipa/rkisp1.h
deleted file mode 100644
index bb824f29..00000000
--- a/include/libcamera/ipa/rkisp1.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2019, Google Inc.
- *
- * rkisp1.h - Image Processing Algorithm interface for RkISP1
- */
-#ifndef __LIBCAMERA_IPA_INTERFACE_RKISP1_H__
-#define __LIBCAMERA_IPA_INTERFACE_RKISP1_H__
-
-#ifndef __DOXYGEN__
-
-enum RkISP1Operations {
-	RKISP1_IPA_ACTION_V4L2_SET = 1,
-	RKISP1_IPA_ACTION_PARAM_FILLED = 2,
-	RKISP1_IPA_ACTION_METADATA = 3,
-	RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER = 4,
-	RKISP1_IPA_EVENT_QUEUE_REQUEST = 5,
-};
-
-#endif /* __DOXYGEN__ */
-
-#endif /* __LIBCAMERA_IPA_INTERFACE_RKISP1_H__ */
diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
new file mode 100644
index 00000000..9270f9c7
--- /dev/null
+++ b/include/libcamera/ipa/rkisp1.mojom
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+module ipa.rkisp1;
+
+import "include/libcamera/ipa/core.mojom";
+
+enum RkISP1Operations {
+	ActionV4L2Set = 1,
+	ActionParamFilled = 2,
+	ActionMetadata = 3,
+	EventSignalStatBuffer = 4,
+	EventQueueRequest = 5,
+};
+
+struct RkISP1Event {
+	RkISP1Operations op;
+	uint32 frame;
+	uint32 bufferId;
+	ControlList controls;
+};
+
+struct RkISP1Action {
+	RkISP1Operations op;
+	ControlList controls;
+};
+
+interface IPARkISP1Interface {
+	init(IPASettings settings) => (int32 ret);
+	start() => (int32 ret);
+	stop();
+
+	configure(CameraSensorInfo sensorInfo,
+		  map<uint32, IPAStream> streamConfig,
+		  map<uint32, ControlInfoMap> entityControls) => ();
+
+	mapBuffers(array<IPABuffer> buffers);
+	unmapBuffers(array<uint32> ids);
+
+	[async] processEvent(RkISP1Event ev);
+};
+
+interface IPARkISP1EventInterface {
+	queueFrameAction(uint32 frame, RkISP1Action action);
+};
diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build
index ed9a6b6b..3061d53c 100644
--- a/src/ipa/rkisp1/meson.build
+++ b/src/ipa/rkisp1/meson.build
@@ -3,7 +3,7 @@
 ipa_name = 'ipa_rkisp1'
 
 mod = shared_module(ipa_name,
-                    'rkisp1.cpp',
+                    ['rkisp1.cpp', libcamera_generated_ipa_headers],
                     name_prefix : '',
                     include_directories : [ipa_includes, libipa_includes],
                     dependencies : libcamera_dep,
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index f4812d8d..67bac986 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -19,7 +19,7 @@
 #include <libcamera/control_ids.h>
 #include <libcamera/ipa/ipa_interface.h>
 #include <libcamera/ipa/ipa_module_info.h>
-#include <libcamera/ipa/rkisp1.h>
+#include <libcamera/ipa/rkisp1_ipa_interface.h>
 #include <libcamera/request.h>
 
 #include "libcamera/internal/log.h"
@@ -28,25 +28,22 @@ namespace libcamera {
 
 LOG_DEFINE_CATEGORY(IPARkISP1)
 
-class IPARkISP1 : public IPAInterface
+class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface
 {
 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;
+		       const std::map<uint32_t, IPAStream> &streamConfig,
+		       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::rkisp1::RkISP1Event &event) override;
 
 private:
 	void queueRequest(unsigned int frame, rkisp1_params_cfg *params,
@@ -79,10 +76,8 @@ private:
  * before accessing them.
  */
 void IPARkISP1::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)
+			  [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
+			  const std::map<uint32_t, ControlInfoMap> &entityControls)
 {
 	if (entityControls.empty())
 		return;
@@ -158,12 +153,12 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
 	}
 }
 
-void IPARkISP1::processEvent(const IPAOperationData &event)
+void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)
 {
-	switch (event.operation) {
-	case RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER: {
-		unsigned int frame = event.data[0];
-		unsigned int bufferId = event.data[1];
+	switch (event.op) {
+	case ipa::rkisp1::EventSignalStatBuffer: {
+		unsigned int frame = event.frame;
+		unsigned int bufferId = event.bufferId;
 
 		const rkisp1_stat_buffer *stats =
 			static_cast<rkisp1_stat_buffer *>(buffersMemory_[bufferId]);
@@ -171,18 +166,18 @@ void IPARkISP1::processEvent(const IPAOperationData &event)
 		updateStatistics(frame, stats);
 		break;
 	}
-	case RKISP1_IPA_EVENT_QUEUE_REQUEST: {
-		unsigned int frame = event.data[0];
-		unsigned int bufferId = event.data[1];
+	case ipa::rkisp1::EventQueueRequest: {
+		unsigned int frame = event.frame;
+		unsigned int bufferId = event.bufferId;
 
 		rkisp1_params_cfg *params =
 			static_cast<rkisp1_params_cfg *>(buffersMemory_[bufferId]);
 
-		queueRequest(frame, params, event.controls[0]);
+		queueRequest(frame, params, event.controls);
 		break;
 	}
 	default:
-		LOG(IPARkISP1, Error) << "Unknown event " << event.operation;
+		LOG(IPARkISP1, Error) << "Unknown event " << event.op;
 		break;
 	}
 }
@@ -202,8 +197,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
 		params->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;
 	}
 
-	IPAOperationData op;
-	op.operation = RKISP1_IPA_ACTION_PARAM_FILLED;
+	ipa::rkisp1::RkISP1Action op;
+	op.op = ipa::rkisp1::ActionParamFilled;
 
 	queueFrameAction.emit(frame, op);
 }
@@ -255,13 +250,13 @@ void IPARkISP1::updateStatistics(unsigned int frame,
 
 void IPARkISP1::setControls(unsigned int frame)
 {
-	IPAOperationData op;
-	op.operation = RKISP1_IPA_ACTION_V4L2_SET;
+	ipa::rkisp1::RkISP1Action op;
+	op.op = ipa::rkisp1::ActionV4L2Set;
 
 	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);
 }
@@ -273,9 +268,9 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)
 	if (aeState)
 		ctrls.set(controls::AeLocked, aeState == 2);
 
-	IPAOperationData op;
-	op.operation = RKISP1_IPA_ACTION_METADATA;
-	op.controls.push_back(ctrls);
+	ipa::rkisp1::RkISP1Action op;
+	op.op = ipa::rkisp1::ActionMetadata;
+	op.controls = ctrls;
 
 	queueFrameAction.emit(frame, op);
 }
@@ -292,9 +287,9 @@ const struct IPAModuleInfo ipaModuleInfo = {
 	"rkisp1",
 };
 
-struct ipa_context *ipaCreate()
+IPAInterface *ipaCreate()
 {
-	return new IPAInterfaceWrapper(std::make_unique<IPARkISP1>());
+	return new IPARkISP1();
 }
 }
 
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index e85979a7..23c95100 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -18,7 +18,9 @@
 #include <libcamera/camera.h>
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
-#include <libcamera/ipa/rkisp1.h>
+#include <libcamera/ipa/core_ipa_interface.h>
+#include <libcamera/ipa/rkisp1_ipa_interface.h>
+#include <libcamera/ipa/rkisp1_ipa_proxy.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
@@ -96,9 +98,11 @@ public:
 	RkISP1MainPath *mainPath_;
 	RkISP1SelfPath *selfPath_;
 
+	std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
+
 private:
 	void queueFrameAction(unsigned int frame,
-			      const IPAOperationData &action);
+			      const ipa::rkisp1::RkISP1Action &action);
 
 	void metadataReady(unsigned int frame, const ControlList &metadata);
 };
@@ -298,7 +302,7 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request)
 
 int RkISP1CameraData::loadIPA()
 {
-	ipa_ = IPAManager::createIPA(pipe_, 1, 1);
+	ipa_ = IPAManager::createIPA<ipa::rkisp1::IPAProxyRkISP1>(pipe_, 1, 1);
 	if (!ipa_)
 		return -ENOENT;
 
@@ -311,15 +315,15 @@ int RkISP1CameraData::loadIPA()
 }
 
 void RkISP1CameraData::queueFrameAction(unsigned int frame,
-					const IPAOperationData &action)
+					const ipa::rkisp1::RkISP1Action &action)
 {
-	switch (action.operation) {
-	case RKISP1_IPA_ACTION_V4L2_SET: {
-		const ControlList &controls = action.controls[0];
+	switch (action.op) {
+	case ipa::rkisp1::ActionV4L2Set: {
+		const ControlList &controls = action.controls;
 		delayedCtrls_->push(controls);
 		break;
 	}
-	case RKISP1_IPA_ACTION_PARAM_FILLED: {
+	case ipa::rkisp1::ActionParamFilled: {
 		PipelineHandlerRkISP1 *pipe = static_cast<PipelineHandlerRkISP1 *>(pipe_);
 		RkISP1FrameInfo *info = frameInfo_.find(frame);
 		if (!info)
@@ -336,11 +340,11 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
 
 		break;
 	}
-	case RKISP1_IPA_ACTION_METADATA:
-		metadataReady(frame, action.controls[0]);
+	case ipa::rkisp1::ActionMetadata:
+		metadataReady(frame, action.controls);
 		break;
 	default:
-		LOG(RkISP1, Error) << "Unknown action " << action.operation;
+		LOG(RkISP1, Error) << "Unknown action " << action.op;
 		break;
 	}
 }
@@ -667,15 +671,15 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 
 	for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {
 		buffer->setCookie(ipaBufferId++);
-		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
-					      .planes = buffer->planes() });
+		data->ipaBuffers_.push_back(IPABuffer(buffer->cookie(),
+						      buffer->planes()));
 		availableParamBuffers_.push(buffer.get());
 	}
 
 	for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {
 		buffer->setCookie(ipaBufferId++);
-		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
-					      .planes = buffer->planes() });
+		data->ipaBuffers_.push_back(IPABuffer(buffer->cookie(),
+						      buffer->planes()));
 		availableStatBuffers_.push(buffer.get());
 	}
 
@@ -729,8 +733,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
 	if (ret)
 		return ret;
 
-	IPAOperationData ipaData = {};
-	ret = data->ipa_->start(ipaData, nullptr);
+	ret = data->ipa_->start();
 	if (ret) {
 		freeBuffers(camera);
 		LOG(RkISP1, Error)
@@ -771,10 +774,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
 			return ret;
 		}
 
-		streamConfig[0] = {
-			.pixelFormat = data->mainPathStream_.configuration().pixelFormat,
-			.size = data->mainPathStream_.configuration().size,
-		};
+		streamConfig[0] = IPAStream(
+			data->mainPathStream_.configuration().pixelFormat,
+			data->mainPathStream_.configuration().size
+		);
 	}
 
 	if (data->selfPath_->isEnabled()) {
@@ -788,10 +791,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
 			return ret;
 		}
 
-		streamConfig[1] = {
-			.pixelFormat = data->selfPathStream_.configuration().pixelFormat,
-			.size = data->selfPathStream_.configuration().size,
-		};
+		streamConfig[1] = IPAStream(
+			data->selfPathStream_.configuration().pixelFormat,
+			data->selfPathStream_.configuration().size
+		);
 	}
 
 	isp_->setFrameStartEnabled(true);
@@ -808,12 +811,10 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
 		ret = 0;
 	}
 
-	std::map<unsigned int, const ControlInfoMap &> entityControls;
+	std::map<uint32_t, ControlInfoMap> entityControls;
 	entityControls.emplace(0, data->sensor_->controls());
 
-	IPAOperationData ipaConfig;
-	data->ipa_->configure(sensorInfo, streamConfig, entityControls,
-			      ipaConfig, nullptr);
+	data->ipa_->configure(sensorInfo, streamConfig, entityControls);
 
 	return ret;
 }
@@ -855,11 +856,12 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
 	if (!info)
 		return -ENOENT;
 
-	IPAOperationData op;
-	op.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST;
-	op.data = { data->frame_, info->paramBuffer->cookie() };
-	op.controls = { request->controls() };
-	data->ipa_->processEvent(op);
+	ipa::rkisp1::RkISP1Event ev;
+	ev.op = ipa::rkisp1::EventQueueRequest;
+	ev.frame = data->frame_;
+	ev.bufferId = info->paramBuffer->cookie();
+	ev.controls = request->controls();
+	data->ipa_->processEvent(ev);
 
 	data->frame_++;
 
@@ -1090,10 +1092,11 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 	if (data->frame_ <= buffer->metadata().sequence)
 		data->frame_ = buffer->metadata().sequence + 1;
 
-	IPAOperationData op;
-	op.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;
-	op.data = { info->frame, info->statBuffer->cookie() };
-	data->ipa_->processEvent(op);
+	ipa::rkisp1::RkISP1Event ev;
+	ev.op = ipa::rkisp1::EventSignalStatBuffer;
+	ev.frame = info->frame;
+	ev.bufferId = info->statBuffer->cookie();
+	data->ipa_->processEvent(ev);
 }
 
 REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)
-- 
2.27.0



More information about the libcamera-devel mailing list