[libcamera-devel,2/2] ipa: raspberrypi: Add support for imx290/imx327 sensors

11499 diff mbox series

imx290 and imx327 share the same kernel driver (imx290.c) and are
therefore both recognised here as "imx290". We add the necessary
CamHelper for these sensors, as well as a camera tuning file.

The tuning was done with an Innomaker STARVIS IMX327LQR module. These
have no IR cut filter so there is no proper colour tuning. However,
you should obtain reasonable results for most modules using this
sensor. Specific tunings for further modules can always be added
subsequently.

To use this sensor on the Raspberry Pi platform, please add

dtoverlay=imx290,clock-frequency=74250000

into your /boot/config.txt file.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
 src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
 src/ipa/raspberrypi/data/meson.build      |   1 +
 src/ipa/raspberrypi/meson.build           |   1 +
 4 files changed, 234 insertions(+)
 create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
 create mode 100644 src/ipa/raspberrypi/data/imx290.json

Hi David,

Thank you for the patch.

On Thu, Mar 04, 2021 at 03:31:20PM +0000, David Plowman wrote:
> imx290 and imx327 share the same kernel driver (imx290.c) and are
> therefore both recognised here as "imx290".

Are there any differences between the two sensors that may need to be
handled anywhere in libcamera ?

> We add the necessary
> CamHelper for these sensors, as well as a camera tuning file.
> 
> The tuning was done with an Innomaker STARVIS IMX327LQR module. These
> have no IR cut filter so there is no proper colour tuning. However,
> you should obtain reasonable results for most modules using this
> sensor. Specific tunings for further modules can always be added
> subsequently.
> 
> To use this sensor on the Raspberry Pi platform, please add
> 
> dtoverlay=imx290,clock-frequency=74250000
> 
> into your /boot/config.txt file.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
>  src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
>  src/ipa/raspberrypi/data/meson.build      |   1 +
>  src/ipa/raspberrypi/meson.build           |   1 +
>  4 files changed, 234 insertions(+)
>  create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
>  create mode 100644 src/ipa/raspberrypi/data/imx290.json
> 
> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> new file mode 100644
> index 00000000..6f412e40
> --- /dev/null
> +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> @@ -0,0 +1,67 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * cam_helper_imx290.cpp - camera helper for imx290 sensor
> + */
> +
> +#include <math.h>
> +
> +#include "cam_helper.hpp"
> +
> +using namespace RPiController;
> +
> +class CamHelperImx290 : public CamHelper
> +{
> +public:
> +	CamHelperImx290();
> +	uint32_t GainCode(double gain) const override;
> +	double Gain(uint32_t gain_code) const override;
> +	void GetDelays(int &exposure_delay, int &gain_delay,
> +		       int &vblank_delay) const override;
> +	unsigned int HideFramesModeSwitch() const override;
> +
> +private:
> +	/*
> +	 * Smallest difference between the frame length and integration time,
> +	 * in units of lines.
> +	 */
> +	static constexpr int frameIntegrationDiff = 2;
> +};
> +
> +CamHelperImx290::CamHelperImx290()
> +	: CamHelper(nullptr, frameIntegrationDiff)
> +{
> +}
> +
> +uint32_t CamHelperImx290::GainCode(double gain) const
> +{
> +	int code = 66.6667 * log10(gain);
> +	return std::max(0, std::min(code, 0xf0));
> +}
> +
> +double CamHelperImx290::Gain(uint32_t gain_code) const
> +{
> +	return pow(10, 0.015 * gain_code);
> +}
> +
> +void CamHelperImx290::GetDelays(int &exposure_delay, int &gain_delay,
> +				int &vblank_delay) const
> +{
> +	exposure_delay = 2;
> +	gain_delay = 2;
> +	vblank_delay = 2;
> +}
> +
> +unsigned int CamHelperImx290::HideFramesModeSwitch() const
> +{
> +	/* After a mode switch, we seem to get 1 bad frame. */
> +	return 1;
> +}
> +
> +static CamHelper *Create()
> +{
> +	return new CamHelperImx290();
> +}
> +
> +static RegisterCamHelper reg("imx290", &Create);
> diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> new file mode 100644
> index 00000000..6fb92cc4
> --- /dev/null
> +++ b/src/ipa/raspberrypi/data/imx290.json
> @@ -0,0 +1,165 @@
> +{
> +    "rpi.black_level":
> +    {
> +        "black_level": 3840
> +    },
> +    "rpi.dpc":
> +    {
> +    },
> +    "rpi.lux":
> +    {
> +        "reference_shutter_speed": 6813,
> +        "reference_gain": 1.0,
> +        "reference_aperture": 1.0,
> +        "reference_lux": 890,
> +        "reference_Y": 12900
> +    },
> +    "rpi.noise":
> +    {
> +        "reference_constant": 0,
> +        "reference_slope": 2.67
> +    },
> +    "rpi.geq":
> +    {
> +        "offset": 187,
> +        "slope": 0.00842
> +    },
> +    "rpi.sdn":
> +    {
> +    },
> +    "rpi.awb":
> +    {
> +	"bayes": 0
> +    },
> +    "rpi.agc":
> +    {
> +	"speed": 0.2,
> +        "metering_modes":
> +        {
> +            "matrix":
> +            {
> +                "weights":
> +                [
> +                    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
> +                ]
> +            },
> +            "centre-weighted":
> +            {
> +                "weights":
> +                [
> +                    3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0
> +                ]
> +            },
> +            "spot":
> +            {
> +                "weights":
> +                [
> +                    2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> +                ]
> +            }
> +        },
> +        "exposure_modes":
> +        {
> +            "normal":
> +            {
> +                "shutter":
> +                [
> +                    10, 30000, 60000
> +                ],
> +                "gain":
> +                [
> +                    1.0,  2.0,   8.0
> +                ]
> +            },
> +            "sport":
> +            {
> +                "shutter":
> +                [
> +                    10, 5000, 10000, 20000, 120000
> +                ],
> +                "gain":
> +                [
> +                    1.0, 2.0, 4.0, 6.0, 6.0
> +                ]
> +            }
> +        },
> +        "constraint_modes":
> +        {
> +            "normal":
> +            [
> +            ],
> +            "highlight":
> +            [
> +                {
> +                    "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> +                    [
> +                        0, 0.5, 1000, 0.5
> +                    ]
> +                },
> +                {
> +                    "bound": "UPPER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> +                    [
> +                        0, 0.8, 1000, 0.8
> +                    ]
> +                }
> +            ]
> +        },
> +        "y_target":
> +        [
> +            0, 0.16, 1000, 0.16, 10000, 0.16
> +        ]
> +    },
> +    "rpi.alsc":
> +    {
> +        "omega": 1.3,
> +        "n_iter": 100,
> +        "luminance_strength": 0.7,
> +        "luminance_lut":
> +        [
> +            2.844, 2.349, 2.018, 1.775, 1.599, 1.466, 1.371, 1.321, 1.306, 1.316, 1.357, 1.439, 1.552, 1.705, 1.915, 2.221,
> +            2.576, 2.151, 1.851, 1.639, 1.478, 1.358, 1.272, 1.231, 1.218, 1.226, 1.262, 1.335, 1.438, 1.571, 1.766, 2.067,
> +            2.381, 2.005, 1.739, 1.545, 1.389, 1.278, 1.204, 1.166, 1.153, 1.161, 1.194, 1.263, 1.356, 1.489, 1.671, 1.943,
> +            2.242, 1.899, 1.658, 1.481, 1.329, 1.225, 1.156, 1.113, 1.096, 1.107, 1.143, 1.201, 1.289, 1.423, 1.607, 1.861,
> +            2.152, 1.831, 1.602, 1.436, 1.291, 1.193, 1.121, 1.069, 1.047, 1.062, 1.107, 1.166, 1.249, 1.384, 1.562, 1.801,
> +            2.104, 1.795, 1.572, 1.407, 1.269, 1.174, 1.099, 1.041, 1.008, 1.029, 1.083, 1.146, 1.232, 1.364, 1.547, 1.766,
> +            2.104, 1.796, 1.572, 1.403, 1.264, 1.171, 1.097, 1.036, 1.001, 1.025, 1.077, 1.142, 1.231, 1.363, 1.549, 1.766,
> +            2.148, 1.827, 1.594, 1.413, 1.276, 1.184, 1.114, 1.062, 1.033, 1.049, 1.092, 1.153, 1.242, 1.383, 1.577, 1.795,
> +            2.211, 1.881, 1.636, 1.455, 1.309, 1.214, 1.149, 1.104, 1.081, 1.089, 1.125, 1.184, 1.273, 1.423, 1.622, 1.846,
> +            2.319, 1.958, 1.698, 1.516, 1.362, 1.262, 1.203, 1.156, 1.137, 1.142, 1.171, 1.229, 1.331, 1.484, 1.682, 1.933,
> +            2.459, 2.072, 1.789, 1.594, 1.441, 1.331, 1.261, 1.219, 1.199, 1.205, 1.232, 1.301, 1.414, 1.571, 1.773, 2.052,
> +            2.645, 2.206, 1.928, 1.728, 1.559, 1.451, 1.352, 1.301, 1.282, 1.289, 1.319, 1.395, 1.519, 1.685, 1.904, 2.227
> +        ],
> +        "sigma": 0.005,
> +        "sigma_Cb": 0.005
> +    },
> +    "rpi.contrast":
> +    {
> +        "ce_enable": 1,
> +        "gamma_curve":
> +        [
> +            0, 0, 1024, 5040, 2048, 9338, 3072, 12356, 4096, 15312, 5120, 18051, 6144, 20790, 7168, 23193,
> +            8192, 25744, 9216, 27942, 10240, 30035, 11264, 32005, 12288, 33975, 13312, 35815, 14336, 37600, 15360, 39168,
> +            16384, 40642, 18432, 43379, 20480, 45749, 22528, 47753, 24576, 49621, 26624, 51253, 28672, 52698, 30720, 53796,
> +            32768, 54876, 36864, 57012, 40960, 58656, 45056, 59954, 49152, 61183, 53248, 62355, 57344, 63419, 61440, 64476,
> +            65535, 65535
> +        ]
> +    },
> +    "rpi.sharpen":
> +    {
> +    },
> +    "rpi.ccm":
> +    {
> +        "ccms":
> +	[
> +            {
> +		"ct": 3900, "ccm":
> +		[
> +		    1.54659, -0.17707, -0.36953, -0.51471, 1.72733, -0.21262, 0.06667, -0.92279, 1.85612
> +		]
> +	    }
> +	]
> +    },
> +    "rpi.focus":
> +    {
> +    }
> +}
> diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> index 5236bf1e..509ad58b 100644
> --- a/src/ipa/raspberrypi/data/meson.build
> +++ b/src/ipa/raspberrypi/data/meson.build
> @@ -3,6 +3,7 @@
>  conf_files = files([
>      'imx219.json',
>      'imx477.json',
> +    'imx290.json',

Alphabetical order please.

>      'ov5647.json',
>      'uncalibrated.json',
>  ])
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index 59e49686..7e88f8e0 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_imx477.cpp',
> +    'cam_helper_imx290.cpp',

Here too.

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

>      'controller/controller.cpp',
>      'controller/histogram.cpp',
>      'controller/algorithm.cpp',

Hi Laurent

Thanks also for this review!

On Mon, 8 Mar 2021 at 19:31, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Thu, Mar 04, 2021 at 03:31:20PM +0000, David Plowman wrote:
> > imx290 and imx327 share the same kernel driver (imx290.c) and are
> > therefore both recognised here as "imx290".
>
> Are there any differences between the two sensors that may need to be
> handled anywhere in libcamera ?

Good question, and the answer is... I don't really know. I don't have
any modules that I know to have an imx290 in them, but at some point
I'm sure we or someone else will come across one and we'll find out.
At some point some we're going to have to start coping with different
modules with the same (or indistinguishable sensors) in them too.

