[RFC,v2] media: v4l2-ctrl: Add gain controls

Commit Message

Lad, Prabhakar Dec. 5, 2012, 11:49 a.m. UTC

From: Lad, Prabhakar <[e-mail]>

add support for per color component digital/analog gain controls
and also their corresponding offset.

Signed-off-by: Lad, Prabhakar <[e-mail]>
Cc: Sakari Ailus <[e-mail]>
Cc: Laurent Pinchart <[e-mail]>
Cc: Kyungmin Park <[e-mail]>
Cc: Guennadi Liakhovetski <[e-mail]>
Cc: Sylwester Nawrocki <[e-mail]>
Cc: Hans Verkuil <[e-mail]>
Cc: Hans de Goede <[e-mail]>
Cc: Chris MacGregor <[e-mail]>
Cc: Rob Landley <[e-mail]>
Cc: Jeongtae Park <[e-mail]>
Cc: Mauro Carvalho Chehab <[e-mail]>
---
 Changes for v2:
 1: Fixed review comments pointed by Laurent.
 2: Rebased on latest tree.

 Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
 include/uapi/linux/v4l2-controls.h           |   11 +++++
 3 files changed, 76 insertions(+), 0 deletions(-)

Comments

Hans Verkuil Dec. 5, 2012, 12:08 p.m. UTC | #1

(resend without HTML formatting)

On Wed 5 December 2012 12:49:29 Prabhakar Lad wrote:
> From: Lad, Prabhakar <[e-mail]>
> 
> add support for per color component digital/analog gain controls
> and also their corresponding offset.

Some obvious questions below...

> 
> Signed-off-by: Lad, Prabhakar <[e-mail]>
> Cc: Sakari Ailus <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Kyungmin Park <[e-mail]>
> Cc: Guennadi Liakhovetski <[e-mail]>
> Cc: Sylwester Nawrocki <[e-mail]>
> Cc: Hans Verkuil <[e-mail]>
> Cc: Hans de Goede <[e-mail]>
> Cc: Chris MacGregor <[e-mail]>
> Cc: Rob Landley <[e-mail]>
> Cc: Jeongtae Park <[e-mail]>
> Cc: Mauro Carvalho Chehab <[e-mail]>
> ---
>  Changes for v2:
>  1: Fixed review comments pointed by Laurent.
>  2: Rebased on latest tree.
> 
>  Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
>  include/uapi/linux/v4l2-controls.h           |   11 +++++
>  3 files changed, 76 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> index 7fe5be1..847a9bb 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -4543,6 +4543,60 @@ interface and may change in the future.</para>
>  	    specific test patterns can be used to test if a device is working
>  	    properly.</entry>
>  	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="descr"> Some capture/sensor devices have
> +	    the capability to set per color component digital/analog gain values.</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
> +	    <entry>integer</entry>
> +	  </row>
> +	  <row>
> +	    <entry spanname="descr"> Some capture/sensor devices have the
> +	    capability to set per color component digital/analog gain offset values.
> +	    V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
> +	    color component gain offsets.</entry>

If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
to interact? Or are they mutually exclusive?

And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
to interact?

This questions should be answered in the documentation...

Regards,

        Hans

