Re: [RFC] [Patch] implement video driver for sur40
On Fri, 19 Dec 2014, Hans Verkuil wrote: drivers/media remains under heavy development, so for video capture drivers like yours you should always patch against either the mainline linux tree or (preferred) the media_tree.git repo (git://linuxtv.org/media_tree.git, master branch). As per your suggestion, I've switched development to 3.18, and now I'm nearly there in terms of v4l2-compliance (also see attachment). There's only one failing test left, which is this one: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(284): g_field() == V4L2_FIELD_ANY fail: v4l2-test-buffers.cpp(611): buf.check(q, last_seq) fail: v4l2-test-buffers.cpp(884): captureBufs(node, q, m2m_q, frame_count, false) test MMAP: FAIL test USERPTR: OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total: 45, Succeeded: 44, Failed: 1, Warnings: 0 Could you give some hints on what this means? On a different note, I'm getting occasional warnings in syslog when I run a regular video streaming application (e.g. cheese): [ cut here ] WARNING: CPU: 1 PID: 4995 at /home/apw/COD/linux/drivers/media/v4l2-core/videobuf2-core.c:2144 __vb2_queue_cancel+0x1d0/0x240 [videobuf2_core]() Modules linked in: sur40(OE) videobuf2_dma_contig videobuf2_memops videobuf2_core v4l2_common videodev media dm_crypt wl(POE) snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel rfcomm bnep joydev input_polldev snd_hda_controller snd_hda_codec snd_hwdep kvm_amd kvm snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi edac_core snd_seq snd_seq_device serio_raw snd_timer sp5100_tco k10temp edac_mce_amd i2c_piix4 snd btusb soundcore bluetooth cfg80211 ipmi_si ppdev lp parport_pc ipmi_msghandler parport tpm_infineon mac_hid shpchp hid_apple usbhid hid uas usb_storage pata_acpi radeon i2c_algo_bit ttm psmouse drm_kms_helper pata_atiixp drm r8169 ahci mii libahci [last unloaded: sur40] CPU: 1 PID: 4995 Comm: cheese Tainted: P OE 3.17.1-031701-generic #201410150735 Hardware name: Samsung SUR40/SDNE-R78BA2-20, BIOS SDNE-R78BA2-2000 11/04/2011 0860 8800c2c1bd28 81796c37 0007 8800c2c1bd68 81074a3c 8800c2c1bd58 fff8800c05904f8 8800c05904d0 8800abd65d38 8800abd65d38 Call Trace: [81796c37] dump_stack+0x46/0x58 [81074a3c] warn_slowpath_common+0x8c/0xc0 [81074a8a] warn_slowpath_null+0x1a/0x20 [c05b7a10] __vb2_queue_cancel+0x1d0/0x240 [videobuf2_core] [c05bb3ee] vb2_queue_release+0x1e/0x40 [videobuf2_core] [c05bb481] _vb2_fop_release+0x71/0xb0 [videobuf2_core] [c05bb4ee] vb2_fop_release+0x2e/0x50 [videobuf2_core] [c0c1f491] v4l2_release+0x41/0x90 [videodev] [811eb34d] __fput+0xbd/0x250 [811eb52e] fput+0xe/0x10 [81091504] task_work_run+0xc4/0xe0 [810776a6] do_exit+0x196/0x470 [81082822] ? zap_other_threads+0x82/0xa0 [81077a14] do_group_exit+0x44/0xa0 [81077a87] SyS_exit_group+0x17/0x20 [817a47ad] system_call_fastpath+0x1a/0x1f ---[ end trace 451ed974170f6e44 ]--- Does this mean the driver consumes too much CPU resources? Thanks for your help best regards, 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_.Driver Info: Driver name : sur40 Card type : Samsung SUR40 Bus info : usb-:00:13.2-1 Driver version: 3.17.1 Capabilities : 0x8521 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x0521 Video Capture Read/Write Streaming Extended Pix Format Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not
Re: [RFC] [Patch] implement video driver for sur40
On 01/06/2015 10:29 AM, Florian Echtler wrote: On Fri, 19 Dec 2014, Hans Verkuil wrote: drivers/media remains under heavy development, so for video capture drivers like yours you should always patch against either the mainline linux tree or (preferred) the media_tree.git repo (git://linuxtv.org/media_tree.git, master branch). As per your suggestion, I've switched development to 3.18, and now I'm nearly there in terms of v4l2-compliance (also see attachment). There's only one failing test left, which is this one: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(284): g_field() == V4L2_FIELD_ANY You're not filling in the 'field' field of struct v4l2_buffer when returning a frame. It should most likely be FIELD_NONE in your case. fail: v4l2-test-buffers.cpp(611): buf.check(q, last_seq) fail: v4l2-test-buffers.cpp(884): captureBufs(node, q, m2m_q, frame_count, false) test MMAP: FAIL test USERPTR: OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total: 45, Succeeded: 44, Failed: 1, Warnings: 0 Could you give some hints on what this means? On a different note, I'm getting occasional warnings in syslog when I run a regular video streaming application (e.g. cheese): [ cut here ] WARNING: CPU: 1 PID: 4995 at /home/apw/COD/linux/drivers/media/v4l2-core/videobuf2-core.c:2144 __vb2_queue_cancel+0x1d0/0x240 [videobuf2_core]() Modules linked in: sur40(OE) videobuf2_dma_contig videobuf2_memops videobuf2_core v4l2_common videodev media dm_crypt wl(POE) snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel rfcomm bnep joydev input_polldev snd_hda_controller snd_hda_codec snd_hwdep kvm_amd kvm snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi edac_core snd_seq snd_seq_device serio_raw snd_timer sp5100_tco k10temp edac_mce_amd i2c_piix4 snd btusb soundcore bluetooth cfg80211 ipmi_si ppdev lp parport_pc ipmi_msghandler parport tpm_infineon mac_hid shpchp hid_apple usbhid hid uas usb_storage pata_acpi radeon i2c_algo_bit ttm psmouse drm_kms_helper pata_atiixp drm r8169 ahci mii libahci [last unloaded: sur40] CPU: 1 PID: 4995 Comm: cheese Tainted: P OE 3.17.1-031701-generic #201410150735 Hardware name: Samsung SUR40/SDNE-R78BA2-20, BIOS SDNE-R78BA2-2000 11/04/2011 0860 8800c2c1bd28 81796c37 0007 8800c2c1bd68 81074a3c 8800c2c1bd58 fff8800c05904f8 8800c05904d0 8800abd65d38 8800abd65d38 Call Trace: [81796c37] dump_stack+0x46/0x58 [81074a3c] warn_slowpath_common+0x8c/0xc0 [81074a8a] warn_slowpath_null+0x1a/0x20 [c05b7a10] __vb2_queue_cancel+0x1d0/0x240 [videobuf2_core] [c05bb3ee] vb2_queue_release+0x1e/0x40 [videobuf2_core] [c05bb481] _vb2_fop_release+0x71/0xb0 [videobuf2_core] [c05bb4ee] vb2_fop_release+0x2e/0x50 [videobuf2_core] [c0c1f491] v4l2_release+0x41/0x90 [videodev] [811eb34d] __fput+0xbd/0x250 [811eb52e] fput+0xe/0x10 [81091504] task_work_run+0xc4/0xe0 [810776a6] do_exit+0x196/0x470 [81082822] ? zap_other_threads+0x82/0xa0 [81077a14] do_group_exit+0x44/0xa0 [81077a87] SyS_exit_group+0x17/0x20 [817a47ad] system_call_fastpath+0x1a/0x1f ---[ end trace 451ed974170f6e44 ]--- Does this mean the driver consumes too much CPU resources? No, it means that your driver is not returning all buffers to vb2. Most likely this is missing in the vb2 stop_streaming op. When that is called your driver must return all buffers it has back to vb2 by calling vb2_buffer_done with state ERROR. The same can happen in the start_streaming op if that returns an error for some reason. In that case all buffers owned by the driver should be returned to vb2 with state QUEUED. See also Documentation/video4linux/v4l2-pci-skeleton.c as reference code. Regards, Hans Thanks for your help best regards, Florian -- 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: [RFC] [Patch] implement video driver for sur40
On 06.01.2015 10:36, Hans Verkuil wrote: On 01/06/2015 10:29 AM, Florian Echtler wrote: There's only one failing test left, which is this one: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(284): g_field() == V4L2_FIELD_ANY You're not filling in the 'field' field of struct v4l2_buffer when returning a frame. It should most likely be FIELD_NONE in your case. fail: v4l2-test-buffers.cpp(611): buf.check(q, last_seq) fail: v4l2-test-buffers.cpp(884): captureBufs(node, q, m2m_q, frame_count, false) OK, easy to fix. This will also influence the other two warnings, I assume? On a different note, I'm getting occasional warnings in syslog when I run a regular video streaming application (e.g. cheese): [ cut here ] ... ---[ end trace 451ed974170f6e44 ]--- Does this mean the driver consumes too much CPU resources? No, it means that your driver is not returning all buffers to vb2. Most likely this is missing in the vb2 stop_streaming op. When that is called your driver must return all buffers it has back to vb2 by calling vb2_buffer_done with state ERROR. The same can happen in the start_streaming op if that returns an error for some reason. In that case all buffers owned by the driver should be returned to vb2 with state QUEUED. See also Documentation/video4linux/v4l2-pci-skeleton.c as reference code. I did actually build my driver code based on v4l2-pci-skeleton.c, and I'm calling the exact same return_all_buffers function (see below) with VB2_BUF_STATE_ERROR from my stop_streaming ioctl. static void return_all_buffers(struct sur40_state *sur40, enum vb2_buffer_state state) { struct sur40_buffer *buf, *node; spin_lock(sur40-qlock); list_for_each_entry_safe(buf, node, sur40-buf_list, list) { vb2_buffer_done(buf-vb, state); list_del(buf-list); } spin_unlock(sur40-qlock); } Is there another possible explanation? Thanks best regards, Florian -- SENT FROM MY DEC VT50 TERMINAL signature.asc Description: OpenPGP digital signature
Re: [RFC] [Patch] implement video driver for sur40
On 01/06/2015 11:17 AM, Florian Echtler wrote: On 06.01.2015 10:36, Hans Verkuil wrote: On 01/06/2015 10:29 AM, Florian Echtler wrote: There's only one failing test left, which is this one: Streaming ioctls: test read/write: OK fail: v4l2-test-buffers.cpp(284): g_field() == V4L2_FIELD_ANY You're not filling in the 'field' field of struct v4l2_buffer when returning a frame. It should most likely be FIELD_NONE in your case. fail: v4l2-test-buffers.cpp(611): buf.check(q, last_seq) fail: v4l2-test-buffers.cpp(884): captureBufs(node, q, m2m_q, frame_count, false) OK, easy to fix. This will also influence the other two warnings, I assume? Most likely, yes. On a different note, I'm getting occasional warnings in syslog when I run a regular video streaming application (e.g. cheese): [ cut here ] ... ---[ end trace 451ed974170f6e44 ]--- Does this mean the driver consumes too much CPU resources? No, it means that your driver is not returning all buffers to vb2. Most likely this is missing in the vb2 stop_streaming op. When that is called your driver must return all buffers it has back to vb2 by calling vb2_buffer_done with state ERROR. The same can happen in the start_streaming op if that returns an error for some reason. In that case all buffers owned by the driver should be returned to vb2 with state QUEUED. See also Documentation/video4linux/v4l2-pci-skeleton.c as reference code. I did actually build my driver code based on v4l2-pci-skeleton.c, and I'm calling the exact same return_all_buffers function (see below) with VB2_BUF_STATE_ERROR from my stop_streaming ioctl. static void return_all_buffers(struct sur40_state *sur40, enum vb2_buffer_state state) { struct sur40_buffer *buf, *node; spin_lock(sur40-qlock); list_for_each_entry_safe(buf, node, sur40-buf_list, list) { vb2_buffer_done(buf-vb, state); list_del(buf-list); } spin_unlock(sur40-qlock); } Is there another possible explanation? No :-) You are still missing a buffer somewhere. I'd have to see your latest source code to see what's wrong. Some drivers (esp. USB drivers) use a separate pointer to the active buffer, so that buffer is no longer part of the buf_list, but still needs to be returned in stop_streaming. Could that be the cause perhaps? Regards, Hans -- 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: [RFC] [Patch] implement video driver for sur40
On 06.01.2015 11:23, Hans Verkuil wrote: On 01/06/2015 11:17 AM, Florian Echtler wrote: You're not filling in the 'field' field of struct v4l2_buffer when returning a frame. It should most likely be FIELD_NONE in your case. fail: v4l2-test-buffers.cpp(611): buf.check(q, last_seq) fail: v4l2-test-buffers.cpp(884): captureBufs(node, q, m2m_q, frame_count, false) OK, easy to fix. This will also influence the other two warnings, I assume? Most likely, yes. Done. I would say that it's nearly ready for submission now (all tests from v4l2-compliance -s pass), I still have to sort out all the warnings from scripts/checkpatch.pl. On a different note, I'm getting occasional warnings in syslog when I run a regular video streaming application (e.g. cheese): Is there another possible explanation? No :-) You are still missing a buffer somewhere. I'd have to see your latest source code to see what's wrong. Weirdly enough, the syslog warning/error doesn't seem to occur anymore since I've fixed the v4l2_buffer field. Perhaps some oddity within cheese? I'm attaching the current source again for you to maybe have another look; I will submit a proper patch in the next days. Thanks again for your help! Best, Florian -- SENT FROM MY DEC VT50 TERMINAL /* * Surface2.0/SUR40/PixelSense input driver * * Copyright (c) 2013 by Florian 'floe' Echtler f...@butterbrot.org * * Derived from the USB Skeleton driver 1.1, * Copyright (c) 2003 Greg Kroah-Hartman (g...@kroah.com) * * and from the Apple USB BCM5974 multitouch driver, * Copyright (c) 2008 Henrik Rydberg (rydb...@euromail.se) * * and from the generic hid-multitouch driver, * Copyright (c) 2010-2012 Stephane Chatty cha...@enac.fr * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation; either version 2 of * the License, or (at your option) any later version. */ #include linux/kernel.h #include linux/errno.h #include linux/delay.h #include linux/init.h #include linux/slab.h #include linux/module.h #include linux/completion.h #include linux/uaccess.h #include linux/usb.h #include linux/printk.h #include linux/input-polldev.h #include linux/input/mt.h #include linux/usb/input.h #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-dev.h #include media/v4l2-ioctl.h #include media/videobuf2-dma-contig.h /* read 512 bytes from endpoint 0x86 - get header + blobs */ struct sur40_header { __le16 type; /* always 0x0001 */ __le16 count; /* count of blobs (if 0: continue prev. packet) */ __le32 packet_id; /* unique ID for all packets in one frame */ __le32 timestamp; /* milliseconds (inc. by 16 or 17 each frame) */ __le32 unknown;/* epoch? always 02/03 00 00 00 */ } __packed; struct sur40_blob { __le16 blob_id; u8 action; /* 0x02 = enter/exit, 0x03 = update (?) */ u8 unknown;/* always 0x01 or 0x02 (no idea what this is?) */ __le16 bb_pos_x; /* upper left corner of bounding box */ __le16 bb_pos_y; __le16 bb_size_x; /* size of bounding box */ __le16 bb_size_y; __le16 pos_x; /* finger tip position */ __le16 pos_y; __le16 ctr_x; /* centroid position */ __le16 ctr_y; __le16 axis_x; /* somehow related to major/minor axis, mostly: */ __le16 axis_y; /* axis_x == bb_size_y axis_y == bb_size_x */ __le32 angle; /* orientation in radians relative to x axis - actually an IEEE754 float, don't use in kernel */ __le32 area; /* size in pixels/pressure (?) */ u8 padding[32]; } __packed; /* combined header/blob data */ struct sur40_data { struct sur40_header header; struct sur40_blob blobs[]; } __packed; /* read 512 bytes from endpoint 0x82 - get header below * continue reading 16k blocks until header.size bytes read */ struct sur40_image_header { __le32 magic; /* SUBF */ __le32 packet_id; __le32 size; /* always 0x0007e900 = 960x540 */ __le32 timestamp; /* milliseconds (increases by 16 or 17 each frame) */ __le32 unknown; /* epoch? always 02/03 00 00 00 */ } __packed; /* version information */ #define DRIVER_SHORT sur40 #define DRIVER_LONGSamsung SUR40 #define DRIVER_AUTHOR Florian 'floe' Echtler f...@butterbrot.org #define DRIVER_DESCSurface2.0/SUR40/PixelSense input driver /* vendor and device IDs */ #define ID_MICROSOFT 0x045e #define ID_SUR40 0x0775 /* sensor resolution */ #define SENSOR_RES_X 1920 #define SENSOR_RES_Y 1080 /* touch data endpoint */ #define TOUCH_ENDPOINT 0x86 /* video data endpoint */ #define VIDEO_ENDPOINT 0x82 /* video header fields */ #define VIDEO_HEADER_MAGIC 0x46425553 #define VIDEO_PACKET_SIZE 16384 /* polling interval (ms) */ #define POLL_INTERVAL 10 /* maximum number of contacts FIXME: this is a guess? */ #define MAX_CONTACTS 64 /* control commands */ #define SUR40_GET_VERSION 0xb0 /* 12 bytes string*/
Re: [RFC] [Patch] implement video driver for sur40
On 01/06/2015 01:09 PM, Florian Echtler wrote: On 06.01.2015 11:23, Hans Verkuil wrote: On 01/06/2015 11:17 AM, Florian Echtler wrote: You're not filling in the 'field' field of struct v4l2_buffer when returning a frame. It should most likely be FIELD_NONE in your case. fail: v4l2-test-buffers.cpp(611): buf.check(q, last_seq) fail: v4l2-test-buffers.cpp(884): captureBufs(node, q, m2m_q, frame_count, false) OK, easy to fix. This will also influence the other two warnings, I assume? Most likely, yes. Done. I would say that it's nearly ready for submission now (all tests from v4l2-compliance -s pass), I still have to sort out all the warnings from scripts/checkpatch.pl. On a different note, I'm getting occasional warnings in syslog when I run a regular video streaming application (e.g. cheese): Is there another possible explanation? No :-) You are still missing a buffer somewhere. I'd have to see your latest source code to see what's wrong. Weirdly enough, the syslog warning/error doesn't seem to occur anymore since I've fixed the v4l2_buffer field. Perhaps some oddity within cheese? I'm attaching the current source again for you to maybe have another look; I will submit a proper patch in the next days. Just a few quick remarks: - run scripts/checkpatch.pl over your source, I'm fairly certain it will complain about several constructs. - use videobuf2-vmalloc instead of dma-contig. There is no DMA involved, so there is no reason to use dma-contig. - Don't set V4L2_CAP_EXT_PIX_FORMAT in querycap: it will be set automatically by the v4l2 core. Regards, Hans -- 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: [RFC] [Patch] implement video driver for sur40
On 12/19/2014 03:30 PM, Florian Echtler wrote: On 18.12.2014 15:11, Hans Verkuil wrote: Run as 'v4l2-compliance -s' (-s starts streaming tests as well and it assumes you have a valid input signal). Mail if you have any questions about the v4l2-compliance output. The failure messages expect you to look at the v4l2-compliance source code as well, but even than it is not always clear what is going on. Ran the most recent version from git master, got a total of 6 fails, 4 of which are probably easy fixes: fail: v4l2-compliance.cpp(306): missing bus_info prefix ('USB:1') test VIDIOC_QUERYCAP: FAIL Changed the relevant code to: usb_make_path(sur40-usbdev, cap-bus_info, sizeof(cap-bus_info)); fail: v4l2-test-input-output.cpp(455): could set input to invalid input 1 test VIDIOC_G/S/ENUMINPUT: FAIL Now returning -EINVAL when S_INPUT called with input != 0. fail: v4l2-test-formats.cpp(322): !colorspace fail: v4l2-test-formats.cpp(429): testColorspace(pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization) test VIDIOC_G_FMT: FAIL Setting colorspace in v4l2_pix_format to V4L2_COLORSPACE_SRGB. fail: v4l2-compliance.cpp(365): doioctl(node, VIDIOC_G_PRIORITY, prio) test VIDIOC_G/S_PRIORITY: FAIL Don't know how to fix this - does this mean VIDIOC_G/S_PRIORITY _must_ be implemented? fail: v4l2-test-buffers.cpp(500): q.has_expbuf(node) test VIDIOC_EXPBUF: FAIL Also not clear how to fix this one. Could you give some hints on the last two? Can you post the driver code you used to run these tests? And which kernel version and git tree did you base your patch on? Regards, Hans Thanks best regards, Florian -- 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: [RFC] [Patch] implement video driver for sur40
On 19.12.2014 15:36, Hans Verkuil wrote: On 12/19/2014 03:30 PM, Florian Echtler wrote: Ran the most recent version from git master, got a total of 6 fails, 4 of which are probably easy fixes: fail: v4l2-compliance.cpp(306): missing bus_info prefix ('USB:1') test VIDIOC_QUERYCAP: FAIL Changed the relevant code to: usb_make_path(sur40-usbdev, cap-bus_info, sizeof(cap-bus_info)); fail: v4l2-test-input-output.cpp(455): could set input to invalid input 1 test VIDIOC_G/S/ENUMINPUT: FAIL Now returning -EINVAL when S_INPUT called with input != 0. fail: v4l2-test-formats.cpp(322): !colorspace fail: v4l2-test-formats.cpp(429): testColorspace(pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization) test VIDIOC_G_FMT: FAIL Setting colorspace in v4l2_pix_format to V4L2_COLORSPACE_SRGB. fail: v4l2-compliance.cpp(365): doioctl(node, VIDIOC_G_PRIORITY, prio) test VIDIOC_G/S_PRIORITY: FAIL Don't know how to fix this - does this mean VIDIOC_G/S_PRIORITY _must_ be implemented? fail: v4l2-test-buffers.cpp(500): q.has_expbuf(node) test VIDIOC_EXPBUF: FAIL Also not clear how to fix this one. Could you give some hints on the last two? Can you post the driver code you used to run these tests? And which kernel version and git tree did you base your patch on? Driver code is attached (should be identical to the one from initial mail). Kernel version used for the tests is 3.16.0-25 from Ubuntu Utopic, git tree for patches is currently https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git I'm building the module standalone on the target machine, since it's not powerful enough that you would want to do a full kernel build. However, since the driver was merged into mainline, no other changes have been made, so I think it shouldn't be a problem to patch against the original git tree? Best, Florian -- SENT FROM MY DEC VT50 TERMINAL /* * Surface2.0/SUR40/PixelSense input driver * * Copyright (c) 2013 by Florian 'floe' Echtler f...@butterbrot.org * * Derived from the USB Skeleton driver 1.1, * Copyright (c) 2003 Greg Kroah-Hartman (g...@kroah.com) * * and from the Apple USB BCM5974 multitouch driver, * Copyright (c) 2008 Henrik Rydberg (rydb...@euromail.se) * * and from the generic hid-multitouch driver, * Copyright (c) 2010-2012 Stephane Chatty cha...@enac.fr * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation; either version 2 of * the License, or (at your option) any later version. */ #include linux/kernel.h #include linux/errno.h #include linux/delay.h #include linux/init.h #include linux/slab.h #include linux/module.h #include linux/completion.h #include linux/uaccess.h #include linux/usb.h #include linux/printk.h #include linux/input-polldev.h #include linux/input/mt.h #include linux/usb/input.h #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-dev.h #include media/v4l2-ioctl.h #include media/videobuf2-dma-contig.h /* read 512 bytes from endpoint 0x86 - get header + blobs */ struct sur40_header { __le16 type; /* always 0x0001 */ __le16 count; /* count of blobs (if 0: continue prev. packet) */ __le32 packet_id; /* unique ID for all packets in one frame */ __le32 timestamp; /* milliseconds (inc. by 16 or 17 each frame) */ __le32 unknown;/* epoch? always 02/03 00 00 00 */ } __packed; struct sur40_blob { __le16 blob_id; u8 action; /* 0x02 = enter/exit, 0x03 = update (?) */ u8 unknown;/* always 0x01 or 0x02 (no idea what this is?) */ __le16 bb_pos_x; /* upper left corner of bounding box */ __le16 bb_pos_y; __le16 bb_size_x; /* size of bounding box */ __le16 bb_size_y; __le16 pos_x; /* finger tip position */ __le16 pos_y; __le16 ctr_x; /* centroid position */ __le16 ctr_y; __le16 axis_x; /* somehow related to major/minor axis, mostly: */ __le16 axis_y; /* axis_x == bb_size_y axis_y == bb_size_x */ __le32 angle; /* orientation in radians relative to x axis - actually an IEEE754 float, don't use in kernel */ __le32 area; /* size in pixels/pressure (?) */ u8 padding[32]; } __packed; /* combined header/blob data */ struct sur40_data { struct sur40_header header; struct sur40_blob blobs[]; } __packed; /* read 512 bytes from endpoint 0x82 - get header below * continue reading 16k blocks until header.size bytes read */ struct sur40_image_header { __le32 magic; /* SUBF */ __le32 packet_id; __le32 size; /* always 0x0007e900 = 960x540 */ __le32 timestamp; /* milliseconds (increases by 16 or 17 each frame) */ __le32 unknown; /* epoch? always 02/03 00 00 00 */ } __packed; /* version information */ #define DRIVER_SHORT sur40 #define DRIVER_LONGSamsung SUR40 #define DRIVER_AUTHOR Florian 'floe' Echtler f...@butterbrot.org #define DRIVER_DESC
Re: [RFC] [Patch] implement video driver for sur40
On 12/19/2014 03:57 PM, Florian Echtler wrote: On 19.12.2014 15:36, Hans Verkuil wrote: On 12/19/2014 03:30 PM, Florian Echtler wrote: Ran the most recent version from git master, got a total of 6 fails, 4 of which are probably easy fixes: fail: v4l2-compliance.cpp(306): missing bus_info prefix ('USB:1') test VIDIOC_QUERYCAP: FAIL Changed the relevant code to: usb_make_path(sur40-usbdev, cap-bus_info, sizeof(cap-bus_info)); fail: v4l2-test-input-output.cpp(455): could set input to invalid input 1 test VIDIOC_G/S/ENUMINPUT: FAIL Now returning -EINVAL when S_INPUT called with input != 0. fail: v4l2-test-formats.cpp(322): !colorspace fail: v4l2-test-formats.cpp(429): testColorspace(pix.pixelformat, pix.colorspace, pix.ycbcr_enc, pix.quantization) test VIDIOC_G_FMT: FAIL Setting colorspace in v4l2_pix_format to V4L2_COLORSPACE_SRGB. fail: v4l2-compliance.cpp(365): doioctl(node, VIDIOC_G_PRIORITY, prio) test VIDIOC_G/S_PRIORITY: FAIL Don't know how to fix this - does this mean VIDIOC_G/S_PRIORITY _must_ be implemented? fail: v4l2-test-buffers.cpp(500): q.has_expbuf(node) test VIDIOC_EXPBUF: FAIL Also not clear how to fix this one. This is most likely fallout from the G_FMT failure above. If you fix that, then this should be OK. Could you give some hints on the last two? Can you post the driver code you used to run these tests? And which kernel version and git tree did you base your patch on? Driver code is attached (should be identical to the one from initial mail). Kernel version used for the tests is 3.16.0-25 from Ubuntu OK, 3.16 explains the PRIO failure. For that kernel you need to set this flag in struct video_device: set_bit(V4L2_FL_USE_FH_PRIO, vdev-flags); This flag went away in 3.17 or 3.18 since it has now become standard behavior. Utopic, git tree for patches is currently https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git I'm building the module standalone on the target machine, since it's not powerful enough that you would want to do a full kernel build. However, since the driver was merged into mainline, no other changes have been made, so I think it shouldn't be a problem to patch against the original git tree? drivers/media remains under heavy development, so for video capture drivers like yours you should always patch against either the mainline linux tree or (preferred) the media_tree.git repo (git://linuxtv.org/media_tree.git, master branch). Regards, Hans -- 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
[RFC] [Patch] implement video driver for sur40
Hello everyone, as promised, I've finally implemented the missing raw video feature for the SUR40 touchscreen. Since this is a bit of a weird hybrid device (multitouch input as well as video), I'm hoping for comments from both communities (linux-input and linux-media). I'm also attaching the full source code for the moment (not yet a proper patch submission) so it's perhaps easier to get a view of the whole thing. There's definitely some obvious cleanup still to be done (e.g. endpoint selection), but I'd like to ask for feedback early so I can get most issues fixed before really submitting a patch. Thanks best regards, Florian -- SENT FROM MY DEC VT50 TERMINAL /* * Surface2.0/SUR40/PixelSense input driver * * Copyright (c) 2013 by Florian 'floe' Echtler f...@butterbrot.org * * Derived from the USB Skeleton driver 1.1, * Copyright (c) 2003 Greg Kroah-Hartman (g...@kroah.com) * * and from the Apple USB BCM5974 multitouch driver, * Copyright (c) 2008 Henrik Rydberg (rydb...@euromail.se) * * and from the generic hid-multitouch driver, * Copyright (c) 2010-2012 Stephane Chatty cha...@enac.fr * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation; either version 2 of * the License, or (at your option) any later version. */ #include linux/kernel.h #include linux/errno.h #include linux/delay.h #include linux/init.h #include linux/slab.h #include linux/module.h #include linux/completion.h #include linux/uaccess.h #include linux/usb.h #include linux/printk.h #include linux/input-polldev.h #include linux/input/mt.h #include linux/usb/input.h #include linux/videodev2.h #include media/v4l2-device.h #include media/v4l2-dev.h #include media/v4l2-ioctl.h #include media/videobuf2-dma-contig.h /* read 512 bytes from endpoint 0x86 - get header + blobs */ struct sur40_header { __le16 type; /* always 0x0001 */ __le16 count; /* count of blobs (if 0: continue prev. packet) */ __le32 packet_id; /* unique ID for all packets in one frame */ __le32 timestamp; /* milliseconds (inc. by 16 or 17 each frame) */ __le32 unknown;/* epoch? always 02/03 00 00 00 */ } __packed; struct sur40_blob { __le16 blob_id; u8 action; /* 0x02 = enter/exit, 0x03 = update (?) */ u8 unknown;/* always 0x01 or 0x02 (no idea what this is?) */ __le16 bb_pos_x; /* upper left corner of bounding box */ __le16 bb_pos_y; __le16 bb_size_x; /* size of bounding box */ __le16 bb_size_y; __le16 pos_x; /* finger tip position */ __le16 pos_y; __le16 ctr_x; /* centroid position */ __le16 ctr_y; __le16 axis_x; /* somehow related to major/minor axis, mostly: */ __le16 axis_y; /* axis_x == bb_size_y axis_y == bb_size_x */ __le32 angle; /* orientation in radians relative to x axis - actually an IEEE754 float, don't use in kernel */ __le32 area; /* size in pixels/pressure (?) */ u8 padding[32]; } __packed; /* combined header/blob data */ struct sur40_data { struct sur40_header header; struct sur40_blob blobs[]; } __packed; /* read 512 bytes from endpoint 0x82 - get header below * continue reading 16k blocks until header.size bytes read */ struct sur40_image_header { __le32 magic; /* SUBF */ __le32 packet_id; __le32 size; /* always 0x0007e900 = 960x540 */ __le32 timestamp; /* milliseconds (increases by 16 or 17 each frame) */ __le32 unknown; /* epoch? always 02/03 00 00 00 */ } __packed; /* version information */ #define DRIVER_SHORT sur40 #define DRIVER_LONGSamsung SUR40 #define DRIVER_AUTHOR Florian 'floe' Echtler f...@butterbrot.org #define DRIVER_DESCSurface2.0/SUR40/PixelSense input driver /* vendor and device IDs */ #define ID_MICROSOFT 0x045e #define ID_SUR40 0x0775 /* sensor resolution */ #define SENSOR_RES_X 1920 #define SENSOR_RES_Y 1080 /* touch data endpoint */ #define TOUCH_ENDPOINT 0x86 /* video data endpoint */ #define VIDEO_ENDPOINT 0x82 /* video header fields */ #define VIDEO_HEADER_MAGIC 0x46425553 #define VIDEO_PACKET_SIZE 16384 /* polling interval (ms) */ #define POLL_INTERVAL 10 /* maximum number of contacts FIXME: this is a guess? */ #define MAX_CONTACTS 64 /* control commands */ #define SUR40_GET_VERSION 0xb0 /* 12 bytes string*/ #define SUR40_UNKNOWN10xb3 /* 5 bytes */ #define SUR40_UNKNOWN20xc1 /* 24 bytes */ #define SUR40_GET_STATE 0xc5 /* 4 bytes state (?) */ #define SUR40_GET_SENSORS 0xb1 /* 8 bytes sensors */ /* * Note: an earlier, non-public version of this driver used USB_RECIP_ENDPOINT * here by mistake which is very likely to have corrupted the firmware EEPROM * on two separate SUR40 devices. Thanks to Alan Stern who spotted this bug. * Should you ever run into a similar problem, the background story to this * incident and instructions on how to fix the corrupted EEPROM are available *
Re: [RFC] [Patch] implement video driver for sur40
On 12/18/14 14:34, Florian Echtler wrote: Hello everyone, as promised, I've finally implemented the missing raw video feature for the SUR40 touchscreen. Since this is a bit of a weird hybrid device (multitouch input as well as video), I'm hoping for comments from both communities (linux-input and linux-media). I'm also attaching the full source code for the moment (not yet a proper patch submission) so it's perhaps easier to get a view of the whole thing. There's definitely some obvious cleanup still to be done (e.g. endpoint selection), but I'd like to ask for feedback early so I can get most issues fixed before really submitting a patch. Thanks best regards, Florian One thing you should do is to run the v4l2-compliance utility. Available here: http://git.linuxtv.org/cgit.cgi/v4l-utils.git/ Compile from the git repo to ensure you have the latest version. Run as 'v4l2-compliance -s' (-s starts streaming tests as well and it assumes you have a valid input signal). The driver shouldn't give any failures. When you post the actual patch, then make sure you also paste in the output of 'v4l2-compliance -s' as I'd like to see what it says. Mail if you have any questions about the v4l2-compliance output. The failure messages expect you to look at the v4l2-compliance source code as well, but even than it is not always clear what is going on. Regards, Hans -- 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