[libcamera-devel] [PATCH 3/4] ipa: raspberrypi: Add camera helper for Sony IMX296 sensor

[Laurent Pinchart]

Hi Naush,

On Mon, Dec 20, 2021 at 07:41:37AM +0000, Naushir Patuck wrote:
> On Sun, 19 Dec 2021 at 23:27, Laurent Pinchart wrote:
> 
> > From: Naushir Patuck <naush at raspberrypi.com>
> >
> > The Sony IMX296 sensor has an exponential gain model, and adds a fixed
> > 14.26µs offset to the exposure time expressed in line units.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/ipa/raspberrypi/cam_helper_imx296.cpp | 126 ++++++++++++++++++++++
> >  src/ipa/raspberrypi/meson.build           |   1 +
> >  2 files changed, 127 insertions(+)
> >  create mode 100644 src/ipa/raspberrypi/cam_helper_imx296.cpp
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper_imx296.cpp
> > b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> > new file mode 100644
> > index 000000000000..9c34582861f1
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> > @@ -0,0 +1,126 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2020, Raspberry Pi (Trading) Limited
> > + *
> > + * cam_helper_imx296.cpp - camera helper for imx296 sensor
> > + */
> > +
> > +#include <assert.h>
> > +#include <cmath>
> > +#include <stddef.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +
> > +/*
> > + * We have observed the imx296 embedded data stream randomly return junk
> > + * reister values.  Do not rely on embedded data until this has been resolved.
> > + */
> > +#define ENABLE_EMBEDDED_DATA 0
> 
> I'm afraid this may not be completely true :-)
> 
> This file was created by doing a search/replace from the imx219 cam helper.
> Perhaps it might be better to completely remove embedded data support
> until we have a chance to properly evaluate this functionality on the
> imx296?

Sounds good to me. I'll drop it.

> Either way,
> 
> Signed-off-by: Naushir Patuck <naush <naushir at gmail.com>@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush <naushir at gmail.com>@raspberrypi.com>
> 
> > +
> > +#include "cam_helper.hpp"
> > +#if ENABLE_EMBEDDED_DATA
> > +#include "md_parser.hpp"
> > +#endif
> > +
> > +//#include "imx296_gain_table.hpp"
> > +
> > +using namespace RPiController;
> > +using libcamera::utils::Duration;
> > +using namespace std::literals::chrono_literals;
> > +
> > +/*
> > + * We care about one gain register and a pair of exposure registers. Their I2C
> > + * addresses from the Sony IMX296 datasheet:
> > + */
> > +constexpr uint32_t gainReg = 0x157;
> > +constexpr uint32_t expHiReg = 0x15a;
> > +constexpr uint32_t expLoReg = 0x15b;
> > +constexpr std::initializer_list<uint32_t> registerList [[maybe_unused]] =
> > { expHiReg, expLoReg, gainReg };
> > +
> > +class CamHelperImx296 : public CamHelper
> > +{
> > +public:
> > +       CamHelperImx296();
> > +       uint32_t GainCode(double gain) const override;
> > +       double Gain(uint32_t gain_code) const override;
> > +       uint32_t ExposureLines(Duration exposure) const override;
> > +       Duration Exposure(uint32_t exposure_lines) const override;
> > +       unsigned int MistrustFramesModeSwitch() const override;
> > +       bool SensorEmbeddedDataPresent() const override;
> > +
> > +private:
> > +       static constexpr uint32_t maxGainCode = 239;
> > +       static constexpr Duration timePerLine = 550.0 / 37.125e6 * 1.0s;
> > +
> > +       /*
> > +        * Smallest difference between the frame length and integration time,
> > +        * in units of lines.
> > +        */
> > +       static constexpr int frameIntegrationDiff = 4;
> > +
> > +       void PopulateMetadata(const MdParser::RegisterMap &registers,
> > +                             Metadata &metadata) const override;
> > +};
> > +
> > +CamHelperImx296::CamHelperImx296()
> > +#if ENABLE_EMBEDDED_DATA
> > +       : CamHelper(std::make_unique<MdParserSmia>(registerList),
> > frameIntegrationDiff)
> > +#else
> > +       : CamHelper(nullptr, frameIntegrationDiff)
> > +#endif
> > +{
> > +}
> > +
> > +uint32_t CamHelperImx296::GainCode(double gain) const
> > +{
> > +       uint32_t code = 20 * std::log10(gain) * 10;
> > +       return std::min(code, maxGainCode);
> > +}
> > +
> > +double CamHelperImx296::Gain(uint32_t gain_code) const
> > +{
> > +       return std::pow(10.0, gain_code / 200.0);
> > +}
> > +
> > +uint32_t CamHelperImx296::ExposureLines(Duration exposure) const
> > +{
> > +       return (exposure - 14.26us) / timePerLine;
> > +}
> > +
> > +Duration CamHelperImx296::Exposure(uint32_t exposure_lines) const
> > +{
> > +       return exposure_lines * timePerLine + 14.26us;
> > +}
> > +
> > +unsigned int CamHelperImx296::MistrustFramesModeSwitch() const
> > +{
> > +       /*
> > +        * For reasons unknown, we do occasionally get a bogus metadata frame
> > +        * at a mode switch (though not at start-up). Possibly warrants some
> > +        * investigation, though not a big deal.
> > +        */
> > +       return 1;
> 
> This may also be something to evaluate (assuming metadata gets enabled).

I'll drop it too.

> > +}
> > +
> > +bool CamHelperImx296::SensorEmbeddedDataPresent() const
> > +{
> > +       return ENABLE_EMBEDDED_DATA;
> > +}
> > +
> > +void CamHelperImx296::PopulateMetadata(const MdParser::RegisterMap &registers,
> > +                                      Metadata &metadata) const
> > +{
> > +       DeviceStatus deviceStatus;
> > +
> > +       deviceStatus.shutter_speed = Exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));
> > +       deviceStatus.analogue_gain = Gain(registers.at(gainReg));
> > +
> > +       metadata.Set("device.status", deviceStatus);
> > +}
> > +
> > +static CamHelper *Create()
> > +{
> > +       return new CamHelperImx296();
> > +}
> > +
> > +static RegisterCamHelper reg("imx296", &Create);
> > diff --git a/src/ipa/raspberrypi/meson.build
> > b/src/ipa/raspberrypi/meson.build
> > index 176055f42248..32897e07dad9 100644
> > --- a/src/ipa/raspberrypi/meson.build
> > +++ b/src/ipa/raspberrypi/meson.build
> > @@ -21,6 +21,7 @@ rpi_ipa_sources = files([
> >      'cam_helper_ov5647.cpp',
> >      'cam_helper_imx219.cpp',
> >      'cam_helper_imx290.cpp',
> > +    'cam_helper_imx296.cpp',
> >      'cam_helper_imx477.cpp',
> >      'cam_helper_imx519.cpp',
> >      'cam_helper_ov9281.cpp',

-- 
Regards,

Laurent Pinchart