Message-ID: <CAFoNnrzHhJVbR1yQrr6o1+1JhyDB6y0NNfyJkK=by9YOJRGusQ@mail.gmail.com>
Date: Mon, 11 Aug 2025 19:31:13 -0700
From: Will Whang <will@...lwhang.com>
To: Dave Stevenson <dave.stevenson@...pberrypi.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, 
	Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-media@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, imx@...ts.linux.dev, 
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 4/4] media: docs: Add userspace-API guide for the
 IMX585 driver

Hi Dave,
Reply inline.
Thanks,
Will Whang

On Mon, Aug 11, 2025 at 7:24 AM Dave Stevenson
<dave.stevenson@...pberrypi.com> wrote:
>
> Hi Will
>
> Thanks for the patches.
>
> On Sun, 10 Aug 2025 at 23:11, Will Whang <will@...lwhang.com> wrote:
> >
> > The new IMX585 V4L2 sub-device driver introduces several
> > driver-specific controls for configuring Clear-HDR blending,
> > gradation compression thresholds, and HCG enabling.  This patch adds
> > an rst document under Documentation/userspace-api/media/drivers/
> > that details each control, allowed values, and their effects.
> >
> > Signed-off-by: Will Whang <will@...lwhang.com>
> > ---
> >  .../userspace-api/media/drivers/imx585.rst    | 122 ++++++++++++++++++
> >  .../userspace-api/media/drivers/index.rst     |   1 +
> >  MAINTAINERS                                   |   1 +
> >  3 files changed, 124 insertions(+)
> >  create mode 100644 Documentation/userspace-api/media/drivers/imx585.rst
> >
> > diff --git a/Documentation/userspace-api/media/drivers/imx585.rst b/Documentation/userspace-api/media/drivers/imx585.rst
> > new file mode 100644
> > index 000000000..9f7c16f30
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/drivers/imx585.rst
> > @@ -0,0 +1,122 @@
> > +.. SPDX-License-Identifier: GPL-2.0-only
> > +
> > +Sony IMX585 driver
> > +==================
> > +
> > +The IMX585 image-sensor driver provides the following *driver-specific*
> > +V4L2 controls.  They are visible only when the IMX585 driver is loaded
> > +and sit in the sensor-private control class.
> > +
> > +HDR data blending
> > +-----------------
> > +
> > +``V4L2_CID_IMX585_HDR_DATASEL_TH``  (``U16[2]``)
> > +    Lower/upper **thresholds** (0 – 4095) that decide which exposure is
> > +    chosen—or blended—for each pixel in Clear-HDR mode.
> > +
> > +``V4L2_CID_IMX585_HDR_DATASEL_BK``  (menu)
> > +    **Blending ratio** between the long-gain (LG) and
> > +    high-gain (HG) read-outs.
> > +
> > +    .. flat-table::
> > +       :stub-columns: 0
> > +       :widths: 1 5
> > +
> > +       * - ``0``
> > +         - HG ½, LG ½
> > +       * - ``1``
> > +         - HG ¾, LG ¼
> > +       * - ``2``     # duplicate ratio present in the datasheet
> > +         - HG ½, LG ½
> > +       * - ``3``
> > +         - HG ⅞, LG ⅛
> > +       * - ``4``
> > +         - HG 15⁄16, LG 1⁄16
> > +       * - ``5``     # second 50/50 entry as documented
> > +         - **2ⁿᵈ** HG ½, LG ½
> > +       * - ``6``
> > +         - HG 1⁄16, LG 15⁄16
> > +       * - ``7``
> > +         - HG ⅛, LG ⅞
> > +       * - ``8``
> > +         - HG ¼, LG ¾
> > +
> > +Gradation compression
> > +---------------------
> > +
> > +``V4L2_CID_IMX585_HDR_GRAD_TH``  (``U32[2]``)
> > +    17-bit **break-points** (0 – 0x1ffff) that shape the 16-bit
> > +    gradation-compression curve.
> > +
> > +``V4L2_CID_IMX585_HDR_GRAD_COMP_L``  (menu)
> > +    See V4L2_CID_IMX585_HDR_GRAD_COMP_H
> > +
> > +``V4L2_CID_IMX585_HDR_GRAD_COMP_H``  (menu)
> > +    **Compression ratios** below the first break-point and between the
> > +    two break-points, respectively.
> > +
> > +    .. flat-table::
> > +        :stub-columns: 0
> > +        :widths: 1 4
> > +
> > +        * - ``0``
> > +          - 1 : 1
> > +        * - ``1``
> > +          - 1 : 2
> > +        * - ``2``
> > +          - 1 : 4   *(default for COMP_L)*
> > +        * - ``3``
> > +          - 1 : 8
> > +        * - ``4``
> > +          - 1 : 16
> > +        * - ``5``
> > +          - 1 : 32
> > +        * - ``6``
> > +          - 1 : 64  *(default for COMP_H)*
> > +        * - ``7``
> > +          - 1 : 128
> > +        * - ``8``
> > +          - 1 : 256
> > +        * - ``9``
> > +          - 1 : 512
> > +        * - ``10``
> > +          - 1 : 1024
> > +        * - ``11``
> > +          - 1 : 2048
> > +
> > +Gain settings
> > +-------------
> > +
> > +``V4L2_CID_IMX585_HDR_GAIN``  (menu)
> > +    **Additional gain** (in dB) applied to the high-gain path when
> > +    Clear-HDR is active.
> > +
> > +    .. flat-table::
> > +        :stub-columns: 0
> > +        :widths: 1 3
> > +
> > +        * - ``0``
> > +          - +0 dB
> > +        * - ``1``
> > +          - +6 dB
> > +        * - ``2``
> > +          - +12 dB *(default)*
> > +        * - ``3``
> > +          - +18 dB
> > +        * - ``4``
> > +          - +24 dB
> > +        * - ``5``
> > +          - +29.1 dB
> > +
> > +``V4L2_CID_IMX585_HCG_GAIN``  (boolean)
>
> HCG stands for High Conversion Gain, so we've got Gain repeated in the name.
>
> Spell it out as V4L2_CID_IMX585_HIGH_CONV_GAIN, or call it
> CONVERSION_GAIN and use an enum control?
>
> > +    Toggle **High-Conversion-Gain** mode.
> > +
> > +    *0 = LCG (default), 1 = HCG.*
>
> An HCG / LCG control would also be applicable for IMX290 [1], so it
> would be nice if this could be a generic control instead of imx585
> specific.
>
> I never got a good description as to the benefit HCG was meant to
> give. The datasheet for IMX290 says the conversion efficiency ratio
> between HCG and LCG is 2, but not why that is any better than adding
> 6dB of analog gain.
>
What I've learned is that HCG is actually a 2nd stage amplifier, so
instead of using one for high gain, you can split it into two lower
gain stages that lower the noise.
This is also why ClearHDR and HCG have the conflict, because it is
basically sampling after the 1st gain stage and 2nd gain stage as
High/Low gain images.

