[libcamera-devel] [PATCH v5 14/19] libcamera: ipu3: Implement camera start/stop

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-03-26 09:38:57 +0100, Jacopo Mondi wrote:
> Start and stop video devices in the pipeline.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 93 ++++++++++++++++++++++++++--
>  1 file changed, 87 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index d3519bb1d536..5b3c44174566 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -111,6 +111,9 @@ public:
>  	int exportBuffers(enum OutputId id, unsigned int count);
>  	void freeBuffers();
>  
> +	int start();
> +	void stop();
> +
>  	unsigned int index_;
>  	std::string name_;
>  	MediaDevice *media_;
> @@ -154,6 +157,9 @@ public:
>  	BufferPool *exportBuffers();
>  	void freeBuffers();
>  
> +	int start();
> +	void stop();
> +
>  	V4L2Device *output_;
>  	V4L2Subdevice *csi2_;
>  	V4L2Subdevice *sensor_;
> @@ -371,12 +377,24 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
>  int PipelineHandlerIPU3::start(Camera *camera)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	V4L2Device *cio2 = data->cio2_.output_;
> +	CIO2Device *cio2 = &data->cio2_;
> +	ImgUDevice *imgu = data->imgu_;
>  	int ret;
>  
> -	ret = cio2->streamOn();
> +	/*
> +	 * Start the ImgU video devices, buffers will be queued to the
> +	 * ImgU output and viewfinder when requests will be queued.
> +	 */
> +	ret = cio2->start();
> +	if (ret) {
> +		cio2->stop();
> +		return ret;
> +	}
> +
> +	ret = imgu->start();
>  	if (ret) {
> -		LOG(IPU3, Info) << "Failed to start camera " << camera->name();
> +		imgu->stop();
> +		cio2->stop();
>  		return ret;
>  	}
>  
> @@ -386,10 +404,9 @@ int PipelineHandlerIPU3::start(Camera *camera)
>  void PipelineHandlerIPU3::stop(Camera *camera)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	V4L2Device *cio2 = data->cio2_.output_;
>  
> -	if (cio2->streamOff())
> -		LOG(IPU3, Info) << "Failed to stop camera " << camera->name();
> +	data->cio2_.stop();
> +	data->imgu_->stop();

Not complaining only pointing out it looks odd that one is a pointer and 
the other one not :-)

With Laurents comments addressed feel free to add,

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

>  
>  	PipelineHandler::stop(camera);
>  }
> @@ -833,6 +850,46 @@ void ImgUDevice::freeBuffers()
>  		LOG(IPU3, Error) << "Failed to release ImgU input buffers";
>  }
>  
> +int ImgUDevice::start()
> +{
> +	int ret;
> +
> +	/* Start the ImgU video devices. */
> +	ret = output_->streamOn();
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to start ImgU output";
> +		return ret;
> +	}
> +
> +	ret = viewfinder_->streamOn();
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to start ImgU viewfinder";
> +		return ret;
> +	}
> +
> +	ret = stat_->streamOn();
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to start ImgU stat";
> +		return ret;
> +	}
> +
> +	ret = input_->streamOn();
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to start ImgU input";
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void ImgUDevice::stop()
> +{
> +	output_->streamOff();
> +	viewfinder_->streamOff();
> +	stat_->streamOff();
> +	input_->streamOff();
> +}
> +
>  /*------------------------------------------------------------------------------
>   * CIO2 Device
>   */
> @@ -1026,6 +1083,30 @@ void CIO2Device::freeBuffers()
>  		LOG(IPU3, Error) << "Failed to release CIO2 buffers";
>  }
>  
> +int CIO2Device::start()
> +{
> +	int ret;
> +
> +	for (Buffer &buffer : pool_.buffers()) {
> +		ret = output_->queueBuffer(&buffer);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = output_->streamOn();
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to start CIO2";
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void CIO2Device::stop()
> +{
> +	output_->streamOff();
> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);
>  
>  } /* namespace libcamera */
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund