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

[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