[libcamera-devel] [PATCH 06/10] libcamera: ipu3: cio2: Move the CIO2Device class to separate files

Jacopo Mondi jacopo at jmondi.org
Tue Jun 2 13:50:03 CEST 2020


Hi Niklas,

On Tue, Jun 02, 2020 at 03:39:05AM +0200, Niklas Söderlund wrote:
> In preparation of refactoring the IPU3 pipeline handler breakout the
> CIO2Device into its own .cpp and .h file, no functional change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

snip

> +
> +V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code)
> +{
> +	switch (code) {
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10);
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10);
> +	case MEDIA_BUS_FMT_SGRBG10_1X10:
> +		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10);
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10);
> +	default:
> +		return {};
> +	}
> +}

Possibly missed in review of the previous patch, but wasn't this
dropped ?

> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> new file mode 100644
> index 0000000000000000..d923038bb4ba356f
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -0,0 +1,64 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * cio2.h - Intel IPU3 CIO2
> + */
> +#ifndef __LIBCAMERA_PIPELINE_IPU3_CIO2_H__
> +#define __LIBCAMERA_PIPELINE_IPU3_CIO2_H__
> +
> +#include <queue>
> +#include <vector>
> +
> +#include <linux/media-bus-format.h>
> +
> +#include "libcamera/internal/camera_sensor.h"
> +#include "libcamera/internal/media_device.h"
> +#include "libcamera/internal/v4l2_subdevice.h"
> +#include "libcamera/internal/v4l2_videodevice.h"
> +
> +namespace libcamera {
> +
> +class CIO2Device
> +{
> +public:
> +	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
> +
> +	CIO2Device()
> +		: output_(nullptr), csi2_(nullptr), sensor_(nullptr)
> +	{
> +	}
> +
> +	~CIO2Device()
> +	{
> +		delete output_;
> +		delete csi2_;
> +		delete sensor_;
> +	}
> +
> +	int init(const MediaDevice *media, unsigned int index);
> +	int configure(const Size &size, V4L2DeviceFormat *outputFormat);
> +
> +	int allocateBuffers();
> +	void freeBuffers();
> +
> +	FrameBuffer *getBuffer();
> +	void putBuffer(FrameBuffer *buffer);
> +
> +	int start();
> +	int stop();
> +
> +	static V4L2PixelFormat mediaBusToFormat(unsigned int code);

Same here

That apart, I like this patch
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> +
> +	V4L2VideoDevice *output_;
> +	V4L2Subdevice *csi2_;
> +	CameraSensor *sensor_;
> +
> +private:
> +	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> +	std::queue<FrameBuffer *> availableBuffers_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_PIPELINE_IPU3_CIO2_H__ */
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f4759715c6ae7572..2047deac299dbf75 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -27,6 +27,8 @@
>  #include "libcamera/internal/v4l2_subdevice.h"
>  #include "libcamera/internal/v4l2_videodevice.h"
>
> +#include "cio2.h"
> +
>  namespace libcamera {
>
>  LOG_DEFINE_CATEGORY(IPU3)
> @@ -103,45 +105,6 @@ public:
>  	/* \todo Add param video device for 3A tuning */
>  };
>
> -class CIO2Device
> -{
> -public:
> -	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
> -
> -	CIO2Device()
> -		: output_(nullptr), csi2_(nullptr), sensor_(nullptr)
> -	{
> -	}
> -
> -	~CIO2Device()
> -	{
> -		delete output_;
> -		delete csi2_;
> -		delete sensor_;
> -	}
> -
> -	int init(const MediaDevice *media, unsigned int index);
> -	int configure(const Size &size,
> -		      V4L2DeviceFormat *outputFormat);
> -
> -	int allocateBuffers();
> -	void freeBuffers();
> -
> -	FrameBuffer *getBuffer();
> -	void putBuffer(FrameBuffer *buffer);
> -
> -	int start();
> -	int stop();
> -
> -	V4L2VideoDevice *output_;
> -	V4L2Subdevice *csi2_;
> -	CameraSensor *sensor_;
> -
> -private:
> -	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> -	std::queue<FrameBuffer *> availableBuffers_;
> -};
> -
>  class IPU3Stream : public Stream
>  {
>  public:
> @@ -1407,214 +1370,6 @@ int ImgUDevice::enableLinks(bool enable)
>  	return linkSetup(name_, PAD_STAT, statName, 0, enable);
>  }
>
> -/*------------------------------------------------------------------------------
> - * CIO2 Device
> - */
> -
> -/**
> - * \brief Initialize components of the CIO2 device with \a index
> - * \param[in] media The CIO2 media device
> - * \param[in] index The CIO2 device index
> - *
> - * Create and open the video device and subdevices in the CIO2 instance at \a
> - * index, if a supported image sensor is connected to the CSI-2 receiver of
> - * this CIO2 instance.  Enable the media links connecting the CIO2 components
> - * to prepare for capture operations and cached the sensor maximum size.
> - *
> - * \return 0 on success or a negative error code otherwise
> - * \retval -ENODEV No supported image sensor is connected to this CIO2 instance
> - */
> -int CIO2Device::init(const MediaDevice *media, unsigned int index)
> -{
> -	int ret;
> -
> -	/*
> -	 * Verify that a sensor subdevice is connected to this CIO2 instance
> -	 * and enable the media link between the two.
> -	 */
> -	std::string csi2Name = "ipu3-csi2 " + std::to_string(index);
> -	MediaEntity *csi2Entity = media->getEntityByName(csi2Name);
> -	const std::vector<MediaPad *> &pads = csi2Entity->pads();
> -	if (pads.empty())
> -		return -ENODEV;
> -
> -	/* IPU3 CSI-2 receivers have a single sink pad at index 0. */
> -	MediaPad *sink = pads[0];
> -	const std::vector<MediaLink *> &links = sink->links();
> -	if (links.empty())
> -		return -ENODEV;
> -
> -	MediaLink *link = links[0];
> -	MediaEntity *sensorEntity = link->source()->entity();
> -	sensor_ = new CameraSensor(sensorEntity);
> -	ret = sensor_->init();
> -	if (ret)
> -		return ret;
> -
> -	ret = link->setEnabled(true);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * Make sure the sensor produces at least one format compatible with
> -	 * the CIO2 requirements.
> -	 *
> -	 * utils::set_overlap requires the ranges to be sorted, keep the
> -	 * cio2Codes vector sorted in ascending order.
> -	 */
> -	const std::vector<unsigned int> cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10,
> -						   MEDIA_BUS_FMT_SGRBG10_1X10,
> -						   MEDIA_BUS_FMT_SGBRG10_1X10,
> -						   MEDIA_BUS_FMT_SRGGB10_1X10 };
> -	const std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();
> -	if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),
> -				cio2Codes.begin(), cio2Codes.end())) {
> -		LOG(IPU3, Error)
> -			<< "Sensor " << sensor_->entity()->name()
> -			<< " has not format compatible with the IPU3";
> -		return -EINVAL;
> -	}
> -
> -	/*
> -	 * \todo Define when to open and close video device nodes, as they
> -	 * might impact on power consumption.
> -	 */
> -
> -	csi2_ = new V4L2Subdevice(csi2Entity);
> -	ret = csi2_->open();
> -	if (ret)
> -		return ret;
> -
> -	std::string cio2Name = "ipu3-cio2 " + std::to_string(index);
> -	output_ = V4L2VideoDevice::fromEntityName(media, cio2Name);
> -	ret = output_->open();
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -/**
> - * \brief Configure the CIO2 unit
> - * \param[in] size The requested CIO2 output frame size
> - * \param[out] outputFormat The CIO2 unit output image format
> - * \return 0 on success or a negative error code otherwise
> - */
> -int CIO2Device::configure(const Size &size,
> -			  V4L2DeviceFormat *outputFormat)
> -{
> -	V4L2SubdeviceFormat sensorFormat;
> -	int ret;
> -
> -	/*
> -	 * Apply the selected format to the sensor, the CSI-2 receiver and
> -	 * the CIO2 output device.
> -	 */
> -	sensorFormat = sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
> -					    MEDIA_BUS_FMT_SGBRG10_1X10,
> -					    MEDIA_BUS_FMT_SGRBG10_1X10,
> -					    MEDIA_BUS_FMT_SRGGB10_1X10 },
> -					  size);
> -	ret = sensor_->setFormat(&sensorFormat);
> -	if (ret)
> -		return ret;
> -
> -	ret = csi2_->setFormat(0, &sensorFormat);
> -	if (ret)
> -		return ret;
> -
> -	V4L2PixelFormat v4l2Format;
> -	switch (sensorFormat.mbus_code) {
> -	case MEDIA_BUS_FMT_SBGGR10_1X10:
> -		v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10);
> -		break;
> -	case MEDIA_BUS_FMT_SGBRG10_1X10:
> -		v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10);
> -		break;
> -	case MEDIA_BUS_FMT_SGRBG10_1X10:
> -		v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10);
> -		break;
> -	case MEDIA_BUS_FMT_SRGGB10_1X10:
> -		v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10);
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	outputFormat->fourcc = v4l2Format;
> -	outputFormat->size = sensorFormat.size;
> -	outputFormat->planesCount = 1;
> -
> -	ret = output_->setFormat(outputFormat);
> -	if (ret)
> -		return ret;
> -
> -	LOG(IPU3, Debug) << "CIO2 output format " << outputFormat->toString();
> -
> -	return 0;
> -}
> -
> -/**
> - * \brief Allocate frame buffers for the CIO2 output
> - *
> - * Allocate frame buffers in the CIO2 video device to be used to capture frames
> - * from the CIO2 output. The buffers are stored in the CIO2Device::buffers_
> - * vector.
> - *
> - * \return Number of buffers allocated or negative error code
> - */
> -int CIO2Device::allocateBuffers()
> -{
> -	int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_);
> -	if (ret < 0)
> -		return ret;
> -
> -	for (std::unique_ptr<FrameBuffer> &buffer : buffers_)
> -		availableBuffers_.push(buffer.get());
> -
> -	return ret;
> -}
> -
> -void CIO2Device::freeBuffers()
> -{
> -	/* The default std::queue constructor is explicit with gcc 5 and 6. */
> -	availableBuffers_ = std::queue<FrameBuffer *>{};
> -
> -	buffers_.clear();
> -
> -	if (output_->releaseBuffers())
> -		LOG(IPU3, Error) << "Failed to release CIO2 buffers";
> -}
> -
> -FrameBuffer *CIO2Device::getBuffer()
> -{
> -	if (availableBuffers_.empty()) {
> -		LOG(IPU3, Error) << "CIO2 buffer underrun";
> -		return nullptr;
> -	}
> -
> -	FrameBuffer *buffer = availableBuffers_.front();
> -
> -	availableBuffers_.pop();
> -
> -	return buffer;
> -}
> -
> -void CIO2Device::putBuffer(FrameBuffer *buffer)
> -{
> -	availableBuffers_.push(buffer);
> -}
> -
> -int CIO2Device::start()
> -{
> -	return output_->streamOn();
> -}
> -
> -int CIO2Device::stop()
> -{
> -	return output_->streamOff();
> -}
> -
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build
> index 0e8c5a14f2b3317b..b2602d30123f908d 100644
> --- a/src/libcamera/pipeline/ipu3/meson.build
> +++ b/src/libcamera/pipeline/ipu3/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>
>  libcamera_sources += files([
> +    'cio2.cpp',
>      'ipu3.cpp',
>  ])
> --
> 2.26.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel


More information about the libcamera-devel mailing list