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

[Jacopo Mondi]

Hi Laurent,

On Tue, Apr 02, 2019 at 08:22:35PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Apr 02, 2019 at 07:13:00PM +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>
> > ---
> >  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)
>
> I wonder why I hadn't thought about this before, would it make sense to
> move this function to the CIO2Device class, make it static, and rename
> it to mediaBusToPixelFormat (or just mediaBusToFormat) ?
>

It might make sense, I guess.

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

With or without the above change?

> > +{
> > +	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)
>
> --
> Regards,
>
> Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190402/8c153a5e/attachment.sig>