[v2,1/2] libcamera: Add gc05a2 camera sensor proprietary for ciri
diff mbox series

Message ID 20240924090051.1617040-2-chenghaoyang@google.com
State Superseded
Headers show
Series
  • Add camera sensor properties for ciri
Related show

Commit Message

Harvey Yang Sept. 24, 2024, 8:26 a.m. UTC
gc05a2 is the first sensor used by ciri.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
Co-developed-by: Xing Gu <xinggu@chromium.org>
Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/ipa/libipa/camera_sensor_helper.cpp         | 17 +++++++++++++++++
 .../sensor/camera_sensor_properties.cpp         |  7 +++++++
 2 files changed, 24 insertions(+)

Comments

Kieran Bingham Sept. 24, 2024, 12:48 p.m. UTC | #1
In $SUBJECT - I think you mean 'properties' not 'proprietary'

But as for the commit message, the subject should say what this patch is
for. libcamera has no knowledge of what a 'ciri' is.

Lets keep it to the details of the image sensor.

Quoting Harvey Yang (2024-09-24 09:26:10)
> gc05a2 is the first sensor used by ciri.

I don't think that's relevant to libcamera.

Please describe what the gc05a2 is instead.

> 
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> Co-developed-by: Xing Gu <xinggu@chromium.org>
> Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp         | 17 +++++++++++++++++
>  .../sensor/camera_sensor_properties.cpp         |  7 +++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index ffc7c1d7..cd7d12d6 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -519,6 +519,23 @@ private:
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("ar0521", CameraSensorHelperAr0521)
>  
> +class CameraSensorHelperGc05a2 : public CameraSensorHelper
> +{
> +public:
> +       uint32_t gainCode(double gain) const override
> +       {
> +               uint32_t aeGain = std::clamp((uint32_t)gain, kAeBaseGain_, kStep_ * kAeBaseGain_);
> +               return aeGain * 0x400 / kAeBaseGain_;
> +       }
> +
> +       // `double gain(uint32_t gainCode)` will not be used.

So don't add it ?

But how do you mean "won't be used" - you mean ... you won't use it? or
it can't be used?

How do we correctly determine the gain *used* from a configured gain
code?

I suspect ... the gain() function should be implemented if the
gainCode() function is implemented. Otherwise callers on the helper will
get inconsistent results.




> +
> +private:
> +       static constexpr uint32_t kStep_ = 16;
> +       static constexpr uint32_t kAeBaseGain_ = 1024;
> +};
> +REGISTER_CAMERA_SENSOR_HELPER("gc05a2", CameraSensorHelperGc05a2)
> +
>  class CameraSensorHelperImx219 : public CameraSensorHelper
>  {
>  public:
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 4e5217ab..3e1bd85e 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 = {
> -- 
> 2.46.0.792.g87dc391469-goog
>
Laurent Pinchart Sept. 24, 2024, 1:19 p.m. UTC | #2
On Tue, Sep 24, 2024 at 01:48:09PM +0100, Kieran Bingham wrote:
> In $SUBJECT - I think you mean 'properties' not 'proprietary'
> 
> But as for the commit message, the subject should say what this patch is
> for. libcamera has no knowledge of what a 'ciri' is.
> 
> Lets keep it to the details of the image sensor.
> 
> Quoting Harvey Yang (2024-09-24 09:26:10)
> > gc05a2 is the first sensor used by ciri.
> 
> I don't think that's relevant to libcamera.
> 
> Please describe what the gc05a2 is instead.
> 
> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > Co-developed-by: Xing Gu <xinggu@chromium.org>
> > Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  src/ipa/libipa/camera_sensor_helper.cpp         | 17 +++++++++++++++++
> >  .../sensor/camera_sensor_properties.cpp         |  7 +++++++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index ffc7c1d7..cd7d12d6 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -519,6 +519,23 @@ private:
> >  };
> >  REGISTER_CAMERA_SENSOR_HELPER("ar0521", CameraSensorHelperAr0521)
> >  
> > +class CameraSensorHelperGc05a2 : public CameraSensorHelper
> > +{
> > +public:
> > +       uint32_t gainCode(double gain) const override
> > +       {
> > +               uint32_t aeGain = std::clamp((uint32_t)gain, kAeBaseGain_, kStep_ * kAeBaseGain_);

This doesn't seem right. 'gain' is expressed as a real number, with 1.0
corresponding to a x1 gain.

> > +               return aeGain * 0x400 / kAeBaseGain_;

If the gain model is linear, you should be able to express it with the
standard gain model, without needing to override gain() and gainCode().

> > +       }
> > +
> > +       // `double gain(uint32_t gainCode)` will not be used.
> 
> So don't add it ?
> 
> But how do you mean "won't be used" - you mean ... you won't use it? or
> it can't be used?
> 
> How do we correctly determine the gain *used* from a configured gain
> code?
> 
> I suspect ... the gain() function should be implemented if the
> gainCode() function is implemented. Otherwise callers on the helper will
> get inconsistent results.

Yes, even if libipa isn't used by the MTK IPA, proper support for the
sensor needs to be implemented. Same for patch 2/2 in the series.

> > +
> > +private:
> > +       static constexpr uint32_t kStep_ = 16;
> > +       static constexpr uint32_t kAeBaseGain_ = 1024;
> > +};
> > +REGISTER_CAMERA_SENSOR_HELPER("gc05a2", CameraSensorHelperGc05a2)
> > +
> >  class CameraSensorHelperImx219 : public CameraSensorHelper
> >  {
> >  public:
> > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > index 4e5217ab..3e1bd85e 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 = {
Harvey Yang Nov. 8, 2024, 6:50 a.m. UTC | #3
Hi Kieran and Laurent,

On Tue, Sep 24, 2024 at 9:19 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Sep 24, 2024 at 01:48:09PM +0100, Kieran Bingham wrote:
> > In $SUBJECT - I think you mean 'properties' not 'proprietary'
> >
> > But as for the commit message, the subject should say what this patch is
> > for. libcamera has no knowledge of what a 'ciri' is.
> >
> > Lets keep it to the details of the image sensor.
> >
> > Quoting Harvey Yang (2024-09-24 09:26:10)
> > > gc05a2 is the first sensor used by ciri.
> >
> > I don't think that's relevant to libcamera.
> >
> > Please describe what the gc05a2 is instead.
> >
> > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > > Co-developed-by: Xing Gu <xinggu@chromium.org>
> > > Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > > ---
> > >  src/ipa/libipa/camera_sensor_helper.cpp         | 17 +++++++++++++++++
> > >  .../sensor/camera_sensor_properties.cpp         |  7 +++++++
> > >  2 files changed, 24 insertions(+)
> > >
> > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > index ffc7c1d7..cd7d12d6 100644
> > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > @@ -519,6 +519,23 @@ private:
> > >  };
> > >  REGISTER_CAMERA_SENSOR_HELPER("ar0521", CameraSensorHelperAr0521)
> > >
> > > +class CameraSensorHelperGc05a2 : public CameraSensorHelper
> > > +{
> > > +public:
> > > +       uint32_t gainCode(double gain) const override
> > > +       {
> > > +               uint32_t aeGain = std::clamp((uint32_t)gain, kAeBaseGain_, kStep_ * kAeBaseGain_);
>
> This doesn't seem right. 'gain' is expressed as a real number, with 1.0
> corresponding to a x1 gain.
>
> > > +               return aeGain * 0x400 / kAeBaseGain_;
>
> If the gain model is linear, you should be able to express it with the
> standard gain model, without needing to override gain() and gainCode().
>
> > > +       }
> > > +
> > > +       // `double gain(uint32_t gainCode)` will not be used.
> >
> > So don't add it ?
> >
> > But how do you mean "won't be used" - you mean ... you won't use it? or
> > it can't be used?
> >
> > How do we correctly determine the gain *used* from a configured gain
> > code?

IIUC, If a configured ae gain is set, IIUC for per-frame control use cases,
in mtkisp7, we feed this information to the IPA proprietary library, which
calculates the corresponding gainCode for v4l2 camera sensor.

We don't have the use case to calculate the ae gain from a configured gain
code. I've seen other pipeline handlers using this function to calculate the
static metadata `controls::AnalogueGain` though. Is this the only use case?

> >
> > I suspect ... the gain() function should be implemented if the
> > gainCode() function is implemented. Otherwise callers on the helper will
> > get inconsistent results.
>
> Yes, even if libipa isn't used by the MTK IPA, proper support for the
> sensor needs to be implemented. Same for patch 2/2 in the series.

I've been contacting MTK's team, while they're quite inconsistent with
themselves and very slow to response...

Previously the conversion is done in their proprietary algorithms, and
we directly use the results in the IPA.

Therefore, we might not end up using these functions even if we finally
dig out MTK's algorithm.
For per-frame control use cases though, it might simplify the task flows,
as we probably could calculate the gain code ourselves without waiting
for IPA proprietary algorithm.

Let's wait for MTK's response for now...

BR,
Harvey

>
> > > +
> > > +private:
> > > +       static constexpr uint32_t kStep_ = 16;
> > > +       static constexpr uint32_t kAeBaseGain_ = 1024;
> > > +};
> > > +REGISTER_CAMERA_SENSOR_HELPER("gc05a2", CameraSensorHelperGc05a2)
> > > +
> > >  class CameraSensorHelperImx219 : public CameraSensorHelper
> > >  {
> > >  public:
> > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > > index 4e5217ab..3e1bd85e 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
Laurent Pinchart Nov. 8, 2024, 10:34 a.m. UTC | #4
Hi Harbey,

On Fri, Nov 08, 2024 at 02:50:33PM +0800, Cheng-Hao Yang wrote:
> On Tue, Sep 24, 2024 at 9:19 PM Laurent Pinchart wrote:
> > On Tue, Sep 24, 2024 at 01:48:09PM +0100, Kieran Bingham wrote:
> > > In $SUBJECT - I think you mean 'properties' not 'proprietary'
> > >
> > > But as for the commit message, the subject should say what this patch is
> > > for. libcamera has no knowledge of what a 'ciri' is.
> > >
> > > Lets keep it to the details of the image sensor.
> > >
> > > Quoting Harvey Yang (2024-09-24 09:26:10)
> > > > gc05a2 is the first sensor used by ciri.
> > >
> > > I don't think that's relevant to libcamera.
> > >
> > > Please describe what the gc05a2 is instead.
> > >
> > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > > > Co-developed-by: Xing Gu <xinggu@chromium.org>
> > > > Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > > > ---
> > > >  src/ipa/libipa/camera_sensor_helper.cpp         | 17 +++++++++++++++++
> > > >  .../sensor/camera_sensor_properties.cpp         |  7 +++++++
> > > >  2 files changed, 24 insertions(+)
> > > >
> > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > index ffc7c1d7..cd7d12d6 100644
> > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > @@ -519,6 +519,23 @@ private:
> > > >  };
> > > >  REGISTER_CAMERA_SENSOR_HELPER("ar0521", CameraSensorHelperAr0521)
> > > >
> > > > +class CameraSensorHelperGc05a2 : public CameraSensorHelper
> > > > +{
> > > > +public:
> > > > +       uint32_t gainCode(double gain) const override
> > > > +       {
> > > > +               uint32_t aeGain = std::clamp((uint32_t)gain, kAeBaseGain_, kStep_ * kAeBaseGain_);
> >
> > This doesn't seem right. 'gain' is expressed as a real number, with 1.0
> > corresponding to a x1 gain.
> >
> > > > +               return aeGain * 0x400 / kAeBaseGain_;
> >
> > If the gain model is linear, you should be able to express it with the
> > standard gain model, without needing to override gain() and gainCode().
> >
> > > > +       }
> > > > +
> > > > +       // `double gain(uint32_t gainCode)` will not be used.
> > >
> > > So don't add it ?
> > >
> > > But how do you mean "won't be used" - you mean ... you won't use it? or
> > > it can't be used?
> > >
> > > How do we correctly determine the gain *used* from a configured gain
> > > code?
> 
> IIUC, If a configured ae gain is set, IIUC for per-frame control use cases,
> in mtkisp7, we feed this information to the IPA proprietary library, which
> calculates the corresponding gainCode for v4l2 camera sensor.
> 
> We don't have the use case to calculate the ae gain from a configured gain
> code. I've seen other pipeline handlers using this function to calculate the
> static metadata `controls::AnalogueGain` though. Is this the only use case?

It's used in open-source IPA modules. To merge support for a new sensor
or a new platform, a fully open source implementation is required. There
will need to be an open-source mtkisp7 IPA module that implements AGC,
so you will need this.

> > > I suspect ... the gain() function should be implemented if the
> > > gainCode() function is implemented. Otherwise callers on the helper will
> > > get inconsistent results.
> >
> > Yes, even if libipa isn't used by the MTK IPA, proper support for the
> > sensor needs to be implemented. Same for patch 2/2 in the series.
> 
> I've been contacting MTK's team, while they're quite inconsistent with
> themselves and very slow to response...
> 
> Previously the conversion is done in their proprietary algorithms, and
> we directly use the results in the IPA.
> 
> Therefore, we might not end up using these functions even if we finally
> dig out MTK's algorithm.
> For per-frame control use cases though, it might simplify the task flows,
> as we probably could calculate the gain code ourselves without waiting
> for IPA proprietary algorithm.
> 
> Let's wait for MTK's response for now...
> 
> > > > +
> > > > +private:
> > > > +       static constexpr uint32_t kStep_ = 16;
> > > > +       static constexpr uint32_t kAeBaseGain_ = 1024;
> > > > +};
> > > > +REGISTER_CAMERA_SENSOR_HELPER("gc05a2", CameraSensorHelperGc05a2)
> > > > +
> > > >  class CameraSensorHelperImx219 : public CameraSensorHelper
> > > >  {
> > > >  public:
> > > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > > > index 4e5217ab..3e1bd85e 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 = {
Harvey Yang Nov. 19, 2024, 7:41 a.m. UTC | #5
Hi Kieran and Laurent,

On Fri, Nov 8, 2024 at 6:34 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Harbey,
>
> On Fri, Nov 08, 2024 at 02:50:33PM +0800, Cheng-Hao Yang wrote:
> > On Tue, Sep 24, 2024 at 9:19 PM Laurent Pinchart wrote:
> > > On Tue, Sep 24, 2024 at 01:48:09PM +0100, Kieran Bingham wrote:
> > > > In $SUBJECT - I think you mean 'properties' not 'proprietary'
> > > >
> > > > But as for the commit message, the subject should say what this patch is
> > > > for. libcamera has no knowledge of what a 'ciri' is.

Updated. Please check again.

> > > >
> > > > Lets keep it to the details of the image sensor.
> > > >
> > > > Quoting Harvey Yang (2024-09-24 09:26:10)
> > > > > gc05a2 is the first sensor used by ciri.
> > > >
> > > > I don't think that's relevant to libcamera.
> > > >
> > > > Please describe what the gc05a2 is instead.

Updated. Please check again.

> > > >
> > > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > > > > Co-developed-by: Xing Gu <xinggu@chromium.org>
> > > > > Co-developed-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > > > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > > > > ---
> > > > >  src/ipa/libipa/camera_sensor_helper.cpp         | 17 +++++++++++++++++
> > > > >  .../sensor/camera_sensor_properties.cpp         |  7 +++++++
> > > > >  2 files changed, 24 insertions(+)
> > > > >
> > > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > > index ffc7c1d7..cd7d12d6 100644
> > > > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > > > @@ -519,6 +519,23 @@ private:
> > > > >  };
> > > > >  REGISTER_CAMERA_SENSOR_HELPER("ar0521", CameraSensorHelperAr0521)
> > > > >
> > > > > +class CameraSensorHelperGc05a2 : public CameraSensorHelper
> > > > > +{
> > > > > +public:
> > > > > +       uint32_t gainCode(double gain) const override
> > > > > +       {
> > > > > +               uint32_t aeGain = std::clamp((uint32_t)gain, kAeBaseGain_, kStep_ * kAeBaseGain_);
> > >
> > > This doesn't seem right. 'gain' is expressed as a real number, with 1.0
> > > corresponding to a x1 gain.
> > >
> > > > > +               return aeGain * 0x400 / kAeBaseGain_;
> > >
> > > If the gain model is linear, you should be able to express it with the
> > > standard gain model, without needing to override gain() and gainCode().
> > >
> > > > > +       }
> > > > > +
> > > > > +       // `double gain(uint32_t gainCode)` will not be used.
> > > >
> > > > So don't add it ?
> > > >
> > > > But how do you mean "won't be used" - you mean ... you won't use it? or
> > > > it can't be used?
> > > >
> > > > How do we correctly determine the gain *used* from a configured gain
> > > > code?
> >
> > IIUC, If a configured ae gain is set, IIUC for per-frame control use cases,
> > in mtkisp7, we feed this information to the IPA proprietary library, which
> > calculates the corresponding gainCode for v4l2 camera sensor.
> >
> > We don't have the use case to calculate the ae gain from a configured gain
> > code. I've seen other pipeline handlers using this function to calculate the
> > static metadata `controls::AnalogueGain` though. Is this the only use case?
>
> It's used in open-source IPA modules. To merge support for a new sensor
> or a new platform, a fully open source implementation is required. There
> will need to be an open-source mtkisp7 IPA module that implements AGC,
> so you will need this.

Got it. I got replies from MediaTek that include the conversion algorithms.
Added in the new version.

BR,
Harvey

>
> > > > I suspect ... the gain() function should be implemented if the
> > > > gainCode() function is implemented. Otherwise callers on the helper will
> > > > get inconsistent results.
> > >
> > > Yes, even if libipa isn't used by the MTK IPA, proper support for the
> > > sensor needs to be implemented. Same for patch 2/2 in the series.
> >
> > I've been contacting MTK's team, while they're quite inconsistent with
> > themselves and very slow to response...
> >
> > Previously the conversion is done in their proprietary algorithms, and
> > we directly use the results in the IPA.
> >
> > Therefore, we might not end up using these functions even if we finally
> > dig out MTK's algorithm.
> > For per-frame control use cases though, it might simplify the task flows,
> > as we probably could calculate the gain code ourselves without waiting
> > for IPA proprietary algorithm.
> >
> > Let's wait for MTK's response for now...
> >
> > > > > +
> > > > > +private:
> > > > > +       static constexpr uint32_t kStep_ = 16;
> > > > > +       static constexpr uint32_t kAeBaseGain_ = 1024;
> > > > > +};
> > > > > +REGISTER_CAMERA_SENSOR_HELPER("gc05a2", CameraSensorHelperGc05a2)
> > > > > +
> > > > >  class CameraSensorHelperImx219 : public CameraSensorHelper
> > > > >  {
> > > > >  public:
> > > > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > > > > index 4e5217ab..3e1bd85e 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

Patch
diff mbox series

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index ffc7c1d7..cd7d12d6 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -519,6 +519,23 @@  private:
 };
 REGISTER_CAMERA_SENSOR_HELPER("ar0521", CameraSensorHelperAr0521)
 
+class CameraSensorHelperGc05a2 : public CameraSensorHelper
+{
+public:
+	uint32_t gainCode(double gain) const override
+	{
+		uint32_t aeGain = std::clamp((uint32_t)gain, kAeBaseGain_, kStep_ * kAeBaseGain_);
+		return aeGain * 0x400 / kAeBaseGain_;
+	}
+
+	// `double gain(uint32_t gainCode)` will not be used.
+
+private:
+	static constexpr uint32_t kStep_ = 16;
+	static constexpr uint32_t kAeBaseGain_ = 1024;
+};
+REGISTER_CAMERA_SENSOR_HELPER("gc05a2", CameraSensorHelperGc05a2)
+
 class CameraSensorHelperImx219 : public CameraSensorHelper
 {
 public:
diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
index 4e5217ab..3e1bd85e 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 = {