[libcamera-devel] [PATCH] libipa: Add CameraSensorHelper for OV13858

[Dave Stevenson]

Hi Laurent

On Wed, 30 Jun 2021 at 14:05, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Dave,
>
> On Wed, Jun 30, 2021 at 01:57:12PM +0100, Dave Stevenson wrote:
> > On Wed, 30 Jun 2021 at 12:26, Jean-Michel Hautbois wrote:
> > > On 30/06/2021 13:21, Kieran Bingham wrote:
> > > > Extend the CameraSensorHelper factory with support for an
> > > > OV13858 sensor as found in the Soraka Chromebook.
> > > >
> > > > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > > ---
> > > >  src/ipa/libipa/camera_sensor_helper.cpp | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > index 23335ed66b02..8e95b8b10102 100644
> > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > @@ -313,6 +313,20 @@ public:
> > > >  };
> > > >  REGISTER_CAMERA_SENSOR_HELPER("ov5693", CameraSensorHelperOv5693)
> > > >
> > > > +/**
> > > > + * \class CameraSensorHelperOv13858
> > > > + * \brief Create and give helpers for the ov13858 sensor
> > > > + */
> > >
> > > As proposed by Laurent, I have a patch which (among other things)
> > > removes the Doxygen comments for the sub-classes as it is not really
> > > needed: https://patchwork.libcamera.org/patch/12736/
> > >
> > > > +class CameraSensorHelperOv13858 : public CameraSensorHelper
> > > > +{
> > > > +public:
> > > > +     CameraSensorHelperOv13858()
> > > > +     {
> > > > +             analogueGainConstants_ = { AnalogueGainLinear, 1, 0, 0, 256 };
> > >
> > > This is based on minimum, maximum and step value, as you don't have the
> > > datasheet...
> >
> > OV13850 appears to be a very similar 10bit 4224x3136 sensor, and there
> > is a datasheet online at
> > http://download.t-firefly.com/product/RK3288/Docs/Peripherals/OV13850%20datasheet/Sensor_OV13850-G04A_OmniVision_SpecificationV1.pdf
> >
> > It matches the driver with analogue gain being registers 0x3508/9, and
>
> Does it ? The OV13850 datasheet lists register 0x3509 as a control
> register, and the manual gain is set in 0x3504/0x3505 or 0x350a/0x350b.
> It's probably similar though.

Doh, that's me going cross-eyed! I'd misread exposure for gain in
about 3 places.
Sorry for the noise.

  Dave

> > confirms that the "Low 4 bits are fraction bits which are not
> > supported and should always be 0".
> > It's making some assumptions, but seems a reasonable match.
> >
> > > That's something we should be thinking about: how to be sure of the
> > > values when we don't have the datasheet.
> > > A tuning code which would set the gains from min to max and compare the
> > > raw pixel values from one gain to another to deduce a linear (or not)
> > > relationship ?
> > >
> > > > +     }
> > > > +};
> > > > +REGISTER_CAMERA_SENSOR_HELPER("ov13858", CameraSensorHelperOv13858)
> > > > +
> > > >  } /* namespace ipa */
> > > >
> > > >  } /* namespace libcamera */
> > > >
>
> --
> Regards,
>
> Laurent Pinchart