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

[Laurent Pinchart]

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.

> 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