[libcamera-devel,v3,3/3] libcamera: ipa: raspberrypi: Add support for ov9281 sensor
diff mbox series

Message ID 20210618220430.31457-4-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • Support monochrome raw sensors
Related show

Commit Message

David Plowman June 18, 2021, 10:04 p.m. UTC 
The necessary tuning file and CamHelper is added for the ov9281 sensor
(which the driver names as the "mov9281").

The ov9281 is a 1280x800 monochrome global shutter sensor. To enable
it, please add

dtoverlay=ov9281

to the /boot/config.txt file and reboot the Pi.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/raspberrypi/cam_helper_mov9281.cpp | 65 +++++++++++++++
 src/ipa/raspberrypi/data/meson.build       |  1 +
 src/ipa/raspberrypi/data/mov9281.json      | 92 ++++++++++++++++++++++
 src/ipa/raspberrypi/meson.build            |  1 +
 4 files changed, 159 insertions(+)
 create mode 100644 src/ipa/raspberrypi/cam_helper_mov9281.cpp
 create mode 100644 src/ipa/raspberrypi/data/mov9281.json

Comments

Kieran Bingham June 18, 2021, 10:23 p.m. UTC | #1 
On 18/06/2021 23:04, David Plowman wrote:
> The necessary tuning file and CamHelper is added for the ov9281 sensor
> (which the driver names as the "mov9281").
> 
> The ov9281 is a 1280x800 monochrome global shutter sensor. To enable
> it, please add
> 
> dtoverlay=ov9281
> 
> to the /boot/config.txt file and reboot the Pi.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

As long as we know that this is destined for upstream integration:

Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>.