> +	  </row>
>  	  <row><entry></entry></row>
>  	</tbody>
>        </tgroup>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index f6ee201..05e3708 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -790,6 +790,17 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_LINK_FREQ:		return "Link Frequency";
>  	case V4L2_CID_PIXEL_RATE:		return "Pixel Rate";
>  	case V4L2_CID_TEST_PATTERN:		return "Test Pattern";
> +	case V4L2_CID_GAIN_RED:			return "Gain Red";
> +	case V4L2_CID_GAIN_GREEN_RED:		return "Gain Green Red";
> +	case V4L2_CID_GAIN_GREEN_BLUE:		return "Gain Green Blue";
> +	case V4L2_CID_GAIN_BLUE:		return "Gain Blue";
> +	case V4L2_CID_GAIN_GREEN:		return "Gain Green";
> +	case V4L2_CID_GAIN_OFFSET:		return "Gain Offset";
> +	case V4L2_CID_BLUE_OFFSET:		return "Gain Blue Offset";
> +	case V4L2_CID_RED_OFFSET:		return "Gain Red Offset";
> +	case V4L2_CID_GREEN_OFFSET:		return "Gain Green Offset";
> +	case V4L2_CID_GREEN_RED_OFFSET:		return "Gain Green Red Offset";
> +	case V4L2_CID_GREEN_BLUE_OFFSET:	return "Gain Green Blue Offset";
>  
>  	/* DV controls */
>  	case V4L2_CID_DV_CLASS:			return "Digital Video Controls";
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index f56c945..9b6b233 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -799,5 +799,16 @@ enum v4l2_jpeg_chroma_subsampling {
>  #define V4L2_CID_LINK_FREQ			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 1)
>  #define V4L2_CID_PIXEL_RATE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
>  #define V4L2_CID_TEST_PATTERN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> +#define V4L2_CID_GAIN_RED			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 4)
> +#define V4L2_CID_GAIN_GREEN_RED			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 5)
> +#define V4L2_CID_GAIN_GREEN_BLUE		(V4L2_CID_IMAGE_PROC_CLASS_BASE + 6)
> +#define V4L2_CID_GAIN_BLUE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 7)
> +#define V4L2_CID_GAIN_GREEN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 8)
> +#define V4L2_CID_GAIN_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 9)
> +#define V4L2_CID_BLUE_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 10)
> +#define V4L2_CID_RED_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 11)
> +#define V4L2_CID_GREEN_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 12)
> +#define V4L2_CID_GREEN_RED_OFFSET		(V4L2_CID_IMAGE_PROC_CLASS_BASE + 13)
> +#define V4L2_CID_GREEN_BLUE_OFFSET		(V4L2_CID_IMAGE_PROC_CLASS_BASE + 14)
>  
>  #endif
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lad, Prabhakar Dec. 6, 2012, 4:54 a.m. UTC | #2

Hi Hans,

On Wed, Dec 5, 2012 at 5:38 PM, Hans Verkuil <[e-mail]> wrote:
> (resend without HTML formatting)
>
> On Wed 5 December 2012 12:49:29 Prabhakar Lad wrote:
>> From: Lad, Prabhakar <[e-mail]>
>>
>> add support for per color component digital/analog gain controls
>> and also their corresponding offset.
>
> Some obvious questions below...
>
>>
>> Signed-off-by: Lad, Prabhakar <[e-mail]>
>> Cc: Sakari Ailus <[e-mail]>
>> Cc: Laurent Pinchart <[e-mail]>
>> Cc: Kyungmin Park <[e-mail]>
>> Cc: Guennadi Liakhovetski <[e-mail]>
>> Cc: Sylwester Nawrocki <[e-mail]>
>> Cc: Hans Verkuil <[e-mail]>
>> Cc: Hans de Goede <[e-mail]>
>> Cc: Chris MacGregor <[e-mail]>
>> Cc: Rob Landley <[e-mail]>
>> Cc: Jeongtae Park <[e-mail]>
>> Cc: Mauro Carvalho Chehab <[e-mail]>
>> ---
>>  Changes for v2:
>>  1: Fixed review comments pointed by Laurent.
>>  2: Rebased on latest tree.
>>
>>  Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
>>  drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
>>  include/uapi/linux/v4l2-controls.h           |   11 +++++
>>  3 files changed, 76 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
>> index 7fe5be1..847a9bb 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -4543,6 +4543,60 @@ interface and may change in the future.</para>
>>           specific test patterns can be used to test if a device is working
>>           properly.</entry>
>>         </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="descr"> Some capture/sensor devices have
>> +         the capability to set per color component digital/analog gain values.</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
>> +         <entry>integer</entry>
>> +       </row>
>> +       <row>
>> +         <entry spanname="descr"> Some capture/sensor devices have the
>> +         capability to set per color component digital/analog gain offset values.
>> +         V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
>> +         color component gain offsets.</entry>
>
> If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
> to interact? Or are they mutually exclusive?
>
> And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
> to interact?
>
> This questions should be answered in the documentation...
>
I haven’t worked on the hardware which supports both, What is the general
behaviour when the hardware supports both per color component and global
and both of them are set ? That could be helpful for me to document.

Regards,
--Prabhakar Lad

> Regards,
>
>         Hans
>
>> +       </row>
>>         <row><entry></entry></row>
>>       </tbody>
>>        </tgroup>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index f6ee201..05e3708 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -790,6 +790,17 @@ const char *v4l2_ctrl_get_name(u32 id)
>>       case V4L2_CID_LINK_FREQ:                return "Link Frequency";
>>       case V4L2_CID_PIXEL_RATE:               return "Pixel Rate";
>>       case V4L2_CID_TEST_PATTERN:             return "Test Pattern";
>> +     case V4L2_CID_GAIN_RED:                 return "Gain Red";
>> +     case V4L2_CID_GAIN_GREEN_RED:           return "Gain Green Red";
>> +     case V4L2_CID_GAIN_GREEN_BLUE:          return "Gain Green Blue";
>> +     case V4L2_CID_GAIN_BLUE:                return "Gain Blue";
>> +     case V4L2_CID_GAIN_GREEN:               return "Gain Green";
>> +     case V4L2_CID_GAIN_OFFSET:              return "Gain Offset";
>> +     case V4L2_CID_BLUE_OFFSET:              return "Gain Blue Offset";
>> +     case V4L2_CID_RED_OFFSET:               return "Gain Red Offset";
>> +     case V4L2_CID_GREEN_OFFSET:             return "Gain Green Offset";
>> +     case V4L2_CID_GREEN_RED_OFFSET:         return "Gain Green Red Offset";
>> +     case V4L2_CID_GREEN_BLUE_OFFSET:        return "Gain Green Blue Offset";
>>
>>       /* DV controls */
>>       case V4L2_CID_DV_CLASS:                 return "Digital Video Controls";
>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>> index f56c945..9b6b233 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -799,5 +799,16 @@ enum v4l2_jpeg_chroma_subsampling {
>>  #define V4L2_CID_LINK_FREQ                   (V4L2_CID_IMAGE_PROC_CLASS_BASE + 1)
>>  #define V4L2_CID_PIXEL_RATE                  (V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
>>  #define V4L2_CID_TEST_PATTERN                        (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
>> +#define V4L2_CID_GAIN_RED                    (V4L2_CID_IMAGE_PROC_CLASS_BASE + 4)
>> +#define V4L2_CID_GAIN_GREEN_RED                      (V4L2_CID_IMAGE_PROC_CLASS_BASE + 5)
>> +#define V4L2_CID_GAIN_GREEN_BLUE             (V4L2_CID_IMAGE_PROC_CLASS_BASE + 6)
>> +#define V4L2_CID_GAIN_BLUE                   (V4L2_CID_IMAGE_PROC_CLASS_BASE + 7)
>> +#define V4L2_CID_GAIN_GREEN                  (V4L2_CID_IMAGE_PROC_CLASS_BASE + 8)
>> +#define V4L2_CID_GAIN_OFFSET                 (V4L2_CID_IMAGE_PROC_CLASS_BASE + 9)
>> +#define V4L2_CID_BLUE_OFFSET                 (V4L2_CID_IMAGE_PROC_CLASS_BASE + 10)
>> +#define V4L2_CID_RED_OFFSET                  (V4L2_CID_IMAGE_PROC_CLASS_BASE + 11)
>> +#define V4L2_CID_GREEN_OFFSET                        (V4L2_CID_IMAGE_PROC_CLASS_BASE + 12)
>> +#define V4L2_CID_GREEN_RED_OFFSET            (V4L2_CID_IMAGE_PROC_CLASS_BASE + 13)
>> +#define V4L2_CID_GREEN_BLUE_OFFSET           (V4L2_CID_IMAGE_PROC_CLASS_BASE + 14)
>>
>>  #endif
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sakari Ailus Dec. 6, 2012, 9:54 a.m. UTC | #3

Hi Prabhakar and Hans,

On Thu, Dec 06, 2012 at 10:24:18AM +0530, Prabhakar Lad wrote:
> Hi Hans,
> 
> On Wed, Dec 5, 2012 at 5:38 PM, Hans Verkuil <[e-mail]> wrote:
> > (resend without HTML formatting)
> >
> > On Wed 5 December 2012 12:49:29 Prabhakar Lad wrote:
> >> From: Lad, Prabhakar <[e-mail]>
> >>
> >> add support for per color component digital/analog gain controls
> >> and also their corresponding offset.
> >
> > Some obvious questions below...
> >
> >>
> >> Signed-off-by: Lad, Prabhakar <[e-mail]>
> >> Cc: Sakari Ailus <[e-mail]>
> >> Cc: Laurent Pinchart <[e-mail]>
> >> Cc: Kyungmin Park <[e-mail]>
> >> Cc: Guennadi Liakhovetski <[e-mail]>
> >> Cc: Sylwester Nawrocki <[e-mail]>
> >> Cc: Hans Verkuil <[e-mail]>
> >> Cc: Hans de Goede <[e-mail]>
> >> Cc: Chris MacGregor <[e-mail]>
> >> Cc: Rob Landley <[e-mail]>
> >> Cc: Jeongtae Park <[e-mail]>
> >> Cc: Mauro Carvalho Chehab <[e-mail]>
> >> ---
> >>  Changes for v2:
> >>  1: Fixed review comments pointed by Laurent.
> >>  2: Rebased on latest tree.
> >>
> >>  Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
> >>  drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
> >>  include/uapi/linux/v4l2-controls.h           |   11 +++++
> >>  3 files changed, 76 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> >> index 7fe5be1..847a9bb 100644
> >> --- a/Documentation/DocBook/media/v4l/controls.xml
> >> +++ b/Documentation/DocBook/media/v4l/controls.xml
> >> @@ -4543,6 +4543,60 @@ interface and may change in the future.</para>
> >>           specific test patterns can be used to test if a device is working
> >>           properly.</entry>
> >>         </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="descr"> Some capture/sensor devices have
> >> +         the capability to set per color component digital/analog gain values.</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="descr"> Some capture/sensor devices have the
> >> +         capability to set per color component digital/analog gain offset values.
> >> +         V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
> >> +         color component gain offsets.</entry>
> >
> > If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
> > to interact? Or are they mutually exclusive?
> >
> > And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
> > to interact?
> >
> > This questions should be answered in the documentation...
> >
> I haven’t worked on the hardware which supports both, What is the general
> behaviour when the hardware supports both per color component and global
> and both of them are set ? That could be helpful for me to document.

I'd guess most of the time only either one is supported, and when someone
thinks of supporting both on the same device, we can start thinking of the
interaction of per-component and global ones. That may be hardware specific
as well, so standardising it might not be possible.

I think it'd be far more important to know which unit is it. Many such
controls are indeed fixed point values but the location of the point varies.
For unstance, u16,u16 and u8,u8 aren't uncommon. We currently have no way to
tell this to the user space. This isn't in any way specific to gain or
offset controls, though.

Hans Verkuil Dec. 11, 2012, 8:56 a.m. UTC | #4

On Thu 6 December 2012 10:54:32 Sakari Ailus wrote:
> Hi Prabhakar and Hans,
> 
> On Thu, Dec 06, 2012 at 10:24:18AM +0530, Prabhakar Lad wrote:
> > Hi Hans,
> > 
> > On Wed, Dec 5, 2012 at 5:38 PM, Hans Verkuil <[e-mail]> wrote:
> > > (resend without HTML formatting)
> > >
> > > On Wed 5 December 2012 12:49:29 Prabhakar Lad wrote:
> > >> From: Lad, Prabhakar <[e-mail]>
> > >>
> > >> add support for per color component digital/analog gain controls
> > >> and also their corresponding offset.
> > >
> > > Some obvious questions below...
> > >
> > >>
> > >> Signed-off-by: Lad, Prabhakar <[e-mail]>
> > >> Cc: Sakari Ailus <[e-mail]>
> > >> Cc: Laurent Pinchart <[e-mail]>
> > >> Cc: Kyungmin Park <[e-mail]>
> > >> Cc: Guennadi Liakhovetski <[e-mail]>
> > >> Cc: Sylwester Nawrocki <[e-mail]>
> > >> Cc: Hans Verkuil <[e-mail]>
> > >> Cc: Hans de Goede <[e-mail]>
> > >> Cc: Chris MacGregor <[e-mail]>
> > >> Cc: Rob Landley <[e-mail]>
> > >> Cc: Jeongtae Park <[e-mail]>
> > >> Cc: Mauro Carvalho Chehab <[e-mail]>
> > >> ---
> > >>  Changes for v2:
> > >>  1: Fixed review comments pointed by Laurent.
> > >>  2: Rebased on latest tree.
> > >>
> > >>  Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
> > >>  drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
> > >>  include/uapi/linux/v4l2-controls.h           |   11 +++++
> > >>  3 files changed, 76 insertions(+), 0 deletions(-)
> > >>
> > >> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> > >> index 7fe5be1..847a9bb 100644
> > >> --- a/Documentation/DocBook/media/v4l/controls.xml
> > >> +++ b/Documentation/DocBook/media/v4l/controls.xml
> > >> @@ -4543,6 +4543,60 @@ interface and may change in the future.</para>
> > >>           specific test patterns can be used to test if a device is working
> > >>           properly.</entry>
> > >>         </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="descr"> Some capture/sensor devices have
> > >> +         the capability to set per color component digital/analog gain values.</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
> > >> +         <entry>integer</entry>
> > >> +       </row>
> > >> +       <row>
> > >> +         <entry spanname="descr"> Some capture/sensor devices have the
> > >> +         capability to set per color component digital/analog gain offset values.
> > >> +         V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
> > >> +         color component gain offsets.</entry>
> > >
> > > If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
> > > to interact? Or are they mutually exclusive?
> > >
> > > And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
> > > to interact?
> > >
> > > This questions should be answered in the documentation...
> > >
> > I haven’t worked on the hardware which supports both, What is the general
> > behaviour when the hardware supports both per color component and global
> > and both of them are set ? That could be helpful for me to document.
> 
> I'd guess most of the time only either one is supported,

Are you talking about GAIN_RED vs GAIN_RED_OFFSET or GAIN_OFFSET vs RED_OFFSET?
Or both?

> and when someone
> thinks of supporting both on the same device, we can start thinking of the
> interaction of per-component and global ones. That may be hardware specific
> as well, so standardising it might not be possible.
> 
> I think it'd be far more important to know which unit is it. Many such
> controls are indeed fixed point values but the location of the point varies.
> For unstance, u16,u16 and u8,u8 aren't uncommon. We currently have no way to
> tell this to the user space. This isn't in any way specific to gain or
> offset controls, though.

There are no standardized units for gain at the moment, and I don't really see
that happening any time soon. Fixed point isn't supported at all as a control
type, so that will have to be converted to an integer anyway.

Prabhakar, which of these controls are actually supported by your hardware?

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hans Verkuil Dec. 11, 2012, 9:05 a.m. UTC | #5

On Tue 11 December 2012 09:56:42 Hans Verkuil wrote:
> On Thu 6 December 2012 10:54:32 Sakari Ailus wrote:
> > Hi Prabhakar and Hans,
> > 
> > On Thu, Dec 06, 2012 at 10:24:18AM +0530, Prabhakar Lad wrote:
> > > Hi Hans,
> > > 
> > > On Wed, Dec 5, 2012 at 5:38 PM, Hans Verkuil <[e-mail]> wrote:
> > > > (resend without HTML formatting)
> > > >
> > > > On Wed 5 December 2012 12:49:29 Prabhakar Lad wrote:
> > > >> From: Lad, Prabhakar <[e-mail]>
> > > >>
> > > >> add support for per color component digital/analog gain controls
> > > >> and also their corresponding offset.
> > > >
> > > > Some obvious questions below...
> > > >
> > > >>
> > > >> Signed-off-by: Lad, Prabhakar <[e-mail]>
> > > >> Cc: Sakari Ailus <[e-mail]>
> > > >> Cc: Laurent Pinchart <[e-mail]>
> > > >> Cc: Kyungmin Park <[e-mail]>
> > > >> Cc: Guennadi Liakhovetski <[e-mail]>
> > > >> Cc: Sylwester Nawrocki <[e-mail]>
> > > >> Cc: Hans Verkuil <[e-mail]>
> > > >> Cc: Hans de Goede <[e-mail]>
> > > >> Cc: Chris MacGregor <[e-mail]>
> > > >> Cc: Rob Landley <[e-mail]>
> > > >> Cc: Jeongtae Park <[e-mail]>
> > > >> Cc: Mauro Carvalho Chehab <[e-mail]>
> > > >> ---
> > > >>  Changes for v2:
> > > >>  1: Fixed review comments pointed by Laurent.
> > > >>  2: Rebased on latest tree.
> > > >>
> > > >>  Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
> > > >>  drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
> > > >>  include/uapi/linux/v4l2-controls.h           |   11 +++++
> > > >>  3 files changed, 76 insertions(+), 0 deletions(-)
> > > >>
> > > >> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> > > >> index 7fe5be1..847a9bb 100644
> > > >> --- a/Documentation/DocBook/media/v4l/controls.xml
> > > >> +++ b/Documentation/DocBook/media/v4l/controls.xml
> > > >> @@ -4543,6 +4543,60 @@ interface and may change in the future.</para>
> > > >>           specific test patterns can be used to test if a device is working
> > > >>           properly.</entry>
> > > >>         </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="descr"> Some capture/sensor devices have
> > > >> +         the capability to set per color component digital/analog gain values.</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
> > > >> +         <entry>integer</entry>
> > > >> +       </row>
> > > >> +       <row>
> > > >> +         <entry spanname="descr"> Some capture/sensor devices have the
> > > >> +         capability to set per color component digital/analog gain offset values.
> > > >> +         V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
> > > >> +         color component gain offsets.</entry>
> > > >
> > > > If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
> > > > to interact? Or are they mutually exclusive?
> > > >
> > > > And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
> > > > to interact?
> > > >
> > > > This questions should be answered in the documentation...
> > > >
> > > I haven’t worked on the hardware which supports both, What is the general
> > > behaviour when the hardware supports both per color component and global
> > > and both of them are set ? That could be helpful for me to document.
> > 
> > I'd guess most of the time only either one is supported,
> 
> Are you talking about GAIN_RED vs GAIN_RED_OFFSET or GAIN_OFFSET vs RED_OFFSET?
> Or both?

I guess GAIN_RED + GAIN_RED_OFFSET is fine as a combination after reading up on it
a bit. It's the global vs per-component that's the problem. After thinking about
it some more I'd say it should be either one or the other, with a possible control
to switch between the two if both modes are supported. It gets very messy otherwise.

Regards,

	Hans

> 
> > and when someone
> > thinks of supporting both on the same device, we can start thinking of the
> > interaction of per-component and global ones. That may be hardware specific
> > as well, so standardising it might not be possible.
> > 
> > I think it'd be far more important to know which unit is it. Many such
> > controls are indeed fixed point values but the location of the point varies.
> > For unstance, u16,u16 and u8,u8 aren't uncommon. We currently have no way to
> > tell this to the user space. This isn't in any way specific to gain or
> > offset controls, though.
> 
> There are no standardized units for gain at the moment, and I don't really see
> that happening any time soon. Fixed point isn't supported at all as a control
> type, so that will have to be converted to an integer anyway.
> 
> Prabhakar, which of these controls are actually supported by your hardware?
> 
> Regards,
> 
> 	Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lad, Prabhakar Dec. 11, 2012, 9:32 a.m. UTC | #6

Hi Hans,

On Tue, Dec 11, 2012 at 2:26 PM, Hans Verkuil <[e-mail]> wrote:
> On Thu 6 December 2012 10:54:32 Sakari Ailus wrote:
>> Hi Prabhakar and Hans,
>>
>> On Thu, Dec 06, 2012 at 10:24:18AM +0530, Prabhakar Lad wrote:
>> > Hi Hans,
>> >
>> > On Wed, Dec 5, 2012 at 5:38 PM, Hans Verkuil <[e-mail]> wrote:
>> > > (resend without HTML formatting)
>> > >
>> > > On Wed 5 December 2012 12:49:29 Prabhakar Lad wrote:
>> > >> From: Lad, Prabhakar <[e-mail]>
>> > >>
>> > >> add support for per color component digital/analog gain controls
>> > >> and also their corresponding offset.
>> > >
>> > > Some obvious questions below...
>> > >
>> > >>
>> > >> Signed-off-by: Lad, Prabhakar <[e-mail]>
>> > >> Cc: Sakari Ailus <[e-mail]>
>> > >> Cc: Laurent Pinchart <[e-mail]>
>> > >> Cc: Kyungmin Park <[e-mail]>
>> > >> Cc: Guennadi Liakhovetski <[e-mail]>
>> > >> Cc: Sylwester Nawrocki <[e-mail]>
>> > >> Cc: Hans Verkuil <[e-mail]>
>> > >> Cc: Hans de Goede <[e-mail]>
>> > >> Cc: Chris MacGregor <[e-mail]>
>> > >> Cc: Rob Landley <[e-mail]>
>> > >> Cc: Jeongtae Park <[e-mail]>
>> > >> Cc: Mauro Carvalho Chehab <[e-mail]>
>> > >> ---
>> > >>  Changes for v2:
>> > >>  1: Fixed review comments pointed by Laurent.
>> > >>  2: Rebased on latest tree.
>> > >>
>> > >>  Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
>> > >>  drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
>> > >>  include/uapi/linux/v4l2-controls.h           |   11 +++++
>> > >>  3 files changed, 76 insertions(+), 0 deletions(-)
>> > >>
>> > >> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
>> > >> index 7fe5be1..847a9bb 100644
>> > >> --- a/Documentation/DocBook/media/v4l/controls.xml
>> > >> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> > >> @@ -4543,6 +4543,60 @@ interface and may change in the future.</para>
>> > >>           specific test patterns can be used to test if a device is working
>> > >>           properly.</entry>
>> > >>         </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="descr"> Some capture/sensor devices have
>> > >> +         the capability to set per color component digital/analog gain values.</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
>> > >> +         <entry>integer</entry>
>> > >> +       </row>
>> > >> +       <row>
>> > >> +         <entry spanname="descr"> Some capture/sensor devices have the
>> > >> +         capability to set per color component digital/analog gain offset values.
>> > >> +         V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
>> > >> +         color component gain offsets.</entry>
>> > >
>> > > If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
>> > > to interact? Or are they mutually exclusive?
>> > >
>> > > And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
>> > > to interact?
>> > >
>> > > This questions should be answered in the documentation...
>> > >
>> > I haven’t worked on the hardware which supports both, What is the general
>> > behaviour when the hardware supports both per color component and global
>> > and both of them are set ? That could be helpful for me to document.
>>
>> I'd guess most of the time only either one is supported,
>
> Are you talking about GAIN_RED vs GAIN_RED_OFFSET or GAIN_OFFSET vs RED_OFFSET?
> Or both?
>
>> and when someone
>> thinks of supporting both on the same device, we can start thinking of the
>> interaction of per-component and global ones. That may be hardware specific
>> as well, so standardising it might not be possible.
>>
>> I think it'd be far more important to know which unit is it. Many such
>> controls are indeed fixed point values but the location of the point varies.
>> For unstance, u16,u16 and u8,u8 aren't uncommon. We currently have no way to
>> tell this to the user space. This isn't in any way specific to gain or
>> offset controls, though.
>
> There are no standardized units for gain at the moment, and I don't really see
> that happening any time soon. Fixed point isn't supported at all as a control
> type, so that will have to be converted to an integer anyway.
>
> Prabhakar, which of these controls are actually supported by your hardware?
>
my hardware supports gain red, gain blue, gain green red, gain green blue and
global gain offset.

Regards,
--Prabhakar Lad

> Regards,
>
>         Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sakari Ailus Dec. 11, 2012, 10:06 p.m. UTC | #7

Hi Hans,

On Tue, Dec 11, 2012 at 09:56:42AM +0100, Hans Verkuil wrote:
...
> > > > If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
> > > > to interact? Or are they mutually exclusive?
> > > >
> > > > And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
> > > > to interact?
> > > >
> > > > This questions should be answered in the documentation...
> > > >
> > > I haven’t worked on the hardware which supports both, What is the general
> > > behaviour when the hardware supports both per color component and global
> > > and both of them are set ? That could be helpful for me to document.
> > 
> > I'd guess most of the time only either one is supported,
> 
> Are you talking about GAIN_RED vs GAIN_RED_OFFSET or GAIN_OFFSET vs RED_OFFSET?
> Or both?

Per-component vs. global controls.

Few devices support both; AFAIR on SMIA the user can choose which one to
use, but the driver implements neither currently.

> > and when someone
> > thinks of supporting both on the same device, we can start thinking of the
> > interaction of per-component and global ones. That may be hardware specific
> > as well, so standardising it might not be possible.
> > 
> > I think it'd be far more important to know which unit is it. Many such
> > controls are indeed fixed point values but the location of the point varies.
> > For unstance, u16,u16 and u8,u8 aren't uncommon. We currently have no way to
> > tell this to the user space. This isn't in any way specific to gain or
> > offset controls, though.
> 
> There are no standardized units for gain at the moment, and I don't really see
> that happening any time soon. Fixed point isn't supported at all as a control
> type, so that will have to be converted to an integer anyway.

Do you think it'd require a new control type? There might be many; some
devices use funny fixed point values, such as u8.u5. I guess you could use
step for those, sure.

For instance, some sensors natively use lines to tell the exposure value
(and in low level sensors control the granularity really matters, so lines
it should be) whereas some SoC ones could use µs instead.

This is about units and prefixes IMO. Fixed point is just a prefix, such as
milli or micro, but instead of being a power of ten it's a power of two.

This would also allow telling the user about a gain control that e.g. the
value 0x100 means "no gain".

I think someone should write an RFC about this. :-)

Patch

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index 7fe5be1..847a9bb 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4543,6 +4543,60 @@  interface and may change in the future.</para>
 	    specific test patterns can be used to test if a device is working
 	    properly.</entry>
 	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr"> Some capture/sensor devices have
+	    the capability to set per color component digital/analog gain values.</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row>
+	    <entry spanname="descr"> Some capture/sensor devices have the
+	    capability to set per color component digital/analog gain offset values.
+	    V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
+	    color component gain offsets.</entry>
+	  </row>
 	  <row><entry></entry></row>
 	</tbody>
       </tgroup>
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index f6ee201..05e3708 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -790,6 +790,17 @@  const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_LINK_FREQ:		return "Link Frequency";
 	case V4L2_CID_PIXEL_RATE:		return "Pixel Rate";
 	case V4L2_CID_TEST_PATTERN:		return "Test Pattern";
+	case V4L2_CID_GAIN_RED:			return "Gain Red";
+	case V4L2_CID_GAIN_GREEN_RED:		return "Gain Green Red";
+	case V4L2_CID_GAIN_GREEN_BLUE:		return "Gain Green Blue";
+	case V4L2_CID_GAIN_BLUE:		return "Gain Blue";
+	case V4L2_CID_GAIN_GREEN:		return "Gain Green";
+	case V4L2_CID_GAIN_OFFSET:		return "Gain Offset";
+	case V4L2_CID_BLUE_OFFSET:		return "Gain Blue Offset";
+	case V4L2_CID_RED_OFFSET:		return "Gain Red Offset";
+	case V4L2_CID_GREEN_OFFSET:		return "Gain Green Offset";
+	case V4L2_CID_GREEN_RED_OFFSET:		return "Gain Green Red Offset";
+	case V4L2_CID_GREEN_BLUE_OFFSET:	return "Gain Green Blue Offset";
 
 	/* DV controls */
 	case V4L2_CID_DV_CLASS:			return "Digital Video Controls";
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index f56c945..9b6b233 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -799,5 +799,16 @@  enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_LINK_FREQ			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 1)
 #define V4L2_CID_PIXEL_RATE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
 #define V4L2_CID_TEST_PATTERN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
+#define V4L2_CID_GAIN_RED			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 4)
+#define V4L2_CID_GAIN_GREEN_RED			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 5)
+#define V4L2_CID_GAIN_GREEN_BLUE		(V4L2_CID_IMAGE_PROC_CLASS_BASE + 6)
+#define V4L2_CID_GAIN_BLUE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 7)
+#define V4L2_CID_GAIN_GREEN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 8)
+#define V4L2_CID_GAIN_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 9)
+#define V4L2_CID_BLUE_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 10)
+#define V4L2_CID_RED_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 11)
+#define V4L2_CID_GREEN_OFFSET			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 12)
+#define V4L2_CID_GREEN_RED_OFFSET		(V4L2_CID_IMAGE_PROC_CLASS_BASE + 13)
+#define V4L2_CID_GREEN_BLUE_OFFSET		(V4L2_CID_IMAGE_PROC_CLASS_BASE + 14)
 
 #endif