[libcamera-devel,v4,6/9] libcamera: ipu3: cio2: Add function to generate configuration
diff mbox series

Message ID 20200625223900.1282164-7-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: ipu3: Allow zero-copy RAW stream
Related show

Commit Message

Niklas Söderlund June 25, 2020, 10:38 p.m. UTC 
Collect the code used to generate configurations for the CIO2 block in
the CIO2Device class. This allows simplifying the code and allow further
changes to only happen at one code location.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v3
- Remove struct MbusInfo and use PixelFormatInfo::info() to find V4L2 fourcc.
- Pass size argument by value to generateConfiguration()
- Make use of Size::isNull()

* Changes since v2
- Remove unneeded code to pick sensor FourCC.
- Remove desiredPixelFormat from generateConfiguration() as it's not
  needed.
- Rename sensorFormat_ cio2Configuration_
- Consolidate all format information in a single table.

* Changes since v1
- Use anonymous namespace instead of static for sensorMbusToPixel map.
- Handle case where the requested mbus code is not supported by the sensor.
- Update commit message.
---
 src/libcamera/pipeline/ipu3/cio2.cpp | 45 +++++++++++++++++++---
 src/libcamera/pipeline/ipu3/cio2.h   |  3 ++
 src/libcamera/pipeline/ipu3/ipu3.cpp | 56 +++++++---------------------
 3 files changed, 56 insertions(+), 48 deletions(-)

Comments

Laurent Pinchart June 26, 2020, 12:41 a.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Fri, Jun 26, 2020 at 12:38:57AM +0200, Niklas Söderlund wrote:
> Collect the code used to generate configurations for the CIO2 block in
> the CIO2Device class. This allows simplifying the code and allow further
> changes to only happen at one code location.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since v3
> - Remove struct MbusInfo and use PixelFormatInfo::info() to find V4L2 fourcc.
> - Pass size argument by value to generateConfiguration()
> - Make use of Size::isNull()

Either you've forgotten to use it, or you've forgotten to remove this
line from the changelog.