>
> > We add the necessary
> > CamHelper for these sensors, as well as a camera tuning file.
> >
> > The tuning was done with an Innomaker STARVIS IMX327LQR module. These
> > have no IR cut filter so there is no proper colour tuning. However,
> > you should obtain reasonable results for most modules using this
> > sensor. Specific tunings for further modules can always be added
> > subsequently.
> >
> > To use this sensor on the Raspberry Pi platform, please add
> >
> > dtoverlay=imx290,clock-frequency=74250000
> >
> > into your /boot/config.txt file.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
> >  src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
> >  src/ipa/raspberrypi/data/meson.build      |   1 +
> >  src/ipa/raspberrypi/meson.build           |   1 +
> >  4 files changed, 234 insertions(+)
> >  create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
> >  create mode 100644 src/ipa/raspberrypi/data/imx290.json
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > new file mode 100644
> > index 00000000..6f412e40
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > @@ -0,0 +1,67 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > + *
> > + * cam_helper_imx290.cpp - camera helper for imx290 sensor
> > + */
> > +
> > +#include <math.h>
> > +
> > +#include "cam_helper.hpp"
> > +
> > +using namespace RPiController;
> > +
> > +class CamHelperImx290 : public CamHelper
> > +{
> > +public:
> > +     CamHelperImx290();
> > +     uint32_t GainCode(double gain) const override;
> > +     double Gain(uint32_t gain_code) const override;
> > +     void GetDelays(int &exposure_delay, int &gain_delay,
> > +                    int &vblank_delay) const override;
> > +     unsigned int HideFramesModeSwitch() const override;
> > +
> > +private:
> > +     /*
> > +      * Smallest difference between the frame length and integration time,
> > +      * in units of lines.
> > +      */
> > +     static constexpr int frameIntegrationDiff = 2;
> > +};
> > +
> > +CamHelperImx290::CamHelperImx290()
> > +     : CamHelper(nullptr, frameIntegrationDiff)
> > +{
> > +}
> > +
> > +uint32_t CamHelperImx290::GainCode(double gain) const
> > +{
> > +     int code = 66.6667 * log10(gain);
> > +     return std::max(0, std::min(code, 0xf0));
> > +}
> > +
> > +double CamHelperImx290::Gain(uint32_t gain_code) const
> > +{
> > +     return pow(10, 0.015 * gain_code);
> > +}
> > +
> > +void CamHelperImx290::GetDelays(int &exposure_delay, int &gain_delay,
> > +                             int &vblank_delay) const
> > +{
> > +     exposure_delay = 2;
> > +     gain_delay = 2;
> > +     vblank_delay = 2;
> > +}
> > +
> > +unsigned int CamHelperImx290::HideFramesModeSwitch() const
> > +{
> > +     /* After a mode switch, we seem to get 1 bad frame. */
> > +     return 1;
> > +}
> > +
> > +static CamHelper *Create()
> > +{
> > +     return new CamHelperImx290();
> > +}
> > +
> > +static RegisterCamHelper reg("imx290", &Create);
> > diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> > new file mode 100644
> > index 00000000..6fb92cc4
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/data/imx290.json
> > @@ -0,0 +1,165 @@
> > +{
> > +    "rpi.black_level":
> > +    {
> > +        "black_level": 3840
> > +    },
> > +    "rpi.dpc":
> > +    {
> > +    },
> > +    "rpi.lux":
> > +    {
> > +        "reference_shutter_speed": 6813,
> > +        "reference_gain": 1.0,
> > +        "reference_aperture": 1.0,
> > +        "reference_lux": 890,
> > +        "reference_Y": 12900
> > +    },
> > +    "rpi.noise":
> > +    {
> > +        "reference_constant": 0,
> > +        "reference_slope": 2.67
> > +    },
> > +    "rpi.geq":
> > +    {
> > +        "offset": 187,
> > +        "slope": 0.00842
> > +    },
> > +    "rpi.sdn":
> > +    {
> > +    },
> > +    "rpi.awb":
> > +    {
> > +     "bayes": 0
> > +    },
> > +    "rpi.agc":
> > +    {
> > +     "speed": 0.2,
> > +        "metering_modes":
> > +        {
> > +            "matrix":
> > +            {
> > +                "weights":
> > +                [
> > +                    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
> > +                ]
> > +            },
> > +            "centre-weighted":
> > +            {
> > +                "weights":
> > +                [
> > +                    3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0
> > +                ]
> > +            },
> > +            "spot":
> > +            {
> > +                "weights":
> > +                [
> > +                    2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> > +                ]
> > +            }
> > +        },
> > +        "exposure_modes":
> > +        {
> > +            "normal":
> > +            {
> > +                "shutter":
> > +                [
> > +                    10, 30000, 60000
> > +                ],
> > +                "gain":
> > +                [
> > +                    1.0,  2.0,   8.0
> > +                ]
> > +            },
> > +            "sport":
> > +            {
> > +                "shutter":
> > +                [
> > +                    10, 5000, 10000, 20000, 120000
> > +                ],
> > +                "gain":
> > +                [
> > +                    1.0, 2.0, 4.0, 6.0, 6.0
> > +                ]
> > +            }
> > +        },
> > +        "constraint_modes":
> > +        {
> > +            "normal":
> > +            [
> > +            ],
> > +            "highlight":
> > +            [
> > +                {
> > +                    "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > +                    [
> > +                        0, 0.5, 1000, 0.5
> > +                    ]
> > +                },
> > +                {
> > +                    "bound": "UPPER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > +                    [
> > +                        0, 0.8, 1000, 0.8
> > +                    ]
> > +                }
> > +            ]
> > +        },
> > +        "y_target":
> > +        [
> > +            0, 0.16, 1000, 0.16, 10000, 0.16
> > +        ]
> > +    },
> > +    "rpi.alsc":
> > +    {
> > +        "omega": 1.3,
> > +        "n_iter": 100,
> > +        "luminance_strength": 0.7,
> > +        "luminance_lut":
> > +        [
> > +            2.844, 2.349, 2.018, 1.775, 1.599, 1.466, 1.371, 1.321, 1.306, 1.316, 1.357, 1.439, 1.552, 1.705, 1.915, 2.221,
> > +            2.576, 2.151, 1.851, 1.639, 1.478, 1.358, 1.272, 1.231, 1.218, 1.226, 1.262, 1.335, 1.438, 1.571, 1.766, 2.067,
> > +            2.381, 2.005, 1.739, 1.545, 1.389, 1.278, 1.204, 1.166, 1.153, 1.161, 1.194, 1.263, 1.356, 1.489, 1.671, 1.943,
> > +            2.242, 1.899, 1.658, 1.481, 1.329, 1.225, 1.156, 1.113, 1.096, 1.107, 1.143, 1.201, 1.289, 1.423, 1.607, 1.861,
> > +            2.152, 1.831, 1.602, 1.436, 1.291, 1.193, 1.121, 1.069, 1.047, 1.062, 1.107, 1.166, 1.249, 1.384, 1.562, 1.801,
> > +            2.104, 1.795, 1.572, 1.407, 1.269, 1.174, 1.099, 1.041, 1.008, 1.029, 1.083, 1.146, 1.232, 1.364, 1.547, 1.766,
> > +            2.104, 1.796, 1.572, 1.403, 1.264, 1.171, 1.097, 1.036, 1.001, 1.025, 1.077, 1.142, 1.231, 1.363, 1.549, 1.766,
> > +            2.148, 1.827, 1.594, 1.413, 1.276, 1.184, 1.114, 1.062, 1.033, 1.049, 1.092, 1.153, 1.242, 1.383, 1.577, 1.795,
> > +            2.211, 1.881, 1.636, 1.455, 1.309, 1.214, 1.149, 1.104, 1.081, 1.089, 1.125, 1.184, 1.273, 1.423, 1.622, 1.846,
> > +            2.319, 1.958, 1.698, 1.516, 1.362, 1.262, 1.203, 1.156, 1.137, 1.142, 1.171, 1.229, 1.331, 1.484, 1.682, 1.933,
> > +            2.459, 2.072, 1.789, 1.594, 1.441, 1.331, 1.261, 1.219, 1.199, 1.205, 1.232, 1.301, 1.414, 1.571, 1.773, 2.052,
> > +            2.645, 2.206, 1.928, 1.728, 1.559, 1.451, 1.352, 1.301, 1.282, 1.289, 1.319, 1.395, 1.519, 1.685, 1.904, 2.227
> > +        ],
> > +        "sigma": 0.005,
> > +        "sigma_Cb": 0.005
> > +    },
> > +    "rpi.contrast":
> > +    {
> > +        "ce_enable": 1,
> > +        "gamma_curve":
> > +        [
> > +            0, 0, 1024, 5040, 2048, 9338, 3072, 12356, 4096, 15312, 5120, 18051, 6144, 20790, 7168, 23193,
> > +            8192, 25744, 9216, 27942, 10240, 30035, 11264, 32005, 12288, 33975, 13312, 35815, 14336, 37600, 15360, 39168,
> > +            16384, 40642, 18432, 43379, 20480, 45749, 22528, 47753, 24576, 49621, 26624, 51253, 28672, 52698, 30720, 53796,
> > +            32768, 54876, 36864, 57012, 40960, 58656, 45056, 59954, 49152, 61183, 53248, 62355, 57344, 63419, 61440, 64476,
> > +            65535, 65535
> > +        ]
> > +    },
> > +    "rpi.sharpen":
> > +    {
> > +    },
> > +    "rpi.ccm":
> > +    {
> > +        "ccms":
> > +     [
> > +            {
> > +             "ct": 3900, "ccm":
> > +             [
> > +                 1.54659, -0.17707, -0.36953, -0.51471, 1.72733, -0.21262, 0.06667, -0.92279, 1.85612
> > +             ]
> > +         }
> > +     ]
> > +    },
> > +    "rpi.focus":
> > +    {
> > +    }
> > +}
> > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > index 5236bf1e..509ad58b 100644
> > --- a/src/ipa/raspberrypi/data/meson.build
> > +++ b/src/ipa/raspberrypi/data/meson.build
> > @@ -3,6 +3,7 @@
> >  conf_files = files([
> >      'imx219.json',
> >      'imx477.json',
> > +    'imx290.json',
>
> Alphabetical order please.
>
> >      'ov5647.json',
> >      'uncalibrated.json',
> >  ])
> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > index 59e49686..7e88f8e0 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_imx477.cpp',
> > +    'cam_helper_imx290.cpp',
>
> Here too.

Will do! Version 2 of this set incoming shortly.

Thanks again

David

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> >      'controller/controller.cpp',
> >      'controller/histogram.cpp',
> >      'controller/algorithm.cpp',
>
> --
> Regards,
>
> Laurent Pinchart

Hi David,

On Mon, Mar 08, 2021 at 10:11:35PM +0000, David Plowman wrote:
> On Mon, 8 Mar 2021 at 19:31, Laurent Pinchart wrote:
> > On Thu, Mar 04, 2021 at 03:31:20PM +0000, David Plowman wrote:
> > > imx290 and imx327 share the same kernel driver (imx290.c) and are
> > > therefore both recognised here as "imx290".
> >
> > Are there any differences between the two sensors that may need to be
> > handled anywhere in libcamera ?
> 
> Good question, and the answer is... I don't really know. I don't have
> any modules that I know to have an imx290 in them, but at some point
> I'm sure we or someone else will come across one and we'll find out.
> At some point some we're going to have to start coping with different
> modules with the same (or indistinguishable sensors) in them too.

There are two issues here, differences between the IMX290 and IMX327
sensors, and differences between modules. For the former, would it make
sense to already add a sony,imx327 compatible string to the imx290
driver, and make use of it ? To be clear, it's not a blocker for this
patch.

