Hi Sakari On Mon, Jul 17, 2023 at 10:24:21AM +0000, Sakari Ailus wrote: > Hi Jacopo, > > On Mon, Jul 10, 2023 at 03:22:40PM +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@xxxxxxxxxxxxxxxx> > > Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > > --- > > .../driver-api/media/camera-sensor.rst | 27 +++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > index cd915ca119ea..67fe77b1edb9 100644 > > --- a/Documentation/driver-api/media/camera-sensor.rst > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > @@ -189,3 +189,30 @@ 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 allow 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 drivers it should be > > +expressed in unit of lines whenever possible. > > This part of the documentation applies to both raw and SoC cameras. > > Should the exposure unit be something more user-friendly for SoC cameras? SoC cameras == YUV/RGB sensors ? Are you thinking about using the actual exposure time for YUV/RGB sensors ? > > We have two exposure controls now, V4L2_CID_EXPOSURE and > V4L2_CID_EXPOSURE_ABSOLUTE. The former doesn't specity a unit whereas the Apparently only 2 drivers in mainline register V4L2_CID_EXPOSURE_ABSOLUTE > latter suggests the unit of 100 µs. > > As exposure is specific to cameras, I think at least a part of this should > make it to the controls documentation. The UVC, for instance, uses > EXPOSURE_ABSOLUTE. > > Could we document V4L2_CID_EXPOSURE is in lines (if possible)? I would indeed be happy with something like "The suggested unit for the control is lines" > > > + > > +To convert lines into units of time, the total line length (visible and > > +not visible pixels) has to be divided by the pixel rate:: > > + > > + line duration = total line length / pixel rate > > + = (image width + horizontal blanking) / pixel rate > > + > > +Camera sensor driver should try whenever possible to distinguish between the > > +analogue and digital gain control functions. Analogue gain is a multiplication > > +factor applied to all color channels on the pixel array before they get > > +converted into the digital domain. It should be made controllable by > > The analogue gain may not be linear. This depends on the sensor. I'd thus > drop the wording related to multiplication factor. > I might have missed why the gain being linear or not has implications on the fact it acts as a multiplication factor for the color channels... > > +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 > > +discouraged from using ``V4L2_CID_GAIN`` as it doesn't allow differentiation of > > +analogue vs digital gain. > > -- > Kind regards, > > Sakari Ailus