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

[Florian Echtler <floe@butterbrot.org>]

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]