[libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure sensor provides a compatible format

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Mar 20, 2019 at 05:30:27PM +0100, Jacopo Mondi wrote:
> When creating a camera, make sure a the image sensor provides images in
> a format compatible with IPU3 CIO2 unit requirements.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 43 ++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 55489c31df2d..2602f89617a3 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -8,6 +8,8 @@
>  #include <memory>
>  #include <vector>
>  
> +#include <linux/media-bus-format.h>
> +
>  #include <libcamera/camera.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
> @@ -78,6 +80,8 @@ private:
>  			PipelineHandler::cameraData(camera));
>  	}
>  
> +	int mediaBusToCIO2Format(unsigned int code);
> +
>  	void registerCameras();
>  
>  	std::shared_ptr<MediaDevice> cio2_;
> @@ -327,6 +331,22 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
>  	return true;
>  }
>  
> +int PipelineHandlerIPU3::mediaBusToCIO2Format(unsigned int code)
> +{
> +	switch (code) {
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +		return V4L2_PIX_FMT_IPU3_SBGGR10;
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +		return V4L2_PIX_FMT_IPU3_SGBRG10;
> +	case MEDIA_BUS_FMT_SGRBG10_1X10:
> +		return V4L2_PIX_FMT_IPU3_SGRBG10;
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +		return V4L2_PIX_FMT_IPU3_SRGGB10;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  /*
>   * Cameras are created associating an image sensor (represented by a
>   * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four
> @@ -404,18 +424,37 @@ void PipelineHandlerIPU3::registerCameras()
>  		if (ret)
>  			continue;
>  
> -		data->cio2_->bufferReady.connect(data.get(), &IPU3CameraData::bufferReady);
> -
> +		/*
> +		 * Make sure the sensor produces at least one image format
> +		 * compatible with IPU3 CIO2 requirements.
> +		 */
>  		data->sensor_ = new V4L2Subdevice(sensor);
>  		ret = data->sensor_->open();
>  		if (ret)
>  			continue;
>  
> +		const FormatEnum formats = data->sensor_->formats(0);
> +		auto it = formats.begin();
> +		for (; it != formats.end(); ++it) {

How about

		for (auto it : formats)

> +			if (mediaBusToCIO2Format(it->first) != -EINVAL)
> +				break;
> +		}
> +		if (it == formats.end()) {
> +			LOG(IPU3, Info)
> +				<< "Sensor '" << data->sensor_->deviceName()

How about printing the entity name instead ?

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

> +				<< "' detected, but no supported image format "
> +				<< " found: skip camera creation";
> +			continue;
> +		}
> +
>  		data->csi2_ = new V4L2Subdevice(csi2);
>  		ret = data->csi2_->open();
>  		if (ret)
>  			continue;
>  
> +		data->cio2_->bufferReady.connect(data.get(),
> +						 &IPU3CameraData::bufferReady);
> +
>  		registerCamera(std::move(camera), std::move(data));
>  
>  		LOG(IPU3, Info)

-- 
Regards,

Laurent Pinchart