diff mbox series

[v2,099/106] v4l: uapi: ccs: Add controls for analogue gain constants

Message ID 20201007084557.25843-90-sakari.ailus@linux.intel.com
State New
Headers show
Series None | expand

Commit Message

Sakari Ailus Oct. 7, 2020, 8:45 a.m. UTC
Add a V4L2 controls for analogue gai constants required to control
analogue gain. The values are device specific and thus need to be obtained
from the driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 MAINTAINERS              |  1 +
 include/uapi/linux/ccs.h | 14 ++++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 include/uapi/linux/ccs.h

Comments

Hans Verkuil Nov. 5, 2020, 12:41 p.m. UTC | #1
On 07/10/2020 10:45, Sakari Ailus wrote:
> Add a V4L2 controls for analogue gai constants required to control

gai -> gain

> analogue gain. The values are device specific and thus need to be obtained
> from the driver.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  MAINTAINERS              |  1 +
>  include/uapi/linux/ccs.h | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
>  create mode 100644 include/uapi/linux/ccs.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b72f666b8b60..c173e503b0b7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11569,6 +11569,7 @@ F:	drivers/media/i2c/ccs/
>  F:	drivers/media/i2c/ccs-pll.c
>  F:	drivers/media/i2c/ccs-pll.h
>  F:	include/uapi/linux/smiapp.h
> +F:	include/uapi/linux/ccs.h
>  
>  MIPS
>  M:	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
> new file mode 100644
> index 000000000000..bcdce95955b0
> --- /dev/null
> +++ b/include/uapi/linux/ccs.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only AND BSD-3-Clause */
> +/* Copyright (C) 2020 Intel Corporation */
> +
> +#ifndef __UAPI_CCS_H__
> +#define __UAPI_CCS_H__
> +
> +#include <linux/videodev2.h>
> +
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_M0		(V4L2_CID_USER_CCS_BASE + 1)
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_C0		(V4L2_CID_USER_CCS_BASE + 2)
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_M1		(V4L2_CID_USER_CCS_BASE + 3)
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)

Please document these controls. Ditto for the later patches that add more
of these controls.

This header is a good place to put the documentation.

Regards,

	Hans

> +
> +#endif
>
Sakari Ailus Nov. 5, 2020, 12:47 p.m. UTC | #2
Hi Hans,

On Thu, Nov 05, 2020 at 01:41:55PM +0100, Hans Verkuil wrote:
> On 07/10/2020 10:45, Sakari Ailus wrote:
> > Add a V4L2 controls for analogue gai constants required to control
> 
> gai -> gain
> 
> > analogue gain. The values are device specific and thus need to be obtained
> > from the driver.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  MAINTAINERS              |  1 +
> >  include/uapi/linux/ccs.h | 14 ++++++++++++++
> >  2 files changed, 15 insertions(+)
> >  create mode 100644 include/uapi/linux/ccs.h
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b72f666b8b60..c173e503b0b7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11569,6 +11569,7 @@ F:	drivers/media/i2c/ccs/
> >  F:	drivers/media/i2c/ccs-pll.c
> >  F:	drivers/media/i2c/ccs-pll.h
> >  F:	include/uapi/linux/smiapp.h
> > +F:	include/uapi/linux/ccs.h
> >  
> >  MIPS
> >  M:	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
> > new file mode 100644
> > index 000000000000..bcdce95955b0
> > --- /dev/null
> > +++ b/include/uapi/linux/ccs.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only AND BSD-3-Clause */
> > +/* Copyright (C) 2020 Intel Corporation */
> > +
> > +#ifndef __UAPI_CCS_H__
> > +#define __UAPI_CCS_H__
> > +
> > +#include <linux/videodev2.h>
> > +
> > +#define V4L2_CID_CCS_ANALOGUE_GAIN_M0		(V4L2_CID_USER_CCS_BASE + 1)
> > +#define V4L2_CID_CCS_ANALOGUE_GAIN_C0		(V4L2_CID_USER_CCS_BASE + 2)
> > +#define V4L2_CID_CCS_ANALOGUE_GAIN_M1		(V4L2_CID_USER_CCS_BASE + 3)
> > +#define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)
> 
> Please document these controls. Ditto for the later patches that add more
> of these controls.
> 
> This header is a good place to put the documentation.

Yes, I'll add that for v3.
Hans Verkuil Nov. 5, 2020, 12:56 p.m. UTC | #3
On 07/10/2020 10:45, Sakari Ailus wrote:
> Add a V4L2 controls for analogue gai constants required to control
> analogue gain. The values are device specific and thus need to be obtained
> from the driver.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  MAINTAINERS              |  1 +
>  include/uapi/linux/ccs.h | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
>  create mode 100644 include/uapi/linux/ccs.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b72f666b8b60..c173e503b0b7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11569,6 +11569,7 @@ F:	drivers/media/i2c/ccs/
>  F:	drivers/media/i2c/ccs-pll.c
>  F:	drivers/media/i2c/ccs-pll.h
>  F:	include/uapi/linux/smiapp.h
> +F:	include/uapi/linux/ccs.h
>  
>  MIPS
>  M:	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
> new file mode 100644
> index 000000000000..bcdce95955b0
> --- /dev/null
> +++ b/include/uapi/linux/ccs.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only AND BSD-3-Clause */
> +/* Copyright (C) 2020 Intel Corporation */
> +
> +#ifndef __UAPI_CCS_H__
> +#define __UAPI_CCS_H__

What does CCS stand for? Provide a reference to the standard as well.

Just looking at this header doesn't give you any clue as to what it
related to.

Regards,

	Hans

> +
> +#include <linux/videodev2.h>
> +
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_M0		(V4L2_CID_USER_CCS_BASE + 1)
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_C0		(V4L2_CID_USER_CCS_BASE + 2)
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_M1		(V4L2_CID_USER_CCS_BASE + 3)
> +#define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)
> +
> +#endif
>
Sakari Ailus Nov. 5, 2020, 12:58 p.m. UTC | #4
Hi Hans,

On Thu, Nov 05, 2020 at 01:56:02PM +0100, Hans Verkuil wrote:
> On 07/10/2020 10:45, Sakari Ailus wrote:
> > Add a V4L2 controls for analogue gai constants required to control
> > analogue gain. The values are device specific and thus need to be obtained
> > from the driver.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  MAINTAINERS              |  1 +
> >  include/uapi/linux/ccs.h | 14 ++++++++++++++
> >  2 files changed, 15 insertions(+)
> >  create mode 100644 include/uapi/linux/ccs.h
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b72f666b8b60..c173e503b0b7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11569,6 +11569,7 @@ F:	drivers/media/i2c/ccs/
> >  F:	drivers/media/i2c/ccs-pll.c
> >  F:	drivers/media/i2c/ccs-pll.h
> >  F:	include/uapi/linux/smiapp.h
> > +F:	include/uapi/linux/ccs.h
> >  
> >  MIPS
> >  M:	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
> > new file mode 100644
> > index 000000000000..bcdce95955b0
> > --- /dev/null
> > +++ b/include/uapi/linux/ccs.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only AND BSD-3-Clause */
> > +/* Copyright (C) 2020 Intel Corporation */
> > +
> > +#ifndef __UAPI_CCS_H__
> > +#define __UAPI_CCS_H__
> 
> What does CCS stand for? Provide a reference to the standard as well.
> 
> Just looking at this header doesn't give you any clue as to what it
> related to.

I'll add references to driver documentation I'm about to write to address
Mauro's review comments. CCS stands for Camera Command Set.

The spec can be found here:

<URL:https://www.mipi.org/specifications/camera-command-set>
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index b72f666b8b60..c173e503b0b7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11569,6 +11569,7 @@  F:	drivers/media/i2c/ccs/
 F:	drivers/media/i2c/ccs-pll.c
 F:	drivers/media/i2c/ccs-pll.h
 F:	include/uapi/linux/smiapp.h
+F:	include/uapi/linux/ccs.h
 
 MIPS
 M:	Thomas Bogendoerfer <tsbogend@alpha.franken.de>
diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
new file mode 100644
index 000000000000..bcdce95955b0
--- /dev/null
+++ b/include/uapi/linux/ccs.h
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only AND BSD-3-Clause */
+/* Copyright (C) 2020 Intel Corporation */
+
+#ifndef __UAPI_CCS_H__
+#define __UAPI_CCS_H__
+
+#include <linux/videodev2.h>
+
+#define V4L2_CID_CCS_ANALOGUE_GAIN_M0		(V4L2_CID_USER_CCS_BASE + 1)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_C0		(V4L2_CID_USER_CCS_BASE + 2)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_M1		(V4L2_CID_USER_CCS_BASE + 3)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)
+
+#endif