> > > We add the necessary
> > > CamHelper for these sensors, as well as a camera tuning file.
> > >
> > > The tuning was done with an Innomaker STARVIS IMX327LQR module. These
> > > have no IR cut filter so there is no proper colour tuning. However,
> > > you should obtain reasonable results for most modules using this
> > > sensor. Specific tunings for further modules can always be added
> > > subsequently.
> > >
> > > To use this sensor on the Raspberry Pi platform, please add
> > >
> > > dtoverlay=imx290,clock-frequency=74250000
> > >
> > > into your /boot/config.txt file.
> > >
> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > ---
> > >  src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
> > >  src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
> > >  src/ipa/raspberrypi/data/meson.build      |   1 +
> > >  src/ipa/raspberrypi/meson.build           |   1 +
> > >  4 files changed, 234 insertions(+)
> > >  create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
> > >  create mode 100644 src/ipa/raspberrypi/data/imx290.json
> > >
> > > diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > new file mode 100644
> > > index 00000000..6f412e40
> > > --- /dev/null
> > > +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > @@ -0,0 +1,67 @@
> > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > +/*
> > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > > + *
> > > + * cam_helper_imx290.cpp - camera helper for imx290 sensor
> > > + */
> > > +
> > > +#include <math.h>
> > > +
> > > +#include "cam_helper.hpp"
> > > +
> > > +using namespace RPiController;
> > > +
> > > +class CamHelperImx290 : public CamHelper
> > > +{
> > > +public:
> > > +     CamHelperImx290();
> > > +     uint32_t GainCode(double gain) const override;
> > > +     double Gain(uint32_t gain_code) const override;
> > > +     void GetDelays(int &exposure_delay, int &gain_delay,
> > > +                    int &vblank_delay) const override;
> > > +     unsigned int HideFramesModeSwitch() const override;
> > > +
> > > +private:
> > > +     /*
> > > +      * Smallest difference between the frame length and integration time,
> > > +      * in units of lines.
> > > +      */
> > > +     static constexpr int frameIntegrationDiff = 2;
> > > +};
> > > +
> > > +CamHelperImx290::CamHelperImx290()
> > > +     : CamHelper(nullptr, frameIntegrationDiff)
> > > +{
> > > +}
> > > +
> > > +uint32_t CamHelperImx290::GainCode(double gain) const
> > > +{
> > > +     int code = 66.6667 * log10(gain);
> > > +     return std::max(0, std::min(code, 0xf0));
> > > +}
> > > +
> > > +double CamHelperImx290::Gain(uint32_t gain_code) const
> > > +{
> > > +     return pow(10, 0.015 * gain_code);
> > > +}
> > > +
> > > +void CamHelperImx290::GetDelays(int &exposure_delay, int &gain_delay,
> > > +                             int &vblank_delay) const
> > > +{
> > > +     exposure_delay = 2;
> > > +     gain_delay = 2;
> > > +     vblank_delay = 2;
> > > +}
> > > +
> > > +unsigned int CamHelperImx290::HideFramesModeSwitch() const
> > > +{
> > > +     /* After a mode switch, we seem to get 1 bad frame. */
> > > +     return 1;
> > > +}
> > > +
> > > +static CamHelper *Create()
> > > +{
> > > +     return new CamHelperImx290();
> > > +}
> > > +
> > > +static RegisterCamHelper reg("imx290", &Create);
> > > diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> > > new file mode 100644
> > > index 00000000..6fb92cc4
> > > --- /dev/null
> > > +++ b/src/ipa/raspberrypi/data/imx290.json
> > > @@ -0,0 +1,165 @@
> > > +{
> > > +    "rpi.black_level":
> > > +    {
> > > +        "black_level": 3840
> > > +    },
> > > +    "rpi.dpc":
> > > +    {
> > > +    },
> > > +    "rpi.lux":
> > > +    {
> > > +        "reference_shutter_speed": 6813,
> > > +        "reference_gain": 1.0,
> > > +        "reference_aperture": 1.0,
> > > +        "reference_lux": 890,
> > > +        "reference_Y": 12900
> > > +    },
> > > +    "rpi.noise":
> > > +    {
> > > +        "reference_constant": 0,
> > > +        "reference_slope": 2.67
> > > +    },
> > > +    "rpi.geq":
> > > +    {
> > > +        "offset": 187,
> > > +        "slope": 0.00842
> > > +    },
> > > +    "rpi.sdn":
> > > +    {
> > > +    },
> > > +    "rpi.awb":
> > > +    {
> > > +     "bayes": 0
> > > +    },
> > > +    "rpi.agc":
> > > +    {
> > > +     "speed": 0.2,
> > > +        "metering_modes":
> > > +        {
> > > +            "matrix":
> > > +            {
> > > +                "weights":
> > > +                [
> > > +                    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
> > > +                ]
> > > +            },
> > > +            "centre-weighted":
> > > +            {
> > > +                "weights":
> > > +                [
> > > +                    3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0
> > > +                ]
> > > +            },
> > > +            "spot":
> > > +            {
> > > +                "weights":
> > > +                [
> > > +                    2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> > > +                ]
> > > +            }
> > > +        },
> > > +        "exposure_modes":
> > > +        {
> > > +            "normal":
> > > +            {
> > > +                "shutter":
> > > +                [
> > > +                    10, 30000, 60000
> > > +                ],
> > > +                "gain":
> > > +                [
> > > +                    1.0,  2.0,   8.0
> > > +                ]
> > > +            },
> > > +            "sport":
> > > +            {
> > > +                "shutter":
> > > +                [
> > > +                    10, 5000, 10000, 20000, 120000
> > > +                ],
> > > +                "gain":
> > > +                [
> > > +                    1.0, 2.0, 4.0, 6.0, 6.0
> > > +                ]
> > > +            }
> > > +        },
> > > +        "constraint_modes":
> > > +        {
> > > +            "normal":
> > > +            [
> > > +            ],
> > > +            "highlight":
> > > +            [
> > > +                {
> > > +                    "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > > +                    [
> > > +                        0, 0.5, 1000, 0.5
> > > +                    ]
> > > +                },
> > > +                {
> > > +                    "bound": "UPPER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > > +                    [
> > > +                        0, 0.8, 1000, 0.8
> > > +                    ]
> > > +                }
> > > +            ]
> > > +        },
> > > +        "y_target":
> > > +        [
> > > +            0, 0.16, 1000, 0.16, 10000, 0.16
> > > +        ]
> > > +    },
> > > +    "rpi.alsc":
> > > +    {
> > > +        "omega": 1.3,
> > > +        "n_iter": 100,
> > > +        "luminance_strength": 0.7,
> > > +        "luminance_lut":
> > > +        [
> > > +            2.844, 2.349, 2.018, 1.775, 1.599, 1.466, 1.371, 1.321, 1.306, 1.316, 1.357, 1.439, 1.552, 1.705, 1.915, 2.221,
> > > +            2.576, 2.151, 1.851, 1.639, 1.478, 1.358, 1.272, 1.231, 1.218, 1.226, 1.262, 1.335, 1.438, 1.571, 1.766, 2.067,
> > > +            2.381, 2.005, 1.739, 1.545, 1.389, 1.278, 1.204, 1.166, 1.153, 1.161, 1.194, 1.263, 1.356, 1.489, 1.671, 1.943,
> > > +            2.242, 1.899, 1.658, 1.481, 1.329, 1.225, 1.156, 1.113, 1.096, 1.107, 1.143, 1.201, 1.289, 1.423, 1.607, 1.861,
> > > +            2.152, 1.831, 1.602, 1.436, 1.291, 1.193, 1.121, 1.069, 1.047, 1.062, 1.107, 1.166, 1.249, 1.384, 1.562, 1.801,
> > > +            2.104, 1.795, 1.572, 1.407, 1.269, 1.174, 1.099, 1.041, 1.008, 1.029, 1.083, 1.146, 1.232, 1.364, 1.547, 1.766,
> > > +            2.104, 1.796, 1.572, 1.403, 1.264, 1.171, 1.097, 1.036, 1.001, 1.025, 1.077, 1.142, 1.231, 1.363, 1.549, 1.766,
> > > +            2.148, 1.827, 1.594, 1.413, 1.276, 1.184, 1.114, 1.062, 1.033, 1.049, 1.092, 1.153, 1.242, 1.383, 1.577, 1.795,
> > > +            2.211, 1.881, 1.636, 1.455, 1.309, 1.214, 1.149, 1.104, 1.081, 1.089, 1.125, 1.184, 1.273, 1.423, 1.622, 1.846,
> > > +            2.319, 1.958, 1.698, 1.516, 1.362, 1.262, 1.203, 1.156, 1.137, 1.142, 1.171, 1.229, 1.331, 1.484, 1.682, 1.933,
> > > +            2.459, 2.072, 1.789, 1.594, 1.441, 1.331, 1.261, 1.219, 1.199, 1.205, 1.232, 1.301, 1.414, 1.571, 1.773, 2.052,
> > > +            2.645, 2.206, 1.928, 1.728, 1.559, 1.451, 1.352, 1.301, 1.282, 1.289, 1.319, 1.395, 1.519, 1.685, 1.904, 2.227
> > > +        ],
> > > +        "sigma": 0.005,
> > > +        "sigma_Cb": 0.005
> > > +    },
> > > +    "rpi.contrast":
> > > +    {
> > > +        "ce_enable": 1,
> > > +        "gamma_curve":
> > > +        [
> > > +            0, 0, 1024, 5040, 2048, 9338, 3072, 12356, 4096, 15312, 5120, 18051, 6144, 20790, 7168, 23193,
> > > +            8192, 25744, 9216, 27942, 10240, 30035, 11264, 32005, 12288, 33975, 13312, 35815, 14336, 37600, 15360, 39168,
> > > +            16384, 40642, 18432, 43379, 20480, 45749, 22528, 47753, 24576, 49621, 26624, 51253, 28672, 52698, 30720, 53796,
> > > +            32768, 54876, 36864, 57012, 40960, 58656, 45056, 59954, 49152, 61183, 53248, 62355, 57344, 63419, 61440, 64476,
> > > +            65535, 65535
> > > +        ]
> > > +    },
> > > +    "rpi.sharpen":
> > > +    {
> > > +    },
> > > +    "rpi.ccm":
> > > +    {
> > > +        "ccms":
> > > +     [
> > > +            {
> > > +             "ct": 3900, "ccm":
> > > +             [
> > > +                 1.54659, -0.17707, -0.36953, -0.51471, 1.72733, -0.21262, 0.06667, -0.92279, 1.85612
> > > +             ]
> > > +         }
> > > +     ]
> > > +    },
> > > +    "rpi.focus":
> > > +    {
> > > +    }
> > > +}
> > > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > > index 5236bf1e..509ad58b 100644
> > > --- a/src/ipa/raspberrypi/data/meson.build
> > > +++ b/src/ipa/raspberrypi/data/meson.build
> > > @@ -3,6 +3,7 @@
> > >  conf_files = files([
> > >      'imx219.json',
> > >      'imx477.json',
> > > +    'imx290.json',
> >
> > Alphabetical order please.
> >
> > >      'ov5647.json',
> > >      'uncalibrated.json',
> > >  ])
> > > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > > index 59e49686..7e88f8e0 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_imx477.cpp',
> > > +    'cam_helper_imx290.cpp',
> >
> > Here too.
> 
> Will do! Version 2 of this set incoming shortly.
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > >      'controller/controller.cpp',
> > >      'controller/histogram.cpp',
> > >      'controller/algorithm.cpp',

Hi Laurent & David

On Mon, 8 Mar 2021 at 22:20, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> On Mon, Mar 08, 2021 at 10:11:35PM +0000, David Plowman wrote:
> > On Mon, 8 Mar 2021 at 19:31, Laurent Pinchart wrote:
> > > On Thu, Mar 04, 2021 at 03:31:20PM +0000, David Plowman wrote:
> > > > imx290 and imx327 share the same kernel driver (imx290.c) and are
> > > > therefore both recognised here as "imx290".
> > >
> > > Are there any differences between the two sensors that may need to be
> > > handled anywhere in libcamera ?
> >
> > Good question, and the answer is... I don't really know. I don't have
> > any modules that I know to have an imx290 in them, but at some point
> > I'm sure we or someone else will come across one and we'll find out.
> > At some point some we're going to have to start coping with different
> > modules with the same (or indistinguishable sensors) in them too.
>
> There are two issues here, differences between the IMX290 and IMX327
> sensors, and differences between modules. For the former, would it make
> sense to already add a sony,imx327 compatible string to the imx290
> driver, and make use of it ? To be clear, it's not a blocker for this
> patch.

I have a mono IMX290 from Vision Components here, as well as several
IMX327's. Vision Components appear to use literally the same PCB for
the two modules - they are both silkscreened IMX327.

IMX290 is the bigger brother to IMX327. It can do 1080p120 (10bit
only, 4lane CSI2 at 891Mb/s/lane), whilst IMX327 can only do 1080p60
(10 or 12bit, 2lane CSI2 at 891Mb/s/lane, or 4lane at 445.5Mb/s/lane).

Programming interface is identical except for allowing frsel=0 for
120fps on IMX290.

The driver is largely Andrey Konovalov's mainline driver with controls
added, and support for an alternate source clock frequency
(74.250MHz).
It switches mode based on link frequency, and doesn't support the
1080p120fps mode, so is actually independent of whether it is 327 or
290. Based on the link frequency numbers it's a max of 1080p30 on
either lane configuration, but I thought I'd had 1080p60.

One to investigate to make the driver more flexible, but shouldn't
affect libcamera (other than wanting mono support!). The Pi ISP won't
support 1080p120 so is actually of limited interest to us.

  Dave

> > > > We add the necessary
> > > > CamHelper for these sensors, as well as a camera tuning file.
> > > >
> > > > The tuning was done with an Innomaker STARVIS IMX327LQR module. These
> > > > have no IR cut filter so there is no proper colour tuning. However,
> > > > you should obtain reasonable results for most modules using this
> > > > sensor. Specific tunings for further modules can always be added
> > > > subsequently.
> > > >
> > > > To use this sensor on the Raspberry Pi platform, please add
> > > >
> > > > dtoverlay=imx290,clock-frequency=74250000
> > > >
> > > > into your /boot/config.txt file.
> > > >
> > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > > ---
> > > >  src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
> > > >  src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
> > > >  src/ipa/raspberrypi/data/meson.build      |   1 +
> > > >  src/ipa/raspberrypi/meson.build           |   1 +
> > > >  4 files changed, 234 insertions(+)
> > > >  create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > >  create mode 100644 src/ipa/raspberrypi/data/imx290.json
> > > >
> > > > diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > > new file mode 100644
> > > > index 00000000..6f412e40
> > > > --- /dev/null
> > > > +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > > @@ -0,0 +1,67 @@
> > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > +/*
> > > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > > > + *
> > > > + * cam_helper_imx290.cpp - camera helper for imx290 sensor
> > > > + */
> > > > +
> > > > +#include <math.h>
> > > > +
> > > > +#include "cam_helper.hpp"
> > > > +
> > > > +using namespace RPiController;
> > > > +
> > > > +class CamHelperImx290 : public CamHelper
> > > > +{
> > > > +public:
> > > > +     CamHelperImx290();
> > > > +     uint32_t GainCode(double gain) const override;
> > > > +     double Gain(uint32_t gain_code) const override;
> > > > +     void GetDelays(int &exposure_delay, int &gain_delay,
> > > > +                    int &vblank_delay) const override;
> > > > +     unsigned int HideFramesModeSwitch() const override;
> > > > +
> > > > +private:
> > > > +     /*
> > > > +      * Smallest difference between the frame length and integration time,
> > > > +      * in units of lines.
> > > > +      */
> > > > +     static constexpr int frameIntegrationDiff = 2;
> > > > +};
> > > > +
> > > > +CamHelperImx290::CamHelperImx290()
> > > > +     : CamHelper(nullptr, frameIntegrationDiff)
> > > > +{
> > > > +}
> > > > +
> > > > +uint32_t CamHelperImx290::GainCode(double gain) const
> > > > +{
> > > > +     int code = 66.6667 * log10(gain);
> > > > +     return std::max(0, std::min(code, 0xf0));
> > > > +}
> > > > +
> > > > +double CamHelperImx290::Gain(uint32_t gain_code) const
> > > > +{
> > > > +     return pow(10, 0.015 * gain_code);
> > > > +}
> > > > +
> > > > +void CamHelperImx290::GetDelays(int &exposure_delay, int &gain_delay,
> > > > +                             int &vblank_delay) const
> > > > +{
> > > > +     exposure_delay = 2;
> > > > +     gain_delay = 2;
> > > > +     vblank_delay = 2;
> > > > +}
> > > > +
> > > > +unsigned int CamHelperImx290::HideFramesModeSwitch() const
> > > > +{
> > > > +     /* After a mode switch, we seem to get 1 bad frame. */
> > > > +     return 1;
> > > > +}
> > > > +
> > > > +static CamHelper *Create()
> > > > +{
> > > > +     return new CamHelperImx290();
> > > > +}
> > > > +
> > > > +static RegisterCamHelper reg("imx290", &Create);
> > > > diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> > > > new file mode 100644
> > > > index 00000000..6fb92cc4
> > > > --- /dev/null
> > > > +++ b/src/ipa/raspberrypi/data/imx290.json
> > > > @@ -0,0 +1,165 @@
> > > > +{
> > > > +    "rpi.black_level":
> > > > +    {
> > > > +        "black_level": 3840
> > > > +    },
> > > > +    "rpi.dpc":
> > > > +    {
> > > > +    },
> > > > +    "rpi.lux":
> > > > +    {
> > > > +        "reference_shutter_speed": 6813,
> > > > +        "reference_gain": 1.0,
> > > > +        "reference_aperture": 1.0,
> > > > +        "reference_lux": 890,
> > > > +        "reference_Y": 12900
> > > > +    },
> > > > +    "rpi.noise":
> > > > +    {
> > > > +        "reference_constant": 0,
> > > > +        "reference_slope": 2.67
> > > > +    },
> > > > +    "rpi.geq":
> > > > +    {
> > > > +        "offset": 187,
> > > > +        "slope": 0.00842
> > > > +    },
> > > > +    "rpi.sdn":
> > > > +    {
> > > > +    },
> > > > +    "rpi.awb":
> > > > +    {
> > > > +     "bayes": 0
> > > > +    },
> > > > +    "rpi.agc":
> > > > +    {
> > > > +     "speed": 0.2,
> > > > +        "metering_modes":
> > > > +        {
> > > > +            "matrix":
> > > > +            {
> > > > +                "weights":
> > > > +                [
> > > > +                    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
> > > > +                ]
> > > > +            },
> > > > +            "centre-weighted":
> > > > +            {
> > > > +                "weights":
> > > > +                [
> > > > +                    3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0
> > > > +                ]
> > > > +            },
> > > > +            "spot":
> > > > +            {
> > > > +                "weights":
> > > > +                [
> > > > +                    2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> > > > +                ]
> > > > +            }
> > > > +        },
> > > > +        "exposure_modes":
> > > > +        {
> > > > +            "normal":
> > > > +            {
> > > > +                "shutter":
> > > > +                [
> > > > +                    10, 30000, 60000
> > > > +                ],
> > > > +                "gain":
> > > > +                [
> > > > +                    1.0,  2.0,   8.0
> > > > +                ]
> > > > +            },
> > > > +            "sport":
> > > > +            {
> > > > +                "shutter":
> > > > +                [
> > > > +                    10, 5000, 10000, 20000, 120000
> > > > +                ],
> > > > +                "gain":
> > > > +                [
> > > > +                    1.0, 2.0, 4.0, 6.0, 6.0
> > > > +                ]
> > > > +            }
> > > > +        },
> > > > +        "constraint_modes":
> > > > +        {
> > > > +            "normal":
> > > > +            [
> > > > +            ],
> > > > +            "highlight":
> > > > +            [
> > > > +                {
> > > > +                    "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > > > +                    [
> > > > +                        0, 0.5, 1000, 0.5
> > > > +                    ]
> > > > +                },
> > > > +                {
> > > > +                    "bound": "UPPER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > > > +                    [
> > > > +                        0, 0.8, 1000, 0.8
> > > > +                    ]
> > > > +                }
> > > > +            ]
> > > > +        },
> > > > +        "y_target":
> > > > +        [
> > > > +            0, 0.16, 1000, 0.16, 10000, 0.16
> > > > +        ]
> > > > +    },
> > > > +    "rpi.alsc":
> > > > +    {
> > > > +        "omega": 1.3,
> > > > +        "n_iter": 100,
> > > > +        "luminance_strength": 0.7,
> > > > +        "luminance_lut":
> > > > +        [
> > > > +            2.844, 2.349, 2.018, 1.775, 1.599, 1.466, 1.371, 1.321, 1.306, 1.316, 1.357, 1.439, 1.552, 1.705, 1.915, 2.221,
> > > > +            2.576, 2.151, 1.851, 1.639, 1.478, 1.358, 1.272, 1.231, 1.218, 1.226, 1.262, 1.335, 1.438, 1.571, 1.766, 2.067,
> > > > +            2.381, 2.005, 1.739, 1.545, 1.389, 1.278, 1.204, 1.166, 1.153, 1.161, 1.194, 1.263, 1.356, 1.489, 1.671, 1.943,
> > > > +            2.242, 1.899, 1.658, 1.481, 1.329, 1.225, 1.156, 1.113, 1.096, 1.107, 1.143, 1.201, 1.289, 1.423, 1.607, 1.861,
> > > > +            2.152, 1.831, 1.602, 1.436, 1.291, 1.193, 1.121, 1.069, 1.047, 1.062, 1.107, 1.166, 1.249, 1.384, 1.562, 1.801,
> > > > +            2.104, 1.795, 1.572, 1.407, 1.269, 1.174, 1.099, 1.041, 1.008, 1.029, 1.083, 1.146, 1.232, 1.364, 1.547, 1.766,
> > > > +            2.104, 1.796, 1.572, 1.403, 1.264, 1.171, 1.097, 1.036, 1.001, 1.025, 1.077, 1.142, 1.231, 1.363, 1.549, 1.766,
> > > > +            2.148, 1.827, 1.594, 1.413, 1.276, 1.184, 1.114, 1.062, 1.033, 1.049, 1.092, 1.153, 1.242, 1.383, 1.577, 1.795,
> > > > +            2.211, 1.881, 1.636, 1.455, 1.309, 1.214, 1.149, 1.104, 1.081, 1.089, 1.125, 1.184, 1.273, 1.423, 1.622, 1.846,
> > > > +            2.319, 1.958, 1.698, 1.516, 1.362, 1.262, 1.203, 1.156, 1.137, 1.142, 1.171, 1.229, 1.331, 1.484, 1.682, 1.933,
> > > > +            2.459, 2.072, 1.789, 1.594, 1.441, 1.331, 1.261, 1.219, 1.199, 1.205, 1.232, 1.301, 1.414, 1.571, 1.773, 2.052,
> > > > +            2.645, 2.206, 1.928, 1.728, 1.559, 1.451, 1.352, 1.301, 1.282, 1.289, 1.319, 1.395, 1.519, 1.685, 1.904, 2.227
> > > > +        ],
> > > > +        "sigma": 0.005,
> > > > +        "sigma_Cb": 0.005
> > > > +    },
> > > > +    "rpi.contrast":
> > > > +    {
> > > > +        "ce_enable": 1,
> > > > +        "gamma_curve":
> > > > +        [
> > > > +            0, 0, 1024, 5040, 2048, 9338, 3072, 12356, 4096, 15312, 5120, 18051, 6144, 20790, 7168, 23193,
> > > > +            8192, 25744, 9216, 27942, 10240, 30035, 11264, 32005, 12288, 33975, 13312, 35815, 14336, 37600, 15360, 39168,
> > > > +            16384, 40642, 18432, 43379, 20480, 45749, 22528, 47753, 24576, 49621, 26624, 51253, 28672, 52698, 30720, 53796,
> > > > +            32768, 54876, 36864, 57012, 40960, 58656, 45056, 59954, 49152, 61183, 53248, 62355, 57344, 63419, 61440, 64476,
> > > > +            65535, 65535
> > > > +        ]
> > > > +    },
> > > > +    "rpi.sharpen":
> > > > +    {
> > > > +    },
> > > > +    "rpi.ccm":
> > > > +    {
> > > > +        "ccms":
> > > > +     [
> > > > +            {
> > > > +             "ct": 3900, "ccm":
> > > > +             [
> > > > +                 1.54659, -0.17707, -0.36953, -0.51471, 1.72733, -0.21262, 0.06667, -0.92279, 1.85612
> > > > +             ]
> > > > +         }
> > > > +     ]
> > > > +    },
> > > > +    "rpi.focus":
> > > > +    {
> > > > +    }
> > > > +}
> > > > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > > > index 5236bf1e..509ad58b 100644
> > > > --- a/src/ipa/raspberrypi/data/meson.build
> > > > +++ b/src/ipa/raspberrypi/data/meson.build
> > > > @@ -3,6 +3,7 @@
> > > >  conf_files = files([
> > > >      'imx219.json',
> > > >      'imx477.json',
> > > > +    'imx290.json',
> > >
> > > Alphabetical order please.
> > >
> > > >      'ov5647.json',
> > > >      'uncalibrated.json',
> > > >  ])
> > > > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > > > index 59e49686..7e88f8e0 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_imx477.cpp',
> > > > +    'cam_helper_imx290.cpp',
> > >
> > > Here too.
> >
> > Will do! Version 2 of this set incoming shortly.
> >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > >      'controller/controller.cpp',
> > > >      'controller/histogram.cpp',
> > > >      'controller/algorithm.cpp',
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Dave,

One correction below...

On 09.03.2021 15:34, Dave Stevenson wrote:
> Hi Laurent & David
> 
> On Mon, 8 Mar 2021 at 22:20, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> Hi David,
>>
>> On Mon, Mar 08, 2021 at 10:11:35PM +0000, David Plowman wrote:
>>> On Mon, 8 Mar 2021 at 19:31, Laurent Pinchart wrote:
>>>> On Thu, Mar 04, 2021 at 03:31:20PM +0000, David Plowman wrote:
>>>>> imx290 and imx327 share the same kernel driver (imx290.c) and are
>>>>> therefore both recognised here as "imx290".
>>>>
>>>> Are there any differences between the two sensors that may need to be
>>>> handled anywhere in libcamera ?
>>>
>>> Good question, and the answer is... I don't really know. I don't have
>>> any modules that I know to have an imx290 in them, but at some point
>>> I'm sure we or someone else will come across one and we'll find out.
>>> At some point some we're going to have to start coping with different
>>> modules with the same (or indistinguishable sensors) in them too.
>>
>> There are two issues here, differences between the IMX290 and IMX327
>> sensors, and differences between modules. For the former, would it make
>> sense to already add a sony,imx327 compatible string to the imx290
>> driver, and make use of it ? To be clear, it's not a blocker for this
>> patch.
> 
> I have a mono IMX290 from Vision Components here, as well as several
> IMX327's. Vision Components appear to use literally the same PCB for
> the two modules - they are both silkscreened IMX327.
> 
> IMX290 is the bigger brother to IMX327. It can do 1080p120 (10bit
> only, 4lane CSI2 at 891Mb/s/lane), whilst IMX327 can only do 1080p60
> (10 or 12bit, 2lane CSI2 at 891Mb/s/lane, or 4lane at 445.5Mb/s/lane).
> 
> Programming interface is identical except for allowing frsel=0 for
> 120fps on IMX290.
> 
> The driver is largely Andrey Konovalov's mainline driver

The original author and the maintainer is Manivannan Sadhasivam.
I did add some changes on top of his work :)

Thanks,
Andrey

> with controls
> added, and support for an alternate source clock frequency
> (74.250MHz).
> It switches mode based on link frequency, and doesn't support the
> 1080p120fps mode, so is actually independent of whether it is 327 or
> 290. Based on the link frequency numbers it's a max of 1080p30 on
> either lane configuration, but I thought I'd had 1080p60.
> 
> One to investigate to make the driver more flexible, but shouldn't
> affect libcamera (other than wanting mono support!). The Pi ISP won't
> support 1080p120 so is actually of limited interest to us.
> 
>    Dave
> 
>>>>> We add the necessary
>>>>> CamHelper for these sensors, as well as a camera tuning file.
>>>>>
>>>>> The tuning was done with an Innomaker STARVIS IMX327LQR module. These
>>>>> have no IR cut filter so there is no proper colour tuning. However,
>>>>> you should obtain reasonable results for most modules using this
>>>>> sensor. Specific tunings for further modules can always be added
>>>>> subsequently.
>>>>>
>>>>> To use this sensor on the Raspberry Pi platform, please add
>>>>>
>>>>> dtoverlay=imx290,clock-frequency=74250000
>>>>>
>>>>> into your /boot/config.txt file.
>>>>>
>>>>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>>>>> ---
>>>>>   src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
>>>>>   src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
>>>>>   src/ipa/raspberrypi/data/meson.build      |   1 +
>>>>>   src/ipa/raspberrypi/meson.build           |   1 +
>>>>>   4 files changed, 234 insertions(+)
>>>>>   create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
>>>>>   create mode 100644 src/ipa/raspberrypi/data/imx290.json
>>>>>
>>>>> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
>>>>> new file mode 100644
>>>>> index 00000000..6f412e40
>>>>> --- /dev/null
>>>>> +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
>>>>> @@ -0,0 +1,67 @@
>>>>> +/* SPDX-License-Identifier: BSD-2-Clause */
>>>>> +/*
>>>>> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
>>>>> + *
>>>>> + * cam_helper_imx290.cpp - camera helper for imx290 sensor
>>>>> + */
>>>>> +
>>>>> +#include <math.h>
>>>>> +
>>>>> +#include "cam_helper.hpp"
>>>>> +
>>>>> +using namespace RPiController;
>>>>> +
>>>>> +class CamHelperImx290 : public CamHelper
>>>>> +{
>>>>> +public:
>>>>> +     CamHelperImx290();
>>>>> +     uint32_t GainCode(double gain) const override;
>>>>> +     double Gain(uint32_t gain_code) const override;
>>>>> +     void GetDelays(int &exposure_delay, int &gain_delay,
>>>>> +                    int &vblank_delay) const override;
>>>>> +     unsigned int HideFramesModeSwitch() const override;
>>>>> +
>>>>> +private:
>>>>> +     /*
>>>>> +      * Smallest difference between the frame length and integration time,
>>>>> +      * in units of lines.
>>>>> +      */
>>>>> +     static constexpr int frameIntegrationDiff = 2;
>>>>> +};
>>>>> +
>>>>> +CamHelperImx290::CamHelperImx290()
>>>>> +     : CamHelper(nullptr, frameIntegrationDiff)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +uint32_t CamHelperImx290::GainCode(double gain) const
>>>>> +{
>>>>> +     int code = 66.6667 * log10(gain);
>>>>> +     return std::max(0, std::min(code, 0xf0));
>>>>> +}
>>>>> +
>>>>> +double CamHelperImx290::Gain(uint32_t gain_code) const
>>>>> +{
>>>>> +     return pow(10, 0.015 * gain_code);
>>>>> +}
>>>>> +
>>>>> +void CamHelperImx290::GetDelays(int &exposure_delay, int &gain_delay,
>>>>> +                             int &vblank_delay) const
>>>>> +{
>>>>> +     exposure_delay = 2;
>>>>> +     gain_delay = 2;
>>>>> +     vblank_delay = 2;
>>>>> +}
>>>>> +
>>>>> +unsigned int CamHelperImx290::HideFramesModeSwitch() const
>>>>> +{
>>>>> +     /* After a mode switch, we seem to get 1 bad frame. */
>>>>> +     return 1;
>>>>> +}
>>>>> +
>>>>> +static CamHelper *Create()
>>>>> +{
>>>>> +     return new CamHelperImx290();
>>>>> +}
>>>>> +
>>>>> +static RegisterCamHelper reg("imx290", &Create);
>>>>> diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
>>>>> new file mode 100644
>>>>> index 00000000..6fb92cc4
>>>>> --- /dev/null
>>>>> +++ b/src/ipa/raspberrypi/data/imx290.json
>>>>> @@ -0,0 +1,165 @@
>>>>> +{
>>>>> +    "rpi.black_level":
>>>>> +    {
>>>>> +        "black_level": 3840
>>>>> +    },
>>>>> +    "rpi.dpc":
>>>>> +    {
>>>>> +    },
>>>>> +    "rpi.lux":
>>>>> +    {
>>>>> +        "reference_shutter_speed": 6813,
>>>>> +        "reference_gain": 1.0,
>>>>> +        "reference_aperture": 1.0,
>>>>> +        "reference_lux": 890,
>>>>> +        "reference_Y": 12900
>>>>> +    },
>>>>> +    "rpi.noise":
>>>>> +    {
>>>>> +        "reference_constant": 0,
>>>>> +        "reference_slope": 2.67
>>>>> +    },
>>>>> +    "rpi.geq":
>>>>> +    {
>>>>> +        "offset": 187,
>>>>> +        "slope": 0.00842
>>>>> +    },
>>>>> +    "rpi.sdn":
>>>>> +    {
>>>>> +    },
>>>>> +    "rpi.awb":
>>>>> +    {
>>>>> +     "bayes": 0
>>>>> +    },
>>>>> +    "rpi.agc":
>>>>> +    {
>>>>> +     "speed": 0.2,
>>>>> +        "metering_modes":
>>>>> +        {
>>>>> +            "matrix":
>>>>> +            {
>>>>> +                "weights":
>>>>> +                [
>>>>> +                    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
>>>>> +                ]
>>>>> +            },
>>>>> +            "centre-weighted":
>>>>> +            {
>>>>> +                "weights":
>>>>> +                [
>>>>> +                    3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0
>>>>> +                ]
>>>>> +            },
>>>>> +            "spot":
>>>>> +            {
>>>>> +                "weights":
>>>>> +                [
>>>>> +                    2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
>>>>> +                ]
>>>>> +            }
>>>>> +        },
>>>>> +        "exposure_modes":
>>>>> +        {
>>>>> +            "normal":
>>>>> +            {
>>>>> +                "shutter":
>>>>> +                [
>>>>> +                    10, 30000, 60000
>>>>> +                ],
>>>>> +                "gain":
>>>>> +                [
>>>>> +                    1.0,  2.0,   8.0
>>>>> +                ]
>>>>> +            },
>>>>> +            "sport":
>>>>> +            {
>>>>> +                "shutter":
>>>>> +                [
>>>>> +                    10, 5000, 10000, 20000, 120000
>>>>> +                ],
>>>>> +                "gain":
>>>>> +                [
>>>>> +                    1.0, 2.0, 4.0, 6.0, 6.0
>>>>> +                ]
>>>>> +            }
>>>>> +        },
>>>>> +        "constraint_modes":
>>>>> +        {
>>>>> +            "normal":
>>>>> +            [
>>>>> +            ],
>>>>> +            "highlight":
>>>>> +            [
>>>>> +                {
>>>>> +                    "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
>>>>> +                    [
>>>>> +                        0, 0.5, 1000, 0.5
>>>>> +                    ]
>>>>> +                },
>>>>> +                {
>>>>> +                    "bound": "UPPER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
>>>>> +                    [
>>>>> +                        0, 0.8, 1000, 0.8
>>>>> +                    ]
>>>>> +                }
>>>>> +            ]
>>>>> +        },
>>>>> +        "y_target":
>>>>> +        [
>>>>> +            0, 0.16, 1000, 0.16, 10000, 0.16
>>>>> +        ]
>>>>> +    },
>>>>> +    "rpi.alsc":
>>>>> +    {
>>>>> +        "omega": 1.3,
>>>>> +        "n_iter": 100,
>>>>> +        "luminance_strength": 0.7,
>>>>> +        "luminance_lut":
>>>>> +        [
>>>>> +            2.844, 2.349, 2.018, 1.775, 1.599, 1.466, 1.371, 1.321, 1.306, 1.316, 1.357, 1.439, 1.552, 1.705, 1.915, 2.221,
>>>>> +            2.576, 2.151, 1.851, 1.639, 1.478, 1.358, 1.272, 1.231, 1.218, 1.226, 1.262, 1.335, 1.438, 1.571, 1.766, 2.067,
>>>>> +            2.381, 2.005, 1.739, 1.545, 1.389, 1.278, 1.204, 1.166, 1.153, 1.161, 1.194, 1.263, 1.356, 1.489, 1.671, 1.943,
>>>>> +            2.242, 1.899, 1.658, 1.481, 1.329, 1.225, 1.156, 1.113, 1.096, 1.107, 1.143, 1.201, 1.289, 1.423, 1.607, 1.861,
>>>>> +            2.152, 1.831, 1.602, 1.436, 1.291, 1.193, 1.121, 1.069, 1.047, 1.062, 1.107, 1.166, 1.249, 1.384, 1.562, 1.801,
>>>>> +            2.104, 1.795, 1.572, 1.407, 1.269, 1.174, 1.099, 1.041, 1.008, 1.029, 1.083, 1.146, 1.232, 1.364, 1.547, 1.766,
>>>>> +            2.104, 1.796, 1.572, 1.403, 1.264, 1.171, 1.097, 1.036, 1.001, 1.025, 1.077, 1.142, 1.231, 1.363, 1.549, 1.766,
>>>>> +            2.148, 1.827, 1.594, 1.413, 1.276, 1.184, 1.114, 1.062, 1.033, 1.049, 1.092, 1.153, 1.242, 1.383, 1.577, 1.795,
>>>>> +            2.211, 1.881, 1.636, 1.455, 1.309, 1.214, 1.149, 1.104, 1.081, 1.089, 1.125, 1.184, 1.273, 1.423, 1.622, 1.846,
>>>>> +            2.319, 1.958, 1.698, 1.516, 1.362, 1.262, 1.203, 1.156, 1.137, 1.142, 1.171, 1.229, 1.331, 1.484, 1.682, 1.933,
>>>>> +            2.459, 2.072, 1.789, 1.594, 1.441, 1.331, 1.261, 1.219, 1.199, 1.205, 1.232, 1.301, 1.414, 1.571, 1.773, 2.052,
>>>>> +            2.645, 2.206, 1.928, 1.728, 1.559, 1.451, 1.352, 1.301, 1.282, 1.289, 1.319, 1.395, 1.519, 1.685, 1.904, 2.227
>>>>> +        ],
>>>>> +        "sigma": 0.005,
>>>>> +        "sigma_Cb": 0.005
>>>>> +    },
>>>>> +    "rpi.contrast":
>>>>> +    {
>>>>> +        "ce_enable": 1,
>>>>> +        "gamma_curve":
>>>>> +        [
>>>>> +            0, 0, 1024, 5040, 2048, 9338, 3072, 12356, 4096, 15312, 5120, 18051, 6144, 20790, 7168, 23193,
>>>>> +            8192, 25744, 9216, 27942, 10240, 30035, 11264, 32005, 12288, 33975, 13312, 35815, 14336, 37600, 15360, 39168,
>>>>> +            16384, 40642, 18432, 43379, 20480, 45749, 22528, 47753, 24576, 49621, 26624, 51253, 28672, 52698, 30720, 53796,
>>>>> +            32768, 54876, 36864, 57012, 40960, 58656, 45056, 59954, 49152, 61183, 53248, 62355, 57344, 63419, 61440, 64476,
>>>>> +            65535, 65535
>>>>> +        ]
>>>>> +    },
>>>>> +    "rpi.sharpen":
>>>>> +    {
>>>>> +    },
>>>>> +    "rpi.ccm":
>>>>> +    {
>>>>> +        "ccms":
>>>>> +     [
>>>>> +            {
>>>>> +             "ct": 3900, "ccm":
>>>>> +             [
>>>>> +                 1.54659, -0.17707, -0.36953, -0.51471, 1.72733, -0.21262, 0.06667, -0.92279, 1.85612
>>>>> +             ]
>>>>> +         }
>>>>> +     ]
>>>>> +    },
>>>>> +    "rpi.focus":
>>>>> +    {
>>>>> +    }
>>>>> +}
>>>>> diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
>>>>> index 5236bf1e..509ad58b 100644
>>>>> --- a/src/ipa/raspberrypi/data/meson.build
>>>>> +++ b/src/ipa/raspberrypi/data/meson.build
>>>>> @@ -3,6 +3,7 @@
>>>>>   conf_files = files([
>>>>>       'imx219.json',
>>>>>       'imx477.json',
>>>>> +    'imx290.json',
>>>>
>>>> Alphabetical order please.
>>>>
>>>>>       'ov5647.json',
>>>>>       'uncalibrated.json',
>>>>>   ])
>>>>> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
>>>>> index 59e49686..7e88f8e0 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_imx477.cpp',
>>>>> +    'cam_helper_imx290.cpp',
>>>>
>>>> Here too.
>>>
>>> Will do! Version 2 of this set incoming shortly.
>>>
>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>
>>>>>       'controller/controller.cpp',
>>>>>       'controller/histogram.cpp',
>>>>>       'controller/algorithm.cpp',
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>

