[libcamera-devel,v1.1,2/2] ipa: rkisp1: Replace event-based ops with dedicated functions
diff mbox series

Message ID 20220323150057.121491-1-umang.jain@ideasonboard.com
State Accepted
Delegated to: Umang Jain
Headers show
Series
  • Untitled series #2990
Related show

Commit Message

Umang Jain March 23, 2022, 3 p.m. UTC 
The IPARkISP1Interface currently uses event-type based structures in
order to communicate with the pipeline-handler (and vice-versa).
Replace the event based structures with dedicated functions associated
to each operation.

The translated naming scheme of operations to dedicated functions:
  ActionV4L2Set         => setSensorControls
  ActionParamFilled     => paramsBufferReady
  ActionMetadata        => metdataReady
  EventSignalStatBuffer => processStatsBuffer()
  EventQueueRequest     => queueRequest()

The lexical of IPARkISP1::metadataReady() will now conflict with the
metadataReady Signal being introduced in this patch as part of the
interface change. Hence, rename IPARkISP1::metadataReady() to
IPARkISP1::prepareReady() to prevent the conflict.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Tested-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/ipa/rkisp1.mojom       | 31 ++-------
 src/ipa/rkisp1/rkisp1.cpp                | 88 +++++++-----------------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 71 +++++++------------
 3 files changed, 59 insertions(+), 131 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
index c3a6d8e1..f50f1e11 100644
--- a/include/libcamera/ipa/rkisp1.mojom
+++ b/include/libcamera/ipa/rkisp1.mojom
@@ -8,28 +8,6 @@  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;
-	libcamera.ControlList controls;
-	libcamera.ControlList sensorControls;
-};
-
-struct RkISP1Action {
-	RkISP1Operations op;
-	libcamera.ControlList controls;
-	libcamera.ControlList sensorControls;
-};
-
 interface IPARkISP1Interface {
 	init(libcamera.IPASettings settings,
 	     uint32 hwRevision)
@@ -45,9 +23,14 @@  interface IPARkISP1Interface {
 	mapBuffers(array<libcamera.IPABuffer> buffers);
 	unmapBuffers(array<uint32> ids);
 
-	[async] processEvent(RkISP1Event ev);
+	[async] queueRequest(uint32 frame, uint32 bufferId,
+			     libcamera.ControlList reqControls);
+	[async] processStatsBuffer(uint32 frame, uint32 bufferId,
+				   libcamera.ControlList sensorControls);
 };
 
 interface IPARkISP1EventInterface {
-	queueFrameAction(uint32 frame, RkISP1Action action);
+	paramsBufferReady(uint32 frame);
+	setSensorControls(uint32 frame, libcamera.ControlList sensorControls);
+	metadataReady(uint32 frame, libcamera.ControlList metadata);
 };
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 129afddd..2225a84d 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -51,16 +51,14 @@  public:
 		      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 RkISP1Event &event) override;
 
+	void queueRequest(const uint32_t frame, const uint32_t bufferId,
+			  const ControlList &controls) override;
+	void processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
+				const ControlList &sensorControls) override;
 private:
-	void queueRequest(unsigned int frame, rkisp1_params_cfg *params,
-			  const ControlList &controls);
-	void updateStatistics(unsigned int frame,
-			      const rkisp1_stat_buffer *stats);
-
 	void setControls(unsigned int frame);
-	void metadataReady(unsigned int frame, unsigned int aeState);
+	void prepareMetadata(unsigned int frame, unsigned int aeState);
 
 	std::map<unsigned int, FrameBuffer> buffers_;
 	std::map<unsigned int, MappedFrameBuffer> mappedBuffers_;
@@ -231,60 +229,34 @@  void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
 	}
 }
 
-void IPARkISP1::processEvent(const RkISP1Event &event)
-{
-	switch (event.op) {
-	case EventSignalStatBuffer: {
-		unsigned int frame = event.frame;
-		unsigned int bufferId = event.bufferId;
-
-		const rkisp1_stat_buffer *stats =
-			reinterpret_cast<rkisp1_stat_buffer *>(
-				mappedBuffers_.at(bufferId).planes()[0].data());
-
-		context_.frameContext.sensor.exposure =
-			event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
-		context_.frameContext.sensor.gain =
-			camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
-
-		updateStatistics(frame, stats);
-		break;
-	}
-	case EventQueueRequest: {
-		unsigned int frame = event.frame;
-		unsigned int bufferId = event.bufferId;
-
-		rkisp1_params_cfg *params =
-			reinterpret_cast<rkisp1_params_cfg *>(
-				mappedBuffers_.at(bufferId).planes()[0].data());
-
-		queueRequest(frame, params, event.controls);
-		break;
-	}
-	default:
-		LOG(IPARkISP1, Error) << "Unknown event " << event.op;
-		break;
-	}
-}
-
-void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
+void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,
 			     [[maybe_unused]] const ControlList &controls)
 {
+	rkisp1_params_cfg *params =
+		reinterpret_cast<rkisp1_params_cfg *>(
+			mappedBuffers_.at(bufferId).planes()[0].data());
+
 	/* Prepare parameters buffer. */
 	memset(params, 0, sizeof(*params));
 
 	for (auto const &algo : algorithms_)
 		algo->prepare(context_, params);
 
-	RkISP1Action op;
-	op.op = ActionParamFilled;
-
-	queueFrameAction.emit(frame, op);
+	paramsBufferReady.emit(frame);
 }
 
