[libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3 pipeline skeleton

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-01-11 17:51:09 +0100, Jacopo Mondi wrote:
> Add a pipeline handler skeleton for the Intel IPU3 device.
> Tested with on Soraka, listing detected cameras on the system and
> verifying the pipeline handler gets properly matched.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> 
> Let's start by simply matching the pipeline handler with the device
> it is running on. The here created single camera gets properly enumerated
> on Soraka by the 'list-cameras' test:
> 
> ./test/list-cameras
> [0:35:35.453249952]   DBG pipeline_handler.cpp:119 Pipeline handler: "PipeHandlerVimc" registered
> [0:35:35.453538626]   DBG pipeline_handler.cpp:119 Pipeline handler: "PipelineHandlerIPU3" registered
> [0:35:35.458316459]   DBG device_enumerator.cpp:214 New media device: ipu3-imgu created from: /dev/media1
> [0:35:35.469071318]   DBG device_enumerator.cpp:214 New media device: ipu3-cio2 created from: /dev/media0
> [0:35:35.475305874]   DBG device_enumerator.cpp:255 Succesfull match for media device: ipu3-cio2
> [0:35:35.475343991]   DBG device_enumerator.cpp:255 Succesfull match for media device: ipu3-imgu
> [0:35:35.475354057]   DBG pipeline_handler.cpp:150 Pipeline handler: "PipelineHandlerIPU3" matched
> - IPU3 Camera
> 
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp    | 119 ++++++++++++++++++++++++
>  src/libcamera/pipeline/ipu3/meson.build |   3 +
>  src/libcamera/pipeline/meson.build      |   2 +
>  3 files changed, 124 insertions(+)
>  create mode 100644 src/libcamera/pipeline/ipu3/ipu3.cpp
>  create mode 100644 src/libcamera/pipeline/ipu3/meson.build
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> new file mode 100644
> index 0000000..477a9a2
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -0,0 +1,119 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipu3.cpp - Pipeline handler for Intel IPU3
> + */
> +
> +#include <libcamera/camera.h>
> +
> +#include "device_enumerator.h"
> +#include "media_device.h"
> +#include "pipeline_handler.h"
> +
> +#include "log.h"
> +
> +namespace libcamera {
> +
> +class PipelineHandlerIPU3 : public PipelineHandler
> +{
> +public:
> +	PipelineHandlerIPU3();
> +	~PipelineHandlerIPU3();
> +
> +	bool match(DeviceEnumerator *enumerator);
> +
> +	unsigned int count();
> +	Camera *camera(unsigned int id) final;
> +
> +private:
> +	MediaDevice *cio2_;
> +	MediaDevice *imgu_;
> +
> +	Camera *camera_;
> +};
> +
> +PipelineHandlerIPU3::PipelineHandlerIPU3()
> +	: cio2_(nullptr), imgu_(nullptr), camera_(nullptr)
> +{
> +}
> +
> +PipelineHandlerIPU3::~PipelineHandlerIPU3()
> +{
> +	if (cio2_)
> +		cio2_->release();
> +	if (imgu_)
> +		imgu_->release();
> +	if (camera_)
> +		camera_->put();
> +
> +	cio2_ = nullptr;
> +	imgu_ = nullptr;
> +	camera_ = nullptr;
> +}
> +
> +unsigned int PipelineHandlerIPU3::count()
> +{
> +	return 1;
> +}
> +
> +Camera *PipelineHandlerIPU3::camera(unsigned int id)
> +{
> +	if (id != 0)
> +		return nullptr;
> +
> +	return camera_;
> +}
> +
> +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> +{
> +	DeviceMatch cio2_dm("ipu3-cio2");
> +	cio2_dm.add("ipu3-csi2 0");
> +	cio2_dm.add("ipu3-cio2 0");
> +	cio2_dm.add("ipu3-csi2 1");
> +	cio2_dm.add("ipu3-cio2 1");
> +	cio2_dm.add("ipu3-csi2 2");
> +	cio2_dm.add("ipu3-cio2 2");
> +	cio2_dm.add("ipu3-csi2 3");
> +	cio2_dm.add("ipu3-cio2 3");
> +
> +	cio2_ = enumerator->search(cio2_dm);
> +	if (!cio2_)
> +		return false;
> +
> +	cio2_->acquire();
> +
> +	DeviceMatch imgu_dm("ipu3-imgu");
> +	imgu_dm.add("ipu3-imgu 0");
> +	imgu_dm.add("ipu3-imgu 0 input");
> +	imgu_dm.add("ipu3-imgu 0 parameters");
> +	imgu_dm.add("ipu3-imgu 0 output");
> +	imgu_dm.add("ipu3-imgu 0 viewfinder");
> +	imgu_dm.add("ipu3-imgu 0 3a stat");
> +	imgu_dm.add("ipu3-imgu 1");
> +	imgu_dm.add("ipu3-imgu 1 input");
> +	imgu_dm.add("ipu3-imgu 1 parameters");
> +	imgu_dm.add("ipu3-imgu 1 output");
> +	imgu_dm.add("ipu3-imgu 1 viewfinder");
> +	imgu_dm.add("ipu3-imgu 1 3a stat");
> +
> +	imgu_ = enumerator->search(imgu_dm);
> +	if (!imgu_) {
> +		cio2_->release();
> +		return false;
> +	}
> +
> +	imgu_->acquire();

I would reorder this a bit.

    ...

    cio2_ = enumerator->search(cio2_dm);
    if (!cio2_)
        return false;

    imgu_ = enumerator->search(imgu_dm);
    if (!imgu_)
        return false;

    cio2_->acquire();
    imgu_->acquire();

    ...

I don't feel strongly about this so if others have a different view I 
will yield to public opinion :-)

> +
> +	/*
> +	 * TODO: create cameras. As of now, just create a dummy one
> +	 * to verify enumeration and matching on IPU3.
> +	 */
> +	camera_ = new Camera("IPU3 Camera");
> +
> +	return true;
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build
> new file mode 100644
> index 0000000..0ab766a
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/meson.build
> @@ -0,0 +1,3 @@
> +libcamera_sources += files([
> +    'ipu3.cpp',
> +])
> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> index 615ecd2..811c075 100644
> --- a/src/libcamera/pipeline/meson.build
> +++ b/src/libcamera/pipeline/meson.build
> @@ -1,3 +1,5 @@
>  libcamera_sources += files([
>      'vimc.cpp',
>  ])
> +
> +subdir('ipu3')
> --
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund