[REVIEW PATCH 0/6] SDR API - V4L implement needed controls and formats

[REVIEW PATCH 0/6] SDR API - V4L implement needed controls and formats

[Antti Palosaari]

Split / group / merge changes as requested by Hans.

Implement needed V4L2 controls and V4L stream formats.

Antti

Antti Palosaari (6):
  v4l: add RF tuner gain controls
  v4l: add RF tuner channel bandwidth control
  v4l: reorganize RF tuner control ID numbers
  v4l: uapi: add SDR formats CU8 and CU16LE
  v4l: add enum_freq_bands support to tuner sub-device
  v4l: add control for RF tuner PLL lock flag

 drivers/media/v4l2-core/v4l2-ctrls.c | 24 ++++++++++++++++++++++++
 include/media/v4l2-subdev.h          |  1 +
 include/uapi/linux/v4l2-controls.h   | 14 ++++++++++++++
 include/uapi/linux/videodev2.h       |  4 ++++
 4 files changed, 43 insertions(+)

-- 
1.8.5.3

[REVIEW PATCH 1/6] v4l: add RF tuner gain controls

[Antti Palosaari]

Modern silicon RF tuners used nowadays has many controllable gain
stages on signal path. Usually, but not always, there is at least
3 gain stages. Also on some cases there could be multiple gain
stages within the ones specified here. However, I think that having
these three controllable gain stages offers enough fine-tuning for
real use cases.

1) LNA gain. That is first gain just after antenna input.
2) Mixer gain. It is located quite middle of the signal path, where
RF signal is down-converted to IF/BB.
3) IF gain. That is last gain in order to adjust output signal level
to optimal level for receiving party (usually demodulator ADC).

Each gain stage could be set rather often both manual or automatic
(AGC) mode. Due to that add separate controls for controlling
operation mode.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
 include/uapi/linux/v4l2-controls.h   | 11 +++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 6ff002b..d201f61 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -857,6 +857,14 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
 	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
 	case V4L2_CID_RDS_RECEPTION:		return "RDS Reception";
+
+	case V4L2_CID_RF_TUNER_CLASS:		return "RF Tuner Controls";
+	case V4L2_CID_LNA_GAIN_AUTO:		return "LNA Gain, Auto";
+	case V4L2_CID_LNA_GAIN:			return "LNA Gain";
+	case V4L2_CID_MIXER_GAIN_AUTO:		return "Mixer Gain, Auto";
+	case V4L2_CID_MIXER_GAIN:		return "Mixer Gain";
+	case V4L2_CID_IF_GAIN_AUTO:		return "IF Gain, Auto";
+	case V4L2_CID_IF_GAIN:			return "IF Gain";
 	default:
 		return NULL;
 	}
@@ -906,6 +914,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_WIDE_DYNAMIC_RANGE:
 	case V4L2_CID_IMAGE_STABILIZATION:
 	case V4L2_CID_RDS_RECEPTION:
+	case V4L2_CID_LNA_GAIN_AUTO:
+	case V4L2_CID_MIXER_GAIN_AUTO:
+	case V4L2_CID_IF_GAIN_AUTO:
 		*type = V4L2_CTRL_TYPE_BOOLEAN;
 		*min = 0;
 		*max = *step = 1;
@@ -991,6 +1002,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_IMAGE_PROC_CLASS:
 	case V4L2_CID_DV_CLASS:
 	case V4L2_CID_FM_RX_CLASS:
+	case V4L2_CID_RF_TUNER_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
@@ -1063,6 +1075,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_PILOT_TONE_FREQUENCY:
 	case V4L2_CID_TUNE_POWER_LEVEL:
 	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
+	case V4L2_CID_LNA_GAIN:
+	case V4L2_CID_MIXER_GAIN:
+	case V4L2_CID_IF_GAIN:
 		*flags |= V4L2_CTRL_FLAG_SLIDER;
 		break;
 	case V4L2_CID_PAN_RELATIVE:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 2cbe605..076fa34 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -60,6 +60,7 @@
 #define V4L2_CTRL_CLASS_IMAGE_PROC	0x009f0000	/* Image processing controls */
 #define V4L2_CTRL_CLASS_DV		0x00a00000	/* Digital Video controls */
 #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
+#define V4L2_CTRL_CLASS_RF_TUNER	0x00a20000	/* RF tuner controls */
 
 /* User-class control IDs */
 