> 
> * Changes since v2
> - Remove unneeded code to pick sensor FourCC.
> - Remove desiredPixelFormat from generateConfiguration() as it's not
>   needed.
> - Rename sensorFormat_ cio2Configuration_
> - Consolidate all format information in a single table.
> 
> * Changes since v1
> - Use anonymous namespace instead of static for sensorMbusToPixel map.
> - Handle case where the requested mbus code is not supported by the sensor.
> - Update commit message.
> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp | 45 +++++++++++++++++++---
>  src/libcamera/pipeline/ipu3/cio2.h   |  3 ++
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 56 +++++++---------------------
>  3 files changed, 56 insertions(+), 48 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index 31eddbc3c7745a32..cf5ccd6013c64d4d 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -9,6 +9,9 @@
>  
>  #include <linux/media-bus-format.h>
>  
> +#include <libcamera/formats.h>
> +#include <libcamera/stream.h>
> +
>  #include "libcamera/internal/camera_sensor.h"
>  #include "libcamera/internal/media_device.h"
>  #include "libcamera/internal/v4l2_subdevice.h"
> @@ -20,11 +23,11 @@ LOG_DECLARE_CATEGORY(IPU3)
>  
>  namespace {
>  
> -static const std::map<uint32_t, V4L2PixelFormat> mbusCodesToInfo = {
> -	{ MEDIA_BUS_FMT_SBGGR10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },
> -	{ MEDIA_BUS_FMT_SGBRG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },
> -	{ MEDIA_BUS_FMT_SGRBG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },
> -	{ MEDIA_BUS_FMT_SRGGB10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },
> +static const std::map<uint32_t, PixelFormat> mbusCodesToInfo = {
> +	{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10_IPU3 },
> +	{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10_IPU3 },
> +	{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10_IPU3 },
> +	{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10_IPU3 },
>  };
>  
>  } /* namespace */
> @@ -159,7 +162,9 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
>  	if (itInfo == mbusCodesToInfo.end())
>  		return -EINVAL;
>  
> -	outputFormat->fourcc = itInfo->second;
> +	const PixelFormatInfo &info = PixelFormatInfo::info(itInfo->second);
> +
> +	outputFormat->fourcc = info.v4l2Format;
>  	outputFormat->size = sensorFormat.size;
>  	outputFormat->planesCount = 1;
>  
> @@ -172,6 +177,34 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
>  	return 0;
>  }
>  
> +StreamConfiguration
> +CIO2Device::generateConfiguration(Size size) const
> +{
> +	StreamConfiguration cfg;
> +
> +	/* If no desired size use the sensor resolution. */
> +	if (!size.width && !size.height)
> +		size = sensor_->resolution();
> +
> +	/* Query the sensor static information for closest match. */
> +	std::vector<unsigned int> mbusCodes;
> +	std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),
> +		       std::back_inserter(mbusCodes),
> +		       [](auto &pair) { return pair.first; });
> +
> +	V4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);
> +	if (!sensorFormat.mbus_code) {
> +		LOG(IPU3, Error) << "Sensor does not support mbus code";
> +		return {};
> +	}
> +
> +	cfg.size = sensorFormat.size;
> +	cfg.pixelFormat = mbusCodesToInfo.at(sensorFormat.mbus_code);
> +	cfg.bufferCount = CIO2_BUFFER_COUNT;
> +
> +	return cfg;
> +}
> +
>  /**
>   * \brief Allocate frame buffers for the CIO2 output
>   *
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> index b2c4f89d682d6cfb..5825433246c7fb89 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.h
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -20,6 +20,7 @@ class V4L2DeviceFormat;
>  class V4L2Subdevice;
>  class V4L2VideoDevice;
>  struct Size;
> +struct StreamConfiguration;
>  
>  class CIO2Device
>  {
> @@ -32,6 +33,8 @@ public:
>  	int init(const MediaDevice *media, unsigned int index);
>  	int configure(const Size &size, V4L2DeviceFormat *outputFormat);
>  
> +	StreamConfiguration generateConfiguration(Size size) const;
> +
>  	int allocateBuffers();
>  	void freeBuffers();
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 6e5eb5609a3c2388..c0e727e592f46883 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -36,13 +36,6 @@ LOG_DEFINE_CATEGORY(IPU3)
>  
>  class IPU3CameraData;
>  
> -static const std::map<uint32_t, PixelFormat> sensorMbusToPixel = {
> -	{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10_IPU3 },
> -	{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10_IPU3 },
> -	{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10_IPU3 },
> -	{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10_IPU3 },
> -};
> -
>  class ImgUDevice
>  {
>  public:
> @@ -147,7 +140,7 @@ public:
>  
>  	Status validate() override;
>  
> -	const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }
> +	const StreamConfiguration &cio2Format() const { return cio2Configuration_; };
>  	const std::vector<const IPU3Stream *> &streams() { return streams_; }
>  
>  private:
> @@ -165,7 +158,7 @@ private:
>  	std::shared_ptr<Camera> camera_;
>  	const IPU3CameraData *data_;
>  
> -	V4L2SubdeviceFormat sensorFormat_;
> +	StreamConfiguration cio2Configuration_;
>  	std::vector<const IPU3Stream *> streams_;
>  };
>  
> @@ -252,7 +245,7 @@ void IPU3CameraConfiguration::assignStreams()
>  
>  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
>  			stream = &data_->rawStream_;
> -		else if (cfg.size == sensorFormat_.size)
> +		else if (cfg.size == cio2Configuration_.size)
>  			stream = &data_->outStream_;
>  		else
>  			stream = &data_->vfStream_;
> @@ -277,8 +270,8 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
>  		 */
>  		if (!cfg.size.width || !cfg.size.height) {
>  			cfg.size.width = 1280;
> -			cfg.size.height = 1280 * sensorFormat_.size.height
> -					/ sensorFormat_.size.width;
> +			cfg.size.height = 1280 * cio2Configuration_.size.height
> +					/ cio2Configuration_.size.width;
>  		}
>  
>  		/*
> @@ -297,7 +290,7 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
>  		 * \todo: Properly support cropping when the ImgU driver
>  		 * interface will be cleaned up.
>  		 */
> -		cfg.size = sensorFormat_.size;
> +		cfg.size = cio2Configuration_.size;
>  	}
>  
>  	/*
> @@ -313,7 +306,6 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
>  
>  CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  {
> -	const CameraSensor *sensor = data_->cio2_.sensor_;
>  	Status status = Valid;
>  
>  	if (config_.empty())
> @@ -340,16 +332,10 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  			size.height = cfg.size.height;
>  	}
>  
> -	if (!size.width || !size.height)
> -		size = sensor->resolution();
> -
> -	sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
> -					    MEDIA_BUS_FMT_SGBRG10_1X10,
> -					    MEDIA_BUS_FMT_SGRBG10_1X10,
> -					    MEDIA_BUS_FMT_SRGGB10_1X10 },
> -					  size);
> -	if (!sensorFormat_.size.width || !sensorFormat_.size.height)
> -		sensorFormat_.size = sensor->resolution();
> +	/* Generate raw configuration from CIO2. */
> +	cio2Configuration_ = data_->cio2_.generateConfiguration(size);
> +	if (!cio2Configuration_.pixelFormat.isValid())
> +		return Invalid;
>  
>  	/* Assign streams to each configuration entry. */
>  	assignStreams();
> @@ -361,20 +347,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  		const IPU3Stream *stream = streams_[i];
>  
>  		if (stream->raw_) {
> -			const auto &itFormat =
> -				sensorMbusToPixel.find(sensorFormat_.mbus_code);
> -			if (itFormat == sensorMbusToPixel.end())
> -				return Invalid;
> -
> -			cfg.pixelFormat = itFormat->second;
> -			cfg.size = sensorFormat_.size;
> +			cfg = cio2Configuration_;
>  		} else {
>  			bool scale = stream == &data_->vfStream_;
>  			adjustStream(config_[i], scale);
> +			cfg.bufferCount = IPU3_BUFFER_COUNT;
>  		}
>  
> -		cfg.bufferCount = IPU3_BUFFER_COUNT;
> -
>  		if (cfg.pixelFormat != oldCfg.pixelFormat ||
>  		    cfg.size != oldCfg.size) {
>  			LOG(IPU3, Debug)
> @@ -454,14 +433,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
>  
>  			cfg.size = data->cio2_.sensor_->resolution();
>  
> -			V4L2SubdeviceFormat sensorFormat =
> -				data->cio2_.sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
> -								 MEDIA_BUS_FMT_SGBRG10_1X10,
> -								 MEDIA_BUS_FMT_SGRBG10_1X10,
> -								 MEDIA_BUS_FMT_SRGGB10_1X10 },
> -							       cfg.size);
> -			cfg.pixelFormat =
> -				sensorMbusToPixel.at(sensorFormat.mbus_code);
> +			cfg = data->cio2_.generateConfiguration(cfg.size);
>  			break;
>  		}
>  
> @@ -575,7 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	 * Pass the requested stream size to the CIO2 unit and get back the
>  	 * adjusted format to be propagated to the ImgU output devices.
>  	 */
> -	const Size &sensorSize = config->sensorFormat().size;
> +	const Size &sensorSize = config->cio2Format().size;
>  	V4L2DeviceFormat cio2Format = {};
>  	ret = cio2->configure(sensorSize, &cio2Format);
>  	if (ret)
Niklas Söderlund June 26, 2020, 12:55 a.m. UTC | #2 
Hi Laurent,