Hi Andrey

On Tue, 9 Mar 2021 at 18:28, Andrey Konovalov
<andrey.konovalov@linaro.org> wrote:
>
> Hi Dave,
>
> One correction below...
>
> On 09.03.2021 15:34, Dave Stevenson wrote:
> > Hi Laurent & David
> >
> > On Mon, 8 Mar 2021 at 22:20, Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> >>
> >> Hi David,
> >>
> >> On Mon, Mar 08, 2021 at 10:11:35PM +0000, David Plowman wrote:
> >>> On Mon, 8 Mar 2021 at 19:31, Laurent Pinchart wrote:
> >>>> On Thu, Mar 04, 2021 at 03:31:20PM +0000, David Plowman wrote:
> >>>>> imx290 and imx327 share the same kernel driver (imx290.c) and are
> >>>>> therefore both recognised here as "imx290".
> >>>>
> >>>> Are there any differences between the two sensors that may need to be
> >>>> handled anywhere in libcamera ?
> >>>
> >>> Good question, and the answer is... I don't really know. I don't have
> >>> any modules that I know to have an imx290 in them, but at some point
> >>> I'm sure we or someone else will come across one and we'll find out.
> >>> At some point some we're going to have to start coping with different
> >>> modules with the same (or indistinguishable sensors) in them too.
> >>
> >> There are two issues here, differences between the IMX290 and IMX327
> >> sensors, and differences between modules. For the former, would it make
> >> sense to already add a sony,imx327 compatible string to the imx290
> >> driver, and make use of it ? To be clear, it's not a blocker for this
> >> patch.
> >
> > I have a mono IMX290 from Vision Components here, as well as several
> > IMX327's. Vision Components appear to use literally the same PCB for
> > the two modules - they are both silkscreened IMX327.
> >
> > IMX290 is the bigger brother to IMX327. It can do 1080p120 (10bit
> > only, 4lane CSI2 at 891Mb/s/lane), whilst IMX327 can only do 1080p60
> > (10 or 12bit, 2lane CSI2 at 891Mb/s/lane, or 4lane at 445.5Mb/s/lane).
> >
> > Programming interface is identical except for allowing frsel=0 for
> > 120fps on IMX290.
> >
> > The driver is largely Andrey Konovalov's mainline driver
>
> The original author and the maintainer is Manivannan Sadhasivam.
> I did add some changes on top of his work :)

Whoops! My apologies to Manivannan, and thanks to you for the correction.
I must confess to having looked at the recent mainline commits to the
driver rather than checking who was listed as the maintainer.

  Dave