I have thought about making it more generic because IMX662 and IMX678
are going to be the next patch once this one passes that also has this
function (the two are basically stripped down versions from IMX585
that the driver can be adapted to easily). I intended for the upcoming
IMX662/IMX678 driver to use IMX585's V4L2 HCG control also.

But given that I'm new to Linux kernel developments, or more
specifically, V4L2, I really don't have an idea how to do so in a way
the patch will be accepted.
Or I can make this more generic name to replace IMX585 for STARVIS2,
for example: V4L2_CID_STARVIS2_HIGH_CONV_GAIN

> Sony's website [2] states
> "Sony’s Super High Conversion Gain technology is designed to amplify
> electrical signals immediately after the conversion from photons, when
> the noise levels are relatively low. In this way, it reduces the
> overall noise after amplification. As a result, lower-noise images,
> compared to conventional technology, can be captured even in a
> low-illuminance environment. Lower noise levels in images also help to
> enhance the accuracy in visual or AI-assisted image recognition."
> From that one would presume you'd always want it on (lower noise =
> good), unless needing the minimum exposure time and the image was
> already over-exposed.
> I'm guessing you have no additional information based on your description text.
>
>   Dave
>
> [1] Also IMX327, IMX462, and IMX662 which are in the same family,
> IMX678 (ratio of 2.6), and quite probably most of the Sony Starvis or
> Starvis 2 ranges.
> [2] https://www.sony-semicon.com/en/technology/security/index.html
Yeah I think Starvis 2 series all have this capability.
>
> > +
> > +Notes
> > +-----
> > +
> > +* Controls are writable while streaming; changes take effect from the
> > +  next frame.
> > +* HDR-specific controls are hidden when HDR is disabled.
> > +* Inter-control dependencies are enforced by the driver.
> > diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
> > index d706cb47b..87912acfb 100644
> > --- a/Documentation/userspace-api/media/drivers/index.rst
> > +++ b/Documentation/userspace-api/media/drivers/index.rst
> > @@ -32,6 +32,7 @@ For more details see the file COPYING in the source distribution of Linux.
> >         cx2341x-uapi
> >         dw100
> >         imx-uapi
> > +       imx585
> >         max2175
> >         npcm-video
> >         omap3isp-uapi
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 175f5236a..42e32b6ba 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -23183,6 +23183,7 @@ M:      Will Whang <will@...lwhang.com>
> >  L:     linux-media@...r.kernel.org
> >  S:     Maintained
> >  F:     Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> > +F:     Documentation/userspace-api/media/drivers/imx585.rst
> >  F:     drivers/media/i2c/imx585.c
> >  F:     include/uapi/linux/imx585.h
> >
> > --
> > 2.39.5
> >
> >

Powered by blists - more mailing lists