> ---
>  src/ipa/raspberrypi/cam_helper_mov9281.cpp | 65 +++++++++++++++
>  src/ipa/raspberrypi/data/meson.build       |  1 +
>  src/ipa/raspberrypi/data/mov9281.json      | 92 ++++++++++++++++++++++
>  src/ipa/raspberrypi/meson.build            |  1 +
>  4 files changed, 159 insertions(+)
>  create mode 100644 src/ipa/raspberrypi/cam_helper_mov9281.cpp
>  create mode 100644 src/ipa/raspberrypi/data/mov9281.json
> 
> diff --git a/src/ipa/raspberrypi/cam_helper_mov9281.cpp b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> new file mode 100644
> index 00000000..29519cb0
> --- /dev/null
> +++ b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * cam_helper_mov9281.cpp - camera information for ov9281 sensor
> + */
> +
> +#include <assert.h>
> +
> +#include "cam_helper.hpp"
> +
> +using namespace RPiController;
> +
> +class CamHelperOv9281 : public CamHelper
> +{
> +public:
> +	CamHelperOv9281();
> +	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;
> +
> +private:
> +	/*
> +	 * Smallest difference between the frame length and integration time,
> +	 * in units of lines.
> +	 */
> +	static constexpr int frameIntegrationDiff = 4;
> +};
> +
> +/*
> + * OV9281 doesn't output metadata, so we have to use the "unicam parser" which
> + * works by counting frames.
> + */
> +
> +CamHelperOv9281::CamHelperOv9281()
> +	: CamHelper(nullptr, frameIntegrationDiff)
> +{
> +}
> +
> +uint32_t CamHelperOv9281::GainCode(double gain) const
> +{
> +	return static_cast<uint32_t>(gain * 16.0);
> +}
> +
> +double CamHelperOv9281::Gain(uint32_t gain_code) const
> +{
> +	return static_cast<double>(gain_code) / 16.0;
> +}
> +
> +void CamHelperOv9281::GetDelays(int &exposure_delay, int &gain_delay,
> +				int &vblank_delay) const
> +{
> +	/* The driver appears to behave as follows: */
> +	exposure_delay = 2;
> +	gain_delay = 2;
> +	vblank_delay = 2;
> +}
> +
> +static CamHelper *Create()
> +{
> +	return new CamHelperOv9281();
> +}
> +
> +static RegisterCamHelper reg("mov9281", &Create);
> diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> index 92ad3272..f8baab6d 100644
> --- a/src/ipa/raspberrypi/data/meson.build
> +++ b/src/ipa/raspberrypi/data/meson.build
> @@ -4,6 +4,7 @@ conf_files = files([
>      'imx219.json',
>      'imx290.json',
>      'imx477.json',
> +    'mov9281.json',
>      'ov5647.json',
>      'se327m12.json',
>      'uncalibrated.json',
> diff --git a/src/ipa/raspberrypi/data/mov9281.json b/src/ipa/raspberrypi/data/mov9281.json
> new file mode 100644
> index 00000000..ecd262be
> --- /dev/null
> +++ b/src/ipa/raspberrypi/data/mov9281.json
> @@ -0,0 +1,92 @@
> +{
> +    "rpi.black_level":
> +    {
> +        "black_level": 4096
> +    },
> +    "rpi.lux":
> +    {
> +        "reference_shutter_speed": 2000,
> +        "reference_gain": 1.0,
> +        "reference_aperture": 1.0,
> +        "reference_lux": 800,
> +        "reference_Y": 20000
> +    },
> +    "rpi.noise":
> +    {
> +        "reference_constant": 0,
> +        "reference_slope": 2.5
> +    },
> +    "rpi.sdn":
> +    {
> +    },
> +    "rpi.agc":
> +    {
> +        "metering_modes":
> +        {
> +            "centre-weighted": {
> +                "weights": [4, 4, 4, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0]
> +            }
> +        },
> +        "exposure_modes":
> +        {
> +            "normal":
> +            {
> +                "shutter": [ 100, 15000, 30000, 60000, 120000 ],
> +                "gain":    [ 1.0, 2.0,   3.0,   4.0,   6.0    ]
> +            }
> +        },
> +        "constraint_modes":
> +        {
> +            "normal":
> +            [
> +                { "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target": [ 0, 0.4, 1000, 0.4 ] }
> +            ]
> +        },
> +        "y_target": [ 0, 0.16, 1000, 0.165, 10000, 0.17 ]
> +    },
> +    "rpi.alsc":
> +    {
> +        "n_iter": 0,
> +        "luminance_strength": 1.0,
> +        "corner_strength": 1.5
> +    },
> +    "rpi.contrast":
> +    {
> +        "ce_enable": 0,
> +        "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
> +        ]
> +    }
> +}
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index 230356d3..93f04e9e 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -22,6 +22,7 @@ rpi_ipa_sources = files([
>      'cam_helper_imx219.cpp',
>      'cam_helper_imx290.cpp',
>      'cam_helper_imx477.cpp',
> +    'cam_helper_mov9281.cpp',
>      'controller/controller.cpp',
>      'controller/histogram.cpp',
>      'controller/algorithm.cpp',
>
Laurent Pinchart June 28, 2021, 6:17 a.m. UTC | #2 
Hi David,

Thank you for the patch.

On Fri, Jun 18, 2021 at 11:04:30PM +0100, David Plowman wrote:
> The necessary tuning file and CamHelper is added for the ov9281 sensor
> (which the driver names as the "mov9281").

This bothers me a little bit, as it will likely need to be renamed to
ov9281 when upstreaming. Why is there an "m" prefix, and could it be
dropped ?

> The ov9281 is a 1280x800 monochrome global shutter sensor. To enable
> it, please add
> 
> dtoverlay=ov9281
> 
> to the /boot/config.txt file and reboot the Pi.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  src/ipa/raspberrypi/cam_helper_mov9281.cpp | 65 +++++++++++++++
>  src/ipa/raspberrypi/data/meson.build       |  1 +
>  src/ipa/raspberrypi/data/mov9281.json      | 92 ++++++++++++++++++++++
>  src/ipa/raspberrypi/meson.build            |  1 +
>  4 files changed, 159 insertions(+)
>  create mode 100644 src/ipa/raspberrypi/cam_helper_mov9281.cpp
>  create mode 100644 src/ipa/raspberrypi/data/mov9281.json
> 
> diff --git a/src/ipa/raspberrypi/cam_helper_mov9281.cpp b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> new file mode 100644
> index 00000000..29519cb0
> --- /dev/null
> +++ b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * cam_helper_mov9281.cpp - camera information for ov9281 sensor
> + */
> +
> +#include <assert.h>
> +
> +#include "cam_helper.hpp"
> +
> +using namespace RPiController;
> +
> +class CamHelperOv9281 : public CamHelper
> +{
> +public:
> +	CamHelperOv9281();
> +	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;
> +
> +private:
> +	/*
> +	 * Smallest difference between the frame length and integration time,
> +	 * in units of lines.
> +	 */
> +	static constexpr int frameIntegrationDiff = 4;
> +};
> +
> +/*
> + * OV9281 doesn't output metadata, so we have to use the "unicam parser" which
> + * works by counting frames.
> + */
> +
> +CamHelperOv9281::CamHelperOv9281()
> +	: CamHelper(nullptr, frameIntegrationDiff)
> +{
> +}
> +
> +uint32_t CamHelperOv9281::GainCode(double gain) const
> +{
> +	return static_cast<uint32_t>(gain * 16.0);
> +}
> +
> +double CamHelperOv9281::Gain(uint32_t gain_code) const
> +{
> +	return static_cast<double>(gain_code) / 16.0;
> +}
> +
> +void CamHelperOv9281::GetDelays(int &exposure_delay, int &gain_delay,
> +				int &vblank_delay) const
> +{
> +	/* The driver appears to behave as follows: */
> +	exposure_delay = 2;
> +	gain_delay = 2;
> +	vblank_delay = 2;
> +}
> +
> +static CamHelper *Create()
> +{
> +	return new CamHelperOv9281();
> +}
> +
> +static RegisterCamHelper reg("mov9281", &Create);
> diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> index 92ad3272..f8baab6d 100644
> --- a/src/ipa/raspberrypi/data/meson.build
> +++ b/src/ipa/raspberrypi/data/meson.build
> @@ -4,6 +4,7 @@ conf_files = files([
>      'imx219.json',
>      'imx290.json',
>      'imx477.json',
> +    'mov9281.json',
>      'ov5647.json',
>      'se327m12.json',
>      'uncalibrated.json',
> diff --git a/src/ipa/raspberrypi/data/mov9281.json b/src/ipa/raspberrypi/data/mov9281.json
> new file mode 100644
> index 00000000..ecd262be
> --- /dev/null
> +++ b/src/ipa/raspberrypi/data/mov9281.json
> @@ -0,0 +1,92 @@
> +{
> +    "rpi.black_level":
> +    {
> +        "black_level": 4096
> +    },
> +    "rpi.lux":
> +    {
> +        "reference_shutter_speed": 2000,
> +        "reference_gain": 1.0,
> +        "reference_aperture": 1.0,
> +        "reference_lux": 800,
> +        "reference_Y": 20000
> +    },
> +    "rpi.noise":
> +    {
> +        "reference_constant": 0,
> +        "reference_slope": 2.5
> +    },
> +    "rpi.sdn":
> +    {
> +    },
> +    "rpi.agc":
> +    {
> +        "metering_modes":
> +        {
> +            "centre-weighted": {
> +                "weights": [4, 4, 4, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0]
> +            }
> +        },
> +        "exposure_modes":
> +        {
> +            "normal":
> +            {
> +                "shutter": [ 100, 15000, 30000, 60000, 120000 ],
> +                "gain":    [ 1.0, 2.0,   3.0,   4.0,   6.0    ]
> +            }
> +        },
> +        "constraint_modes":
> +        {
> +            "normal":
> +            [
> +                { "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target": [ 0, 0.4, 1000, 0.4 ] }
> +            ]
> +        },
> +        "y_target": [ 0, 0.16, 1000, 0.165, 10000, 0.17 ]
> +    },
> +    "rpi.alsc":
> +    {
> +        "n_iter": 0,
> +        "luminance_strength": 1.0,
> +        "corner_strength": 1.5
> +    },
> +    "rpi.contrast":
> +    {
> +        "ce_enable": 0,
> +        "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
> +        ]
> +    }
> +}
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index 230356d3..93f04e9e 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -22,6 +22,7 @@ rpi_ipa_sources = files([
>      'cam_helper_imx219.cpp',
>      'cam_helper_imx290.cpp',
>      'cam_helper_imx477.cpp',
> +    'cam_helper_mov9281.cpp',
>      'controller/controller.cpp',
>      'controller/histogram.cpp',
>      'controller/algorithm.cpp',
David Plowman June 28, 2021, 6:47 a.m. UTC | #3 
Hi Laurent

Thanks for the comments.

On Mon, 28 Jun 2021 at 07:17, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Fri, Jun 18, 2021 at 11:04:30PM +0100, David Plowman wrote:
> > The necessary tuning file and CamHelper is added for the ov9281 sensor
> > (which the driver names as the "mov9281").
>
> This bothers me a little bit, as it will likely need to be renamed to
> ov9281 when upstreaming. Why is there an "m" prefix, and could it be
> dropped ?

Yes, I'm suspicious that the "m" is actually a reference to "mono",
thereby opening the way for colour versions of this sensor. In fact it
would be helpful for the driver to advertise a different name in such
cases so that a more appropriate tuning file can be identified (though
as we know, that still isn't really good enough...!) But maybe
something a little less cryptic than just "m" would help?

Best regards
David

>
> > The ov9281 is a 1280x800 monochrome global shutter sensor. To enable
> > it, please add
> >
> > dtoverlay=ov9281
> >
> > to the /boot/config.txt file and reboot the Pi.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  src/ipa/raspberrypi/cam_helper_mov9281.cpp | 65 +++++++++++++++
> >  src/ipa/raspberrypi/data/meson.build       |  1 +
> >  src/ipa/raspberrypi/data/mov9281.json      | 92 ++++++++++++++++++++++
> >  src/ipa/raspberrypi/meson.build            |  1 +
> >  4 files changed, 159 insertions(+)
> >  create mode 100644 src/ipa/raspberrypi/cam_helper_mov9281.cpp
> >  create mode 100644 src/ipa/raspberrypi/data/mov9281.json
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper_mov9281.cpp b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > new file mode 100644
> > index 00000000..29519cb0
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > @@ -0,0 +1,65 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > + *
> > + * cam_helper_mov9281.cpp - camera information for ov9281 sensor
> > + */
> > +
> > +#include <assert.h>
> > +
> > +#include "cam_helper.hpp"
> > +
> > +using namespace RPiController;
> > +
> > +class CamHelperOv9281 : public CamHelper
> > +{
> > +public:
> > +     CamHelperOv9281();
> > +     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;
> > +
> > +private:
> > +     /*
> > +      * Smallest difference between the frame length and integration time,
> > +      * in units of lines.
> > +      */
> > +     static constexpr int frameIntegrationDiff = 4;
> > +};
> > +
> > +/*
> > + * OV9281 doesn't output metadata, so we have to use the "unicam parser" which
> > + * works by counting frames.
> > + */
> > +
> > +CamHelperOv9281::CamHelperOv9281()
> > +     : CamHelper(nullptr, frameIntegrationDiff)
> > +{
> > +}
> > +
> > +uint32_t CamHelperOv9281::GainCode(double gain) const
> > +{
> > +     return static_cast<uint32_t>(gain * 16.0);
> > +}
> > +
> > +double CamHelperOv9281::Gain(uint32_t gain_code) const
> > +{
> > +     return static_cast<double>(gain_code) / 16.0;
> > +}
> > +
> > +void CamHelperOv9281::GetDelays(int &exposure_delay, int &gain_delay,
> > +                             int &vblank_delay) const
> > +{
> > +     /* The driver appears to behave as follows: */
> > +     exposure_delay = 2;
> > +     gain_delay = 2;
> > +     vblank_delay = 2;
> > +}
> > +
> > +static CamHelper *Create()
> > +{
> > +     return new CamHelperOv9281();
> > +}
> > +
> > +static RegisterCamHelper reg("mov9281", &Create);
> > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > index 92ad3272..f8baab6d 100644
> > --- a/src/ipa/raspberrypi/data/meson.build
> > +++ b/src/ipa/raspberrypi/data/meson.build
> > @@ -4,6 +4,7 @@ conf_files = files([
> >      'imx219.json',
> >      'imx290.json',
> >      'imx477.json',
> > +    'mov9281.json',
> >      'ov5647.json',
> >      'se327m12.json',
> >      'uncalibrated.json',
> > diff --git a/src/ipa/raspberrypi/data/mov9281.json b/src/ipa/raspberrypi/data/mov9281.json
> > new file mode 100644
> > index 00000000..ecd262be
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/data/mov9281.json
> > @@ -0,0 +1,92 @@
> > +{
> > +    "rpi.black_level":
> > +    {
> > +        "black_level": 4096
> > +    },
> > +    "rpi.lux":
> > +    {
> > +        "reference_shutter_speed": 2000,
> > +        "reference_gain": 1.0,
> > +        "reference_aperture": 1.0,
> > +        "reference_lux": 800,
> > +        "reference_Y": 20000
> > +    },
> > +    "rpi.noise":
> > +    {
> > +        "reference_constant": 0,
> > +        "reference_slope": 2.5
> > +    },
> > +    "rpi.sdn":
> > +    {
> > +    },
> > +    "rpi.agc":
> > +    {
> > +        "metering_modes":
> > +        {
> > +            "centre-weighted": {
> > +                "weights": [4, 4, 4, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0]
> > +            }
> > +        },
> > +        "exposure_modes":
> > +        {
> > +            "normal":
> > +            {
> > +                "shutter": [ 100, 15000, 30000, 60000, 120000 ],
> > +                "gain":    [ 1.0, 2.0,   3.0,   4.0,   6.0    ]
> > +            }
> > +        },
> > +        "constraint_modes":
> > +        {
> > +            "normal":
> > +            [
> > +                { "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target": [ 0, 0.4, 1000, 0.4 ] }
> > +            ]
> > +        },
> > +        "y_target": [ 0, 0.16, 1000, 0.165, 10000, 0.17 ]
> > +    },
> > +    "rpi.alsc":
> > +    {
> > +        "n_iter": 0,
> > +        "luminance_strength": 1.0,
> > +        "corner_strength": 1.5
> > +    },
> > +    "rpi.contrast":
> > +    {
> > +        "ce_enable": 0,
> > +        "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
> > +        ]
> > +    }
> > +}
> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > index 230356d3..93f04e9e 100644
> > --- a/src/ipa/raspberrypi/meson.build
> > +++ b/src/ipa/raspberrypi/meson.build
> > @@ -22,6 +22,7 @@ rpi_ipa_sources = files([
> >      'cam_helper_imx219.cpp',
> >      'cam_helper_imx290.cpp',
> >      'cam_helper_imx477.cpp',
> > +    'cam_helper_mov9281.cpp',
> >      'controller/controller.cpp',
> >      'controller/histogram.cpp',
> >      'controller/algorithm.cpp',
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart June 28, 2021, 7:02 a.m. UTC | #4 
Hi David,

On Mon, Jun 28, 2021 at 07:47:39AM +0100, David Plowman wrote:
> On Mon, 28 Jun 2021 at 07:17, Laurent Pinchart wrote:
> > On Fri, Jun 18, 2021 at 11:04:30PM +0100, David Plowman wrote:
> > > The necessary tuning file and CamHelper is added for the ov9281 sensor
> > > (which the driver names as the "mov9281").
> >
> > This bothers me a little bit, as it will likely need to be renamed to
> > ov9281 when upstreaming. Why is there an "m" prefix, and could it be
> > dropped ?
> 
> Yes, I'm suspicious that the "m" is actually a reference to "mono",
> thereby opening the way for colour versions of this sensor. In fact it
> would be helpful for the driver to advertise a different name in such
> cases so that a more appropriate tuning file can be identified (though
> as we know, that still isn't really good enough...!) But maybe
> something a little less cryptic than just "m" would help?

Is there a colour version of the OV9281 ? If not, I'd just drop the
prefix. If there is, the media entity should be named according to the
Omnivision naming scheme.

> > > The ov9281 is a 1280x800 monochrome global shutter sensor. To enable
> > > it, please add
> > >
> > > dtoverlay=ov9281
> > >
> > > to the /boot/config.txt file and reboot the Pi.
> > >
> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> > > ---
> > >  src/ipa/raspberrypi/cam_helper_mov9281.cpp | 65 +++++++++++++++
> > >  src/ipa/raspberrypi/data/meson.build       |  1 +
> > >  src/ipa/raspberrypi/data/mov9281.json      | 92 ++++++++++++++++++++++
> > >  src/ipa/raspberrypi/meson.build            |  1 +
> > >  4 files changed, 159 insertions(+)
> > >  create mode 100644 src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > >  create mode 100644 src/ipa/raspberrypi/data/mov9281.json
> > >
> > > diff --git a/src/ipa/raspberrypi/cam_helper_mov9281.cpp b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > new file mode 100644
> > > index 00000000..29519cb0
> > > --- /dev/null
> > > +++ b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > @@ -0,0 +1,65 @@
> > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > +/*
> > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > > + *
> > > + * cam_helper_mov9281.cpp - camera information for ov9281 sensor
> > > + */
> > > +
> > > +#include <assert.h>
> > > +
> > > +#include "cam_helper.hpp"
> > > +
> > > +using namespace RPiController;
> > > +
> > > +class CamHelperOv9281 : public CamHelper
> > > +{
> > > +public:
> > > +     CamHelperOv9281();
> > > +     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;
> > > +
> > > +private:
> > > +     /*
> > > +      * Smallest difference between the frame length and integration time,
> > > +      * in units of lines.
> > > +      */
> > > +     static constexpr int frameIntegrationDiff = 4;
> > > +};
> > > +
> > > +/*
> > > + * OV9281 doesn't output metadata, so we have to use the "unicam parser" which
> > > + * works by counting frames.
> > > + */
> > > +
> > > +CamHelperOv9281::CamHelperOv9281()
> > > +     : CamHelper(nullptr, frameIntegrationDiff)
> > > +{
> > > +}
> > > +
> > > +uint32_t CamHelperOv9281::GainCode(double gain) const
> > > +{
> > > +     return static_cast<uint32_t>(gain * 16.0);
> > > +}
> > > +
> > > +double CamHelperOv9281::Gain(uint32_t gain_code) const
> > > +{
> > > +     return static_cast<double>(gain_code) / 16.0;
> > > +}
> > > +
> > > +void CamHelperOv9281::GetDelays(int &exposure_delay, int &gain_delay,
> > > +                             int &vblank_delay) const
> > > +{
> > > +     /* The driver appears to behave as follows: */
> > > +     exposure_delay = 2;
> > > +     gain_delay = 2;
> > > +     vblank_delay = 2;
> > > +}
> > > +
> > > +static CamHelper *Create()
> > > +{
> > > +     return new CamHelperOv9281();
> > > +}
> > > +
> > > +static RegisterCamHelper reg("mov9281", &Create);
> > > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > > index 92ad3272..f8baab6d 100644
> > > --- a/src/ipa/raspberrypi/data/meson.build
> > > +++ b/src/ipa/raspberrypi/data/meson.build
> > > @@ -4,6 +4,7 @@ conf_files = files([
> > >      'imx219.json',
> > >      'imx290.json',
> > >      'imx477.json',
> > > +    'mov9281.json',
> > >      'ov5647.json',
> > >      'se327m12.json',
> > >      'uncalibrated.json',
> > > diff --git a/src/ipa/raspberrypi/data/mov9281.json b/src/ipa/raspberrypi/data/mov9281.json
> > > new file mode 100644
> > > index 00000000..ecd262be
> > > --- /dev/null
> > > +++ b/src/ipa/raspberrypi/data/mov9281.json
> > > @@ -0,0 +1,92 @@
> > > +{
> > > +    "rpi.black_level":
> > > +    {
> > > +        "black_level": 4096
> > > +    },
> > > +    "rpi.lux":
> > > +    {
> > > +        "reference_shutter_speed": 2000,
> > > +        "reference_gain": 1.0,
> > > +        "reference_aperture": 1.0,
> > > +        "reference_lux": 800,
> > > +        "reference_Y": 20000
> > > +    },
> > > +    "rpi.noise":
> > > +    {
> > > +        "reference_constant": 0,
> > > +        "reference_slope": 2.5
> > > +    },
> > > +    "rpi.sdn":
> > > +    {
> > > +    },
> > > +    "rpi.agc":
> > > +    {
> > > +        "metering_modes":
> > > +        {
> > > +            "centre-weighted": {
> > > +                "weights": [4, 4, 4, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0]
> > > +            }
> > > +        },
> > > +        "exposure_modes":
> > > +        {
> > > +            "normal":
> > > +            {
> > > +                "shutter": [ 100, 15000, 30000, 60000, 120000 ],
> > > +                "gain":    [ 1.0, 2.0,   3.0,   4.0,   6.0    ]
> > > +            }
> > > +        },
> > > +        "constraint_modes":
> > > +        {
> > > +            "normal":
> > > +            [
> > > +                { "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target": [ 0, 0.4, 1000, 0.4 ] }
> > > +            ]
> > > +        },
> > > +        "y_target": [ 0, 0.16, 1000, 0.165, 10000, 0.17 ]
> > > +    },
> > > +    "rpi.alsc":
> > > +    {
> > > +        "n_iter": 0,
> > > +        "luminance_strength": 1.0,
> > > +        "corner_strength": 1.5
> > > +    },
> > > +    "rpi.contrast":
> > > +    {
> > > +        "ce_enable": 0,
> > > +        "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
> > > +        ]
> > > +    }
> > > +}
> > > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > > index 230356d3..93f04e9e 100644
> > > --- a/src/ipa/raspberrypi/meson.build
> > > +++ b/src/ipa/raspberrypi/meson.build
> > > @@ -22,6 +22,7 @@ rpi_ipa_sources = files([
> > >      'cam_helper_imx219.cpp',
> > >      'cam_helper_imx290.cpp',
> > >      'cam_helper_imx477.cpp',
> > > +    'cam_helper_mov9281.cpp',
> > >      'controller/controller.cpp',
> > >      'controller/histogram.cpp',
> > >      'controller/algorithm.cpp',
Dave Stevenson June 28, 2021, 9:15 a.m. UTC | #5 
On Mon, 28 Jun 2021 at 08:02, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi David,
>
> On Mon, Jun 28, 2021 at 07:47:39AM +0100, David Plowman wrote:
> > On Mon, 28 Jun 2021 at 07:17, Laurent Pinchart wrote:
> > > On Fri, Jun 18, 2021 at 11:04:30PM +0100, David Plowman wrote:
> > > > The necessary tuning file and CamHelper is added for the ov9281 sensor
> > > > (which the driver names as the "mov9281").
> > >
> > > This bothers me a little bit, as it will likely need to be renamed to
> > > ov9281 when upstreaming. Why is there an "m" prefix, and could it be
> > > dropped ?
> >
> > Yes, I'm suspicious that the "m" is actually a reference to "mono",
> > thereby opening the way for colour versions of this sensor. In fact it
> > would be helpful for the driver to advertise a different name in such
> > cases so that a more appropriate tuning file can be identified (though
> > as we know, that still isn't really good enough...!) But maybe
> > something a little less cryptic than just "m" would help?
>
> Is there a colour version of the OV9281 ? If not, I'd just drop the
> prefix. If there is, the media entity should be named according to the
> Omnivision naming scheme.

No colour version that I'm aware of.

The m was present in the Rockchip original driver that I based this on
- https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/media/i2c/ov9281.c#L1103
No issues at all in removing it.

  Dave

> > > > The ov9281 is a 1280x800 monochrome global shutter sensor. To enable
> > > > it, please add
> > > >
> > > > dtoverlay=ov9281
> > > >
> > > > to the /boot/config.txt file and reboot the Pi.
> > > >
> > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> > > > ---
> > > >  src/ipa/raspberrypi/cam_helper_mov9281.cpp | 65 +++++++++++++++
> > > >  src/ipa/raspberrypi/data/meson.build       |  1 +
> > > >  src/ipa/raspberrypi/data/mov9281.json      | 92 ++++++++++++++++++++++
> > > >  src/ipa/raspberrypi/meson.build            |  1 +
> > > >  4 files changed, 159 insertions(+)
> > > >  create mode 100644 src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > >  create mode 100644 src/ipa/raspberrypi/data/mov9281.json
> > > >
> > > > diff --git a/src/ipa/raspberrypi/cam_helper_mov9281.cpp b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > > new file mode 100644
> > > > index 00000000..29519cb0
> > > > --- /dev/null
> > > > +++ b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > > @@ -0,0 +1,65 @@
> > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > +/*
> > > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > > > + *
> > > > + * cam_helper_mov9281.cpp - camera information for ov9281 sensor
> > > > + */
> > > > +
> > > > +#include <assert.h>
> > > > +
> > > > +#include "cam_helper.hpp"
> > > > +
> > > > +using namespace RPiController;
> > > > +
> > > > +class CamHelperOv9281 : public CamHelper
> > > > +{
> > > > +public:
> > > > +     CamHelperOv9281();
> > > > +     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;
> > > > +
> > > > +private:
> > > > +     /*
> > > > +      * Smallest difference between the frame length and integration time,
> > > > +      * in units of lines.
> > > > +      */
> > > > +     static constexpr int frameIntegrationDiff = 4;
> > > > +};
> > > > +
> > > > +/*
> > > > + * OV9281 doesn't output metadata, so we have to use the "unicam parser" which
> > > > + * works by counting frames.
> > > > + */
> > > > +
> > > > +CamHelperOv9281::CamHelperOv9281()
> > > > +     : CamHelper(nullptr, frameIntegrationDiff)
> > > > +{
> > > > +}
> > > > +
> > > > +uint32_t CamHelperOv9281::GainCode(double gain) const
> > > > +{
> > > > +     return static_cast<uint32_t>(gain * 16.0);
> > > > +}
> > > > +
> > > > +double CamHelperOv9281::Gain(uint32_t gain_code) const
> > > > +{
> > > > +     return static_cast<double>(gain_code) / 16.0;
> > > > +}
> > > > +
> > > > +void CamHelperOv9281::GetDelays(int &exposure_delay, int &gain_delay,
> > > > +                             int &vblank_delay) const
> > > > +{
> > > > +     /* The driver appears to behave as follows: */
> > > > +     exposure_delay = 2;
> > > > +     gain_delay = 2;
> > > > +     vblank_delay = 2;
> > > > +}
> > > > +
> > > > +static CamHelper *Create()
> > > > +{
> > > > +     return new CamHelperOv9281();
> > > > +}
> > > > +
> > > > +static RegisterCamHelper reg("mov9281", &Create);
> > > > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > > > index 92ad3272..f8baab6d 100644
> > > > --- a/src/ipa/raspberrypi/data/meson.build
> > > > +++ b/src/ipa/raspberrypi/data/meson.build
> > > > @@ -4,6 +4,7 @@ conf_files = files([
> > > >      'imx219.json',
> > > >      'imx290.json',
> > > >      'imx477.json',
> > > > +    'mov9281.json',
> > > >      'ov5647.json',
> > > >      'se327m12.json',
> > > >      'uncalibrated.json',
> > > > diff --git a/src/ipa/raspberrypi/data/mov9281.json b/src/ipa/raspberrypi/data/mov9281.json
> > > > new file mode 100644
> > > > index 00000000..ecd262be
> > > > --- /dev/null
> > > > +++ b/src/ipa/raspberrypi/data/mov9281.json
> > > > @@ -0,0 +1,92 @@
> > > > +{
> > > > +    "rpi.black_level":
> > > > +    {
> > > > +        "black_level": 4096
> > > > +    },
> > > > +    "rpi.lux":
> > > > +    {
> > > > +        "reference_shutter_speed": 2000,
> > > > +        "reference_gain": 1.0,
> > > > +        "reference_aperture": 1.0,
> > > > +        "reference_lux": 800,
> > > > +        "reference_Y": 20000
> > > > +    },
> > > > +    "rpi.noise":
> > > > +    {
> > > > +        "reference_constant": 0,
> > > > +        "reference_slope": 2.5
> > > > +    },
> > > > +    "rpi.sdn":
> > > > +    {
> > > > +    },
> > > > +    "rpi.agc":
> > > > +    {
> > > > +        "metering_modes":
> > > > +        {
> > > > +            "centre-weighted": {
> > > > +                "weights": [4, 4, 4, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0]
> > > > +            }
> > > > +        },
> > > > +        "exposure_modes":
> > > > +        {
> > > > +            "normal":
> > > > +            {
> > > > +                "shutter": [ 100, 15000, 30000, 60000, 120000 ],
> > > > +                "gain":    [ 1.0, 2.0,   3.0,   4.0,   6.0    ]
> > > > +            }
> > > > +        },
> > > > +        "constraint_modes":
> > > > +        {
> > > > +            "normal":
> > > > +            [
> > > > +                { "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target": [ 0, 0.4, 1000, 0.4 ] }
> > > > +            ]
> > > > +        },
> > > > +        "y_target": [ 0, 0.16, 1000, 0.165, 10000, 0.17 ]
> > > > +    },
> > > > +    "rpi.alsc":
> > > > +    {
> > > > +        "n_iter": 0,
> > > > +        "luminance_strength": 1.0,
> > > > +        "corner_strength": 1.5
> > > > +    },
> > > > +    "rpi.contrast":
> > > > +    {
> > > > +        "ce_enable": 0,
> > > > +        "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
> > > > +        ]
> > > > +    }
> > > > +}
> > > > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > > > index 230356d3..93f04e9e 100644
> > > > --- a/src/ipa/raspberrypi/meson.build
> > > > +++ b/src/ipa/raspberrypi/meson.build
> > > > @@ -22,6 +22,7 @@ rpi_ipa_sources = files([
> > > >      'cam_helper_imx219.cpp',
> > > >      'cam_helper_imx290.cpp',
> > > >      'cam_helper_imx477.cpp',
> > > > +    'cam_helper_mov9281.cpp',
> > > >      'controller/controller.cpp',
> > > >      'controller/histogram.cpp',
> > > >      'controller/algorithm.cpp',
>
> --
> Regards,
>
> Laurent Pinchart
David Plowman June 28, 2021, 9:20 a.m. UTC | #6 
Thanks, Dave.

I'll prepare another version that removes the offending "m"!

David

On Mon, 28 Jun 2021 at 10:16, Dave Stevenson <dave.stevenson@raspberrypi.com>
wrote:

> On Mon, 28 Jun 2021 at 08:02, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi David,
> >
> > On Mon, Jun 28, 2021 at 07:47:39AM +0100, David Plowman wrote:
> > > On Mon, 28 Jun 2021 at 07:17, Laurent Pinchart wrote:
> > > > On Fri, Jun 18, 2021 at 11:04:30PM +0100, David Plowman wrote:
> > > > > The necessary tuning file and CamHelper is added for the ov9281
> sensor
> > > > > (which the driver names as the "mov9281").
> > > >
> > > > This bothers me a little bit, as it will likely need to be renamed to
> > > > ov9281 when upstreaming. Why is there an "m" prefix, and could it be
> > > > dropped ?
> > >
> > > Yes, I'm suspicious that the "m" is actually a reference to "mono",
> > > thereby opening the way for colour versions of this sensor. In fact it
> > > would be helpful for the driver to advertise a different name in such
> > > cases so that a more appropriate tuning file can be identified (though
> > > as we know, that still isn't really good enough...!) But maybe
> > > something a little less cryptic than just "m" would help?
> >
> > Is there a colour version of the OV9281 ? If not, I'd just drop the
> > prefix. If there is, the media entity should be named according to the
> > Omnivision naming scheme.
>
> No colour version that I'm aware of.
>
> The m was present in the Rockchip original driver that I based this on
> -
> https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/media/i2c/ov9281.c#L1103
> No issues at all in removing it.
>
>   Dave
>
> > > > > The ov9281 is a 1280x800 monochrome global shutter sensor. To
> enable
> > > > > it, please add
> > > > >
> > > > > dtoverlay=ov9281
> > > > >
> > > > > to the /boot/config.txt file and reboot the Pi.
> > > > >
> > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> > > > > ---
> > > > >  src/ipa/raspberrypi/cam_helper_mov9281.cpp | 65 +++++++++++++++
> > > > >  src/ipa/raspberrypi/data/meson.build       |  1 +
> > > > >  src/ipa/raspberrypi/data/mov9281.json      | 92
> ++++++++++++++++++++++
> > > > >  src/ipa/raspberrypi/meson.build            |  1 +
> > > > >  4 files changed, 159 insertions(+)
> > > > >  create mode 100644 src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > > >  create mode 100644 src/ipa/raspberrypi/data/mov9281.json
> > > > >
> > > > > diff --git a/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > > > new file mode 100644
> > > > > index 00000000..29519cb0
> > > > > --- /dev/null
> > > > > +++ b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
> > > > > @@ -0,0 +1,65 @@
> > > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > > +/*
> > > > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> > > > > + *
> > > > > + * cam_helper_mov9281.cpp - camera information for ov9281 sensor
> > > > > + */
> > > > > +
> > > > > +#include <assert.h>
> > > > > +
> > > > > +#include "cam_helper.hpp"
> > > > > +
> > > > > +using namespace RPiController;
> > > > > +
> > > > > +class CamHelperOv9281 : public CamHelper
> > > > > +{
> > > > > +public:
> > > > > +     CamHelperOv9281();
> > > > > +     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;
> > > > > +
> > > > > +private:
> > > > > +     /*
> > > > > +      * Smallest difference between the frame length and
> integration time,
> > > > > +      * in units of lines.
> > > > > +      */
> > > > > +     static constexpr int frameIntegrationDiff = 4;
> > > > > +};
> > > > > +
> > > > > +/*
> > > > > + * OV9281 doesn't output metadata, so we have to use the "unicam
> parser" which
> > > > > + * works by counting frames.
> > > > > + */
> > > > > +
> > > > > +CamHelperOv9281::CamHelperOv9281()
> > > > > +     : CamHelper(nullptr, frameIntegrationDiff)
> > > > > +{
> > > > > +}
> > > > > +
> > > > > +uint32_t CamHelperOv9281::GainCode(double gain) const
> > > > > +{
> > > > > +     return static_cast<uint32_t>(gain * 16.0);
> > > > > +}
> > > > > +
> > > > > +double CamHelperOv9281::Gain(uint32_t gain_code) const
> > > > > +{
> > > > > +     return static_cast<double>(gain_code) / 16.0;
> > > > > +}
> > > > > +
> > > > > +void CamHelperOv9281::GetDelays(int &exposure_delay, int
> &gain_delay,
> > > > > +                             int &vblank_delay) const
> > > > > +{
> > > > > +     /* The driver appears to behave as follows: */
> > > > > +     exposure_delay = 2;
> > > > > +     gain_delay = 2;
> > > > > +     vblank_delay = 2;
> > > > > +}
> > > > > +
> > > > > +static CamHelper *Create()
> > > > > +{
> > > > > +     return new CamHelperOv9281();
> > > > > +}
> > > > > +
> > > > > +static RegisterCamHelper reg("mov9281", &Create);
> > > > > diff --git a/src/ipa/raspberrypi/data/meson.build
> b/src/ipa/raspberrypi/data/meson.build
> > > > > index 92ad3272..f8baab6d 100644
> > > > > --- a/src/ipa/raspberrypi/data/meson.build
> > > > > +++ b/src/ipa/raspberrypi/data/meson.build
> > > > > @@ -4,6 +4,7 @@ conf_files = files([
> > > > >      'imx219.json',
> > > > >      'imx290.json',
> > > > >      'imx477.json',
> > > > > +    'mov9281.json',
> > > > >      'ov5647.json',
> > > > >      'se327m12.json',
> > > > >      'uncalibrated.json',
> > > > > diff --git a/src/ipa/raspberrypi/data/mov9281.json
> b/src/ipa/raspberrypi/data/mov9281.json
> > > > > new file mode 100644
> > > > > index 00000000..ecd262be
> > > > > --- /dev/null
> > > > > +++ b/src/ipa/raspberrypi/data/mov9281.json
> > > > > @@ -0,0 +1,92 @@
> > > > > +{
> > > > > +    "rpi.black_level":
> > > > > +    {
> > > > > +        "black_level": 4096
> > > > > +    },
> > > > > +    "rpi.lux":
> > > > > +    {
> > > > > +        "reference_shutter_speed": 2000,
> > > > > +        "reference_gain": 1.0,
> > > > > +        "reference_aperture": 1.0,
> > > > > +        "reference_lux": 800,
> > > > > +        "reference_Y": 20000
> > > > > +    },
> > > > > +    "rpi.noise":
> > > > > +    {
> > > > > +        "reference_constant": 0,
> > > > > +        "reference_slope": 2.5
> > > > > +    },
> > > > > +    "rpi.sdn":
> > > > > +    {
> > > > > +    },
> > > > > +    "rpi.agc":
> > > > > +    {
> > > > > +        "metering_modes":
> > > > > +        {
> > > > > +            "centre-weighted": {
> > > > > +                "weights": [4, 4, 4, 2, 2, 2, 2, 1, 1, 1, 1, 0,
> 0, 0, 0]
> > > > > +            }
> > > > > +        },
> > > > > +        "exposure_modes":
> > > > > +        {
> > > > > +            "normal":
> > > > > +            {
> > > > > +                "shutter": [ 100, 15000, 30000, 60000, 120000 ],
> > > > > +                "gain":    [ 1.0, 2.0,   3.0,   4.0,   6.0    ]
> > > > > +            }
> > > > > +        },
> > > > > +        "constraint_modes":
> > > > > +        {
> > > > > +            "normal":
> > > > > +            [
> > > > > +                { "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0,
> "y_target": [ 0, 0.4, 1000, 0.4 ] }
> > > > > +            ]
> > > > > +        },
> > > > > +        "y_target": [ 0, 0.16, 1000, 0.165, 10000, 0.17 ]
> > > > > +    },
> > > > > +    "rpi.alsc":
> > > > > +    {
> > > > > +        "n_iter": 0,
> > > > > +        "luminance_strength": 1.0,
> > > > > +        "corner_strength": 1.5
> > > > > +    },
> > > > > +    "rpi.contrast":
> > > > > +    {
> > > > > +        "ce_enable": 0,
> > > > > +        "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
> > > > > +        ]
> > > > > +    }
> > > > > +}
> > > > > diff --git a/src/ipa/raspberrypi/meson.build
> b/src/ipa/raspberrypi/meson.build
> > > > > index 230356d3..93f04e9e 100644
> > > > > --- a/src/ipa/raspberrypi/meson.build
> > > > > +++ b/src/ipa/raspberrypi/meson.build
> > > > > @@ -22,6 +22,7 @@ rpi_ipa_sources = files([
> > > > >      'cam_helper_imx219.cpp',
> > > > >      'cam_helper_imx290.cpp',
> > > > >      'cam_helper_imx477.cpp',
> > > > > +    'cam_helper_mov9281.cpp',
> > > > >      'controller/controller.cpp',
> > > > >      'controller/histogram.cpp',
> > > > >      'controller/algorithm.cpp',
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/cam_helper_mov9281.cpp b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
new file mode 100644
index 00000000..29519cb0
--- /dev/null
+++ b/src/ipa/raspberrypi/cam_helper_mov9281.cpp
@@ -0,0 +1,65 @@ 
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (C) 2021, Raspberry Pi (Trading) Limited
+ *
+ * cam_helper_mov9281.cpp - camera information for ov9281 sensor
+ */
+
+#include <assert.h>
+
+#include "cam_helper.hpp"
+
+using namespace RPiController;
+
+class CamHelperOv9281 : public CamHelper
+{
+public:
+	CamHelperOv9281();
+	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;
+
+private:
+	/*
+	 * Smallest difference between the frame length and integration time,
+	 * in units of lines.
+	 */
+	static constexpr int frameIntegrationDiff = 4;
+};
+
+/*
+ * OV9281 doesn't output metadata, so we have to use the "unicam parser" which
+ * works by counting frames.
+ */
+
+CamHelperOv9281::CamHelperOv9281()
+	: CamHelper(nullptr, frameIntegrationDiff)
+{
+}
+
+uint32_t CamHelperOv9281::GainCode(double gain) const
+{
+	return static_cast<uint32_t>(gain * 16.0);
+}
+
+double CamHelperOv9281::Gain(uint32_t gain_code) const
+{
+	return static_cast<double>(gain_code) / 16.0;
+}
+
+void CamHelperOv9281::GetDelays(int &exposure_delay, int &gain_delay,
+				int &vblank_delay) const
+{
+	/* The driver appears to behave as follows: */
+	exposure_delay = 2;
+	gain_delay = 2;
+	vblank_delay = 2;
+}
+
+static CamHelper *Create()
+{
+	return new CamHelperOv9281();
+}
+
+static RegisterCamHelper reg("mov9281", &Create);
diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
index 92ad3272..f8baab6d 100644
--- a/src/ipa/raspberrypi/data/meson.build
+++ b/src/ipa/raspberrypi/data/meson.build
@@ -4,6 +4,7 @@  conf_files = files([
     'imx219.json',
     'imx290.json',
     'imx477.json',
+    'mov9281.json',
     'ov5647.json',
     'se327m12.json',
     'uncalibrated.json',
diff --git a/src/ipa/raspberrypi/data/mov9281.json b/src/ipa/raspberrypi/data/mov9281.json
new file mode 100644
index 00000000..ecd262be
--- /dev/null
+++ b/src/ipa/raspberrypi/data/mov9281.json
@@ -0,0 +1,92 @@ 
+{
+    "rpi.black_level":
+    {
+        "black_level": 4096
+    },
+    "rpi.lux":
+    {
+        "reference_shutter_speed": 2000,
+        "reference_gain": 1.0,
+        "reference_aperture": 1.0,
+        "reference_lux": 800,
+        "reference_Y": 20000
+    },
+    "rpi.noise":
+    {
+        "reference_constant": 0,
+        "reference_slope": 2.5
+    },
+    "rpi.sdn":
+    {
+    },
+    "rpi.agc":
+    {
+        "metering_modes":
+        {
+            "centre-weighted": {
+                "weights": [4, 4, 4, 2, 2, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0]
+            }
+        },
+        "exposure_modes":
+        {
+            "normal":
+            {
+                "shutter": [ 100, 15000, 30000, 60000, 120000 ],
+                "gain":    [ 1.0, 2.0,   3.0,   4.0,   6.0    ]
+            }
+        },
+        "constraint_modes":
+        {
+            "normal":
+            [
+                { "bound": "LOWER", "q_lo": 0.98, "q_hi": 1.0, "y_target": [ 0, 0.4, 1000, 0.4 ] }
+            ]
+        },
+        "y_target": [ 0, 0.16, 1000, 0.165, 10000, 0.17 ]
+    },
+    "rpi.alsc":
+    {
+        "n_iter": 0,
+        "luminance_strength": 1.0,
+        "corner_strength": 1.5
+    },
+    "rpi.contrast":
+    {
+        "ce_enable": 0,
+        "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
+        ]
+    }
+}
diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
index 230356d3..93f04e9e 100644
--- a/src/ipa/raspberrypi/meson.build
+++ b/src/ipa/raspberrypi/meson.build
@@ -22,6 +22,7 @@  rpi_ipa_sources = files([
     'cam_helper_imx219.cpp',
     'cam_helper_imx290.cpp',
     'cam_helper_imx477.cpp',
+    'cam_helper_mov9281.cpp',
     'controller/controller.cpp',
     'controller/histogram.cpp',
     'controller/algorithm.cpp',