[libcamera-devel] [PATCH v7 04/13] libcamera: ipu3: Cache the sensor sizes

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-04-02 19:13:00 +0200, Jacopo Mondi wrote:
> Cache the sensor maximum size and the media bus code used to produce
> it.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

With or without addressing Laurents comment about moving 
mediaBusToCIO2Format(),

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

I actually prefer it the way it is, a file local static helper function, 
but I don't feel strongly about it :-)

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 47 ++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index a870b325f4b3..1e315048738b 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>
> @@ -24,6 +26,22 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPU3)
>  
> +static int 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;
> +	}
> +}
> +
>  class CIO2Device
>  {
>  public:
> @@ -44,6 +62,10 @@ public:
>  	V4L2Device *output_;
>  	V4L2Subdevice *csi2_;
>  	V4L2Subdevice *sensor_;
> +
> +	/* Maximum sizes and the mbus code used to produce them. */
> +	unsigned int mbusCode_;
> +	Size maxSize_;
>  };
>  
>  class PipelineHandlerIPU3 : public PipelineHandler
> @@ -442,6 +464,10 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
>  		return ret;
>  
>  	/*
> +	 * Now that we're sure a sensor subdevice is connected, make sure it
> +	 * produces at least one image format compatible with CIO2 requirements
> +	 * and cache the camera maximum size.
> +	 *
>  	 * \todo Define when to open and close video device nodes, as they
>  	 * might impact on power consumption.
>  	 */
> @@ -450,6 +476,27 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)
>  	if (ret)
>  		return ret;
>  
> +	for (auto it : sensor_->formats(0)) {
> +		int mbusCode = mediaBusToCIO2Format(it.first);
> +		if (mbusCode < 0)
> +			continue;
> +
> +		for (const SizeRange &size : it.second) {
> +			if (maxSize_.width < size.maxWidth &&
> +			    maxSize_.height < size.maxHeight) {
> +				maxSize_.width = size.maxWidth;
> +				maxSize_.height = size.maxHeight;
> +				mbusCode_ = mbusCode;
> +			}
> +		}
> +	}
> +	if (maxSize_.width == 0) {
> +		LOG(IPU3, Info) << "Sensor '" << sensor_->entityName()
> +				<< "' detected, but no supported image format "
> +				<< " found: skip camera creation";
> +		return -ENODEV;
> +	}
> +
>  	csi2_ = new V4L2Subdevice(csi2Entity);
>  	ret = csi2_->open();
>  	if (ret)
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund