All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/2] V4L2 SDR stream format
@ 2013-12-12 16:57 Antti Palosaari
  2013-12-12 16:57 ` [PATCH RFC 1/2] v4l2: add stream format for SDR receiver Antti Palosaari
  2013-12-12 16:57 ` [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR Antti Palosaari
  0 siblings, 2 replies; 8+ messages in thread
From: Antti Palosaari @ 2013-12-12 16:57 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil, Antti Palosaari

I think that needs device capability flag too (struct v4l2_capability)
but bits on that struct are quite short. Add it after V4L2_CAP_MODULATOR ?


OK, now all the API pieces seems to be there, so I am converting my existing
SDR drivers to that API and make some tests.

Antti Palosaari (2):
  v4l2: add stream format for SDR receiver
  v4l2: enable FMT IOCTLs for SDR

 drivers/media/v4l2-core/v4l2-dev.c   | 12 ++++++++++++
 drivers/media/v4l2-core/v4l2-ioctl.c | 27 +++++++++++++++++++++++++++
 include/trace/events/v4l2.h          |  1 +
 include/uapi/linux/videodev2.h       | 11 +++++++++++
 4 files changed, 51 insertions(+)

-- 
1.8.4.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH RFC 1/2] v4l2: add stream format for SDR receiver
  2013-12-12 16:57 [PATCH RFC 0/2] V4L2 SDR stream format Antti Palosaari
@ 2013-12-12 16:57 ` Antti Palosaari
  2013-12-13 14:36   ` Hans Verkuil
  2013-12-12 16:57 ` [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR Antti Palosaari
  1 sibling, 1 reply; 8+ messages in thread
From: Antti Palosaari @ 2013-12-12 16:57 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil, Antti Palosaari

Add new V4L2 stream format definition, named V4L2_BUF_TYPE_SDR_RX,
for SDR receiver.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/v4l2-core/v4l2-ioctl.c |  1 +
 include/trace/events/v4l2.h          |  1 +
 include/uapi/linux/videodev2.h       | 11 +++++++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index ee91a9f..5b6e0e8 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -149,6 +149,7 @@ const char *v4l2_type_names[] = {
 	[V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY] = "vid-out-overlay",
 	[V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE] = "vid-cap-mplane",
 	[V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE] = "vid-out-mplane",
+	[V4L2_BUF_TYPE_SDR_RX]             = "sdr-rx",
 };
 EXPORT_SYMBOL(v4l2_type_names);
 
diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
index ef94eca..d2ddd82 100644
--- a/include/trace/events/v4l2.h
+++ b/include/trace/events/v4l2.h
@@ -18,6 +18,7 @@
 		{ V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY, "VIDEO_OUTPUT_OVERLAY" },\
 		{ V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, "VIDEO_CAPTURE_MPLANE" },\
 		{ V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,  "VIDEO_OUTPUT_MPLANE" }, \
+		{ V4L2_BUF_TYPE_SDR_RX,               "SDR_RX" }, \
 		{ V4L2_BUF_TYPE_PRIVATE,	      "PRIVATE" })
 
 #define show_field(field)						\
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 1bac6c4..694694a 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -139,6 +139,7 @@ enum v4l2_buf_type {
 #endif
 	V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE = 9,
 	V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE  = 10,
+	V4L2_BUF_TYPE_SDR_RX               = 11,
 	/* Deprecated, do not use */
 	V4L2_BUF_TYPE_PRIVATE              = 0x80,
 };
@@ -1703,6 +1704,15 @@ struct v4l2_pix_format_mplane {
 } __attribute__ ((packed));
 
 /**
+ * struct v4l2_format_sdr - SDR format definition
+ * @pixelformat:	little endian four character code (fourcc)
+ */
+struct v4l2_format_sdr {
+	__u32				pixelformat;
+	__u8				reserved[28];
+} __attribute__ ((packed));
+
+/**
  * struct v4l2_format - stream data format
  * @type:	enum v4l2_buf_type; type of the data stream
  * @pix:	definition of an image format
@@ -1720,6 +1730,7 @@ struct v4l2_format {
 		struct v4l2_window		win;     /* V4L2_BUF_TYPE_VIDEO_OVERLAY */
 		struct v4l2_vbi_format		vbi;     /* V4L2_BUF_TYPE_VBI_CAPTURE */
 		struct v4l2_sliced_vbi_format	sliced;  /* V4L2_BUF_TYPE_SLICED_VBI_CAPTURE */
+		struct v4l2_format_sdr		sdr;     /* V4L2_BUF_TYPE_SDR_RX */
 		__u8	raw_data[200];                   /* user-defined */
 	} fmt;
 };
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR
  2013-12-12 16:57 [PATCH RFC 0/2] V4L2 SDR stream format Antti Palosaari
  2013-12-12 16:57 ` [PATCH RFC 1/2] v4l2: add stream format for SDR receiver Antti Palosaari
