[v9,1/4] V4L2: Add seek spacing and FM RX class.

  Add spacing field to v4l2_hw_freq_seek, add V4L2_CAP_RAW_RDS_ONLY
to driver capabilities and also add FM RX class to control classes.

Signed-off-by: Matti J. Aaltonen <matti.j.aaltonen@nokia.com>
---
 drivers/media/video/v4l2-ctrls.c |   12 ++++++++++++
 include/linux/videodev2.h        |   16 +++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletions(-)
  

Em 30-08-2010 08:38, Matti J. Aaltonen escreveu:
> Add spacing field to v4l2_hw_freq_seek, add V4L2_CAP_RAW_RDS_ONLY
> to driver capabilities and also add FM RX class to control classes.
> 
> Signed-off-by: Matti J. Aaltonen <matti.j.aaltonen@nokia.com>
> ---
>  drivers/media/video/v4l2-ctrls.c |   12 ++++++++++++
>  include/linux/videodev2.h        |   16 +++++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
> index ea8d32c..15749f1 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -216,6 +216,12 @@ const char **v4l2_ctrl_get_menu(u32 id)
>  		"75 useconds",
>  		NULL,
>  	};
> +	static const char *fm_band[] = {
> +		"87.5 - 108. MHz",
> +		"76. - 90. MHz, Japan",
> +		"65. - 74. MHz, OIRT",
> +		NULL,
> +	};
>  
>  	switch (id) {
>  	case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> @@ -256,6 +262,8 @@ const char **v4l2_ctrl_get_menu(u32 id)
>  		return colorfx;
>  	case V4L2_CID_TUNE_PREEMPHASIS:
>  		return tune_preemphasis;
> +	case V4L2_CID_FM_BAND:
> +		return fm_band;
>  	default:
>  		return NULL;
>  	}
> @@ -386,6 +394,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_TUNE_PREEMPHASIS:		return "Pre-emphasis settings";
>  	case V4L2_CID_TUNE_POWER_LEVEL:		return "Tune Power Level";
>  	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:	return "Tune Antenna Capacitor";
> +	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Tuner Controls";

> +	case V4L2_CID_FM_BAND:			return "FM Band";