@@ -895,4 +896,14 @@ enum v4l2_deemphasis {
 
 #define V4L2_CID_RDS_RECEPTION			(V4L2_CID_FM_RX_CLASS_BASE + 2)
 
+#define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
+#define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
+
+#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
+#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
+#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
+#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
+#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
+#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
+
 #endif
-- 
1.8.5.3

[REVIEW PATCH 2/6] v4l: add RF tuner channel bandwidth control

[Antti Palosaari]

Modern silicon RF tuners has one or more adjustable filters on
signal path, in order to filter noise from desired radio channel.

Add channel bandwidth control to tell the driver which is radio
channel width we want receive. Filters could be then adjusted by
the driver or hardware, using RF frequency and channel bandwidth
as a base of filter calculations.

On automatic mode (normal mode), bandwidth is calculated from sampling
rate or tuning info got from userspace. That new control gives
possibility to set manual mode and let user have more control for
filters.

Cc: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++
 include/uapi/linux/v4l2-controls.h   | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index d201f61..e44722b 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -865,6 +865,8 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MIXER_GAIN:		return "Mixer Gain";
 	case V4L2_CID_IF_GAIN_AUTO:		return "IF Gain, Auto";
 	case V4L2_CID_IF_GAIN:			return "IF Gain";
+	case V4L2_CID_BANDWIDTH_AUTO:		return "Channel Bandwidth, Auto";
+	case V4L2_CID_BANDWIDTH:		return "Channel Bandwidth";
 	default:
 		return NULL;
 	}
@@ -917,6 +919,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_LNA_GAIN_AUTO:
 	case V4L2_CID_MIXER_GAIN_AUTO:
 	case V4L2_CID_IF_GAIN_AUTO:
+	case V4L2_CID_BANDWIDTH_AUTO:
 		*type = V4L2_CTRL_TYPE_BOOLEAN;
 		*min = 0;
 		*max = *step = 1;
@@ -1078,6 +1081,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_LNA_GAIN:
 	case V4L2_CID_MIXER_GAIN:
 	case V4L2_CID_IF_GAIN:
+	case V4L2_CID_BANDWIDTH:
 		*flags |= V4L2_CTRL_FLAG_SLIDER;
 		break;
 	case V4L2_CID_PAN_RELATIVE:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 076fa34..3cf68a6 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -905,5 +905,7 @@ enum v4l2_deemphasis {
 #define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
 #define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
 #define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
+#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 7)
+#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 8)
 
 #endif
-- 
1.8.5.3

[REVIEW PATCH 3/6] v4l: reorganize RF tuner control ID numbers

[Antti Palosaari]

It appears that controls are ordered by ID number. Change order of
controls by reorganizing assigned IDs now as we can. It is not
reasonable possible after the API is released. Leave some spare
space between IDs too for future extensions.

Cc: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 include/uapi/linux/v4l2-controls.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 3cf68a6..cc488c3 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -899,13 +899,13 @@ enum v4l2_deemphasis {
 #define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
 #define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
 
-#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
-#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
-#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
-#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
-#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
-#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
-#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 7)
-#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 8)
+#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 11)
+#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 12)
+#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 41)
+#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 42)
+#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 51)
+#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 52)
+#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 61)
+#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 62)
 
 #endif
-- 
1.8.5.3

[REVIEW PATCH 4/6] v4l: uapi: add SDR formats CU8 and CU16LE

[Antti Palosaari]

V4L2_SDR_FMT_CU8 — Complex unsigned 8-bit IQ sample
V4L2_SDR_FMT_CU16LE — Complex unsigned 16-bit little endian IQ sample

Cc: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 include/uapi/linux/videodev2.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 27fedfe..3411215 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -436,6 +436,10 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_SE401      v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
 #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
 
+/* SDR formats - used only for Software Defined Radio devices */
+#define V4L2_SDR_FMT_CU8          v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */
+#define V4L2_SDR_FMT_CU16LE       v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */
+
 /*
  *	F O R M A T   E N U M E R A T I O N
  */
-- 
1.8.5.3

[REVIEW PATCH 5/6] v4l: add enum_freq_bands support to tuner sub-device

[Antti Palosaari]

Add VIDIOC_ENUM_FREQ_BANDS, enumerate supported frequency bands,
IOCTL support for sub-device tuners too.

Cc: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 include/media/v4l2-subdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d67210a..4682aad 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -192,6 +192,7 @@ struct v4l2_subdev_tuner_ops {
 	int (*s_radio)(struct v4l2_subdev *sd);
 	int (*s_frequency)(struct v4l2_subdev *sd, const struct v4l2_frequency *freq);
 	int (*g_frequency)(struct v4l2_subdev *sd, struct v4l2_frequency *freq);
+	int (*enum_freq_bands)(struct v4l2_subdev *sd, struct v4l2_frequency_band *band);
 	int (*g_tuner)(struct v4l2_subdev *sd, struct v4l2_tuner *vt);
 	int (*s_tuner)(struct v4l2_subdev *sd, const struct v4l2_tuner *vt);
 	int (*g_modulator)(struct v4l2_subdev *sd, struct v4l2_modulator *vm);
-- 
1.8.5.3

[REVIEW PATCH 6/6] v4l: add control for RF tuner PLL lock flag

[Antti Palosaari]

Add volatile boolean control to indicate if tuner frequency synthesizer
is locked to requested frequency. That means tuner is able to receive
given frequency. Control is named as "PLL lock", since frequency
synthesizers are based of phase-locked-loop. Maybe more general name
could be wise still?

Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 5 +++++
 include/uapi/linux/v4l2-controls.h   | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index e44722b..dc6cba4 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -867,6 +867,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_IF_GAIN:			return "IF Gain";
 	case V4L2_CID_BANDWIDTH_AUTO:		return "Channel Bandwidth, Auto";
 	case V4L2_CID_BANDWIDTH:		return "Channel Bandwidth";
+	case V4L2_CID_PLL_LOCK:			return "PLL Lock";
 	default:
 		return NULL;
 	}
@@ -920,6 +921,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MIXER_GAIN_AUTO:
 	case V4L2_CID_IF_GAIN_AUTO:
 	case V4L2_CID_BANDWIDTH_AUTO:
+	case V4L2_CID_PLL_LOCK:
 		*type = V4L2_CTRL_TYPE_BOOLEAN;
 		*min = 0;
 		*max = *step = 1;
@@ -1100,6 +1102,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_DV_RX_POWER_PRESENT:
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
 		break;
+	case V4L2_CID_PLL_LOCK:
+		*flags |= V4L2_CTRL_FLAG_VOLATILE;
+		break;
 	}
 }
 EXPORT_SYMBOL(v4l2_ctrl_fill);
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index cc488c3..06918c9 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -907,5 +907,6 @@ enum v4l2_deemphasis {
 #define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 52)
 #define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 61)
 #define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 62)
+#define V4L2_CID_PLL_LOCK			(V4L2_CID_RF_TUNER_CLASS_BASE + 91)
 
 #endif
-- 
1.8.5.3

Re: [REVIEW PATCH 3/6] v4l: reorganize RF tuner control ID numbers

[Hans Verkuil]

Hi Antti,

On 02/10/2014 05:17 PM, Antti Palosaari wrote:
> It appears that controls are ordered by ID number. Change order of
> controls by reorganizing assigned IDs now as we can. It is not
> reasonable possible after the API is released. Leave some spare
> space between IDs too for future extensions.

Am I missing something? I see no reason for this patch or for adding the
spare space.

Regards,

	Hans

> 
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  include/uapi/linux/v4l2-controls.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 3cf68a6..cc488c3 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -899,13 +899,13 @@ enum v4l2_deemphasis {
>  #define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
>  #define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
>  
> -#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
> -#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
> -#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
> -#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
> -#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
> -#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
> -#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 7)
> -#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 8)
> +#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 11)
> +#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 12)
> +#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 41)
> +#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 42)
> +#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 51)
> +#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 52)
> +#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 61)
> +#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 62)
>  
>  #endif
> 

Re: [REVIEW PATCH 1/6] v4l: add RF tuner gain controls

[Hans Verkuil]

On 02/10/2014 05:17 PM, Antti Palosaari wrote:
> Modern silicon RF tuners used nowadays has many controllable gain
> stages on signal path. Usually, but not always, there is at least
> 3 gain stages. Also on some cases there could be multiple gain
> stages within the ones specified here. However, I think that having
> these three controllable gain stages offers enough fine-tuning for
> real use cases.
> 
> 1) LNA gain. That is first gain just after antenna input.
> 2) Mixer gain. It is located quite middle of the signal path, where
> RF signal is down-converted to IF/BB.
> 3) IF gain. That is last gain in order to adjust output signal level
> to optimal level for receiving party (usually demodulator ADC).
> 
> Each gain stage could be set rather often both manual or automatic
> (AGC) mode. Due to that add separate controls for controlling
> operation mode.
> 
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
>  include/uapi/linux/v4l2-controls.h   | 11 +++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 6ff002b..d201f61 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -857,6 +857,14 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
>  	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
>  	case V4L2_CID_RDS_RECEPTION:		return "RDS Reception";
> +
> +	case V4L2_CID_RF_TUNER_CLASS:		return "RF Tuner Controls";
> +	case V4L2_CID_LNA_GAIN_AUTO:		return "LNA Gain, Auto";
> +	case V4L2_CID_LNA_GAIN:			return "LNA Gain";
> +	case V4L2_CID_MIXER_GAIN_AUTO:		return "Mixer Gain, Auto";
> +	case V4L2_CID_MIXER_GAIN:		return "Mixer Gain";
> +	case V4L2_CID_IF_GAIN_AUTO:		return "IF Gain, Auto";
> +	case V4L2_CID_IF_GAIN:			return "IF Gain";
>  	default:
>  		return NULL;
>  	}
> @@ -906,6 +914,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_WIDE_DYNAMIC_RANGE:
>  	case V4L2_CID_IMAGE_STABILIZATION:
>  	case V4L2_CID_RDS_RECEPTION:
> +	case V4L2_CID_LNA_GAIN_AUTO:
> +	case V4L2_CID_MIXER_GAIN_AUTO:
> +	case V4L2_CID_IF_GAIN_AUTO:
>  		*type = V4L2_CTRL_TYPE_BOOLEAN;
>  		*min = 0;
>  		*max = *step = 1;
> @@ -991,6 +1002,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_IMAGE_PROC_CLASS:
>  	case V4L2_CID_DV_CLASS:
>  	case V4L2_CID_FM_RX_CLASS:
> +	case V4L2_CID_RF_TUNER_CLASS:
>  		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
>  		/* You can neither read not write these */
>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
> @@ -1063,6 +1075,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_PILOT_TONE_FREQUENCY:
>  	case V4L2_CID_TUNE_POWER_LEVEL:
>  	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
> +	case V4L2_CID_LNA_GAIN:
> +	case V4L2_CID_MIXER_GAIN:
> +	case V4L2_CID_IF_GAIN:
>  		*flags |= V4L2_CTRL_FLAG_SLIDER;
>  		break;
>  	case V4L2_CID_PAN_RELATIVE:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 2cbe605..076fa34 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -60,6 +60,7 @@
>  #define V4L2_CTRL_CLASS_IMAGE_PROC	0x009f0000	/* Image processing controls */
>  #define V4L2_CTRL_CLASS_DV		0x00a00000	/* Digital Video controls */
>  #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
> +#define V4L2_CTRL_CLASS_RF_TUNER	0x00a20000	/* RF tuner controls */
>  
>  /* User-class control IDs */
>  
> @@ -895,4 +896,14 @@ enum v4l2_deemphasis {
>  
>  #define V4L2_CID_RDS_RECEPTION			(V4L2_CID_FM_RX_CLASS_BASE + 2)
>  
> +#define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
> +#define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
> +
> +#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
> +#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
> +#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
> +#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
> +#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
> +#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)

I would prefer to give these control defines a prefix:

V4L2_CID_RF_TUNER_MIXER_GAIN_AUTO (or possibly V4L2_CID_RF_TNR_...)

'MIXER_GAIN' by itself does not make it clear it relates to a tuner, it
might just as well refer to audio mixing or video mixing.

Thinking this over I am wondering whether these controls might not fit
just as well with the FM_RX class. Yeah, I know, the 'FM' part is a bit
unfortunate in that context, but it is about radio receivers as well.

Unless there is a good reason to keep these controls in their own class?

Regards,

	Hans

> +
>  #endif
> 

Re: [REVIEW PATCH 2/6] v4l: add RF tuner channel bandwidth control

[Hans Verkuil]

On 02/10/2014 05:17 PM, Antti Palosaari wrote:
> Modern silicon RF tuners has one or more adjustable filters on
> signal path, in order to filter noise from desired radio channel.
> 
> Add channel bandwidth control to tell the driver which is radio
> channel width we want receive. Filters could be then adjusted by
> the driver or hardware, using RF frequency and channel bandwidth
> as a base of filter calculations.
> 
> On automatic mode (normal mode), bandwidth is calculated from sampling
> rate or tuning info got from userspace. That new control gives
> possibility to set manual mode and let user have more control for
> filters.
> 
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++
>  include/uapi/linux/v4l2-controls.h   | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index d201f61..e44722b 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -865,6 +865,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MIXER_GAIN:		return "Mixer Gain";
>  	case V4L2_CID_IF_GAIN_AUTO:		return "IF Gain, Auto";
>  	case V4L2_CID_IF_GAIN:			return "IF Gain";
> +	case V4L2_CID_BANDWIDTH_AUTO:		return "Channel Bandwidth, Auto";
> +	case V4L2_CID_BANDWIDTH:		return "Channel Bandwidth";
>  	default:
>  		return NULL;
>  	}
> @@ -917,6 +919,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_LNA_GAIN_AUTO:
>  	case V4L2_CID_MIXER_GAIN_AUTO:
>  	case V4L2_CID_IF_GAIN_AUTO:
> +	case V4L2_CID_BANDWIDTH_AUTO:
>  		*type = V4L2_CTRL_TYPE_BOOLEAN;
>  		*min = 0;
>  		*max = *step = 1;
> @@ -1078,6 +1081,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_LNA_GAIN:
>  	case V4L2_CID_MIXER_GAIN:
>  	case V4L2_CID_IF_GAIN:
> +	case V4L2_CID_BANDWIDTH:
>  		*flags |= V4L2_CTRL_FLAG_SLIDER;
>  		break;
>  	case V4L2_CID_PAN_RELATIVE:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 076fa34..3cf68a6 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -905,5 +905,7 @@ enum v4l2_deemphasis {
>  #define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
>  #define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
>  #define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
> +#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 7)
> +#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 8)

This definitely needs a prefix. Bandwidth can refer to so many things that
we have to be clear here.

	Hans

>  
>  #endif
> 

Re: [REVIEW PATCH 3/6] v4l: reorganize RF tuner control ID numbers

[Antti Palosaari]

On 14.02.2014 16:20, Hans Verkuil wrote:
> Hi Antti,
>
> On 02/10/2014 05:17 PM, Antti Palosaari wrote:
>> It appears that controls are ordered by ID number. Change order of
>> controls by reorganizing assigned IDs now as we can. It is not
>> reasonable possible after the API is released. Leave some spare
>> space between IDs too for future extensions.
>
> Am I missing something? I see no reason for this patch or for adding the
> spare space.

"It appears that controls are ordered by ID number"

I used app called v4l2ucp and at least it seems to organize those 
controls by ID. Is there some way to say application how those are 
organized?

regards
Antti


>
> Regards,
>
> 	Hans
>
>>
>> Cc: Hans Verkuil <hverkuil@xs4all.nl>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>>   include/uapi/linux/v4l2-controls.h | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>> index 3cf68a6..cc488c3 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -899,13 +899,13 @@ enum v4l2_deemphasis {
>>   #define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
>>   #define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
>>
>> -#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
>> -#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
>> -#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
>> -#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
>> -#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
>> -#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
>> -#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 7)
>> -#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 8)
>> +#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 11)
>> +#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 12)
>> +#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 41)
>> +#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 42)
>> +#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 51)
>> +#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 52)
>> +#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 61)
>> +#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 62)
>>
>>   #endif
>>
>


-- 
http://palosaari.fi/

Re: [REVIEW PATCH 1/6] v4l: add RF tuner gain controls

[Antti Palosaari]

On 14.02.2014 16:30, Hans Verkuil wrote:
> On 02/10/2014 05:17 PM, Antti Palosaari wrote:
>> Modern silicon RF tuners used nowadays has many controllable gain
>> stages on signal path. Usually, but not always, there is at least
>> 3 gain stages. Also on some cases there could be multiple gain
>> stages within the ones specified here. However, I think that having
>> these three controllable gain stages offers enough fine-tuning for
>> real use cases.
>>
>> 1) LNA gain. That is first gain just after antenna input.
>> 2) Mixer gain. It is located quite middle of the signal path, where
>> RF signal is down-converted to IF/BB.
>> 3) IF gain. That is last gain in order to adjust output signal level
>> to optimal level for receiving party (usually demodulator ADC).
>>
>> Each gain stage could be set rather often both manual or automatic
>> (AGC) mode. Due to that add separate controls for controlling
>> operation mode.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>>   drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
>>   include/uapi/linux/v4l2-controls.h   | 11 +++++++++++
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 6ff002b..d201f61 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -857,6 +857,14 @@ const char *v4l2_ctrl_get_name(u32 id)
>>   	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
>>   	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
>>   	case V4L2_CID_RDS_RECEPTION:		return "RDS Reception";
>> +
>> +	case V4L2_CID_RF_TUNER_CLASS:		return "RF Tuner Controls";
>> +	case V4L2_CID_LNA_GAIN_AUTO:		return "LNA Gain, Auto";
>> +	case V4L2_CID_LNA_GAIN:			return "LNA Gain";
>> +	case V4L2_CID_MIXER_GAIN_AUTO:		return "Mixer Gain, Auto";
>> +	case V4L2_CID_MIXER_GAIN:		return "Mixer Gain";
>> +	case V4L2_CID_IF_GAIN_AUTO:		return "IF Gain, Auto";
>> +	case V4L2_CID_IF_GAIN:			return "IF Gain";
>>   	default:
>>   		return NULL;
>>   	}
>> @@ -906,6 +914,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>   	case V4L2_CID_WIDE_DYNAMIC_RANGE:
>>   	case V4L2_CID_IMAGE_STABILIZATION:
>>   	case V4L2_CID_RDS_RECEPTION:
>> +	case V4L2_CID_LNA_GAIN_AUTO:
>> +	case V4L2_CID_MIXER_GAIN_AUTO:
>> +	case V4L2_CID_IF_GAIN_AUTO:
>>   		*type = V4L2_CTRL_TYPE_BOOLEAN;
>>   		*min = 0;
>>   		*max = *step = 1;
>> @@ -991,6 +1002,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>   	case V4L2_CID_IMAGE_PROC_CLASS:
>>   	case V4L2_CID_DV_CLASS:
>>   	case V4L2_CID_FM_RX_CLASS:
>> +	case V4L2_CID_RF_TUNER_CLASS:
>>   		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
>>   		/* You can neither read not write these */
>>   		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
>> @@ -1063,6 +1075,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>   	case V4L2_CID_PILOT_TONE_FREQUENCY:
>>   	case V4L2_CID_TUNE_POWER_LEVEL:
>>   	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
>> +	case V4L2_CID_LNA_GAIN:
>> +	case V4L2_CID_MIXER_GAIN:
>> +	case V4L2_CID_IF_GAIN:
>>   		*flags |= V4L2_CTRL_FLAG_SLIDER;
>>   		break;
>>   	case V4L2_CID_PAN_RELATIVE:
>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>> index 2cbe605..076fa34 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -60,6 +60,7 @@
>>   #define V4L2_CTRL_CLASS_IMAGE_PROC	0x009f0000	/* Image processing controls */
>>   #define V4L2_CTRL_CLASS_DV		0x00a00000	/* Digital Video controls */
>>   #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
>> +#define V4L2_CTRL_CLASS_RF_TUNER	0x00a20000	/* RF tuner controls */
>>
>>   /* User-class control IDs */
>>
>> @@ -895,4 +896,14 @@ enum v4l2_deemphasis {
>>
>>   #define V4L2_CID_RDS_RECEPTION			(V4L2_CID_FM_RX_CLASS_BASE + 2)
>>
>> +#define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
>> +#define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
>> +
>> +#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
>> +#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
>> +#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
>> +#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
>> +#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
>> +#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
>
> I would prefer to give these control defines a prefix:
>
> V4L2_CID_RF_TUNER_MIXER_GAIN_AUTO (or possibly V4L2_CID_RF_TNR_...)
>
> 'MIXER_GAIN' by itself does not make it clear it relates to a tuner, it
> might just as well refer to audio mixing or video mixing.
>
> Thinking this over I am wondering whether these controls might not fit
> just as well with the FM_RX class. Yeah, I know, the 'FM' part is a bit
> unfortunate in that context, but it is about radio receivers as well.
>
> Unless there is a good reason to keep these controls in their own class?

Those controls in FM RX class are way layer or two upper level. Controls 
in FM RX are from the demulation whilst the controls I added to RF tuner 
are from RF tuner and are suitable for every device having RF tuner, 
like radio, television, GPS, etc.

regards
Antti

-- 
http://palosaari.fi/

Re: [REVIEW PATCH 3/6] v4l: reorganize RF tuner control ID numbers

[Hans Verkuil]

On 02/14/2014 03:36 PM, Antti Palosaari wrote:
> On 14.02.2014 16:20, Hans Verkuil wrote:
>> Hi Antti,
>>
>> On 02/10/2014 05:17 PM, Antti Palosaari wrote:
>>> It appears that controls are ordered by ID number. Change order of
>>> controls by reorganizing assigned IDs now as we can. It is not
>>> reasonable possible after the API is released. Leave some spare
>>> space between IDs too for future extensions.
>>
>> Am I missing something? I see no reason for this patch or for adding the
>> spare space.
> 
> "It appears that controls are ordered by ID number"
> 
> I used app called v4l2ucp and at least it seems to organize those 
> controls by ID. Is there some way to say application how those are 
> organized?

Ah, now I get it. When enumerating controls they are enumerated by
increasing control ID. Yes, that's correct.

The commit message could be improved since I completely missed the point :-)