@ 2013-12-12 16:57 ` Antti Palosaari
  2013-12-13 14:45   ` Hans Verkuil
  1 sibling, 1 reply; 8+ messages in thread
From: Antti Palosaari @ 2013-12-12 16:57 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil, Antti Palosaari

Enable format IOCTLs for SDR use. There are used for negotiate used
data stream format.

Signed-off-by: Antti Palosaari <crope@iki.fi>
---
 drivers/media/v4l2-core/v4l2-dev.c   | 12 ++++++++++++
 drivers/media/v4l2-core/v4l2-ioctl.c | 26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index c9cf54c..d67286ba 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -563,6 +563,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER;
 	bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
 	bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
+	bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
 	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
 
@@ -612,6 +613,17 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
 		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
 
+	if (is_sdr && is_rx) {
+		/* SDR specific ioctls */
+		if (ops->vidioc_enum_fmt_vid_cap)
+			set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
+		if (ops->vidioc_g_fmt_vid_cap)
+			set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
+		if (ops->vidioc_s_fmt_vid_cap)
+			set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
+		if (ops->vidioc_try_fmt_vid_cap)
+			set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
+	}
 	if (is_vid) {
 		/* video specific ioctls */
 		if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 5b6e0e8..2471179 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -879,6 +879,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
 	const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
 	bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
+	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 
@@ -928,6 +929,10 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
 		if (is_vbi && is_tx && ops->vidioc_g_fmt_sliced_vbi_out)
 			return 0;
 		break;
+	case V4L2_BUF_TYPE_SDR_RX:
+		if (is_sdr && is_rx && ops->vidioc_g_fmt_vid_cap)
+			return 0;
+		break;
 	default:
 		break;
 	}
@@ -1047,6 +1052,10 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
 		if (unlikely(!is_tx || !ops->vidioc_enum_fmt_vid_out_mplane))
 			break;
 		return ops->vidioc_enum_fmt_vid_out_mplane(file, fh, arg);
+	case V4L2_BUF_TYPE_SDR_RX:
+		if (unlikely(!is_rx || !ops->vidioc_enum_fmt_vid_cap))
+			break;
+		return ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
 	}
 	return -EINVAL;
 }
@@ -1057,6 +1066,7 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 	struct v4l2_format *p = arg;
 	struct video_device *vfd = video_devdata(file);
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
+	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 
@@ -1101,6 +1111,10 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 		if (unlikely(!is_tx || is_vid || !ops->vidioc_g_fmt_sliced_vbi_out))
 			break;
 		return ops->vidioc_g_fmt_sliced_vbi_out(file, fh, arg);
+	case V4L2_BUF_TYPE_SDR_RX:
+		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_g_fmt_vid_cap))
+			break;
+		return ops->vidioc_g_fmt_vid_cap(file, fh, arg);
 	}
 	return -EINVAL;
 }
@@ -1111,6 +1125,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 	struct v4l2_format *p = arg;
 	struct video_device *vfd = video_devdata(file);
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
+	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 
@@ -1165,6 +1180,11 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.sliced);
 		return ops->vidioc_s_fmt_sliced_vbi_out(file, fh, arg);
