[PATCH 1/1] libcamera: Add camera sensor properties for ciri

[Cheng-Hao Yang]

Thanks Laurent for the quick review,

On Mon, Sep 23, 2024 at 4:48 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Harvey and Han-Lin,
>
> Thank you for the patch.
>
> On Mon, Sep 23, 2024 at 07:09:56AM +0000, Harvey Yang wrote:
> > From: Han-Lin Chen <hanlinchen at chromium.org>
> >
> > ciri has sensors: hi1339, gc08a3, and gc05a2.
> >
> > 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>
> > ---
> >  .../sensor/camera_sensor_properties.cpp       | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > index 4e5217ab..a224f8d2 100644
> > --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> > @@ -276,6 +276,27 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> >                               { controls::draft::TestPatternModeColorBars, 1 },
> >                       },
> >               } },
> > +             { "hi1339", {
> > +                     .unitCellSize = { 1120, 1120 },
> > +                     .testPatternModes = {
> > +                             { controls::draft::TestPatternModeOff, 0 },
> > +                             { controls::draft::TestPatternModeColorBars, 2 },
> > +                     },
> > +             } },
>
> Unless I'm mistaken, the driver for this sensor hasn't been posted to
> the linux-media mailing list. The policy in libcamera is that drivers
> need to be on their way to upstream. You could split this patch in two
> to merge support for the gc08a3 and gc05a2 already, and address the
> hi1339 when the driver gets posted.

Yeah right. hi1339 is the old sensor used on an old ciri model in the
development process. Therefore, I don't think we're using it in
production and MTK may just skip upstreaming it.

I'll drop this in the next version.


>
> > +             { "gc08a3", {
> > +                     .unitCellSize = { 1120, 1120 },
> > +                     .testPatternModes = {
> > +                             { controls::draft::TestPatternModeOff, 0 },
> > +                             { controls::draft::TestPatternModeColorBars, 2 },
> > +                     },
> > +             } },
>
> Sensor support also requires adding a sensor helper in
> src/ipa/libipa/camera_sensor_helper.cpp.

Ah got it, while I'm not sure if we want to use it in
mtkisp7's ipa, as the algorithm returns an integer
as the analogue gain [1], instead of double.
The algo also doesn't need the gain fed back. It
keeps the recent values searchable by request
ids.

[1]: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/ipa/mtkisp7/hal3a/hal_3a.cpp;l=946

>
> > +             { "gc05a2", {
> > +                     .unitCellSize = { 1120, 1120 },
>
> Do those three sensors really have a pixel size of 1.12µm, or was that
> by any chance copied from the previous entry in the table ?

It's 1.12um for "gc05a2" [1] and "gc08a3" [2] at least.

[1]: https://en.gcoreinc.com/products/index?cid=2&subcid=5
[2]: https://en.gcoreinc.com/products/index?cid=2&subcid=4

>
> > +                     .testPatternModes = {
> > +                             { controls::draft::TestPatternModeOff, 0 },
> > +                             { controls::draft::TestPatternModeColorBars, 1 },
> > +                     },
> > +             } },
>
> Please sort entries alphabetically.

Will do.

BR,
Harvey


>
> >       };
> >
> >       const auto it = sensorProps.find(sensor);
>
> --
> Regards,
>
> Laurent Pinchart