Regards,

	Hans

> 
> regards
> Antti
> 
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Cc: Hans Verkuil <hverkuil@xs4all.nl>
>>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>>> ---
>>>   include/uapi/linux/v4l2-controls.h | 16 ++++++++--------
>>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>> index 3cf68a6..cc488c3 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -899,13 +899,13 @@ enum v4l2_deemphasis {
>>>   #define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
>>>   #define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
>>>
>>> -#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
>>> -#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
>>> -#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
>>> -#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
>>> -#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
>>> -#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
>>> -#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 7)
>>> -#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 8)
>>> +#define V4L2_CID_BANDWIDTH_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 11)
>>> +#define V4L2_CID_BANDWIDTH			(V4L2_CID_RF_TUNER_CLASS_BASE + 12)
>>> +#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 41)
>>> +#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 42)
>>> +#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 51)
>>> +#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 52)
>>> +#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 61)
>>> +#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 62)
>>>
>>>   #endif
>>>
>>
> 
> 

Re: [REVIEW PATCH 1/6] v4l: add RF tuner gain controls

[Hans Verkuil]

On 02/14/2014 03:44 PM, Antti Palosaari wrote:
> On 14.02.2014 16:30, Hans Verkuil wrote:
>> On 02/10/2014 05:17 PM, Antti Palosaari wrote:
>>> Modern silicon RF tuners used nowadays has many controllable gain
>>> stages on signal path. Usually, but not always, there is at least
>>> 3 gain stages. Also on some cases there could be multiple gain
>>> stages within the ones specified here. However, I think that having
>>> these three controllable gain stages offers enough fine-tuning for
>>> real use cases.
>>>
>>> 1) LNA gain. That is first gain just after antenna input.
>>> 2) Mixer gain. It is located quite middle of the signal path, where
>>> RF signal is down-converted to IF/BB.
>>> 3) IF gain. That is last gain in order to adjust output signal level
>>> to optimal level for receiving party (usually demodulator ADC).
>>>
>>> Each gain stage could be set rather often both manual or automatic
>>> (AGC) mode. Due to that add separate controls for controlling
>>> operation mode.
>>>
>>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>>> ---
>>>   drivers/media/v4l2-core/v4l2-ctrls.c | 15 +++++++++++++++
>>>   include/uapi/linux/v4l2-controls.h   | 11 +++++++++++
>>>   2 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index 6ff002b..d201f61 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -857,6 +857,14 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>   	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
>>>   	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
>>>   	case V4L2_CID_RDS_RECEPTION:		return "RDS Reception";
>>> +
>>> +	case V4L2_CID_RF_TUNER_CLASS:		return "RF Tuner Controls";
>>> +	case V4L2_CID_LNA_GAIN_AUTO:		return "LNA Gain, Auto";
>>> +	case V4L2_CID_LNA_GAIN:			return "LNA Gain";
>>> +	case V4L2_CID_MIXER_GAIN_AUTO:		return "Mixer Gain, Auto";
>>> +	case V4L2_CID_MIXER_GAIN:		return "Mixer Gain";
>>> +	case V4L2_CID_IF_GAIN_AUTO:		return "IF Gain, Auto";
>>> +	case V4L2_CID_IF_GAIN:			return "IF Gain";
>>>   	default:
>>>   		return NULL;
>>>   	}
>>> @@ -906,6 +914,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>   	case V4L2_CID_WIDE_DYNAMIC_RANGE:
>>>   	case V4L2_CID_IMAGE_STABILIZATION:
>>>   	case V4L2_CID_RDS_RECEPTION:
>>> +	case V4L2_CID_LNA_GAIN_AUTO:
>>> +	case V4L2_CID_MIXER_GAIN_AUTO:
>>> +	case V4L2_CID_IF_GAIN_AUTO:
>>>   		*type = V4L2_CTRL_TYPE_BOOLEAN;
>>>   		*min = 0;
>>>   		*max = *step = 1;
>>> @@ -991,6 +1002,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>   	case V4L2_CID_IMAGE_PROC_CLASS:
>>>   	case V4L2_CID_DV_CLASS:
>>>   	case V4L2_CID_FM_RX_CLASS:
>>> +	case V4L2_CID_RF_TUNER_CLASS:
>>>   		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
>>>   		/* You can neither read not write these */
>>>   		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
>>> @@ -1063,6 +1075,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>   	case V4L2_CID_PILOT_TONE_FREQUENCY:
>>>   	case V4L2_CID_TUNE_POWER_LEVEL:
>>>   	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
>>> +	case V4L2_CID_LNA_GAIN:
>>> +	case V4L2_CID_MIXER_GAIN:
>>> +	case V4L2_CID_IF_GAIN:
>>>   		*flags |= V4L2_CTRL_FLAG_SLIDER;
>>>   		break;
>>>   	case V4L2_CID_PAN_RELATIVE:
>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>> index 2cbe605..076fa34 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -60,6 +60,7 @@
>>>   #define V4L2_CTRL_CLASS_IMAGE_PROC	0x009f0000	/* Image processing controls */
>>>   #define V4L2_CTRL_CLASS_DV		0x00a00000	/* Digital Video controls */
>>>   #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
>>> +#define V4L2_CTRL_CLASS_RF_TUNER	0x00a20000	/* RF tuner controls */
>>>
>>>   /* User-class control IDs */
>>>
>>> @@ -895,4 +896,14 @@ enum v4l2_deemphasis {
>>>
>>>   #define V4L2_CID_RDS_RECEPTION			(V4L2_CID_FM_RX_CLASS_BASE + 2)
>>>
>>> +#define V4L2_CID_RF_TUNER_CLASS_BASE		(V4L2_CTRL_CLASS_RF_TUNER | 0x900)
>>> +#define V4L2_CID_RF_TUNER_CLASS			(V4L2_CTRL_CLASS_RF_TUNER | 1)
>>> +
>>> +#define V4L2_CID_LNA_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 1)
>>> +#define V4L2_CID_LNA_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 2)
>>> +#define V4L2_CID_MIXER_GAIN_AUTO		(V4L2_CID_RF_TUNER_CLASS_BASE + 3)
>>> +#define V4L2_CID_MIXER_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 4)
>>> +#define V4L2_CID_IF_GAIN_AUTO			(V4L2_CID_RF_TUNER_CLASS_BASE + 5)
>>> +#define V4L2_CID_IF_GAIN			(V4L2_CID_RF_TUNER_CLASS_BASE + 6)
>>
>> I would prefer to give these control defines a prefix:
>>
>> V4L2_CID_RF_TUNER_MIXER_GAIN_AUTO (or possibly V4L2_CID_RF_TNR_...)
>>
>> 'MIXER_GAIN' by itself does not make it clear it relates to a tuner, it
>> might just as well refer to audio mixing or video mixing.
>>
>> Thinking this over I am wondering whether these controls might not fit
>> just as well with the FM_RX class. Yeah, I know, the 'FM' part is a bit
>> unfortunate in that context, but it is about radio receivers as well.
>>
>> Unless there is a good reason to keep these controls in their own class?
> 
> Those controls in FM RX class are way layer or two upper level. Controls 
> in FM RX are from the demulation whilst the controls I added to RF tuner 
> are from RF tuner and are suitable for every device having RF tuner, 
> like radio, television, GPS, etc.

Fair enough. It would be good though if this could be clarified in the description
of this control class in DocBook. It's fairly specialized stuff, so some background
information or perhaps a link to wikipedia (if there is a suitable article there)
can be helpful.

Regards,

	Hans