[libcamera-devel,v10,3/3] libcamera: pipeline: ipu3: Apply a requested test pattern mode
diff mbox series

Message ID 20211206054918.2467049-3-hiroh@chromium.org
State Accepted
Headers show
Series
  • [libcamera-devel,v10,1/3] libcamera: camera_sensor: Reference test pattern modes by enum type
Related show

Commit Message

Hirokazu Honda Dec. 6, 2021, 5:49 a.m. UTC
This introduces a way to set controls immediately for a capture
in ipu3 pipeline handler. It enables to apply a test pattern mode
per frame.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 59 +++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Dec. 6, 2021, 11:54 a.m. UTC | #1
Hi Hiro,

Thank you for the patch.

On Mon, Dec 06, 2021 at 02:49:18PM +0900, Hirokazu Honda wrote:
> This introduces a way to set controls immediately for a capture
> in ipu3 pipeline handler. It enables to apply a test pattern mode
> per frame.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 59 +++++++++++++++++++++++++++-
>  1 file changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 25490dcf..ee1ad27e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -59,6 +59,7 @@ public:
>  	void statBufferReady(FrameBuffer *buffer);
>  	void queuePendingRequests();
>  	void cancelPendingRequests();
> +	void frameStart(uint32_t sequence);
>  
>  	CIO2Device cio2_;
>  	ImgUDevice *imgu_;
> @@ -76,7 +77,10 @@ public:
>  
>  	std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
>  
> +	/* Requests before queueing cio2 device. */
>  	std::queue<Request *> pendingRequests_;
> +	/* Requests queued in cio2 device and before passing imgu device. */
> +	std::queue<Request *> processingRequests_;
>  
>  	ControlInfoMap ipaControls_;
>  
> @@ -564,6 +568,11 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	if (ret)
>  		return ret;
>  
> +	ret = cio2->sensor()->setTestPatternMode(
> +		controls::draft::TestPatternModeEnum::TestPatternModeOff);
> +	if (ret)
> +		return ret;

Shouldn't this be done at start() time instead ?

> +
>  	IPACameraSensorInfo sensorInfo;
>  	cio2->sensor()->sensorInfo(&sensorInfo);
>  	data->cropRegion_ = sensorInfo.analogCrop;
> @@ -811,6 +820,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  
>  void IPU3CameraData::cancelPendingRequests()
>  {
> +	processingRequests_ = {};
> +
>  	while (!pendingRequests_.empty()) {
>  		Request *request = pendingRequests_.front();
>  
> @@ -853,6 +864,8 @@ void IPU3CameraData::queuePendingRequests()
>  
>  		info->rawBuffer = rawBuffer;
>  
> +		processingRequests_.push(request);
> +

I would have moved this to the end, after the pendingRequests_.pop()
call, to group code that moves the request from one queue to the other,
but it likely makes no real difference in practice.

>  		ipa::ipu3::IPU3Event ev;
>  		ev.op = ipa::ipu3::EventProcessControls;
>  		ev.frame = info->id;
> @@ -1121,8 +1134,8 @@ int PipelineHandlerIPU3::registerCameras()
>  		data->delayedCtrls_ =
>  			std::make_unique<DelayedControls>(cio2->sensor()->device(),
>  							  params);
> -		data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
> -						 &DelayedControls::applyControls);
> +		data->cio2_.frameStart().connect(data.get(),
> +						 &IPU3CameraData::frameStart);
>  
>  		/* Convert the sensor rotation to a transformation */
>  		int32_t rotation = 0;
> @@ -1414,6 +1427,48 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>  	ipa_->processEvent(ev);
>  }
>  
> +/*
> + * \brief Handle the start of frame exposure signal
> + * \param[in] sequence The sequence number of frame
> + *
> + * Inspect the list of pending requests waiting for a RAW frame to be
> + * produced and apply controls for the 'next' one.
> + *
> + * Some controls need to be applied immediately, such as the
> + * TestPatternMode one. Other controls are handled through the delayed
> + * controls class.
> + */
> +void IPU3CameraData::frameStart(uint32_t sequence)
> +{
> +	delayedCtrls_->applyControls(sequence);
> +
> +	if (processingRequests_.empty())
> +		return;
> +
> +	/* Handle controls which are to be set ready for the next frame to start. */
> +	Request *request = processingRequests_.front();
> +	processingRequests_.pop();

There's no synchronization with the sequence number, I can imagine many
ways how this could go wrong. I don't want to delay this series
endlessly, so with a

	/* \todo Synchronize with the sequence number */

comment,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	/* Takes effect applying the test pattern mode affects immediately. */
> +	if (!request->controls().contains(controls::draft::TestPatternMode))
> +		return;
> +
> +	const int32_t testPatternMode = request->controls().get(
> +		controls::draft::TestPatternMode);
> +
> +	int ret = cio2_.sensor()->setTestPatternMode(
> +		static_cast<controls::draft::TestPatternModeEnum>(
> +			testPatternMode));
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to set test pattern mode: "
> +				 << ret;
> +		return;
> +	}
> +
> +	request->metadata().set(controls::draft::TestPatternMode,
> +				testPatternMode);
> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
>  
>  } /* namespace libcamera */

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 25490dcf..ee1ad27e 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -59,6 +59,7 @@  public:
 	void statBufferReady(FrameBuffer *buffer);
 	void queuePendingRequests();
 	void cancelPendingRequests();
