Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1425126 > unrolled thread

[PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Nick Dyer <nick.dyer@itdev.co.uk> - 2016-06-17 16:20 +0200
    Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Hans Verkuil <hverkuil@xs4all.nl> - 2016-06-20 14:10 +0200
      Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Florian Echtler <floe@butterbrot.org> - 2016-06-22 14:10 +0200
        Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Nick Dyer <nick.dyer@itdev.co.uk> - 2016-06-22 14:10 +0200
          Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Florian Echtler <floe@butterbrot.org> - 2016-06-22 22:40 +0200
            Re: [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR Nick Dyer <nick.dyer@itdev.co.uk> - 2016-06-22 23:30 +0200

#1425126 — [PATCH v4 2/9] [media] v4l2-core: Add VFL_TYPE_TOUCH_SENSOR

Some touch controllers send out raw touch data in a similar way to a
greyscale frame grabber. Add a new device type for these devices.

Use a new device prefix v4l-touch for these devices, to stop generic
capture software from treating them as webcams.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/sur40.c    |  4 ++--
 drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
 drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
 include/media/v4l2-dev.h             |  3 ++-
 include/uapi/linux/videodev2.h       |  1 +
 5 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 880c40b..2daa786 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -599,7 +599,7 @@ static int sur40_probe(struct usb_interface *interface,
 	sur40->vdev.queue = &sur40->queue;
 	video_set_drvdata(&sur40->vdev, sur40);
 
-	error = video_register_device(&sur40->vdev, VFL_TYPE_GRABBER, -1);
+	error = video_register_device(&sur40->vdev, VFL_TYPE_TOUCH_SENSOR, -1);
 	if (error) {
 		dev_err(&interface->dev,
 			"Unable to register video subdevice.");
@@ -763,7 +763,7 @@ static int sur40_vidioc_enum_input(struct file *file, void *priv,
 {
 	if (i->index != 0)
 		return -EINVAL;
-	i->type = V4L2_INPUT_TYPE_CAMERA;
+	i->type = V4L2_INPUT_TYPE_TOUCH_SENSOR;
 	i->std = V4L2_STD_UNKNOWN;
 	strlcpy(i->name, "In-Cell Sensor", sizeof(i->name));
 	i->capabilities = 0;
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 70b559d..7068e12 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -529,6 +529,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	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;
+	bool is_touch = vdev->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 
 	bitmap_zero(valid_ioctls, BASE_VIDIOC_PRIVATE);
 
@@ -573,7 +574,7 @@ 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_vid) {
+	if (is_vid || is_touch) {
 		/* video specific ioctls */
 		if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
 			       ops->vidioc_enum_fmt_vid_cap_mplane ||
@@ -662,7 +663,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 			set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
 	}
 
-	if (is_vid || is_vbi || is_sdr) {
+	if (is_vid || is_vbi || is_sdr || is_touch) {
 		/* ioctls valid for video, vbi or sdr */
 		SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
 		SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
@@ -675,7 +676,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
 	}
 
-	if (is_vid || is_vbi) {
+	if (is_vid || is_vbi || is_touch) {
 		/* ioctls valid for video or vbi */
 		if (ops->vidioc_s_std)
 			set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
@@ -739,6 +740,7 @@ static int video_register_media_controller(struct video_device *vdev, int type)
 	vdev->entity.function = MEDIA_ENT_F_UNKNOWN;
 
 	switch (type) {
+	case VFL_TYPE_TOUCH_SENSOR:
 	case VFL_TYPE_GRABBER:
 		intf_type = MEDIA_INTF_T_V4L_VIDEO;
 		vdev->entity.function = MEDIA_ENT_F_IO_V4L;
@@ -845,6 +847,8 @@ static int video_register_media_controller(struct video_device *vdev, int type)
  *	%VFL_TYPE_SUBDEV - A subdevice
  *
  *	%VFL_TYPE_SDR - Software Defined Radio
+ *
+ *	%VFL_TYPE_TOUCH_SENSOR - A touch sensor
  */
 int __video_register_device(struct video_device *vdev, int type, int nr,
 		int warn_if_nr_in_use, struct module *owner)
@@ -888,6 +892,9 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 		/* Use device name 'swradio' because 'sdr' was already taken. */
 		name_base = "swradio";
 		break;
+	case VFL_TYPE_TOUCH_SENSOR:
+		name_base = "v4l-touch";
+		break;
 	default:
 		printk(KERN_ERR "%s called with unknown type: %d\n",
 		       __func__, type);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index ecf7e0b..352c4d0 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -924,6 +924,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
 	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_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 
@@ -932,7 +933,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
 
 	switch (type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (is_vid && is_rx &&
+		if ((is_vid || is_touch) && is_rx &&
 		    (ops->vidioc_g_fmt_vid_cap || ops->vidioc_g_fmt_vid_cap_mplane))
 			return 0;
 		break;
@@ -1310,13 +1311,14 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
 	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_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret = -EINVAL;
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_enum_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_enum_fmt_vid_cap))
 			break;
 		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
 		break;
@@ -1363,6 +1365,7 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 	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_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
@@ -1393,7 +1396,7 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_g_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_g_fmt_vid_cap))
 			break;
 		p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
 		ret = ops->vidioc_g_fmt_vid_cap(file, fh, arg);
@@ -1459,6 +1462,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 	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_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
@@ -1470,7 +1474,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_s_fmt_vid_cap))
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.pix);
 		ret = ops->vidioc_s_fmt_vid_cap(file, fh, arg);
@@ -1546,6 +1550,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
 	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_touch = vfd->vfl_type == VFL_TYPE_TOUCH_SENSOR;
 	bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
 	bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
 	int ret;
@@ -1554,7 +1559,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		if (unlikely(!is_rx || !is_vid || !ops->vidioc_try_fmt_vid_cap))
+		if (unlikely(!is_rx || (!is_vid && !is_touch) || !ops->vidioc_try_fmt_vid_cap))
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.pix);
 		ret = ops->vidioc_try_fmt_vid_cap(file, fh, arg);
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 25a3190..99eb87a 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -25,7 +25,8 @@
 #define VFL_TYPE_RADIO		2
 #define VFL_TYPE_SUBDEV		3
 #define VFL_TYPE_SDR		4
-#define VFL_TYPE_MAX		5
+#define VFL_TYPE_TOUCH_SENSOR	5
+#define VFL_TYPE_MAX		6
 
 /* Is this a receiver, transmitter or mem-to-mem? */
 /* Ignored for VFL_TYPE_SUBDEV. */
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index e0125cf..765a6cb 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1400,6 +1400,7 @@ struct v4l2_input {
 /*  Values for the 'type' field */
 #define V4L2_INPUT_TYPE_TUNER		1
 #define V4L2_INPUT_TYPE_CAMERA		2
+#define V4L2_INPUT_TYPE_TOUCH_SENSOR	3
 
 /* field 'status' - general */
 #define V4L2_IN_ST_NO_POWER    0x00000001  /* Attached device is off */
-- 
2.5.0

[toc] | [next] | [standalone]

#1426527

On 06/17/2016 04:16 PM, Nick Dyer wrote:
> Some touch controllers send out raw touch data in a similar way to a
> greyscale frame grabber. Add a new device type for these devices.
> 
> Use a new device prefix v4l-touch for these devices, to stop generic
> capture software from treating them as webcams.
> 
> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
> ---
>  drivers/input/touchscreen/sur40.c    |  4 ++--
>  drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
>  include/media/v4l2-dev.h             |  3 ++-
>  include/uapi/linux/videodev2.h       |  1 +

The new interface also needs to be documented in the media DocBook: section 4
describes all the interfaces, so a new one should be added here.

Sorry, just realized this.

I recommend that you grep for swradio:

git grep swradio Documentation/DocBook/media

and add a line there for v4l-touch as well. It also looks like a new interface
define is needed as a media type.

Regards,

	Hans

[toc] | [prev] | [next] | [standalone]

#1428721

[Multipart message — attachments visible in raw view] — view raw

On 20.06.2016 14:00, Hans Verkuil wrote:
> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>> Some touch controllers send out raw touch data in a similar way to a
>> greyscale frame grabber. Add a new device type for these devices.
>>
>> Use a new device prefix v4l-touch for these devices, to stop generic
>> capture software from treating them as webcams.
>>
>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>> ---
>>  drivers/input/touchscreen/sur40.c    |  4 ++--
>>  drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
>>  include/media/v4l2-dev.h             |  3 ++-
>>  include/uapi/linux/videodev2.h       |  1 +

Generally a good idea in my opinion, but I think the SUR40 is a special
case: the whole point of putting in a V4L2 driver was that software like
reacTIVision, which already has a V4L2 interface, can then use that
device like any other camera.

Come to think of it, wouldn't it make sense to expose the other touch
devices as generic frame grabbers, too, so you can easily view the debug
output with any generic tool like cheese?

Best, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL

[toc] | [prev] | [next] | [standalone]

#1428731

On 22/06/2016 12:48, Florian Echtler wrote:
> On 20.06.2016 14:00, Hans Verkuil wrote:
>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>> Some touch controllers send out raw touch data in a similar way to a
>>> greyscale frame grabber. Add a new device type for these devices.
>>>
>>> Use a new device prefix v4l-touch for these devices, to stop generic
>>> capture software from treating them as webcams.
>>>
>>> Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
>>> ---
>>>  drivers/input/touchscreen/sur40.c    |  4 ++--
>>>  drivers/media/v4l2-core/v4l2-dev.c   | 13 ++++++++++---
>>>  drivers/media/v4l2-core/v4l2-ioctl.c | 15 ++++++++++-----
>>>  include/media/v4l2-dev.h             |  3 ++-
>>>  include/uapi/linux/videodev2.h       |  1 +
> 
> Generally a good idea in my opinion, but I think the SUR40 is a special
> case: the whole point of putting in a V4L2 driver was that software like
> reacTIVision, which already has a V4L2 interface, can then use that
> device like any other camera.

Thanks. I see that reactivision definitely uses this already
(https://github.com/mkalten/reacTIVision/issues/3 ) and we don't want to
break it - I've split the sur40.c change out of this patch now so it can be
considered separately.

> Come to think of it, wouldn't it make sense to expose the other touch
> devices as generic frame grabbers, too, so you can easily view the debug
> output with any generic tool like cheese?

While I like the idea of being able to use the generic tools, I think we
needed to do something to stop these devices turning up in e.g. video
conferencing software - it would cause a lot of confusion. There's nothing
stopping particular tools adding the necessary code to handle touch devices
if they feel their users want it.

Also, the RMI4 and Atmel mXT touchscreens output signed data, which
unfortunately would confuse the generic tools.

[toc] | [prev] | [next] | [standalone]

#1429098

On Wed, 22 Jun 2016, Nick Dyer wrote:

> On 22/06/2016 12:48, Florian Echtler wrote:
>> On 20.06.2016 14:00, Hans Verkuil wrote:
>>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>>>
>>>> Use a new device prefix v4l-touch for these devices, to stop generic
>>>> capture software from treating them as webcams.
>
>> Come to think of it, wouldn't it make sense to expose the other touch
>> devices as generic frame grabbers, too, so you can easily view the debug
>> output with any generic tool like cheese?
>
> While I like the idea of being able to use the generic tools, I think we
> needed to do something to stop these devices turning up in e.g. video
> conferencing software - it would cause a lot of confusion. There's nothing
> stopping particular tools adding the necessary code to handle touch devices
> if they feel their users want it.

Just to clarify: from the userspace point-of-view, would this change 
simply modify the prefix of the device node (i.e. /dev/video1 -> 
/dev/v4l-touch1), or would it somehow affect the API? If it's just the 
device node name, then that shouldn't be a problem after all, because e.g. 
reacTIVision requires you to specify the exact camera to be used anyway.

Florian
-- 
"_Nothing_ brightens up my morning. Coffee simply provides a shade of
grey just above the pitch-black of the infinite depths of the _abyss_."

[toc] | [prev] | [next] | [standalone]

#1429122

On 22/06/2016 21:38, Florian Echtler wrote:
> On Wed, 22 Jun 2016, Nick Dyer wrote:
> 
>> On 22/06/2016 12:48, Florian Echtler wrote:
>>> On 20.06.2016 14:00, Hans Verkuil wrote:
>>>> On 06/17/2016 04:16 PM, Nick Dyer wrote:
>>>>>
>>>>> Use a new device prefix v4l-touch for these devices, to stop generic
>>>>> capture software from treating them as webcams.
>>
>>> Come to think of it, wouldn't it make sense to expose the other touch
>>> devices as generic frame grabbers, too, so you can easily view the debug
>>> output with any generic tool like cheese?
>>
>> While I like the idea of being able to use the generic tools, I think we
>> needed to do something to stop these devices turning up in e.g. video
>> conferencing software - it would cause a lot of confusion. There's nothing
>> stopping particular tools adding the necessary code to handle touch devices
>> if they feel their users want it.
> 
> Just to clarify: from the userspace point-of-view, would this change simply
> modify the prefix of the device node (i.e. /dev/video1 -> /dev/v4l-touch1),
> or would it somehow affect the API? If it's just the device node name, then
> that shouldn't be a problem after all, because e.g. reacTIVision requires
> you to specify the exact camera to be used anyway.

With the changes that Hans Verkuil has asked me to do:
* The device node is /dev/v4l-touch0-255
* There are several new formats eg. V4L2_TCH_FMT_DELTA_TD16 (16 bit deltas)
* I've defined new types VFL_TYPE_TOUCH, V4L2_BUF_TYPE_TOUCH_CAPTURE,
V4L2_INPUT_TYPE_TOUCH

We're just testing the changes, I hope to post an updated version soon.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web