+	case V4L2_BUF_TYPE_SDR_RX:
+		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_s_fmt_vid_cap))
+			break;
+		CLEAR_AFTER_FIELD(p, fmt.sdr);
+		return ops->vidioc_s_fmt_vid_cap(file, fh, arg);
 	}
 	return -EINVAL;
 }
@@ -1175,6 +1195,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
 	struct v4l2_format *p = arg;
 	struct video_device *vfd = video_devdata(file);
 	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
+	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 
@@ -1229,6 +1250,11 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.sliced);
 		return ops->vidioc_try_fmt_sliced_vbi_out(file, fh, arg);
+	case V4L2_BUF_TYPE_SDR_RX:
+		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_try_fmt_vid_cap))
+			break;
+		CLEAR_AFTER_FIELD(p, fmt.sdr);
+		return ops->vidioc_try_fmt_vid_cap(file, fh, arg);
 	}
 	return -EINVAL;
 }
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 1/2] v4l2: add stream format for SDR receiver
  2013-12-12 16:57 ` [PATCH RFC 1/2] v4l2: add stream format for SDR receiver Antti Palosaari
@ 2013-12-13 14:36   ` Hans Verkuil
  2013-12-13 14:44     ` Antti Palosaari
  0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2013-12-13 14:36 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media, Mauro Carvalho Chehab

On 12/12/2013 05:57 PM, Antti Palosaari wrote:
> Add new V4L2 stream format definition, named V4L2_BUF_TYPE_SDR_RX,
> for SDR receiver.
> 
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c |  1 +
>  include/trace/events/v4l2.h          |  1 +
>  include/uapi/linux/videodev2.h       | 11 +++++++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index ee91a9f..5b6e0e8 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -149,6 +149,7 @@ const char *v4l2_type_names[] = {
>  	[V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY] = "vid-out-overlay",
>  	[V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE] = "vid-cap-mplane",
>  	[V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE] = "vid-out-mplane",
> +	[V4L2_BUF_TYPE_SDR_RX]             = "sdr-rx",

Make this SDR_CAPTURE and sdr-cap to be consistent with existing naming
conventions.

>  };
>  EXPORT_SYMBOL(v4l2_type_names);
>  
> diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
> index ef94eca..d2ddd82 100644
> --- a/include/trace/events/v4l2.h
> +++ b/include/trace/events/v4l2.h
> @@ -18,6 +18,7 @@
>  		{ V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY, "VIDEO_OUTPUT_OVERLAY" },\
>  		{ V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, "VIDEO_CAPTURE_MPLANE" },\
>  		{ V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,  "VIDEO_OUTPUT_MPLANE" }, \
> +		{ V4L2_BUF_TYPE_SDR_RX,               "SDR_RX" }, \

"SDR_CAPTURE"

Regards,

	Hans

>  		{ V4L2_BUF_TYPE_PRIVATE,	      "PRIVATE" })
>  
>  #define show_field(field)						\
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 1bac6c4..694694a 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -139,6 +139,7 @@ enum v4l2_buf_type {
>  #endif
>  	V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE = 9,
>  	V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE  = 10,
> +	V4L2_BUF_TYPE_SDR_RX               = 11,
>  	/* Deprecated, do not use */
>  	V4L2_BUF_TYPE_PRIVATE              = 0x80,
>  };
> @@ -1703,6 +1704,15 @@ struct v4l2_pix_format_mplane {
>  } __attribute__ ((packed));
>  
>  /**
> + * struct v4l2_format_sdr - SDR format definition
> + * @pixelformat:	little endian four character code (fourcc)
> + */
> +struct v4l2_format_sdr {
> +	__u32				pixelformat;
> +	__u8				reserved[28];
> +} __attribute__ ((packed));
> +
> +/**
>   * struct v4l2_format - stream data format
>   * @type:	enum v4l2_buf_type; type of the data stream
>   * @pix:	definition of an image format
> @@ -1720,6 +1730,7 @@ struct v4l2_format {
>  		struct v4l2_window		win;     /* V4L2_BUF_TYPE_VIDEO_OVERLAY */
>  		struct v4l2_vbi_format		vbi;     /* V4L2_BUF_TYPE_VBI_CAPTURE */
>  		struct v4l2_sliced_vbi_format	sliced;  /* V4L2_BUF_TYPE_SLICED_VBI_CAPTURE */
> +		struct v4l2_format_sdr		sdr;     /* V4L2_BUF_TYPE_SDR_RX */
>  		__u8	raw_data[200];                   /* user-defined */
>  	} fmt;
>  };
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 1/2] v4l2: add stream format for SDR receiver
  2013-12-13 14:36   ` Hans Verkuil