On 2020-06-26 03:41:05 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Fri, Jun 26, 2020 at 12:38:57AM +0200, Niklas Söderlund wrote:
> > Collect the code used to generate configurations for the CIO2 block in
> > the CIO2Device class. This allows simplifying the code and allow further
> > changes to only happen at one code location.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > * Changes since v3
> > - Remove struct MbusInfo and use PixelFormatInfo::info() to find V4L2 fourcc.
> > - Pass size argument by value to generateConfiguration()
> > - Make use of Size::isNull()
> 
> Either you've forgotten to use it, or you've forgotten to remove this
> line from the changelog.

I added it to the code and then removed it again and forgot it in the 
changelog. I think isNull() shall be used here but since the name is 
still debated I wanted this series to be applicable on master so 
reverted back. If isNull() is merged before this series I will switch to 
it. Same goes for the map_keys() helper a bit further in this series.

> 
> > 
> > * Changes since v2
> > - Remove unneeded code to pick sensor FourCC.
> > - Remove desiredPixelFormat from generateConfiguration() as it's not
> >   needed.
> > - Rename sensorFormat_ cio2Configuration_
> > - Consolidate all format information in a single table.
> > 
> > * Changes since v1
> > - Use anonymous namespace instead of static for sensorMbusToPixel map.
> > - Handle case where the requested mbus code is not supported by the sensor.
> > - Update commit message.
> > ---
> >  src/libcamera/pipeline/ipu3/cio2.cpp | 45 +++++++++++++++++++---
> >  src/libcamera/pipeline/ipu3/cio2.h   |  3 ++
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 56 +++++++---------------------
> >  3 files changed, 56 insertions(+), 48 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> > index 31eddbc3c7745a32..cf5ccd6013c64d4d 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> > @@ -9,6 +9,9 @@
> >  
> >  #include <linux/media-bus-format.h>
> >  
> > +#include <libcamera/formats.h>
> > +#include <libcamera/stream.h>
> > +
> >  #include "libcamera/internal/camera_sensor.h"
> >  #include "libcamera/internal/media_device.h"
> >  #include "libcamera/internal/v4l2_subdevice.h"
> > @@ -20,11 +23,11 @@ LOG_DECLARE_CATEGORY(IPU3)
> >  
> >  namespace {
> >  
> > -static const std::map<uint32_t, V4L2PixelFormat> mbusCodesToInfo = {
> > -	{ MEDIA_BUS_FMT_SBGGR10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },
> > -	{ MEDIA_BUS_FMT_SGBRG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },
> > -	{ MEDIA_BUS_FMT_SGRBG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },
> > -	{ MEDIA_BUS_FMT_SRGGB10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },
> > +static const std::map<uint32_t, PixelFormat> mbusCodesToInfo = {
> > +	{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10_IPU3 },
> > +	{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10_IPU3 },
> > +	{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10_IPU3 },
> > +	{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10_IPU3 },
> >  };
> >  
> >  } /* namespace */
> > @@ -159,7 +162,9 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
> >  	if (itInfo == mbusCodesToInfo.end())
> >  		return -EINVAL;
> >  
> > -	outputFormat->fourcc = itInfo->second;
> > +	const PixelFormatInfo &info = PixelFormatInfo::info(itInfo->second);
> > +
> > +	outputFormat->fourcc = info.v4l2Format;
> >  	outputFormat->size = sensorFormat.size;
> >  	outputFormat->planesCount = 1;
> >  
> > @@ -172,6 +177,34 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
> >  	return 0;
> >  }
> >  
> > +StreamConfiguration
> > +CIO2Device::generateConfiguration(Size size) const
> > +{
> > +	StreamConfiguration cfg;
> > +
> > +	/* If no desired size use the sensor resolution. */
> > +	if (!size.width && !size.height)
> > +		size = sensor_->resolution();
> > +
> > +	/* Query the sensor static information for closest match. */
> > +	std::vector<unsigned int> mbusCodes;
> > +	std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),
> > +		       std::back_inserter(mbusCodes),
> > +		       [](auto &pair) { return pair.first; });
> > +
> > +	V4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);
> > +	if (!sensorFormat.mbus_code) {
> > +		LOG(IPU3, Error) << "Sensor does not support mbus code";
> > +		return {};
> > +	}
> > +
> > +	cfg.size = sensorFormat.size;
> > +	cfg.pixelFormat = mbusCodesToInfo.at(sensorFormat.mbus_code);
> > +	cfg.bufferCount = CIO2_BUFFER_COUNT;
> > +
> > +	return cfg;
> > +}
> > +
> >  /**
> >   * \brief Allocate frame buffers for the CIO2 output
> >   *
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> > index b2c4f89d682d6cfb..5825433246c7fb89 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.h
> > +++ b/src/libcamera/pipeline/ipu3/cio2.h
> > @@ -20,6 +20,7 @@ class V4L2DeviceFormat;
> >  class V4L2Subdevice;
> >  class V4L2VideoDevice;
> >  struct Size;
> > +struct StreamConfiguration;
> >  
> >  class CIO2Device
> >  {
> > @@ -32,6 +33,8 @@ public:
> >  	int init(const MediaDevice *media, unsigned int index);
> >  	int configure(const Size &size, V4L2DeviceFormat *outputFormat);
> >  
> > +	StreamConfiguration generateConfiguration(Size size) const;
> > +
> >  	int allocateBuffers();
> >  	void freeBuffers();
> >  
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 6e5eb5609a3c2388..c0e727e592f46883 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -36,13 +36,6 @@ LOG_DEFINE_CATEGORY(IPU3)
> >  
> >  class IPU3CameraData;
> >  
> > -static const std::map<uint32_t, PixelFormat> sensorMbusToPixel = {
> > -	{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10_IPU3 },
> > -	{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10_IPU3 },
> > -	{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10_IPU3 },
> > -	{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10_IPU3 },
> > -};
> > -
> >  class ImgUDevice
> >  {
> >  public:
> > @@ -147,7 +140,7 @@ public:
> >  
> >  	Status validate() override;
> >  
> > -	const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }
> > +	const StreamConfiguration &cio2Format() const { return cio2Configuration_; };
> >  	const std::vector<const IPU3Stream *> &streams() { return streams_; }
> >  
> >  private:
> > @@ -165,7 +158,7 @@ private:
> >  	std::shared_ptr<Camera> camera_;
> >  	const IPU3CameraData *data_;
> >  
> > -	V4L2SubdeviceFormat sensorFormat_;
> > +	StreamConfiguration cio2Configuration_;
> >  	std::vector<const IPU3Stream *> streams_;
> >  };
> >  
> > @@ -252,7 +245,7 @@ void IPU3CameraConfiguration::assignStreams()
> >  
> >  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> >  			stream = &data_->rawStream_;
> > -		else if (cfg.size == sensorFormat_.size)
> > +		else if (cfg.size == cio2Configuration_.size)
> >  			stream = &data_->outStream_;
> >  		else
> >  			stream = &data_->vfStream_;
> > @@ -277,8 +270,8 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
> >  		 */
> >  		if (!cfg.size.width || !cfg.size.height) {
> >  			cfg.size.width = 1280;
> > -			cfg.size.height = 1280 * sensorFormat_.size.height
> > -					/ sensorFormat_.size.width;
> > +			cfg.size.height = 1280 * cio2Configuration_.size.height
> > +					/ cio2Configuration_.size.width;
> >  		}
> >  
> >  		/*
> > @@ -297,7 +290,7 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
> >  		 * \todo: Properly support cropping when the ImgU driver
> >  		 * interface will be cleaned up.
> >  		 */
> > -		cfg.size = sensorFormat_.size;
> > +		cfg.size = cio2Configuration_.size;
> >  	}
> >  
> >  	/*
> > @@ -313,7 +306,6 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
> >  
> >  CameraConfiguration::Status IPU3CameraConfiguration::validate()
> >  {
> > -	const CameraSensor *sensor = data_->cio2_.sensor_;
> >  	Status status = Valid;
> >  
> >  	if (config_.empty())
> > @@ -340,16 +332,10 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> >  			size.height = cfg.size.height;
> >  	}
> >  
> > -	if (!size.width || !size.height)
> > -		size = sensor->resolution();
> > -
> > -	sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
> > -					    MEDIA_BUS_FMT_SGBRG10_1X10,
> > -					    MEDIA_BUS_FMT_SGRBG10_1X10,
> > -					    MEDIA_BUS_FMT_SRGGB10_1X10 },
> > -					  size);
> > -	if (!sensorFormat_.size.width || !sensorFormat_.size.height)
> > -		sensorFormat_.size = sensor->resolution();
> > +	/* Generate raw configuration from CIO2. */
> > +	cio2Configuration_ = data_->cio2_.generateConfiguration(size);
> > +	if (!cio2Configuration_.pixelFormat.isValid())
> > +		return Invalid;
> >  
> >  	/* Assign streams to each configuration entry. */
> >  	assignStreams();
> > @@ -361,20 +347,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> >  		const IPU3Stream *stream = streams_[i];
> >  
> >  		if (stream->raw_) {
> > -			const auto &itFormat =
> > -				sensorMbusToPixel.find(sensorFormat_.mbus_code);
> > -			if (itFormat == sensorMbusToPixel.end())
> > -				return Invalid;
> > -
> > -			cfg.pixelFormat = itFormat->second;
> > -			cfg.size = sensorFormat_.size;
> > +			cfg = cio2Configuration_;
> >  		} else {
> >  			bool scale = stream == &data_->vfStream_;
> >  			adjustStream(config_[i], scale);
> > +			cfg.bufferCount = IPU3_BUFFER_COUNT;
> >  		}
> >  
> > -		cfg.bufferCount = IPU3_BUFFER_COUNT;
> > -
> >  		if (cfg.pixelFormat != oldCfg.pixelFormat ||
> >  		    cfg.size != oldCfg.size) {
> >  			LOG(IPU3, Debug)
> > @@ -454,14 +433,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> >  
> >  			cfg.size = data->cio2_.sensor_->resolution();
> >  
> > -			V4L2SubdeviceFormat sensorFormat =
> > -				data->cio2_.sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
> > -								 MEDIA_BUS_FMT_SGBRG10_1X10,
> > -								 MEDIA_BUS_FMT_SGRBG10_1X10,
> > -								 MEDIA_BUS_FMT_SRGGB10_1X10 },
> > -							       cfg.size);
> > -			cfg.pixelFormat =
> > -				sensorMbusToPixel.at(sensorFormat.mbus_code);
> > +			cfg = data->cio2_.generateConfiguration(cfg.size);
> >  			break;
> >  		}
> >  
> > @@ -575,7 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >  	 * Pass the requested stream size to the CIO2 unit and get back the
> >  	 * adjusted format to be propagated to the ImgU output devices.
> >  	 */
> > -	const Size &sensorSize = config->sensorFormat().size;
> > +	const Size &sensorSize = config->cio2Format().size;
> >  	V4L2DeviceFormat cio2Format = {};
> >  	ret = cio2->configure(sensorSize, &cio2Format);
> >  	if (ret)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 31eddbc3c7745a32..cf5ccd6013c64d4d 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -9,6 +9,9 @@ 
 
 #include <linux/media-bus-format.h>
 
