[v2,3/4] ipa: rpi: Update black level tuning for IMX662
diff mbox series

Message ID 20260806152701.4179-4-david.plowman@raspberrypi.com
State New
Headers show
Series
  • Black level and other fixes for IMX662
Related show

Commit Message

David Plowman Aug. 6, 2026, 3:19 p.m. UTC 
At very high gains, the measured black level increases noticeably
from the nominal value of 3200.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/rpi/pisp/data/imx662.json | 11 ++++++++++-
 src/ipa/rpi/vc4/data/imx662.json  | 11 ++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Aug. 10, 2026, 10:46 a.m. UTC | #1 
Hi David,

Thank you for the patch.

On Thu, Aug 06, 2026 at 04:19:43PM +0100, David Plowman wrote:
> At very high gains, the measured black level increases noticeably
> from the nominal value of 3200.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/ipa/rpi/pisp/data/imx662.json | 11 ++++++++++-
>  src/ipa/rpi/vc4/data/imx662.json  | 11 ++++++++++-
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rpi/pisp/data/imx662.json b/src/ipa/rpi/pisp/data/imx662.json
> index b146d6b59..1aa5cf533 100644
> --- a/src/ipa/rpi/pisp/data/imx662.json
> +++ b/src/ipa/rpi/pisp/data/imx662.json
> @@ -5,7 +5,16 @@
>          {
>              "rpi.black_level":
>              {
> -                "black_level": 3200
> +                "black_level_func":
> +                [
> +                    1.0, 3200,
> +                    192.0, 3200,
> +                    256.0, 3245,
> +                    320.0, 3331,
> +                    384.0, 3443,
> +                    480.0, 3606,
> +                    512.0, 3623

The IMX662 gain register supports values up to 240. What am I missing ?

Furthermore, gains above 100 max out the analogue gain and add a digital
gain. Digital gain is typically applied in the ISP in libcamera, not in
the sensor. What's your use case for applying digital gain in the sensor
here ?

> +                ]
>              }
>          },
>          {
> diff --git a/src/ipa/rpi/vc4/data/imx662.json b/src/ipa/rpi/vc4/data/imx662.json
> index cec937d62..dc28b53c4 100644
> --- a/src/ipa/rpi/vc4/data/imx662.json
> +++ b/src/ipa/rpi/vc4/data/imx662.json
> @@ -5,7 +5,16 @@
>          {
>              "rpi.black_level":
>              {
> -                "black_level": 3200
> +                "black_level_func":
> +                [
> +                    1.0, 3200,
> +                    192.0, 3200,
> +                    256.0, 3245,
> +                    320.0, 3331,
> +                    384.0, 3443,
> +                    480.0, 3606,
> +                    512.0, 3623
> +                ]
>              }
>          },
>          {
Jai Luthra Aug. 10, 2026, 11:20 a.m. UTC | #2 
Hi Laurent, David,

+ Dave,

Quoting Laurent Pinchart (2026-08-10 16:16:50)
> Hi David,
> 
> Thank you for the patch.
> 
> On Thu, Aug 06, 2026 at 04:19:43PM +0100, David Plowman wrote:
> > At very high gains, the measured black level increases noticeably
> > from the nominal value of 3200.
> > 
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> > Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  src/ipa/rpi/pisp/data/imx662.json | 11 ++++++++++-
> >  src/ipa/rpi/vc4/data/imx662.json  | 11 ++++++++++-
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/ipa/rpi/pisp/data/imx662.json b/src/ipa/rpi/pisp/data/imx662.json
> > index b146d6b59..1aa5cf533 100644
> > --- a/src/ipa/rpi/pisp/data/imx662.json
> > +++ b/src/ipa/rpi/pisp/data/imx662.json
> > @@ -5,7 +5,16 @@
> >          {
> >              "rpi.black_level":
> >              {
> > -                "black_level": 3200
> > +                "black_level_func":
> > +                [
> > +                    1.0, 3200,
> > +                    192.0, 3200,
> > +                    256.0, 3245,
> > +                    320.0, 3331,
> > +                    384.0, 3443,
> > +                    480.0, 3606,
> > +                    512.0, 3623
> 
> The IMX662 gain register supports values up to 240. What am I missing ?
> 
> Furthermore, gains above 100 max out the analogue gain and add a digital
> gain. Digital gain is typically applied in the ISP in libcamera, not in
> the sensor. What's your use case for applying digital gain in the sensor
> here ?

I see the downstream driver in Dave's tree uses the full 240 range and
optionally enables HCG mode on top using a custom V4L2 control:

https://github.com/6by9/linux/blob/rpi-6.18.y-imx662-mainline/drivers/media/i2c/imx662.c

Could you please point me to the relevant sensor documentation that
explains how much does the effective gain change when the HCG bit is set,
and if it is a constant multiplier or something?

I see two separate problems that need a solution for the shared Starvis2
sensor driver that we are planning for IMX662, IMX678, etc.

1. Exposing controls when Analogue and Digital gains are controlled through
   a single register

2. Exposing a standard control for HCG enable/disable, possibly updating
   the values for normal gain control so userspace can use a single
   gain-model?

Cheers,
    Jai
David Plowman Aug. 11, 2026, 8:39 a.m. UTC | #3 
Hi

Don't know a huge amount about this sensor as I didn't write the
driver, but I'll answer as best as I can (others are away).

On Mon, 10 Aug 2026 at 12:20, Jai Luthra <jai.luthra@ideasonboard.com> wrote:
>
> Hi Laurent, David,
>
> + Dave,
>
> Quoting Laurent Pinchart (2026-08-10 16:16:50)
> > Hi David,
> >
> > Thank you for the patch.
> >
> > On Thu, Aug 06, 2026 at 04:19:43PM +0100, David Plowman wrote:
> > > At very high gains, the measured black level increases noticeably
> > > from the nominal value of 3200.
> > >
> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> > > Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > >  src/ipa/rpi/pisp/data/imx662.json | 11 ++++++++++-
> > >  src/ipa/rpi/vc4/data/imx662.json  | 11 ++++++++++-
> > >  2 files changed, 20 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/ipa/rpi/pisp/data/imx662.json b/src/ipa/rpi/pisp/data/imx662.json
> > > index b146d6b59..1aa5cf533 100644
> > > --- a/src/ipa/rpi/pisp/data/imx662.json
> > > +++ b/src/ipa/rpi/pisp/data/imx662.json
> > > @@ -5,7 +5,16 @@
> > >          {
> > >              "rpi.black_level":
> > >              {
> > > -                "black_level": 3200
> > > +                "black_level_func":
> > > +                [
> > > +                    1.0, 3200,
> > > +                    192.0, 3200,
> > > +                    256.0, 3245,
> > > +                    320.0, 3331,
> > > +                    384.0, 3443,
> > > +                    480.0, 3606,
> > > +                    512.0, 3623
> >
> > The IMX662 gain register supports values up to 240. What am I missing ?

We're not planning to use this sensor beyond 512x. Even here, the
black level is starting to go a bit strange, and it gets *way* weirder
if you go higher. There may be explanations for all this, but no one
has investigated.

Also, 512x already turns near total darkness (light levels at which a
human, even after several minutes, can basically not see a thing) into
daylight, so it's more than enough. The noise levels are absolutely
extreme at this gain, to the point that we're expecting only neural
networks will be able to make it usable.

> >
> > Furthermore, gains above 100 max out the analogue gain and add a digital
> > gain. Digital gain is typically applied in the ISP in libcamera, not in
> > the sensor. What's your use case for applying digital gain in the sensor
> > here ?

I think the main reason is really that we want the digital gain
pre-applied to the raw data for the neural networks. Letting the
sensor do it saves us the trouble of doing it in the CPU (which is
likely to be very busy). It also means it's been pre-applied in DNG
files, which is what we need for training.

Again, I don't think anyone has looked into it carefully, but images
produced by sensor digital gain vs. ISP digital gain are not the same.
I would say the sensor ones probably have slightly less colour
banding, but again, no one really knows why.

>
> I see the downstream driver in Dave's tree uses the full 240 range and
> optionally enables HCG mode on top using a custom V4L2 control:
>
> https://github.com/6by9/linux/blob/rpi-6.18.y-imx662-mainline/drivers/media/i2c/imx662.c
>
> Could you please point me to the relevant sensor documentation that
> explains how much does the effective gain change when the HCG bit is set,
> and if it is a constant multiplier or something?
>
> I see two separate problems that need a solution for the shared Starvis2
> sensor driver that we are planning for IMX662, IMX678, etc.
>
> 1. Exposing controls when Analogue and Digital gains are controlled through
>    a single register
>
> 2. Exposing a standard control for HCG enable/disable, possibly updating
>    the values for normal gain control so userspace can use a single
>    gain-model?
>
> Cheers,
>     Jai

I don't have a whole lot to go on here. There's something in the
guides that talks about an extra 10.2dB of gain in the HCG mode, which
corresponds to 3.236x (= 10^0.51). This is actually the lowest gain
the HCG mode will own up to (that is, set the gain to 1x and it comes
back in the embedded data as this value).

However, empirical evidence suggests the LCG images need a gain about
8x the HCG one for equivalent brightness, so I don't really understand
what's happening there.

We haven't really come to any conclusion about how to present the
LCG/HCG mode distinction. For the moment, we're treating them as,
effectively, different sensors. You choose one or other, and don't
change it. In reality they might even need different tuning files. But
I don't know what the best answer is.

David

Patch
diff mbox series 

diff --git a/src/ipa/rpi/pisp/data/imx662.json b/src/ipa/rpi/pisp/data/imx662.json
index b146d6b59..1aa5cf533 100644
--- a/src/ipa/rpi/pisp/data/imx662.json
+++ b/src/ipa/rpi/pisp/data/imx662.json
@@ -5,7 +5,16 @@ 
         {
             "rpi.black_level":
             {
-                "black_level": 3200
+                "black_level_func":
+                [
+                    1.0, 3200,
+                    192.0, 3200,
+                    256.0, 3245,
+                    320.0, 3331,
+                    384.0, 3443,
+                    480.0, 3606,
+                    512.0, 3623
+                ]
             }
         },
         {
diff --git a/src/ipa/rpi/vc4/data/imx662.json b/src/ipa/rpi/vc4/data/imx662.json
index cec937d62..dc28b53c4 100644
--- a/src/ipa/rpi/vc4/data/imx662.json
+++ b/src/ipa/rpi/vc4/data/imx662.json
@@ -5,7 +5,16 @@ 
         {
             "rpi.black_level":
             {
-                "black_level": 3200
+                "black_level_func":
+                [
+                    1.0, 3200,
+                    192.0, 3200,
+                    256.0, 3245,
+                    320.0, 3331,
+                    384.0, 3443,
+                    480.0, 3606,
+                    512.0, 3623
+                ]
             }
         },
         {