Re: [RFC] [Patch] implement video driver for sur40

2015-01-06 Thread Florian Echtler

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

2015-01-06 Thread Hans Verkuil
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

2015-01-06 Thread Florian Echtler
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

2015-01-06 Thread Hans Verkuil
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

2015-01-06 Thread Florian Echtler
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

2015-01-06 Thread Hans Verkuil
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

2014-12-19 Thread Hans Verkuil


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

2014-12-19 Thread Florian Echtler
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

2014-12-19 Thread Hans Verkuil


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

2014-12-18 Thread Florian Echtler
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

2014-12-18 Thread Hans Verkuil
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