+#include <libcamera/formats.h>
+#include <libcamera/stream.h>
+
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/v4l2_subdevice.h"
@@ -20,11 +23,11 @@  LOG_DECLARE_CATEGORY(IPU3)
 
 namespace {
 
-static const std::map<uint32_t, V4L2PixelFormat> mbusCodesToInfo = {
-	{ MEDIA_BUS_FMT_SBGGR10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },
-	{ MEDIA_BUS_FMT_SGBRG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },
-	{ MEDIA_BUS_FMT_SGRBG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },
-	{ MEDIA_BUS_FMT_SRGGB10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },
+static const std::map<uint32_t, PixelFormat> mbusCodesToInfo = {
+	{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10_IPU3 },
+	{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10_IPU3 },
+	{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10_IPU3 },
+	{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10_IPU3 },
 };
 
 } /* namespace */
@@ -159,7 +162,9 @@  int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
 	if (itInfo == mbusCodesToInfo.end())
 		return -EINVAL;
 
-	outputFormat->fourcc = itInfo->second;
+	const PixelFormatInfo &info = PixelFormatInfo::info(itInfo->second);
+
+	outputFormat->fourcc = info.v4l2Format;
 	outputFormat->size = sensorFormat.size;
 	outputFormat->planesCount = 1;
 
@@ -172,6 +177,34 @@  int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)
 	return 0;
 }
 