There's no need for a FM control, as there's already an ioctl pair that allows get/set the frequency
bandwidth: VIDIOC_S_TUNER and VIDIOC_G_TUNER. So, the entire patch here seems uneeded/unwanted.
>  
>  	default:
>  		return NULL;
> @@ -448,6 +458,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_EXPOSURE_AUTO:
>  	case V4L2_CID_COLORFX:
>  	case V4L2_CID_TUNE_PREEMPHASIS:
> +	case V4L2_CID_FM_BAND:
>  		*type = V4L2_CTRL_TYPE_MENU;
>  		break;
>  	case V4L2_CID_RDS_TX_PS_NAME:
> @@ -458,6 +469,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_CAMERA_CLASS:
>  	case V4L2_CID_MPEG_CLASS:
>  	case V4L2_CID_FM_TX_CLASS:
> +	case V4L2_CID_FM_RX_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;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 61490c6..7d6511e 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -244,6 +244,7 @@ struct v4l2_capability {
>  #define V4L2_CAP_VIDEO_OUTPUT_OVERLAY	0x00000200  /* Can do video output overlay */
>  #define V4L2_CAP_HW_FREQ_SEEK		0x00000400  /* Can do hardware frequency seek  */
>  #define V4L2_CAP_RDS_OUTPUT		0x00000800  /* Is an RDS encoder */
> +#define V4L2_CAP_RAW_RDS_ONLY		0x00001000  /* Does not interpret RDS data */
>  
>  #define V4L2_CAP_TUNER			0x00010000  /* has a tuner */
>  #define V4L2_CAP_AUDIO			0x00020000  /* has audio support */
> @@ -930,6 +931,7 @@ struct v4l2_ext_controls {
>  #define V4L2_CTRL_CLASS_MPEG 0x00990000	/* MPEG-compression controls */
>  #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
>  #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
> +#define V4L2_CTRL_CLASS_FM_RX 0x009c0000	/* FM Tuner control class */
>  
>  #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
>  #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
> @@ -1328,6 +1330,17 @@ enum v4l2_preemphasis {
>  #define V4L2_CID_TUNE_POWER_LEVEL		(V4L2_CID_FM_TX_CLASS_BASE + 113)
>  #define V4L2_CID_TUNE_ANTENNA_CAPACITOR		(V4L2_CID_FM_TX_CLASS_BASE + 114)
>  
> +/* FM Tuner class control IDs */
> +#define V4L2_CID_FM_RX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_RX | 0x900)
> +#define V4L2_CID_FM_RX_CLASS			(V4L2_CTRL_CLASS_FM_RX | 1)
> +
> +#define V4L2_CID_FM_BAND			(V4L2_CID_FM_RX_CLASS_BASE + 1)
> +enum v4l2_fm_band {
> +	V4L2_FM_BAND_OTHER		= 0,
> +	V4L2_FM_BAND_JAPAN		= 1,
> +	V4L2_FM_BAND_OIRT		= 2
> +};
> +
>  /*
>   *	T U N I N G
>   */
> @@ -1392,7 +1405,8 @@ struct v4l2_hw_freq_seek {
>  	enum v4l2_tuner_type  type;
>  	__u32		      seek_upward;
>  	__u32		      wrap_around;
> -	__u32		      reserved[8];
> +	__u32		      spacing;
> +	__u32		      reserved[7];
>  };
>  
>  /*

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

Hello.

And thanks for the comments.

On Wed, 2010-09-08 at 20:35 +0200, ext Mauro Carvalho 
> >  	}
> > @@ -386,6 +394,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> >  	case V4L2_CID_TUNE_PREEMPHASIS:		return "Pre-emphasis settings";
> >  	case V4L2_CID_TUNE_POWER_LEVEL:		return "Tune Power Level";
> >  	case V4L2_CID_TUNE_ANTENNA_CAPACITOR:	return "Tune Antenna Capacitor";
> > +	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Tuner Controls";
> 
> > +	case V4L2_CID_FM_BAND:			return "FM Band";
> 
> 
> There's no need for a FM control, as there's already an ioctl pair that allows get/set the frequency
> bandwidth: VIDIOC_S_TUNER and VIDIOC_G_TUNER. So, the entire patch here seems uneeded/unwanted.

Sometime ago Hans said the following in his comment to v5 patches:

> Based on this article: 
> http://en.wikipedia.org/wiki/FM_broadcasting, there
> seem to be only three bands for FM radio in
> practice: 87.5-108, 76-90 (Japan)
> and 65-74 (former Soviet republics, some former
> eastern bloc countries).
>
> So this should be a standard control. Since the
> latter band is being phased
> out I think a menu control with just the two other
> bands is sufficient.
>
> And I also think this should be a control of a new
> FM_RX class.
>
> I know, that's not what I said last time. So sue 
> me :-)

But if we now have a consensus when we are at v9 that's good news...

Thanks,
Matti

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

Hello.

On Wed, 2010-09-08 at 20:35 +0200, ext Mauro Carvalho Chehab wrote:

> > +	case V4L2_CID_FM_BAND:			return "FM Band";
> 
> There's no need for a FM control, as there's already an ioctl pair that allows get/set the frequency
> bandwidth: VIDIOC_S_TUNER and VIDIOC_G_TUNER. So, the entire patch here seems uneeded/unwanted.

Yes I agree, we can manage without having BAND support, but it isn't
completely covered by VIDIOC_S_TUNER and VIDIOC_G_TUNER. Actually it
would have had the biggest effect on the HW seek operation. For example
without V4L2_CID_FM_BAND using wl1273  FM radio, which supports two
bands, you need to do two seeks and band switch for every HW_SEEK IOCTL
and those seeks are kind of slow operations to begin with. You have
probably taken this into consideration but I wanted point this out
anyway... Could the seek struct have a band indicator?

B.R.
Matti


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