-void IPARkISP1::updateStatistics(unsigned int frame,
-				 const rkisp1_stat_buffer *stats)
+void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
+				   const ControlList &sensorControls)
 {
+	const rkisp1_stat_buffer *stats =
+		reinterpret_cast<rkisp1_stat_buffer *>(
+			mappedBuffers_.at(bufferId).planes()[0].data());
+
+	context_.frameContext.sensor.exposure =
+		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
+	context_.frameContext.sensor.gain =
+		camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
+
 	unsigned int aeState = 0;
 
 	for (auto const &algo : algorithms_)
@@ -292,37 +264,29 @@  void IPARkISP1::updateStatistics(unsigned int frame,
 
 	setControls(frame);
 
-	metadataReady(frame, aeState);
+	prepareMetadata(frame, aeState);
 }
 
 void IPARkISP1::setControls(unsigned int frame)
 {
-	RkISP1Action op;
-	op.op = ActionV4L2Set;
-
 	uint32_t exposure = context_.frameContext.agc.exposure;
 	uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);
 
 	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.sensorControls = ctrls;
 
-	queueFrameAction.emit(frame, op);
+	setSensorControls.emit(frame, ctrls);
 }
 
-void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)
+void IPARkISP1::prepareMetadata(unsigned int frame, unsigned int aeState)
 {
 	ControlList ctrls(controls::controls);
 
 	if (aeState)
 		ctrls.set(controls::AeLocked, aeState == 2);
 
-	RkISP1Action op;
-	op.op = ActionMetadata;
-	op.controls = ctrls;
-
-	queueFrameAction.emit(frame, op);
+	metadataReady.emit(frame, ctrls);
 }
 
 } /* namespace ipa::rkisp1 */
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 8cca8a15..e6fc582b 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -104,8 +104,9 @@  public:
 	std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
 
 private:
-	void queueFrameAction(unsigned int frame,
-			      const ipa::rkisp1::RkISP1Action &action);
+	void paramFilled(unsigned int frame);
+	void setSensorControls(unsigned int frame,
+			       const ControlList &sensorControls);
 
 	void metadataReady(unsigned int frame, const ControlList &metadata);
 };
@@ -316,8 +317,9 @@  int RkISP1CameraData::loadIPA(unsigned int hwRevision)
 	if (!ipa_)
 		return -ENOENT;
 
-	ipa_->queueFrameAction.connect(this,
-				       &RkISP1CameraData::queueFrameAction);
+	ipa_->setSensorControls.connect(this, &RkISP1CameraData::setSensorControls);
+	ipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);
+	ipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);
 
 	int ret = ipa_->init(IPASettings{ "", sensor_->model() }, hwRevision);
 	if (ret < 0) {
@@ -328,39 +330,27 @@  int RkISP1CameraData::loadIPA(unsigned int hwRevision)
 	return 0;
 }
 
-void RkISP1CameraData::queueFrameAction(unsigned int frame,
-					const ipa::rkisp1::RkISP1Action &action)
+void RkISP1CameraData::paramFilled(unsigned int frame)
 {
-	switch (action.op) {
-	case ipa::rkisp1::ActionV4L2Set: {
-		const ControlList &controls = action.sensorControls;
-		delayedCtrls_->push(controls);
-		break;
-	}
-	case ipa::rkisp1::ActionParamFilled: {
-		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
-		RkISP1FrameInfo *info = frameInfo_.find(frame);
-		if (!info)
-			break;
+	PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
+	RkISP1FrameInfo *info = frameInfo_.find(frame);
+	if (!info)
+		return;
 
-		pipe->param_->queueBuffer(info->paramBuffer);
-		pipe->stat_->queueBuffer(info->statBuffer);
+	pipe->param_->queueBuffer(info->paramBuffer);
+	pipe->stat_->queueBuffer(info->statBuffer);
 
-		if (info->mainPathBuffer)
-			mainPath_->queueBuffer(info->mainPathBuffer);
+	if (info->mainPathBuffer)
+		mainPath_->queueBuffer(info->mainPathBuffer);
 
-		if (info->selfPathBuffer)
-			selfPath_->queueBuffer(info->selfPathBuffer);
+	if (info->selfPathBuffer)
+		selfPath_->queueBuffer(info->selfPathBuffer);
+}
 
-		break;
-	}
-	case ipa::rkisp1::ActionMetadata:
-		metadataReady(frame, action.controls);
-		break;
-	default:
-		LOG(RkISP1, Error) << "Unknown action " << action.op;
-		break;
-	}
+void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
+					 const ControlList &sensorControls)
+{
+	delayedCtrls_->push(sensorControls);
 }
 
 void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
@@ -865,13 +855,8 @@  int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
 	if (!info)
 		return -ENOENT;
 
-	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->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),
+				 request->controls());
 	data->frame_++;
 
 	return 0;
@@ -1120,12 +1105,8 @@  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 	if (data->frame_ <= buffer->metadata().sequence)
 		data->frame_ = buffer->metadata().sequence + 1;
 
-	ipa::rkisp1::RkISP1Event ev;
-	ev.op = ipa::rkisp1::EventSignalStatBuffer;
-	ev.frame = info->frame;
-	ev.bufferId = info->statBuffer->cookie();
-	ev.sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);
-	data->ipa_->processEvent(ev);
+	data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
+				       data->delayedCtrls_->get(buffer->metadata().sequence));
 }
 
 REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)