[libcamera-devel] [PATCH v4 6/9] libcamera: ipu3: cio2: Add function to generate configuration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Jun 26 02:41:05 CEST 2020
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 at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at 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)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list