+StreamConfiguration
+CIO2Device::generateConfiguration(Size size) const
+{
+	StreamConfiguration cfg;
+
+	/* If no desired size use the sensor resolution. */
+	if (!size.width && !size.height)
+		size = sensor_->resolution();
+
+	/* Query the sensor static information for closest match. */
+	std::vector<unsigned int> mbusCodes;
+	std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),
+		       std::back_inserter(mbusCodes),
+		       [](auto &pair) { return pair.first; });
+
+	V4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);
+	if (!sensorFormat.mbus_code) {
+		LOG(IPU3, Error) << "Sensor does not support mbus code";
+		return {};
+	}
+
+	cfg.size = sensorFormat.size;
+	cfg.pixelFormat = mbusCodesToInfo.at(sensorFormat.mbus_code);
+	cfg.bufferCount = CIO2_BUFFER_COUNT;
+
+	return cfg;
+}
+
 /**
  * \brief Allocate frame buffers for the CIO2 output
  *
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index b2c4f89d682d6cfb..5825433246c7fb89 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -20,6 +20,7 @@  class V4L2DeviceFormat;
 class V4L2Subdevice;
 class V4L2VideoDevice;
 struct Size;
+struct StreamConfiguration;
 
 class CIO2Device
 {
@@ -32,6 +33,8 @@  public:
 	int init(const MediaDevice *media, unsigned int index);
 	int configure(const Size &size, V4L2DeviceFormat *outputFormat);
 
+	StreamConfiguration generateConfiguration(Size size) const;
+
 	int allocateBuffers();
 	void freeBuffers();
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 6e5eb5609a3c2388..c0e727e592f46883 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -36,13 +36,6 @@  LOG_DEFINE_CATEGORY(IPU3)
 
 class IPU3CameraData;
 
-static const std::map<uint32_t, PixelFormat> sensorMbusToPixel = {
-	{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10_IPU3 },
-	{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10_IPU3 },
-	{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10_IPU3 },
-	{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10_IPU3 },
-};
-
 class ImgUDevice
 {
 public:
@@ -147,7 +140,7 @@  public:
 
 	Status validate() override;
 
-	const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }
+	const StreamConfiguration &cio2Format() const { return cio2Configuration_; };
 	const std::vector<const IPU3Stream *> &streams() { return streams_; }
 
 private:
@@ -165,7 +158,7 @@  private:
 	std::shared_ptr<Camera> camera_;
 	const IPU3CameraData *data_;
 
-	V4L2SubdeviceFormat sensorFormat_;
+	StreamConfiguration cio2Configuration_;
 	std::vector<const IPU3Stream *> streams_;
 };
 
@@ -252,7 +245,7 @@  void IPU3CameraConfiguration::assignStreams()
 
 		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
 			stream = &data_->rawStream_;
-		else if (cfg.size == sensorFormat_.size)
+		else if (cfg.size == cio2Configuration_.size)
 			stream = &data_->outStream_;
 		else
 			stream = &data_->vfStream_;
@@ -277,8 +270,8 @@  void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
 		 */
 		if (!cfg.size.width || !cfg.size.height) {
 			cfg.size.width = 1280;
-			cfg.size.height = 1280 * sensorFormat_.size.height
-					/ sensorFormat_.size.width;
+			cfg.size.height = 1280 * cio2Configuration_.size.height
+					/ cio2Configuration_.size.width;
 		}
 
 		/*
@@ -297,7 +290,7 @@  void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
 		 * \todo: Properly support cropping when the ImgU driver
 		 * interface will be cleaned up.
 		 */