@ 2013-12-13 14:44     ` Antti Palosaari
  0 siblings, 0 replies; 8+ messages in thread
From: Antti Palosaari @ 2013-12-13 14:44 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Mauro Carvalho Chehab

Thanks for the comments Hans!

On 13.12.2013 16:36, Hans Verkuil wrote:
> On 12/12/2013 05:57 PM, Antti Palosaari wrote:
>> +	[V4L2_BUF_TYPE_SDR_RX]             = "sdr-rx",
>

> Make this SDR_CAPTURE and sdr-cap to be consistent with existing naming
> conventions.

>> +		{ V4L2_BUF_TYPE_SDR_RX,               "SDR_RX" }, \
>
> "SDR_CAPTURE"

I will change device name to from RX to CAP/CAPTURE in order to keep it 
like existing devices. I actually thought that name too, but decided to 
use RX (and TX for SDR transmitter) is it sounds more suitable for radio 
devices. But capture is not bad at all.


regards
Antti

-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR
  2013-12-12 16:57 ` [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR Antti Palosaari
@ 2013-12-13 14:45   ` Hans Verkuil
  2013-12-13 15:04     ` Antti Palosaari
  0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2013-12-13 14:45 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media, Mauro Carvalho Chehab

On 12/12/2013 05:57 PM, Antti Palosaari wrote:
> Enable format IOCTLs for SDR use. There are used for negotiate used
> data stream format.
> 
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
>  drivers/media/v4l2-core/v4l2-dev.c   | 12 ++++++++++++
>  drivers/media/v4l2-core/v4l2-ioctl.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index c9cf54c..d67286ba 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -563,6 +563,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>  	bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER;
>  	bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
>  	bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
> +	bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
>  	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
>  	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>  
> @@ -612,6 +613,17 @@ static void determine_valid_ioctls(struct video_device *vdev)
>  	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
>  		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>  
> +	if (is_sdr && is_rx) {

I would drop the is_rx part. If there even is something like a SDR transmitter,
then I would still expect that the same ioctls are needed.

> +		/* SDR specific ioctls */
> +		if (ops->vidioc_enum_fmt_vid_cap)
> +			set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> +		if (ops->vidioc_g_fmt_vid_cap)
> +			set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> +		if (ops->vidioc_s_fmt_vid_cap)
> +			set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> +		if (ops->vidioc_try_fmt_vid_cap)
> +			set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);

We need sdr-specific ops: vidioc_enum/g/s/try_sdr_cap.

> +	}
>  	if (is_vid) {
>  		/* video specific ioctls */
>  		if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||

You also need to split up the large 'if (!is_radio)' part:

        if (!is_radio) {
                /* ioctls valid for video, vbi or sdr */
                SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
                SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
                SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
                SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
                SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
                SET_VALID_IOCTL(ops, VIDIOC_CREATE_BUFS, vidioc_create_bufs);
                SET_VALID_IOCTL(ops, VIDIOC_PREPARE_BUF, vidioc_prepare_buf);
	}
	if (!is_radio && !is_sdr) {

Regards,

	Hans

> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 5b6e0e8..2471179 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -879,6 +879,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>  	const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
>  	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
>  	bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>  	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>  	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>  
> @@ -928,6 +929,10 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>  		if (is_vbi && is_tx && ops->vidioc_g_fmt_sliced_vbi_out)
>  			return 0;
>  		break;
> +	case V4L2_BUF_TYPE_SDR_RX:
> +		if (is_sdr && is_rx && ops->vidioc_g_fmt_vid_cap)
> +			return 0;
> +		break;
>  	default:
>  		break;
>  	}
> @@ -1047,6 +1052,10 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>  		if (unlikely(!is_tx || !ops->vidioc_enum_fmt_vid_out_mplane))
>  			break;
>  		return ops->vidioc_enum_fmt_vid_out_mplane(file, fh, arg);
> +	case V4L2_BUF_TYPE_SDR_RX:
> +		if (unlikely(!is_rx || !ops->vidioc_enum_fmt_vid_cap))
> +			break;
> +		return ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
>  	}
>  	return -EINVAL;
>  }
> @@ -1057,6 +1066,7 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
>  	struct v4l2_format *p = arg;
>  	struct video_device *vfd = video_devdata(file);
>  	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>  	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>  	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>  
> @@ -1101,6 +1111,10 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
>  		if (unlikely(!is_tx || is_vid || !ops->vidioc_g_fmt_sliced_vbi_out))
>  			break;
>  		return ops->vidioc_g_fmt_sliced_vbi_out(file, fh, arg);
> +	case V4L2_BUF_TYPE_SDR_RX:
> +		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_g_fmt_vid_cap))
> +			break;
> +		return ops->vidioc_g_fmt_vid_cap(file, fh, arg);
>  	}
>  	return -EINVAL;
>  }
> @@ -1111,6 +1125,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
>  	struct v4l2_format *p = arg;
>  	struct video_device *vfd = video_devdata(file);
>  	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>  	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>  	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>  
> @@ -1165,6 +1180,11 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
>  			break;
>  		CLEAR_AFTER_FIELD(p, fmt.sliced);
>  		return ops->vidioc_s_fmt_sliced_vbi_out(file, fh, arg);
> +	case V4L2_BUF_TYPE_SDR_RX:
> +		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_s_fmt_vid_cap))
> +			break;
> +		CLEAR_AFTER_FIELD(p, fmt.sdr);
> +		return ops->vidioc_s_fmt_vid_cap(file, fh, arg);
>  	}
>  	return -EINVAL;
>  }
> @@ -1175,6 +1195,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
>  	struct v4l2_format *p = arg;
>  	struct video_device *vfd = video_devdata(file);
>  	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>  	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>  	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>  
> @@ -1229,6 +1250,11 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
>  			break;
>  		CLEAR_AFTER_FIELD(p, fmt.sliced);
>  		return ops->vidioc_try_fmt_sliced_vbi_out(file, fh, arg);
> +	case V4L2_BUF_TYPE_SDR_RX:
> +		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_try_fmt_vid_cap))
> +			break;
> +		CLEAR_AFTER_FIELD(p, fmt.sdr);
> +		return ops->vidioc_try_fmt_vid_cap(file, fh, arg);
>  	}
>  	return -EINVAL;
>  }
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR
  2013-12-13 14:45   ` Hans Verkuil
@ 2013-12-13 15:04     ` Antti Palosaari
  2013-12-13 15:29       ` Hans Verkuil
  0 siblings, 1 reply; 8+ messages in thread
From: Antti Palosaari @ 2013-12-13 15:04 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Mauro Carvalho Chehab

On 13.12.2013 16:45, Hans Verkuil wrote:
> On 12/12/2013 05:57 PM, Antti Palosaari wrote:
>> Enable format IOCTLs for SDR use. There are used for negotiate used
>> data stream format.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>>   drivers/media/v4l2-core/v4l2-dev.c   | 12 ++++++++++++
>>   drivers/media/v4l2-core/v4l2-ioctl.c | 26 ++++++++++++++++++++++++++
>>   2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index c9cf54c..d67286ba 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -563,6 +563,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>>   	bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER;
>>   	bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
>>   	bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
>> +	bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
>>   	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
>>   	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>>
>> @@ -612,6 +613,17 @@ static void determine_valid_ioctls(struct video_device *vdev)
>>   	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
>>   		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>>
>> +	if (is_sdr && is_rx) {
>
> I would drop the is_rx part. If there even is something like a SDR transmitter,
> then I would still expect that the same ioctls are needed.

There is TX devices too, I am looking it later, maybe on March at earliest.

>> +		/* SDR specific ioctls */
>> +		if (ops->vidioc_enum_fmt_vid_cap)
>> +			set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
>> +		if (ops->vidioc_g_fmt_vid_cap)
>> +			set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
>> +		if (ops->vidioc_s_fmt_vid_cap)
>> +			set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
>> +		if (ops->vidioc_try_fmt_vid_cap)
>> +			set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
>
> We need sdr-specific ops: vidioc_enum/g/s/try_sdr_cap.

Yes. But it could be done very easily later as it does not have effect 
to V4L2 API.

>
>> +	}
>>   	if (is_vid) {
>>   		/* video specific ioctls */
>>   		if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
>
> You also need to split up the large 'if (!is_radio)' part:
>
>          if (!is_radio) {
>                  /* ioctls valid for video, vbi or sdr */
>                  SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
>                  SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
>                  SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
>                  SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
>                  SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
>                  SET_VALID_IOCTL(ops, VIDIOC_CREATE_BUFS, vidioc_create_bufs);
>                  SET_VALID_IOCTL(ops, VIDIOC_PREPARE_BUF, vidioc_prepare_buf);
> 	}
> 	if (!is_radio && !is_sdr) {

I will change it to limit only to those relevant VB2 IOCTLs.

regards
Antti

>
> Regards,
>
> 	Hans
>
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 5b6e0e8..2471179 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -879,6 +879,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>>   	const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
>>   	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
>>   	bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
>> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>>   	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>>   	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>>
>> @@ -928,6 +929,10 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>>   		if (is_vbi && is_tx && ops->vidioc_g_fmt_sliced_vbi_out)
>>   			return 0;
>>   		break;
>> +	case V4L2_BUF_TYPE_SDR_RX:
>> +		if (is_sdr && is_rx && ops->vidioc_g_fmt_vid_cap)
>> +			return 0;
>> +		break;
>>   	default:
>>   		break;
>>   	}
>> @@ -1047,6 +1052,10 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>>   		if (unlikely(!is_tx || !ops->vidioc_enum_fmt_vid_out_mplane))
>>   			break;
>>   		return ops->vidioc_enum_fmt_vid_out_mplane(file, fh, arg);
>> +	case V4L2_BUF_TYPE_SDR_RX:
>> +		if (unlikely(!is_rx || !ops->vidioc_enum_fmt_vid_cap))
>> +			break;
>> +		return ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
>>   	}
>>   	return -EINVAL;
>>   }
>> @@ -1057,6 +1066,7 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
>>   	struct v4l2_format *p = arg;
>>   	struct video_device *vfd = video_devdata(file);
>>   	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
>> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>>   	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>>   	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>>
>> @@ -1101,6 +1111,10 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
>>   		if (unlikely(!is_tx || is_vid || !ops->vidioc_g_fmt_sliced_vbi_out))
>>   			break;
>>   		return ops->vidioc_g_fmt_sliced_vbi_out(file, fh, arg);
>> +	case V4L2_BUF_TYPE_SDR_RX:
>> +		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_g_fmt_vid_cap))
>> +			break;
>> +		return ops->vidioc_g_fmt_vid_cap(file, fh, arg);
>>   	}
>>   	return -EINVAL;
>>   }
>> @@ -1111,6 +1125,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
>>   	struct v4l2_format *p = arg;
>>   	struct video_device *vfd = video_devdata(file);
>>   	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
>> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>>   	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>>   	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>>
>> @@ -1165,6 +1180,11 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
>>   			break;
>>   		CLEAR_AFTER_FIELD(p, fmt.sliced);
>>   		return ops->vidioc_s_fmt_sliced_vbi_out(file, fh, arg);
>> +	case V4L2_BUF_TYPE_SDR_RX:
>> +		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_s_fmt_vid_cap))
>> +			break;
>> +		CLEAR_AFTER_FIELD(p, fmt.sdr);
>> +		return ops->vidioc_s_fmt_vid_cap(file, fh, arg);
>>   	}
>>   	return -EINVAL;
>>   }
>> @@ -1175,6 +1195,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
>>   	struct v4l2_format *p = arg;
>>   	struct video_device *vfd = video_devdata(file);
>>   	bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
>> +	bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>>   	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>>   	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>>
>> @@ -1229,6 +1250,11 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
>>   			break;
>>   		CLEAR_AFTER_FIELD(p, fmt.sliced);
>>   		return ops->vidioc_try_fmt_sliced_vbi_out(file, fh, arg);
>> +	case V4L2_BUF_TYPE_SDR_RX:
>> +		if (unlikely(!is_rx || !is_sdr || !ops->vidioc_try_fmt_vid_cap))
>> +			break;
>> +		CLEAR_AFTER_FIELD(p, fmt.sdr);
>> +		return ops->vidioc_try_fmt_vid_cap(file, fh, arg);
>>   	}
>>   	return -EINVAL;
>>   }
>>
>
> --
> 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
>


-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR
  2013-12-13 15:04     ` Antti Palosaari
