Re: [PATCH v2 4/8] Input: atmel_mxt_ts - output diagnostic debug via v4l2 device
On 27/05/2016 13:54, Hans Verkuil wrote: > Hi Nick, Thanks for the useful review. Most of it is straightforward and I've updated it in a new version of these patches which I will post now. I think the open question is whether you're happy with the V4L2_PIX_FMT_YS16 or whether I need to rename it. For what it's worth, Synaptics RMI4 also emits 16 bit signed, see https://github.com/wanam/Adam-Kernel-GS4/blob/master/drivers/input/touchscreen/rmi_f54.c#L1831 > On 05/04/2016 07:07 PM, Nick Dyer wrote: > BTW, did you run v4l2-compliance? I think it should work if you just do: > > v4l2-compliance -d /dev/v4l-touch0 Yes, and I've now fixed all issues that I could find with it. I had to do some minor updates to support the touch sensor stuff, see: https://github.com/ndyer/v4l-utils/commits/touch-sensor -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/8] Input: atmel_mxt_ts - output diagnostic debug via v4l2 device
Hi Nick, On 05/04/2016 07:07 PM, Nick Dyer wrote: > Register a video device to output T37 diagnostic data. > > Signed-off-by: Nick Dyer> --- > drivers/input/touchscreen/Kconfig| 2 + > drivers/input/touchscreen/atmel_mxt_ts.c | 271 > +++ > 2 files changed, 273 insertions(+) > > diff --git a/drivers/input/touchscreen/Kconfig > b/drivers/input/touchscreen/Kconfig > index 1f99e7f..4aa7609 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -105,6 +105,8 @@ config TOUCHSCREEN_AR1021_I2C > config TOUCHSCREEN_ATMEL_MXT > tristate "Atmel mXT I2C Touchscreen" > depends on I2C > + depends on VIDEO_V4L2 > + select VIDEOBUF2_VMALLOC > select FW_LOADER > help > Say Y here if you have Atmel mXT series I2C touchscreen, > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c > b/drivers/input/touchscreen/atmel_mxt_ts.c > index cd97713..8945235 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -28,6 +28,10 @@ > #include > #include > #include > +#include > +#include > +#include > +#include > > /* Firmware files */ > #define MXT_FW_NAME "maxtouch.fw" > @@ -222,6 +226,23 @@ struct mxt_dbg { > struct t37_debug *t37_buf; > unsigned int t37_pages; > unsigned int t37_nodes; > + > + struct v4l2_device v4l2; > + struct v4l2_pix_format format; > + struct video_device vdev; > + struct vb2_queue queue; > + struct mutex lock; > + int input; > +}; > + > +static const struct v4l2_file_operations mxt_video_fops = { > + .owner = THIS_MODULE, > + .open = v4l2_fh_open, > + .release = vb2_fop_release, > + .unlocked_ioctl = video_ioctl2, > + .read = vb2_fop_read, > + .mmap = vb2_fop_mmap, > + .poll = vb2_fop_poll, > }; > > /* Each client has this additional data */ > @@ -277,6 +298,11 @@ struct mxt_data { > struct completion crc_completion; > }; > > +struct mxt_vb2_buffer { > + struct vb2_buffer vb; > + struct list_headlist; > +}; > + > static size_t mxt_obj_size(const struct mxt_object *obj) > { > return obj->size_minus_one + 1; > @@ -1523,6 +1549,9 @@ static void mxt_free_input_device(struct mxt_data *data) > > static void mxt_free_object_table(struct mxt_data *data) > { > + video_unregister_device(>dbg.vdev); > + v4l2_device_unregister(>dbg.v4l2); > + > kfree(data->object_table); > data->object_table = NULL; > kfree(data->msg_buf); > @@ -2154,10 +2183,215 @@ wait_cmd: > return mxt_convert_debug_pages(data, outbuf); > } > > +static int mxt_queue_setup(struct vb2_queue *q, > +unsigned int *nbuffers, unsigned int *nplanes, > +unsigned int sizes[], void *alloc_ctxs[]) > +{ > + struct mxt_data *data = q->drv_priv; > + > + *nbuffers = 1; That's not right. You can just drop this line since you've already set min_buffers_needed below. This line forces vb2 to always allocate just one buffer, even if userspace wants more. And in order to correctly support VIDIOC_CREATE_BUFS you need this: if (*nplanes) return sizes[0] < data->dbg.t37_nodes * sizeof(u16) ? -EINVAL : 0; > + *nplanes = 1; > + sizes[0] = data->dbg.t37_nodes * sizeof(u16); > + > + return 0; > +} > + > +static int mxt_buffer_prepare(struct vb2_buffer *vb) > +{ > + return 0; > +} Just drop this function since it doesn't do anything. > + > +static void mxt_buffer_queue(struct vb2_buffer *vb) > +{ > + struct mxt_data *data = vb2_get_drv_priv(vb->vb2_queue); > + u16 *ptr; > + int ret; > + > + ptr = vb2_plane_vaddr(vb, 0); > + if (!ptr) { > + dev_err(>client->dev, "Error acquiring frame ptr\n"); > + goto fault; > + } > + > + ret = mxt_read_diagnostic_debug(data, MXT_DIAGNOSTIC_DELTAS, ptr); > + if (ret) > + goto fault; > + > + vb2_set_plane_payload(vb, 0, data->dbg.t37_nodes * sizeof(u16)); > + vb2_buffer_done(vb, VB2_BUF_STATE_DONE); > + return; > + > +fault: > + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR); > +} > + > +/* V4L2 structures */ > +static const struct vb2_ops mxt_queue_ops = { > + .queue_setup= mxt_queue_setup, > + .buf_prepare= mxt_buffer_prepare, > + .buf_queue = mxt_buffer_queue, > + .wait_prepare = vb2_ops_wait_prepare, > + .wait_finish= vb2_ops_wait_finish, > +}; > + > +static const struct vb2_queue mxt_queue = { > + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, > + .io_modes = VB2_MMAP, Add VB2_USERPTR | VB2_DMABUF | VB2_READ here as well. You get that for free, so why not? > + .buf_struct_size = sizeof(struct mxt_vb2_buffer), > + .ops = _queue_ops, > + .mem_ops = _vmalloc_memops, > + .timestamp_flags =
[PATCH v2 4/8] Input: atmel_mxt_ts - output diagnostic debug via v4l2 device
Register a video device to output T37 diagnostic data. Signed-off-by: Nick Dyer--- drivers/input/touchscreen/Kconfig| 2 + drivers/input/touchscreen/atmel_mxt_ts.c | 271 +++ 2 files changed, 273 insertions(+) diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 1f99e7f..4aa7609 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -105,6 +105,8 @@ config TOUCHSCREEN_AR1021_I2C config TOUCHSCREEN_ATMEL_MXT tristate "Atmel mXT I2C Touchscreen" depends on I2C + depends on VIDEO_V4L2 + select VIDEOBUF2_VMALLOC select FW_LOADER help Say Y here if you have Atmel mXT series I2C touchscreen, diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index cd97713..8945235 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -28,6 +28,10 @@ #include #include #include +#include +#include +#include +#include /* Firmware files */ #define MXT_FW_NAME"maxtouch.fw" @@ -222,6 +226,23 @@ struct mxt_dbg { struct t37_debug *t37_buf; unsigned int t37_pages; unsigned int t37_nodes; + + struct v4l2_device v4l2; + struct v4l2_pix_format format; + struct video_device vdev; + struct vb2_queue queue; + struct mutex lock; + int input; +}; + +static const struct v4l2_file_operations mxt_video_fops = { + .owner = THIS_MODULE, + .open = v4l2_fh_open, + .release = vb2_fop_release, + .unlocked_ioctl = video_ioctl2, + .read = vb2_fop_read, + .mmap = vb2_fop_mmap, + .poll = vb2_fop_poll, }; /* Each client has this additional data */ @@ -277,6 +298,11 @@ struct mxt_data { struct completion crc_completion; }; +struct mxt_vb2_buffer { + struct vb2_buffer vb; + struct list_headlist; +}; + static size_t mxt_obj_size(const struct mxt_object *obj) { return obj->size_minus_one + 1; @@ -1523,6 +1549,9 @@ static void mxt_free_input_device(struct mxt_data *data) static void mxt_free_object_table(struct mxt_data *data) { + video_unregister_device(>dbg.vdev); + v4l2_device_unregister(>dbg.v4l2); + kfree(data->object_table); data->object_table = NULL; kfree(data->msg_buf); @@ -2154,10 +2183,215 @@ wait_cmd: return mxt_convert_debug_pages(data, outbuf); } +static int mxt_queue_setup(struct vb2_queue *q, + unsigned int *nbuffers, unsigned int *nplanes, + unsigned int sizes[], void *alloc_ctxs[]) +{ + struct mxt_data *data = q->drv_priv; + + *nbuffers = 1; + *nplanes = 1; + sizes[0] = data->dbg.t37_nodes * sizeof(u16); + + return 0; +} + +static int mxt_buffer_prepare(struct vb2_buffer *vb) +{ + return 0; +} + +static void mxt_buffer_queue(struct vb2_buffer *vb) +{ + struct mxt_data *data = vb2_get_drv_priv(vb->vb2_queue); + u16 *ptr; + int ret; + + ptr = vb2_plane_vaddr(vb, 0); + if (!ptr) { + dev_err(>client->dev, "Error acquiring frame ptr\n"); + goto fault; + } + + ret = mxt_read_diagnostic_debug(data, MXT_DIAGNOSTIC_DELTAS, ptr); + if (ret) + goto fault; + + vb2_set_plane_payload(vb, 0, data->dbg.t37_nodes * sizeof(u16)); + vb2_buffer_done(vb, VB2_BUF_STATE_DONE); + return; + +fault: + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR); +} + +/* V4L2 structures */ +static const struct vb2_ops mxt_queue_ops = { + .queue_setup= mxt_queue_setup, + .buf_prepare= mxt_buffer_prepare, + .buf_queue = mxt_buffer_queue, + .wait_prepare = vb2_ops_wait_prepare, + .wait_finish= vb2_ops_wait_finish, +}; + +static const struct vb2_queue mxt_queue = { + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, + .io_modes = VB2_MMAP, + .buf_struct_size = sizeof(struct mxt_vb2_buffer), + .ops = _queue_ops, + .mem_ops = _vmalloc_memops, + .timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC, + .min_buffers_needed = 1, +}; + +static int mxt_vidioc_querycap(struct file *file, void *priv, +struct v4l2_capability *cap) +{ + struct mxt_data *data = video_drvdata(file); + + strlcpy(cap->driver, "atmel_mxt_ts", sizeof(cap->driver)); + strlcpy(cap->card, "atmel_mxt_ts touch", sizeof(cap->card)); + strlcpy(cap->bus_info, data->phys, sizeof(cap->bus_info)); + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | + V4L2_CAP_READWRITE | + V4L2_CAP_STREAMING; + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; + + return 0; +} + +static int mxt_vidioc_enum_input(struct file *file, void