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:

quoted

On 01/06/2015 10:29 AM, Florian Echtler wrote:

quoted

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.

quoted

 		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.

quoted

quoted

On a different note, I'm getting occasional warnings in syslog when I run 
a regular video streaming application (e.g. cheese):

------------[ cut here ]------------

...

quoted

quoted

---[ 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