[libcamera-devel,2/2] libipa: Correct IMX219 in CameraSensorHelper
diff mbox series

Message ID 20211119102559.56522-3-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • libipa: Fix doc and imx219
Related show

Commit Message

Jean-Michel Hautbois Nov. 19, 2021, 10:25 a.m. UTC
The equation is badly reported in the CameraSensorHelper, as m1 and c0
are inverted. Correct it to have a proper gain calculation.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/libipa/camera_sensor_helper.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kieran Bingham Nov. 19, 2021, 11:14 a.m. UTC | #1
Quoting Jean-Michel Hautbois (2021-11-19 10:25:59)
> The equation is badly reported in the CameraSensorHelper, as m1 and c0
> are inverted. Correct it to have a proper gain calculation.
> 

I feel like this would be easier to spot if this was all constructed
from a single table. But perhaps that's not as extensible in the future,
which is why we've got the individual constructors.

Anyway, this update matches more closely to the IMX258 now, so it looks
better from that comparative perspective at least...

I can't really add more value than that and I don't know what to check
specifically on the datasheets so

Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index 67cf6913..5e22f9c3 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -292,7 +292,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper
>  public:
>         CameraSensorHelperImx219()
>         {
> -               analogueGainConstants_ = { AnalogueGainLinear, 0, -1, 256, 256 };
> +               analogueGainConstants_ = { AnalogueGainLinear, 0, 256, -1, 256 };
>         }
>  };
>  REGISTER_CAMERA_SENSOR_HELPER("imx219", CameraSensorHelperImx219)
> -- 
> 2.32.0
>
Laurent Pinchart Nov. 19, 2021, 12:39 p.m. UTC | #2
On Fri, Nov 19, 2021 at 11:14:34AM +0000, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-11-19 10:25:59)
> > The equation is badly reported in the CameraSensorHelper, as m1 and c0
> > are inverted. Correct it to have a proper gain calculation.
> > 
> 
> I feel like this would be easier to spot if this was all constructed
> from a single table. But perhaps that's not as extensible in the future,
> which is why we've got the individual constructors.

Agreed, there's a bit of room for improvements in the syntax.

> Anyway, this update matches more closely to the IMX258 now, so it looks
> better from that comparative perspective at least...
> 
> I can't really add more value than that and I don't know what to check
> specifically on the datasheets so

Registers 0x008c to 0x0093: m0 = 0x0000, c0 = 0x0100, m1 = 0xffff,
c1 = 0x0100.

> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > ---
> >  src/ipa/libipa/camera_sensor_helper.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index 67cf6913..5e22f9c3 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -292,7 +292,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper
> >  public:
> >         CameraSensorHelperImx219()
> >         {
> > -               analogueGainConstants_ = { AnalogueGainLinear, 0, -1, 256, 256 };
> > +               analogueGainConstants_ = { AnalogueGainLinear, 0, 256, -1, 256 };
> >         }
> >  };
> >  REGISTER_CAMERA_SENSOR_HELPER("imx219", CameraSensorHelperImx219)

Patch
diff mbox series

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index 67cf6913..5e22f9c3 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -292,7 +292,7 @@  class CameraSensorHelperImx219 : public CameraSensorHelper
 public:
 	CameraSensorHelperImx219()
 	{
-		analogueGainConstants_ = { AnalogueGainLinear, 0, -1, 256, 256 };
+		analogueGainConstants_ = { AnalogueGainLinear, 0, 256, -1, 256 };
 	}
 };
 REGISTER_CAMERA_SENSOR_HELPER("imx219", CameraSensorHelperImx219)