+	void frameStart(uint32_t sequence);
 
 	CIO2Device cio2_;
 	ImgUDevice *imgu_;
@@ -76,7 +77,10 @@  public:
 
 	std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
 
+	/* Requests before queueing cio2 device. */
 	std::queue<Request *> pendingRequests_;
+	/* Requests queued in cio2 device and before passing imgu device. */
+	std::queue<Request *> processingRequests_;
 
 	ControlInfoMap ipaControls_;
 
@@ -564,6 +568,11 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	if (ret)
 		return ret;
 
+	ret = cio2->sensor()->setTestPatternMode(
+		controls::draft::TestPatternModeEnum::TestPatternModeOff);
+	if (ret)
+		return ret;
+
 	IPACameraSensorInfo sensorInfo;
 	cio2->sensor()->sensorInfo(&sensorInfo);
 	data->cropRegion_ = sensorInfo.analogCrop;
@@ -811,6 +820,8 @@  void PipelineHandlerIPU3::stop(Camera *camera)
 
 void IPU3CameraData::cancelPendingRequests()
 {
+	processingRequests_ = {};
+
 	while (!pendingRequests_.empty()) {
 		Request *request = pendingRequests_.front();
 
@@ -853,6 +864,8 @@  void IPU3CameraData::queuePendingRequests()
 
 		info->rawBuffer = rawBuffer;
 
+		processingRequests_.push(request);
+
 		ipa::ipu3::IPU3Event ev;
 		ev.op = ipa::ipu3::EventProcessControls;
 		ev.frame = info->id;
@@ -1121,8 +1134,8 @@  int PipelineHandlerIPU3::registerCameras()
 		data->delayedCtrls_ =
 			std::make_unique<DelayedControls>(cio2->sensor()->device(),
 							  params);
-		data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
-						 &DelayedControls::applyControls);
+		data->cio2_.frameStart().connect(data.get(),
+						 &IPU3CameraData::frameStart);
 
 		/* Convert the sensor rotation to a transformation */
 		int32_t rotation = 0;
@@ -1414,6 +1427,48 @@  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
 	ipa_->processEvent(ev);
 }
 
+/*
+ * \brief Handle the start of frame exposure signal
+ * \param[in] sequence The sequence number of frame
+ *
+ * Inspect the list of pending requests waiting for a RAW frame to be
+ * produced and apply controls for the 'next' one.
+ *
+ * Some controls need to be applied immediately, such as the
+ * TestPatternMode one. Other controls are handled through the delayed
+ * controls class.
+ */
+void IPU3CameraData::frameStart(uint32_t sequence)
+{
+	delayedCtrls_->applyControls(sequence);
+
+	if (processingRequests_.empty())
+		return;
+
+	/* Handle controls which are to be set ready for the next frame to start. */
+	Request *request = processingRequests_.front();
+	processingRequests_.pop();
+
+	/* Takes effect applying the test pattern mode affects immediately. */
+	if (!request->controls().contains(controls::draft::TestPatternMode))
+		return;
+
+	const int32_t testPatternMode = request->controls().get(
+		controls::draft::TestPatternMode);
+
+	int ret = cio2_.sensor()->setTestPatternMode(
+		static_cast<controls::draft::TestPatternModeEnum>(
+			testPatternMode));
+	if (ret) {
+		LOG(IPU3, Error) << "Failed to set test pattern mode: "
+				 << ret;
+		return;
+	}
+
+	request->metadata().set(controls::draft::TestPatternMode,
+				testPatternMode);
+}
+
 REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
 
 } /* namespace libcamera */