[libcamera-devel] [PATCH v3 3/3] libcamera: ipa: raspberrypi: Add support for ov9281 sensor
Dave Stevenson
dave.stevenson at raspberrypi.com
Mon Jun 28 11:15:48 CEST 2021
On Mon, 28 Jun 2021 at 08:02, Laurent Pinchart
<laurent.pinchart at 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 at raspberrypi.com>
> > > > Reviewed-by: Naushir Patuck <naush at 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
More information about the libcamera-devel
mailing list