diff mbox series

[2/2] Documentation: v4l: Exposure/gain for camera sensor

Message ID 20230703202910.31142-3-jacopo.mondi@ideasonboard.com
State New
Headers show
Series Documentation: v4l: more camera sensor doc | expand

Commit Message

Jacopo Mondi July 3, 2023, 8:29 p.m. UTC
Document the suggested way to exposure controls for exposure and gain
for camera sensor drivers.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 .../driver-api/media/camera-sensor.rst        | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Dave Stevenson July 4, 2023, 11:06 a.m. UTC | #1
On Tue, 4 Jul 2023 at 11:28, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Dave
>
> On Tue, Jul 04, 2023 at 11:05:42AM +0100, Dave Stevenson wrote:
> > Hi Jacopo
> >
> > On Mon, 3 Jul 2023 at 21:29, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
> > >
> > > Document the suggested way to exposure controls for exposure and gain
> > > for camera sensor drivers.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > >  .../driver-api/media/camera-sensor.rst        | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > >
> > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > > index ee4a7fe5f72a..dfe8f35aecea 100644
> > > --- a/Documentation/driver-api/media/camera-sensor.rst
> > > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > > @@ -189,3 +189,22 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the
> > >  a flip can potentially change the output buffer content layout. Flips should
> > >  also be taken into account when enumerating and handling media bus formats
> > >  on the camera sensor source pads.
> > > +
> > > +Exposure and Gain Control
> > > +-------------------------
> > > +
> > > +Camera sensor drivers that allows applications to control the image exposure
> >
> > s/allows/allow
> >
> > > +and gain should do so by exposing dedicated controls to applications.
> > > +
> > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control.
> > > +The control definition does not specify a unit to allow maximum flexibility
> > > +for multiple device types, but when used for camera sensor driver it should be
> >
> > s/driver/drivers
> >
> > > +expressed in unit of lines whenever possible.
> >
> > s/unit/units
> >
> > Possibly clarify that lines can be converted into units of time by
> > using V4L2_CID_PIXEL_RATE and (image width + V4L2_CID_HBLANK).
> >
>
> I think this might be useful yes. A few paragraph above the frame
> duration calculation formula is expressed as well, so I guess adding
> one for this is helpful too

Possibly just reference that paragraph then for computing the line period then.

> > > +
> > > +Camera sensor driver should try whenever possible to distinguish between the
> > > +analogue and digital gain control functions. Analogue gain is a multiplier
> > > +factor applied to all color channels on the pixel array before they get
> > > +converted in the digital domain. It should be be made controllable by
> > > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device
> > > +specific gain code. Digital gain control is optional and should be exposed to
> > > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are
> >
> > Something appears to be truncated here.
>
> Clearly a bad rebase. What I meant (I lost the original version) was
>
> Camera sensor drivers are discouraged from using ``V4L2_CID_GAIN``
> which doesn't allow to control the analogue and digital gain

as it doesn't allow differentiation of analogue vs digital gain

> components individually.
>
> How does this sound ?

Otherwise it sounds fine.

  Dave
Tommaso Merciai July 4, 2023, 1:51 p.m. UTC | #2
Hi Jacopo,

On Mon, Jul 03, 2023 at 10:29:10PM +0200, Jacopo Mondi wrote:
> Document the suggested way to exposure controls for exposure and gain
> for camera sensor drivers.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  .../driver-api/media/camera-sensor.rst        | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> index ee4a7fe5f72a..dfe8f35aecea 100644
> --- a/Documentation/driver-api/media/camera-sensor.rst
> +++ b/Documentation/driver-api/media/camera-sensor.rst
> @@ -189,3 +189,22 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the
>  a flip can potentially change the output buffer content layout. Flips should
>  also be taken into account when enumerating and handling media bus formats
>  on the camera sensor source pads.
> +
> +Exposure and Gain Control
> +-------------------------
> +
> +Camera sensor drivers that allows applications to control the image exposure
> +and gain should do so by exposing dedicated controls to applications.
> +
> +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control.
> +The control definition does not specify a unit to allow maximum flexibility
> +for multiple device types, but when used for camera sensor driver it should be

> +expressed in unit of lines whenever possible.

Same comment here. 

Can you add formula/references about this point I think you are referring on "tline" units (maybe I'm completely wrong :) ),
but to be honest checking also the some sensors datasheet I don't find to much infos about this units.
Would be really helpfull to add some details on this point.

> +
> +Camera sensor driver should try whenever possible to distinguish between the
> +analogue and digital gain control functions. Analogue gain is a multiplier
> +factor applied to all color channels on the pixel array before they get
> +converted in the digital domain. It should be be made controllable by
> +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device
> +specific gain code. Digital gain control is optional and should be exposed to
> +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are
> -- 

This part looks good to me.

Thanks,
Tommaso

> 2.40.1
>
Jacopo Mondi July 4, 2023, 2:05 p.m. UTC | #3
Hi Tommaso

On Tue, Jul 04, 2023 at 03:51:43PM +0200, Tommaso Merciai wrote:
> Hi Jacopo,
>
> On Mon, Jul 03, 2023 at 10:29:10PM +0200, Jacopo Mondi wrote:
> > Document the suggested way to exposure controls for exposure and gain
> > for camera sensor drivers.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  .../driver-api/media/camera-sensor.rst        | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > index ee4a7fe5f72a..dfe8f35aecea 100644
> > --- a/Documentation/driver-api/media/camera-sensor.rst
> > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > @@ -189,3 +189,22 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the
> >  a flip can potentially change the output buffer content layout. Flips should
> >  also be taken into account when enumerating and handling media bus formats
> >  on the camera sensor source pads.
> > +
> > +Exposure and Gain Control
> > +-------------------------
> > +
> > +Camera sensor drivers that allows applications to control the image exposure
> > +and gain should do so by exposing dedicated controls to applications.
> > +
> > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control.
> > +The control definition does not specify a unit to allow maximum flexibility
> > +for multiple device types, but when used for camera sensor driver it should be
>
> > +expressed in unit of lines whenever possible.
>
> Same comment here.

I might have missed what other comment you are referring to :)

>
> Can you add formula/references about this point I think you are referring on "tline" units (maybe I'm completely wrong :) ),

Is "tline" the line duration ? If that's the case then no, I am
referring to the number of lines, not their duration.

> but to be honest checking also the some sensors datasheet I don't find to much infos about this units.
> Would be really helpfull to add some details on this point.
>
> > +
> > +Camera sensor driver should try whenever possible to distinguish between the
> > +analogue and digital gain control functions. Analogue gain is a multiplier
> > +factor applied to all color channels on the pixel array before they get
> > +converted in the digital domain. It should be be made controllable by
> > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device
> > +specific gain code. Digital gain control is optional and should be exposed to
> > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are
> > --
>
> This part looks good to me.
>
> Thanks,
> Tommaso
>
> > 2.40.1
> >
Jacopo Mondi July 4, 2023, 3 p.m. UTC | #4
Hi Tommaso

On Tue, Jul 04, 2023 at 04:30:09PM +0200, Tommaso Merciai wrote:
> Hi Jacopo,
>
> On Tue, Jul 04, 2023 at 04:05:44PM +0200, Jacopo Mondi wrote:
> > Hi Tommaso
> >
> > On Tue, Jul 04, 2023 at 03:51:43PM +0200, Tommaso Merciai wrote:
> > > Hi Jacopo,
> > >
> > > On Mon, Jul 03, 2023 at 10:29:10PM +0200, Jacopo Mondi wrote:
> > > > Document the suggested way to exposure controls for exposure and gain
> > > > for camera sensor drivers.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > > ---
> > > >  .../driver-api/media/camera-sensor.rst        | 19 +++++++++++++++++++
> > > >  1 file changed, 19 insertions(+)
> > > >
> > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > > > index ee4a7fe5f72a..dfe8f35aecea 100644
> > > > --- a/Documentation/driver-api/media/camera-sensor.rst
> > > > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > > > @@ -189,3 +189,22 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the
> > > >  a flip can potentially change the output buffer content layout. Flips should
> > > >  also be taken into account when enumerating and handling media bus formats
> > > >  on the camera sensor source pads.
> > > > +
> > > > +Exposure and Gain Control
> > > > +-------------------------
> > > > +
> > > > +Camera sensor drivers that allows applications to control the image exposure
> > > > +and gain should do so by exposing dedicated controls to applications.
> > > > +
> > > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control.
> > > > +The control definition does not specify a unit to allow maximum flexibility
> > > > +for multiple device types, but when used for camera sensor driver it should be
> > >
> > > > +expressed in unit of lines whenever possible.
> > >
> > > Same comment here.
> >
> > I might have missed what other comment you are referring to :)
>
> Sorry, I'm referring to your comment:
>
> "I think this might be useful yes. A few paragraph above the frame
> duration calculation formula is expressed as well, so I guess adding
> one for this is helpful too"
>

Ah ok, see v2 for that

> >
> > >
> > > Can you add formula/references about this point I think you are referring on "tline" units (maybe I'm completely wrong :) ),
> >
> > Is "tline" the line duration ? If that's the case then no, I am
> > referring to the number of lines, not their duration.
>
> Ok. Thanks, need to find some docs regarding this units :'(
> For this I think having some good reference/formula here would help users to find the
> corrispective value into the sensor datasheet.
>

I'm still not sure if you're talking about the formula to convert from
a time duration to the number of lines as Dave suggested (which I
added in v2) or a formula to convert from the number of lines provided
by userspace as the value of the V4L2_CID_EXPOSURE control to the
actual value to be set in the sensor's registers that control the
exposure time. If you're referring to the latter I'm afraid this is
device specific and putting any example here might actually be
mis-leading. As far as I can tell the sensors I had dealt with,
internally represents the exposure control in number of lines or
fractions of lines. Doing the conversion in the driver is usually
trivial (I'm sure there are devices where this is less trivial of
course).

Did I get your question right or am I still missing something ?

> I just proved this to you :)
>
> Regards,
> Tommaso
>
> >
> > > but to be honest checking also the some sensors datasheet I don't find to much infos about this units.
> > > Would be really helpfull to add some details on this point.
> > >
> > > > +
> > > > +Camera sensor driver should try whenever possible to distinguish between the
> > > > +analogue and digital gain control functions. Analogue gain is a multiplier
> > > > +factor applied to all color channels on the pixel array before they get
> > > > +converted in the digital domain. It should be be made controllable by
> > > > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device
> > > > +specific gain code. Digital gain control is optional and should be exposed to
> > > > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are
> > > > --
> > >
> > > This part looks good to me.
> > >
> > > Thanks,
> > > Tommaso
> > >
> > > > 2.40.1
> > > >
Tommaso Merciai July 7, 2023, 4:13 p.m. UTC | #5
Hi Jacopo,

On Tue, Jul 04, 2023 at 05:00:24PM +0200, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Tue, Jul 04, 2023 at 04:30:09PM +0200, Tommaso Merciai wrote:
> > Hi Jacopo,
> >
> > On Tue, Jul 04, 2023 at 04:05:44PM +0200, Jacopo Mondi wrote:
> > > Hi Tommaso
> > >
> > > On Tue, Jul 04, 2023 at 03:51:43PM +0200, Tommaso Merciai wrote:
> > > > Hi Jacopo,
> > > >
> > > > On Mon, Jul 03, 2023 at 10:29:10PM +0200, Jacopo Mondi wrote:
> > > > > Document the suggested way to exposure controls for exposure and gain
> > > > > for camera sensor drivers.
> > > > >
> > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > > > ---
> > > > >  .../driver-api/media/camera-sensor.rst        | 19 +++++++++++++++++++
> > > > >  1 file changed, 19 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > > > > index ee4a7fe5f72a..dfe8f35aecea 100644
> > > > > --- a/Documentation/driver-api/media/camera-sensor.rst
> > > > > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > > > > @@ -189,3 +189,22 @@ the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the
> > > > >  a flip can potentially change the output buffer content layout. Flips should
> > > > >  also be taken into account when enumerating and handling media bus formats
> > > > >  on the camera sensor source pads.
> > > > > +
> > > > > +Exposure and Gain Control
> > > > > +-------------------------
> > > > > +
> > > > > +Camera sensor drivers that allows applications to control the image exposure
> > > > > +and gain should do so by exposing dedicated controls to applications.
> > > > > +
> > > > > +Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control.
> > > > > +The control definition does not specify a unit to allow maximum flexibility
> > > > > +for multiple device types, but when used for camera sensor driver it should be
> > > >
> > > > > +expressed in unit of lines whenever possible.
> > > >
> > > > Same comment here.
> > >
> > > I might have missed what other comment you are referring to :)
> >
> > Sorry, I'm referring to your comment:
> >
> > "I think this might be useful yes. A few paragraph above the frame
> > duration calculation formula is expressed as well, so I guess adding
> > one for this is helpful too"
> >
> 
> Ah ok, see v2 for that
> 
> > >
> > > >
> > > > Can you add formula/references about this point I think you are referring on "tline" units (maybe I'm completely wrong :) ),
> > >
> > > Is "tline" the line duration ? If that's the case then no, I am
> > > referring to the number of lines, not their duration.
> >
> > Ok. Thanks, need to find some docs regarding this units :'(
> > For this I think having some good reference/formula here would help users to find the
> > corrispective value into the sensor datasheet.
> >
> 

> I'm still not sure if you're talking about the formula to convert from
> a time duration to the number of lines as Dave suggested (which I
> added in v2) 

This one. I checked v2 thanks for the clarification!

Regards,
Tommaso


> or a formula to convert from the number of lines provided
> by userspace as the value of the V4L2_CID_EXPOSURE control to the
> actual value to be set in the sensor's registers that control the
> exposure time. If you're referring to the latter I'm afraid this is
> device specific and putting any example here might actually be
> mis-leading. As far as I can tell the sensors I had dealt with,
> internally represents the exposure control in number of lines or
> fractions of lines. Doing the conversion in the driver is usually
> trivial (I'm sure there are devices where this is less trivial of
> course).
> 
> Did I get your question right or am I still missing something ?
> 
> > I just proved this to you :)
> >
> > Regards,
> > Tommaso
> >
> > >
> > > > but to be honest checking also the some sensors datasheet I don't find to much infos about this units.
> > > > Would be really helpfull to add some details on this point.
> > > >
> > > > > +
> > > > > +Camera sensor driver should try whenever possible to distinguish between the
> > > > > +analogue and digital gain control functions. Analogue gain is a multiplier
> > > > > +factor applied to all color channels on the pixel array before they get
> > > > > +converted in the digital domain. It should be be made controllable by
> > > > > +registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device
> > > > > +specific gain code. Digital gain control is optional and should be exposed to
> > > > > +applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are
> > > > > --
> > > >
> > > > This part looks good to me.
> > > >
> > > > Thanks,
> > > > Tommaso
> > > >
> > > > > 2.40.1
> > > > >
diff mbox series

Patch

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index ee4a7fe5f72a..dfe8f35aecea 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -189,3 +189,22 @@  the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the
 a flip can potentially change the output buffer content layout. Flips should
 also be taken into account when enumerating and handling media bus formats
 on the camera sensor source pads.
+
+Exposure and Gain Control
+-------------------------
+
+Camera sensor drivers that allows applications to control the image exposure
+and gain should do so by exposing dedicated controls to applications.
+
+Exposure time is controlled by registering the ``V4L2_CID_EXPOSURE`` control.
+The control definition does not specify a unit to allow maximum flexibility
+for multiple device types, but when used for camera sensor driver it should be
+expressed in unit of lines whenever possible.
+
+Camera sensor driver should try whenever possible to distinguish between the
+analogue and digital gain control functions. Analogue gain is a multiplier
+factor applied to all color channels on the pixel array before they get
+converted in the digital domain. It should be be made controllable by
+registering the ``V4L2_CID_ANALOGUE_GAIN`` control, expressed as a device
+specific gain code. Digital gain control is optional and should be exposed to
+applications by registering ``V4L2_CID_DIGITAL_GAIN``. Camera sensor drivers are