@ 2013-12-13 15:29       ` Hans Verkuil
  0 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2013-12-13 15:29 UTC (permalink / raw)
  To: Antti Palosaari; +Cc: linux-media, Mauro Carvalho Chehab

On 12/13/2013 04:04 PM, Antti Palosaari wrote:
> On 13.12.2013 16:45, Hans Verkuil wrote:
>> On 12/12/2013 05:57 PM, Antti Palosaari wrote:
>>> Enable format IOCTLs for SDR use. There are used for negotiate used
>>> data stream format.
>>>
>>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>>> ---
>>>   drivers/media/v4l2-core/v4l2-dev.c   | 12 ++++++++++++
>>>   drivers/media/v4l2-core/v4l2-ioctl.c | 26 ++++++++++++++++++++++++++
>>>   2 files changed, 38 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>>> index c9cf54c..d67286ba 100644
>>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>>> @@ -563,6 +563,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>>>   	bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER;
>>>   	bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
>>>   	bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
>>> +	bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
>>>   	bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
>>>   	bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>>>
>>> @@ -612,6 +613,17 @@ static void determine_valid_ioctls(struct video_device *vdev)
>>>   	if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
>>>   		set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>>>
>>> +	if (is_sdr && is_rx) {
>>
>> I would drop the is_rx part. If there even is something like a SDR transmitter,
>> then I would still expect that the same ioctls are needed.
> 
> There is TX devices too, I am looking it later, maybe on March at earliest.
> 
>>> +		/* SDR specific ioctls */
>>> +		if (ops->vidioc_enum_fmt_vid_cap)
>>> +			set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
>>> +		if (ops->vidioc_g_fmt_vid_cap)
>>> +			set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
>>> +		if (ops->vidioc_s_fmt_vid_cap)
>>> +			set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
>>> +		if (ops->vidioc_try_fmt_vid_cap)
>>> +			set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
>>
>> We need sdr-specific ops: vidioc_enum/g/s/try_sdr_cap.
> 
> Yes. But it could be done very easily later as it does not have effect 
> to V4L2 API.

It's much easier to do it right the first time, then to add it in later :-)

Spoken from personal experience...

Anyway, this really should be implemented like that. Things can get very
confusing otherwise.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-12-13 15:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-12 16:57 [PATCH RFC 0/2] V4L2 SDR stream format Antti Palosaari
2013-12-12 16:57 ` [PATCH RFC 1/2] v4l2: add stream format for SDR receiver Antti Palosaari
2013-12-13 14:36   ` Hans Verkuil
2013-12-13 14:44     ` Antti Palosaari
2013-12-12 16:57 ` [PATCH RFC 2/2] v4l2: enable FMT IOCTLs for SDR Antti Palosaari
2013-12-13 14:45   ` Hans Verkuil
2013-12-13 15:04     ` Antti Palosaari
2013-12-13 15:29       ` Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.