> Thanks,
> Andrey
>
> > with controls
> > added, and support for an alternate source clock frequency
> > (74.250MHz).
> > It switches mode based on link frequency, and doesn't support the
> > 1080p120fps mode, so is actually independent of whether it is 327 or
> > 290. Based on the link frequency numbers it's a max of 1080p30 on
> > either lane configuration, but I thought I'd had 1080p60.
> >
> > One to investigate to make the driver more flexible, but shouldn't
> > affect libcamera (other than wanting mono support!). The Pi ISP won't
> > support 1080p120 so is actually of limited interest to us.
> >
> >    Dave
> >
> >>>>> We add the necessary
> >>>>> CamHelper for these sensors, as well as a camera tuning file.
> >>>>>
> >>>>> The tuning was done with an Innomaker STARVIS IMX327LQR module. These
> >>>>> have no IR cut filter so there is no proper colour tuning. However,
> >>>>> you should obtain reasonable results for most modules using this
> >>>>> sensor. Specific tunings for further modules can always be added
> >>>>> subsequently.
> >>>>>
> >>>>> To use this sensor on the Raspberry Pi platform, please add
> >>>>>
> >>>>> dtoverlay=imx290,clock-frequency=74250000
> >>>>>
> >>>>> into your /boot/config.txt file.
> >>>>>
> >>>>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> >>>>> ---
> >>>>>   src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
> >>>>>   src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
> >>>>>   src/ipa/raspberrypi/data/meson.build      |   1 +
> >>>>>   src/ipa/raspberrypi/meson.build           |   1 +
> >>>>>   4 files changed, 234 insertions(+)
> >>>>>   create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
> >>>>>   create mode 100644 src/ipa/raspberrypi/data/imx290.json
> >>>>>
> >>>>> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> >>>>> new file mode 100644
> >>>>> index 00000000..6f412e40
> >>>>> --- /dev/null
> >>>>> +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> >>>>> @@ -0,0 +1,67 @@
> >>>>> +/* SPDX-License-Identifier: BSD-2-Clause */
> >>>>> +/*
> >>>>> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> >>>>> + *
> >>>>> + * cam_helper_imx290.cpp - camera helper for imx290 sensor
> >>>>> + */
> >>>>> +
> >>>>> +#include <math.h>
> >>>>> +
> >>>>> +#include "cam_helper.hpp"
> >>>>> +
> >>>>> +using namespace RPiController;
> >>>>> +
> >>>>> +class CamHelperImx290 : public CamHelper
> >>>>> +{
> >>>>> +public:
> >>>>> +     CamHelperImx290();
> >>>>> +     uint32_t GainCode(double gain) const override;
> >>>>> +     double Gain(uint32_t gain_code) const override;
> >>>>> +     void GetDelays(int &exposure_delay, int &gain_delay,
> >>>>> +                    int &vblank_delay) const override;
> >>>>> +     unsigned int HideFramesModeSwitch() const override;
> >>>>> +
> >>>>> +private:
> >>>>> +     /*
> >>>>> +      * Smallest difference between the frame length and integration time,
> >>>>> +      * in units of lines.
> >>>>> +      */
> >>>>> +     static constexpr int frameIntegrationDiff = 2;
> >>>>> +};
> >>>>> +
> >>>>> +CamHelperImx290::CamHelperImx290()
> >>>>> +     : CamHelper(nullptr, frameIntegrationDiff)
> >>>>> +{
> >>>>> +}
> >>>>> +
> >>>>> +uint32_t CamHelperImx290::GainCode(double gain) const
> >>>>> +{
> >>>>> +     int code = 66.6667 * log10(gain);
> >>>>> +     return std::max(0, std::min(code, 0xf0));
> >>>>> +}
> >>>>> +
> >>>>> +double CamHelperImx290::Gain(uint32_t gain_code) const
> >>>>> +{
> >>>>> +     return pow(10, 0.015 * gain_code);
> >>>>> +}
> >>>>> +
> >>>>> +void CamHelperImx290::GetDelays(int &exposure_delay, int &gain_delay,
> >>>>> +                             int &vblank_delay) const
> >>>>> +{
> >>>>> +     exposure_delay = 2;
> >>>>> +     gain_delay = 2;
> >>>>> +     vblank_delay = 2;
> >>>>> +}
> >>>>> +
> >>>>> +unsigned int CamHelperImx290::HideFramesModeSwitch() const
> >>>>> +{
> >>>>> +     /* After a mode switch, we seem to get 1 bad frame. */
> >>>>> +     return 1;
> >>>>> +}
> >>>>> +
> >>>>> +static CamHelper *Create()
> >>>>> +{
> >>>>> +     return new CamHelperImx290();
> >>>>> +}
> >>>>> +
> >>>>> +static RegisterCamHelper reg("imx290", &Create);
> >>>>> diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> >>>>> new file mode 100644
> >>>>> index 00000000..6fb92cc4
> >>>>> --- /dev/null
> >>>>> +++ b/src/ipa/raspberrypi/data/imx290.json
> >>>>> @@ -0,0 +1,165 @@
> >>>>> +{
> >>>>> +    "rpi.black_level":
> >>>>> +    {
> >>>>> +        "black_level": 3840
> >>>>> +    },
> >>>>> +    "rpi.dpc":
> >>>>> +    {
> >>>>> +    },
> >>>>> +    "rpi.lux":
> >>>>> +    {
> >>>>> +        "reference_shutter_speed": 6813,
> >>>>> +        "reference_gain": 1.0,
> >>>>> +        "reference_aperture": 1.0,
> >>>>> +        "reference_lux": 890,
> >>>>> +        "reference_Y": 12900
> >>>>> +    },
> >>>>> +    "rpi.noise":
> >>>>> +    {
> >>>>> +        "reference_constant": 0,
> >>>>> +        "reference_slope": 2.67
> >>>>> +    },
> >>>>> +    "rpi.geq":
> >>>>> +    {
> >>>>> +        "offset": 187,
> >>>>> +        "slope": 0.00842
> >>>>> +    },
> >>>>> +    "rpi.sdn":
> >>>>> +    {
> >>>>> +    },
> >>>>> +    "rpi.awb":
> >>>>> +    {
> >>>>> +     "bayes": 0
> >>>>> +    },
> >>>>> +    "rpi.agc":
> >>>>> +    {
> >>>>> +     "speed": 0.2,
> >>>>> +        "metering_modes":
> >>>>> +        {
> >>>>> +            "matrix":
> >>>>> +            {
> >>>>> +                "weights":
> >>>>> +                [
> >>>>> +                    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
> >>>>> +                ]
> >>>>> +            },
> >>>>> +            "centre-weighted":
> >>>>> +            {
> >>>>> +                "weights":
> >>>>> +                [
> >>>>> +                    3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0
> >>>>> +                ]
> >>>>> +            },
> >>>>> +            "spot":
> >>>>> +            {
> >>>>> +                "weights":
> >>>>> +                [
> >>>>> +                    2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> >>>>> +                ]
> >>>>> +            }
> >>>>> +        },
> >>>>> +        "exposure_modes":
> >>>>> +        {
> >>>>> +            "normal":
> >>>>> +            {
> >>>>> +                "shutter":
> >>>>> +                [
> >>>>> +                    10, 30000, 60000
> >>>>> +                ],
> >>>>> +                "gain":
> >>>>> +                [
> >>>>> +                    1.0,  2.0,   8.0
> >>>>> +                ]
> >>>>> +            },
> >>>>> +            "sport":
> >>>>> +            {
> >>>>> +                "shutter":
> >>>>> +                [
> >>>>> +                    10, 5000, 10000, 20000, 120000
> >>>>> +                ],
> >>>>> +                "gain":
> >>>>> +                [
> >>>>> +                    1.0, 2.0, 4.0, 6.0, 6.0
> >>>>> +                ]
> >>>>> +            }
> >>>>> +        },
> >>>>> +        "constraint_modes":
> >>>>> +        {
> >>>>> +            "normal":
> >>>>> +            [
> >>>>> +            ],
> >>>>> +            "highlight":
> >>>>> +            [
> >>>>> +                {
> >>>>> +                    "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> >>>>> +                    [
> >>>>> +                        0, 0.5, 1000, 0.5
> >>>>> +                    ]
> >>>>> +                },
> >>>>> +                {
> >>>>> +                    "bound": "UPPER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> >>>>> +                    [
> >>>>> +                        0, 0.8, 1000, 0.8
> >>>>> +                    ]
> >>>>> +                }
> >>>>> +            ]
> >>>>> +        },
> >>>>> +        "y_target":
> >>>>> +        [
> >>>>> +            0, 0.16, 1000, 0.16, 10000, 0.16
> >>>>> +        ]
> >>>>> +    },
> >>>>> +    "rpi.alsc":
> >>>>> +    {
> >>>>> +        "omega": 1.3,
> >>>>> +        "n_iter": 100,
> >>>>> +        "luminance_strength": 0.7,
> >>>>> +        "luminance_lut":
> >>>>> +        [
> >>>>> +            2.844, 2.349, 2.018, 1.775, 1.599, 1.466, 1.371, 1.321, 1.306, 1.316, 1.357, 1.439, 1.552, 1.705, 1.915, 2.221,
> >>>>> +            2.576, 2.151, 1.851, 1.639, 1.478, 1.358, 1.272, 1.231, 1.218, 1.226, 1.262, 1.335, 1.438, 1.571, 1.766, 2.067,
> >>>>> +            2.381, 2.005, 1.739, 1.545, 1.389, 1.278, 1.204, 1.166, 1.153, 1.161, 1.194, 1.263, 1.356, 1.489, 1.671, 1.943,
> >>>>> +            2.242, 1.899, 1.658, 1.481, 1.329, 1.225, 1.156, 1.113, 1.096, 1.107, 1.143, 1.201, 1.289, 1.423, 1.607, 1.861,
> >>>>> +            2.152, 1.831, 1.602, 1.436, 1.291, 1.193, 1.121, 1.069, 1.047, 1.062, 1.107, 1.166, 1.249, 1.384, 1.562, 1.801,
> >>>>> +            2.104, 1.795, 1.572, 1.407, 1.269, 1.174, 1.099, 1.041, 1.008, 1.029, 1.083, 1.146, 1.232, 1.364, 1.547, 1.766,
> >>>>> +            2.104, 1.796, 1.572, 1.403, 1.264, 1.171, 1.097, 1.036, 1.001, 1.025, 1.077, 1.142, 1.231, 1.363, 1.549, 1.766,
> >>>>> +            2.148, 1.827, 1.594, 1.413, 1.276, 1.184, 1.114, 1.062, 1.033, 1.049, 1.092, 1.153, 1.242, 1.383, 1.577, 1.795,
> >>>>> +            2.211, 1.881, 1.636, 1.455, 1.309, 1.214, 1.149, 1.104, 1.081, 1.089, 1.125, 1.184, 1.273, 1.423, 1.622, 1.846,
> >>>>> +            2.319, 1.958, 1.698, 1.516, 1.362, 1.262, 1.203, 1.156, 1.137, 1.142, 1.171, 1.229, 1.331, 1.484, 1.682, 1.933,
> >>>>> +            2.459, 2.072, 1.789, 1.594, 1.441, 1.331, 1.261, 1.219, 1.199, 1.205, 1.232, 1.301, 1.414, 1.571, 1.773, 2.052,
> >>>>> +            2.645, 2.206, 1.928, 1.728, 1.559, 1.451, 1.352, 1.301, 1.282, 1.289, 1.319, 1.395, 1.519, 1.685, 1.904, 2.227
> >>>>> +        ],
> >>>>> +        "sigma": 0.005,
> >>>>> +        "sigma_Cb": 0.005
> >>>>> +    },
> >>>>> +    "rpi.contrast":
> >>>>> +    {
> >>>>> +        "ce_enable": 1,
> >>>>> +        "gamma_curve":
> >>>>> +        [
> >>>>> +            0, 0, 1024, 5040, 2048, 9338, 3072, 12356, 4096, 15312, 5120, 18051, 6144, 20790, 7168, 23193,
> >>>>> +            8192, 25744, 9216, 27942, 10240, 30035, 11264, 32005, 12288, 33975, 13312, 35815, 14336, 37600, 15360, 39168,
> >>>>> +            16384, 40642, 18432, 43379, 20480, 45749, 22528, 47753, 24576, 49621, 26624, 51253, 28672, 52698, 30720, 53796,
> >>>>> +            32768, 54876, 36864, 57012, 40960, 58656, 45056, 59954, 49152, 61183, 53248, 62355, 57344, 63419, 61440, 64476,
> >>>>> +            65535, 65535
> >>>>> +        ]
> >>>>> +    },
> >>>>> +    "rpi.sharpen":
> >>>>> +    {
> >>>>> +    },
> >>>>> +    "rpi.ccm":
> >>>>> +    {
> >>>>> +        "ccms":
> >>>>> +     [
> >>>>> +            {
> >>>>> +             "ct": 3900, "ccm":
> >>>>> +             [
> >>>>> +                 1.54659, -0.17707, -0.36953, -0.51471, 1.72733, -0.21262, 0.06667, -0.92279, 1.85612
> >>>>> +             ]
> >>>>> +         }
> >>>>> +     ]
> >>>>> +    },
> >>>>> +    "rpi.focus":
> >>>>> +    {
> >>>>> +    }
> >>>>> +}
> >>>>> diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> >>>>> index 5236bf1e..509ad58b 100644
> >>>>> --- a/src/ipa/raspberrypi/data/meson.build
> >>>>> +++ b/src/ipa/raspberrypi/data/meson.build
> >>>>> @@ -3,6 +3,7 @@
> >>>>>   conf_files = files([
> >>>>>       'imx219.json',
> >>>>>       'imx477.json',
> >>>>> +    'imx290.json',
> >>>>
> >>>> Alphabetical order please.
> >>>>
> >>>>>       'ov5647.json',
> >>>>>       'uncalibrated.json',
> >>>>>   ])
> >>>>> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> >>>>> index 59e49686..7e88f8e0 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_imx477.cpp',
> >>>>> +    'cam_helper_imx290.cpp',
> >>>>
> >>>> Here too.
> >>>
> >>> Will do! Version 2 of this set incoming shortly.
> >>>
> >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>
> >>>>>       'controller/controller.cpp',
> >>>>>       'controller/histogram.cpp',
> >>>>>       'controller/algorithm.cpp',
> >>
> >> --
> >> Regards,
> >>
> >> Laurent Pinchart
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel@lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> >

Hi Dave,

On Tue, Mar 09, 2021 at 12:34:48PM +0000, Dave Stevenson wrote:
> On Mon, 8 Mar 2021 at 22:20, Laurent Pinchart wrote:
> > On Mon, Mar 08, 2021 at 10:11:35PM +0000, David Plowman wrote:
> > > On Mon, 8 Mar 2021 at 19:31, Laurent Pinchart wrote:
> > > > On Thu, Mar 04, 2021 at 03:31:20PM +0000, David Plowman wrote:
> > > > > imx290 and imx327 share the same kernel driver (imx290.c) and are
> > > > > therefore both recognised here as "imx290".
> > > >
> > > > Are there any differences between the two sensors that may need to be
> > > > handled anywhere in libcamera ?
> > >
> > > Good question, and the answer is... I don't really know. I don't have
> > > any modules that I know to have an imx290 in them, but at some point
> > > I'm sure we or someone else will come across one and we'll find out.
> > > At some point some we're going to have to start coping with different
> > > modules with the same (or indistinguishable sensors) in them too.
> >
> > There are two issues here, differences between the IMX290 and IMX327
> > sensors, and differences between modules. For the former, would it make
> > sense to already add a sony,imx327 compatible string to the imx290
> > driver, and make use of it ? To be clear, it's not a blocker for this
> > patch.
> 
> I have a mono IMX290 from Vision Components here, as well as several
> IMX327's. Vision Components appear to use literally the same PCB for
> the two modules - they are both silkscreened IMX327.
> 
> IMX290 is the bigger brother to IMX327. It can do 1080p120 (10bit
> only, 4lane CSI2 at 891Mb/s/lane), whilst IMX327 can only do 1080p60
> (10 or 12bit, 2lane CSI2 at 891Mb/s/lane, or 4lane at 445.5Mb/s/lane).
> 
> Programming interface is identical except for allowing frsel=0 for
> 120fps on IMX290.
> 
> The driver is largely Andrey Konovalov's mainline driver with controls
> added, and support for an alternate source clock frequency
> (74.250MHz).
> It switches mode based on link frequency, and doesn't support the
> 1080p120fps mode, so is actually independent of whether it is 327 or
> 290. Based on the link frequency numbers it's a max of 1080p30 on
> either lane configuration, but I thought I'd had 1080p60.
> 
> One to investigate to make the driver more flexible, but shouldn't
> affect libcamera (other than wanting mono support!). The Pi ISP won't
> support 1080p120 so is actually of limited interest to us.

Thank you for the detailed information. Based on the difference you
outlined between the IMX290 and IMX327, I think different compatible
strings would be useful, to enable the 120fps mode on IMX290 only (we
don't have to add 120fps support to the driver just yet, just prepare
for it). I've read that the IMX327 has a better light sensitivity than
the IMX290, so maybe this would have an influence on algorithms, and
would also be worth reporting the correct sensor model to userspace.