-		cfg.size = sensorFormat_.size;
+		cfg.size = cio2Configuration_.size;
 	}
 
 	/*
@@ -313,7 +306,6 @@  void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
 
 CameraConfiguration::Status IPU3CameraConfiguration::validate()
 {
-	const CameraSensor *sensor = data_->cio2_.sensor_;
 	Status status = Valid;
 
 	if (config_.empty())
@@ -340,16 +332,10 @@  CameraConfiguration::Status IPU3CameraConfiguration::validate()
 			size.height = cfg.size.height;
 	}
 
-	if (!size.width || !size.height)
-		size = sensor->resolution();
-
-	sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
-					    MEDIA_BUS_FMT_SGBRG10_1X10,
-					    MEDIA_BUS_FMT_SGRBG10_1X10,
-					    MEDIA_BUS_FMT_SRGGB10_1X10 },
-					  size);
-	if (!sensorFormat_.size.width || !sensorFormat_.size.height)
-		sensorFormat_.size = sensor->resolution();
+	/* Generate raw configuration from CIO2. */
+	cio2Configuration_ = data_->cio2_.generateConfiguration(size);
+	if (!cio2Configuration_.pixelFormat.isValid())
+		return Invalid;
 
 	/* Assign streams to each configuration entry. */
 	assignStreams();
