[PATCH v3 1/2] libcamera: libipa: camera_sensor: Add MediaTek gc05a2 sensor properties

[Cheng-Hao Yang]

Hi Kieran,

On Thu, Nov 21, 2024 at 12:15 AM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Cheng-Hao Yang (2024-11-20 15:56:31)
> > Hi Kieran,
> >
> > On Wed, Nov 20, 2024 at 6:18 PM Kieran Bingham
> > <kieran.bingham at ideasonboard.com> wrote:
> > >
> > > Quoting Cheng-Hao Yang (2024-11-20 07:39:16)
> > > > Hi Laurent,
> > > >
> > > > On Tue, Nov 19, 2024 at 3:56 PM Laurent Pinchart
> > > > <laurent.pinchart at ideasonboard.com> wrote:
> > > > >
> > > > > Hi Harvey,
> > > > >
> > > > > Thank you for the patch.
> > > > >
> > > > > On Tue, Nov 19, 2024 at 07:37:35AM +0000, Harvey Yang wrote:
> > > > > > From: Harvey Yang <chenghaoyang at google.com>
> > > > > >
> > > > > > Provide the MediaTek gc05a2 camera sensor properties and registration
> > > > >
> > > > > I think you meant GalaxyCore, not MediaTek, both in the subject line and
> > > > > here.
> > > >
> > > > Yes, thanks for the catch.
> > > >
> > > > >
> > > > > > with libipa for the gain code helpers.
> > > > > >
> > > > > > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > > > > > Co-developed-by: Xing Gu <xinggu at chromium.org>
> > > > > > Co-developed-by: Yudhistira Erlandinata <yerlandinata at chromium.org>
> > > > > > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > > > > > ---
> > > > > >  src/ipa/libipa/camera_sensor_helper.cpp           | 11 +++++++++++
> > > > > >  src/libcamera/sensor/camera_sensor_properties.cpp |  7 +++++++
> > > > > >  2 files changed, 18 insertions(+)
> > > > > >
> > > > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > > > index c6169bdc7..36555680b 100644
> > > > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > > > @@ -519,6 +519,17 @@ private:
> > > > > >  };
> > > > > >  REGISTER_CAMERA_SENSOR_HELPER("ar0521", CameraSensorHelperAr0521)
> > > > > >
> > > > > > +class CameraSensorHelperGc05a2 : public CameraSensorHelper
> > > > > > +{
> > > > > > +public:
> > > > > > +     CameraSensorHelperGc05a2()
> > > > > > +     {
> > > > > > +             gainType_ = AnalogueGainLinear;
> > > > > > +             gainConstants_.linear = { 100, 0, 0, 1024 };
> > > > >
> > > > > We also need to know the black level (a.k.a. data pedestal).
> > > > >
> > > > > The rest of the patch looks good to me. With the black level value
> > > > > added, I expect this can be merged.
> > > > >
> > > > > Same comment for 2/2.
> > > >
> > > > Confirmed with MediaTek that both sensors have a black level 0x40.
> > > > Updated in the next version.
> > >
> > > At what bit depth? We store the black level as a 16 bit value in
> > > libcamera so it needs to be bit-shifted accordingly.
> >
> > According to the docs that they provided to me, the field only takes
> > 8 bits. 0x40 must mean 64 in decimal, IIUC.
>
> Taking a guess from
> https://en.gcoreinc.com/products/index?cid=2&subcid=5 Does the sensor
> output RAW10 ?
>
> If so - then this is 0x40 at 10 bits. And therefore storing it in
> libcamera we would represent this as a 16 bit value and shift
> accordingly.
>
> Therefore it would be:
>
>                 /* From datasheet: 64 at 10bits. */
>                 blackLevel_ = 4096;
>
> or
>
>                 /* From datasheet: 64 at 10bits. */
>                 blackLevel_ = 64 << 6;

Ah got it. It was RAW10, while libcamera uses RAW16, so
we need to shift 6 bits
Thanks, will be updated in v5.

BR,
Harvey

>
> --
> Kieran
>
>
>
>
> >
> > BR,
> > Harvey
> >
> > >
> > > --
> > > Kieran
> > >
> > > >
> > > > BR,
> > > > Harvey
> > > >
> > > > >
> > > > > > +     }
> > > > > > +};
> > > > > > +REGISTER_CAMERA_SENSOR_HELPER("gc05a2", CameraSensorHelperGc05a2)
> > > > > > +
> > > > > >  class CameraSensorHelperImx214 : public CameraSensorHelper
> > > > > >  {
> > > > > >  public:
> > > > > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > > > > > index 6d4136d03..2f048d4a3 100644
> > > > > > --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> > > > > > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> > > > > > @@ -70,6 +70,13 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > > > > >                               { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
> > > > > >                       },
> > > > > >               } },
> > > > > > +             { "gc05a2", {
> > > > > > +                     .unitCellSize = { 1120, 1120 },
> > > > > > +                     .testPatternModes = {
> > > > > > +                             { controls::draft::TestPatternModeOff, 0 },
> > > > > > +                             { controls::draft::TestPatternModeColorBars, 1 },
> > > > > > +                     },
> > > > > > +             } },
> > > > > >               { "hi846", {
> > > > > >                       .unitCellSize = { 1120, 1120 },
> > > > > >                       .testPatternModes = {
> > > > >
> > > > > --
> > > > > Regards,
> > > > >
> > > > > Laurent Pinchart