> > > > > We add the necessary
> > > > > CamHelper for these sensors, as well as a camera tuning file.
> > > > >
> > > > > The tuning was done with an Innomaker STARVIS IMX327LQR module. These
> > > > > have no IR cut filter so there is no proper colour tuning. However,
> > > > > you should obtain reasonable results for most modules using this
> > > > > sensor. Specific tunings for further modules can always be added
> > > > > subsequently.
> > > > >
> > > > > To use this sensor on the Raspberry Pi platform, please add
> > > > >
> > > > > dtoverlay=imx290,clock-frequency=74250000
> > > > >
> > > > > into your /boot/config.txt file.
> > > > >
> > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > > > ---
> > > > >  src/ipa/raspberrypi/cam_helper_imx290.cpp |  67 +++++++++
> > > > >  src/ipa/raspberrypi/data/imx290.json      | 165 ++++++++++++++++++++++
> > > > >  src/ipa/raspberrypi/data/meson.build      |   1 +
> > > > >  src/ipa/raspberrypi/meson.build           |   1 +
> > > > >  4 files changed, 234 insertions(+)
> > > > >  create mode 100644 src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > > >  create mode 100644 src/ipa/raspberrypi/data/imx290.json
> > > > >
> > > > > diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > > > new file mode 100644
> > > > > index 00000000..6f412e40
> > > > > --- /dev/null
> > > > > +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> > > > > @@ -0,0 +1,67 @@
> > > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > > +/*
> > > > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > > > > + *
> > > > > + * cam_helper_imx290.cpp - camera helper for imx290 sensor
> > > > > + */
> > > > > +
> > > > > +#include <math.h>
> > > > > +
> > > > > +#include "cam_helper.hpp"
> > > > > +
> > > > > +using namespace RPiController;
> > > > > +
> > > > > +class CamHelperImx290 : public CamHelper
> > > > > +{
> > > > > +public:
> > > > > +     CamHelperImx290();
> > > > > +     uint32_t GainCode(double gain) const override;
> > > > > +     double Gain(uint32_t gain_code) const override;
> > > > > +     void GetDelays(int &exposure_delay, int &gain_delay,
> > > > > +                    int &vblank_delay) const override;
> > > > > +     unsigned int HideFramesModeSwitch() const override;
> > > > > +
> > > > > +private:
> > > > > +     /*
> > > > > +      * Smallest difference between the frame length and integration time,
> > > > > +      * in units of lines.
> > > > > +      */
> > > > > +     static constexpr int frameIntegrationDiff = 2;
> > > > > +};
> > > > > +
> > > > > +CamHelperImx290::CamHelperImx290()
> > > > > +     : CamHelper(nullptr, frameIntegrationDiff)
> > > > > +{
> > > > > +}
> > > > > +
> > > > > +uint32_t CamHelperImx290::GainCode(double gain) const
> > > > > +{
> > > > > +     int code = 66.6667 * log10(gain);
> > > > > +     return std::max(0, std::min(code, 0xf0));
> > > > > +}
> > > > > +
> > > > > +double CamHelperImx290::Gain(uint32_t gain_code) const
> > > > > +{
> > > > > +     return pow(10, 0.015 * gain_code);
> > > > > +}
> > > > > +
> > > > > +void CamHelperImx290::GetDelays(int &exposure_delay, int &gain_delay,
> > > > > +                             int &vblank_delay) const
> > > > > +{
> > > > > +     exposure_delay = 2;
> > > > > +     gain_delay = 2;
> > > > > +     vblank_delay = 2;
> > > > > +}
> > > > > +
> > > > > +unsigned int CamHelperImx290::HideFramesModeSwitch() const
> > > > > +{
> > > > > +     /* After a mode switch, we seem to get 1 bad frame. */
> > > > > +     return 1;
> > > > > +}
> > > > > +
> > > > > +static CamHelper *Create()
> > > > > +{
> > > > > +     return new CamHelperImx290();
> > > > > +}
> > > > > +
> > > > > +static RegisterCamHelper reg("imx290", &Create);
> > > > > diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json
> > > > > new file mode 100644
> > > > > index 00000000..6fb92cc4
> > > > > --- /dev/null
> > > > > +++ b/src/ipa/raspberrypi/data/imx290.json
> > > > > @@ -0,0 +1,165 @@
> > > > > +{
> > > > > +    "rpi.black_level":
> > > > > +    {
> > > > > +        "black_level": 3840
> > > > > +    },
> > > > > +    "rpi.dpc":
> > > > > +    {
> > > > > +    },
> > > > > +    "rpi.lux":
> > > > > +    {
> > > > > +        "reference_shutter_speed": 6813,
> > > > > +        "reference_gain": 1.0,
> > > > > +        "reference_aperture": 1.0,
> > > > > +        "reference_lux": 890,
> > > > > +        "reference_Y": 12900
> > > > > +    },
> > > > > +    "rpi.noise":
> > > > > +    {
> > > > > +        "reference_constant": 0,
> > > > > +        "reference_slope": 2.67
> > > > > +    },
> > > > > +    "rpi.geq":
> > > > > +    {
> > > > > +        "offset": 187,
> > > > > +        "slope": 0.00842
> > > > > +    },
> > > > > +    "rpi.sdn":
> > > > > +    {
> > > > > +    },
> > > > > +    "rpi.awb":
> > > > > +    {
> > > > > +     "bayes": 0
> > > > > +    },
> > > > > +    "rpi.agc":
> > > > > +    {
> > > > > +     "speed": 0.2,
> > > > > +        "metering_modes":
> > > > > +        {
> > > > > +            "matrix":
> > > > > +            {
> > > > > +                "weights":
> > > > > +                [
> > > > > +                    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1
> > > > > +                ]
> > > > > +            },
> > > > > +            "centre-weighted":
> > > > > +            {
> > > > > +                "weights":
> > > > > +                [
> > > > > +                    3, 3, 3, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0
> > > > > +                ]
> > > > > +            },
> > > > > +            "spot":
> > > > > +            {
> > > > > +                "weights":
> > > > > +                [
> > > > > +                    2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> > > > > +                ]
> > > > > +            }
> > > > > +        },
> > > > > +        "exposure_modes":
> > > > > +        {
> > > > > +            "normal":
> > > > > +            {
> > > > > +                "shutter":
> > > > > +                [
> > > > > +                    10, 30000, 60000
> > > > > +                ],
> > > > > +                "gain":
> > > > > +                [
> > > > > +                    1.0,  2.0,   8.0
> > > > > +                ]
> > > > > +            },
> > > > > +            "sport":
> > > > > +            {
> > > > > +                "shutter":
> > > > > +                [
> > > > > +                    10, 5000, 10000, 20000, 120000
> > > > > +                ],
> > > > > +                "gain":
> > > > > +                [
> > > > > +                    1.0, 2.0, 4.0, 6.0, 6.0
> > > > > +                ]
> > > > > +            }
> > > > > +        },
> > > > > +        "constraint_modes":
> > > > > +        {
> > > > > +            "normal":
> > > > > +            [
> > > > > +            ],
> > > > > +            "highlight":
> > > > > +            [
> > > > > +                {
> > > > > +                    "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > > > > +                    [
> > > > > +                        0, 0.5, 1000, 0.5
> > > > > +                    ]
> > > > > +                },
> > > > > +                {
> > > > > +                    "bound": "UPPER", "q_lo": 0.98, "q_hi": 1.0, "y_target":
> > > > > +                    [
> > > > > +                        0, 0.8, 1000, 0.8
> > > > > +                    ]
> > > > > +                }
> > > > > +            ]
> > > > > +        },
> > > > > +        "y_target":
> > > > > +        [
> > > > > +            0, 0.16, 1000, 0.16, 10000, 0.16
> > > > > +        ]
> > > > > +    },
> > > > > +    "rpi.alsc":
> > > > > +    {
> > > > > +        "omega": 1.3,
> > > > > +        "n_iter": 100,
> > > > > +        "luminance_strength": 0.7,
> > > > > +        "luminance_lut":
> > > > > +        [
> > > > > +            2.844, 2.349, 2.018, 1.775, 1.599, 1.466, 1.371, 1.321, 1.306, 1.316, 1.357, 1.439, 1.552, 1.705, 1.915, 2.221,
> > > > > +            2.576, 2.151, 1.851, 1.639, 1.478, 1.358, 1.272, 1.231, 1.218, 1.226, 1.262, 1.335, 1.438, 1.571, 1.766, 2.067,
> > > > > +            2.381, 2.005, 1.739, 1.545, 1.389, 1.278, 1.204, 1.166, 1.153, 1.161, 1.194, 1.263, 1.356, 1.489, 1.671, 1.943,
> > > > > +            2.242, 1.899, 1.658, 1.481, 1.329, 1.225, 1.156, 1.113, 1.096, 1.107, 1.143, 1.201, 1.289, 1.423, 1.607, 1.861,
> > > > > +            2.152, 1.831, 1.602, 1.436, 1.291, 1.193, 1.121, 1.069, 1.047, 1.062, 1.107, 1.166, 1.249, 1.384, 1.562, 1.801,
> > > > > +            2.104, 1.795, 1.572, 1.407, 1.269, 1.174, 1.099, 1.041, 1.008, 1.029, 1.083, 1.146, 1.232, 1.364, 1.547, 1.766,
> > > > > +            2.104, 1.796, 1.572, 1.403, 1.264, 1.171, 1.097, 1.036, 1.001, 1.025, 1.077, 1.142, 1.231, 1.363, 1.549, 1.766,
> > > > > +            2.148, 1.827, 1.594, 1.413, 1.276, 1.184, 1.114, 1.062, 1.033, 1.049, 1.092, 1.153, 1.242, 1.383, 1.577, 1.795,
> > > > > +            2.211, 1.881, 1.636, 1.455, 1.309, 1.214, 1.149, 1.104, 1.081, 1.089, 1.125, 1.184, 1.273, 1.423, 1.622, 1.846,
> > > > > +            2.319, 1.958, 1.698, 1.516, 1.362, 1.262, 1.203, 1.156, 1.137, 1.142, 1.171, 1.229, 1.331, 1.484, 1.682, 1.933,
> > > > > +            2.459, 2.072, 1.789, 1.594, 1.441, 1.331, 1.261, 1.219, 1.199, 1.205, 1.232, 1.301, 1.414, 1.571, 1.773, 2.052,
> > > > > +            2.645, 2.206, 1.928, 1.728, 1.559, 1.451, 1.352, 1.301, 1.282, 1.289, 1.319, 1.395, 1.519, 1.685, 1.904, 2.227
> > > > > +        ],
> > > > > +        "sigma": 0.005,
> > > > > +        "sigma_Cb": 0.005
> > > > > +    },
> > > > > +    "rpi.contrast":
> > > > > +    {
> > > > > +        "ce_enable": 1,
> > > > > +        "gamma_curve":
> > > > > +        [
> > > > > +            0, 0, 1024, 5040, 2048, 9338, 3072, 12356, 4096, 15312, 5120, 18051, 6144, 20790, 7168, 23193,
> > > > > +            8192, 25744, 9216, 27942, 10240, 30035, 11264, 32005, 12288, 33975, 13312, 35815, 14336, 37600, 15360, 39168,
> > > > > +            16384, 40642, 18432, 43379, 20480, 45749, 22528, 47753, 24576, 49621, 26624, 51253, 28672, 52698, 30720, 53796,
> > > > > +            32768, 54876, 36864, 57012, 40960, 58656, 45056, 59954, 49152, 61183, 53248, 62355, 57344, 63419, 61440, 64476,
> > > > > +            65535, 65535
> > > > > +        ]
> > > > > +    },
> > > > > +    "rpi.sharpen":
> > > > > +    {
> > > > > +    },
> > > > > +    "rpi.ccm":
> > > > > +    {
> > > > > +        "ccms":
> > > > > +     [
> > > > > +            {
> > > > > +             "ct": 3900, "ccm":
> > > > > +             [
> > > > > +                 1.54659, -0.17707, -0.36953, -0.51471, 1.72733, -0.21262, 0.06667, -0.92279, 1.85612
> > > > > +             ]
> > > > > +         }
> > > > > +     ]
> > > > > +    },
> > > > > +    "rpi.focus":
> > > > > +    {
> > > > > +    }
> > > > > +}
> > > > > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > > > > index 5236bf1e..509ad58b 100644
> > > > > --- a/src/ipa/raspberrypi/data/meson.build
> > > > > +++ b/src/ipa/raspberrypi/data/meson.build
> > > > > @@ -3,6 +3,7 @@
> > > > >  conf_files = files([
> > > > >      'imx219.json',
> > > > >      'imx477.json',
> > > > > +    'imx290.json',
> > > >
> > > > Alphabetical order please.
> > > >
> > > > >      'ov5647.json',
> > > > >      'uncalibrated.json',
> > > > >  ])
> > > > > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > > > > index 59e49686..7e88f8e0 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_imx477.cpp',
> > > > > +    'cam_helper_imx290.cpp',
> > > >
> > > > Here too.
> > >
> > > Will do! Version 2 of this set incoming shortly.
> > >
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > >      'controller/controller.cpp',
> > > > >      'controller/histogram.cpp',
> > > > >      'controller/algorithm.cpp',