@@ -361,20 +347,13 @@  CameraConfiguration::Status IPU3CameraConfiguration::validate()
 		const IPU3Stream *stream = streams_[i];
 
 		if (stream->raw_) {
-			const auto &itFormat =
-				sensorMbusToPixel.find(sensorFormat_.mbus_code);
-			if (itFormat == sensorMbusToPixel.end())
-				return Invalid;
-
-			cfg.pixelFormat = itFormat->second;
-			cfg.size = sensorFormat_.size;
+			cfg = cio2Configuration_;
 		} else {
 			bool scale = stream == &data_->vfStream_;
 			adjustStream(config_[i], scale);
+			cfg.bufferCount = IPU3_BUFFER_COUNT;
 		}
 
-		cfg.bufferCount = IPU3_BUFFER_COUNT;
-
 		if (cfg.pixelFormat != oldCfg.pixelFormat ||
 		    cfg.size != oldCfg.size) {
 			LOG(IPU3, Debug)
@@ -454,14 +433,7 @@  CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
 
 			cfg.size = data->cio2_.sensor_->resolution();
 
-			V4L2SubdeviceFormat sensorFormat =
-				data->cio2_.sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10,
-								 MEDIA_BUS_FMT_SGBRG10_1X10,
-								 MEDIA_BUS_FMT_SGRBG10_1X10,
-								 MEDIA_BUS_FMT_SRGGB10_1X10 },
-							       cfg.size);
-			cfg.pixelFormat =
-				sensorMbusToPixel.at(sensorFormat.mbus_code);
+			cfg = data->cio2_.generateConfiguration(cfg.size);
 			break;
 		}
 
@@ -575,7 +547,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	 * Pass the requested stream size to the CIO2 unit and get back the
 	 * adjusted format to be propagated to the ImgU output devices.
 	 */
-	const Size &sensorSize = config->sensorFormat().size;
+	const Size &sensorSize = config->cio2Format().size;
 	V4L2DeviceFormat cio2Format = {};
 	ret = cio2->configure(sensorSize, &cio2Format);
 	if (ret)