[libcamera-devel] [PATCH v4 6/9] libcamera: ipu3: cio2: Add function to generate configuration
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Jun 26 02:55:03 CEST 2020
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 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.
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
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list