Hi Michael,
On Monday 04 February 2013 11:58:27 Michael Grzeschik wrote:
> On Thu, Jan 17, 2013 at 04:23:51PM +0530, Bhupesh Sharma wrote:
> > This patch reworks the videobuffer management logic present in the UVC
> > webcam gadget and ports it to use the "more apt" videobuf2 framework for
> > video buffer management.
> >
> > To support routing video data captured from a real V4L2 video capture
> > device with a "zero copy" operation on videobuffers (as they pass from
> > the V4L2 domain to UVC domain via a user-space application), we need to
> > support USER_PTR IO method at the UVC gadget side.
> >
> > So the V4L2 capture device driver can still continue to use MMAP IO
> > method and now the user-space application can just pass a pointer to the
> > video buffers being dequeued from the V4L2 device side while queueing
> > them at the UVC gadget end. This ensures that we have a "zero-copy"
> > design as the videobuffers pass from the V4L2 capture device to the UVC
> > gadget.
> >
> > Note that there will still be a need to apply UVC specific payload
> > headers on top of each UVC payload data, which will still require a copy
> > operation to be performed in the 'encode' routines of the UVC gadget.
> >
> > This patch also addresses one issue found out while porting the UVC
> > gadget to videobuf2 framework:
> > - In case the usb requests queued by the gadget get completed
> > with a status of -ESHUTDOWN (disconnected from host),
> > the queue of videobuf2 should be cancelled to ensure that the
> > application space daemon is not left in a state waiting for
> > a vb2 to be successfully absorbed at the USB side.
> >
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@xxxxxx>
> > ---
> >
> > drivers/usb/gadget/Kconfig | 1 +
> > drivers/usb/gadget/uvc_queue.c | 537 +++++++++++------------------------
> > drivers/usb/gadget/uvc_queue.h | 25 +--
> > drivers/usb/gadget/uvc_v4l2.c | 27 +--
> > drivers/usb/gadget/uvc_video.c | 18 +-
> > 5 files changed, 189 insertions(+), 419 deletions(-)
[snip]
> > diff --git a/drivers/usb/gadget/uvc_queue.c
> > b/drivers/usb/gadget/uvc_queue.c index 104ae9c..bd20fab 100644
> > --- a/drivers/usb/gadget/uvc_queue.c
> > +++ b/drivers/usb/gadget/uvc_queue.c
[snip]
> > -static int uvc_queue_waiton(struct uvc_buffer *buf, int nonblocking)
> > +static int uvc_queue_buffer(struct uvc_video_queue *queue,
> > + struct v4l2_buffer *buf)
> > {
> > - if (nonblocking) {
> > - return (buf->state != UVC_BUF_STATE_QUEUED &&
> > - buf->state != UVC_BUF_STATE_ACTIVE)
> > - ? 0 : -EAGAIN;
> > - }
> > + int ret;
> >
> > - return wait_event_interruptible(buf->wait,
> > - buf->state != UVC_BUF_STATE_QUEUED &&
> > - buf->state != UVC_BUF_STATE_ACTIVE);
> > + mutex_lock(&queue->mutex);
> > + ret = vb2_qbuf(&queue->queue, buf);
> > + mutex_unlock(&queue->mutex);
> > +
>
> How is the UVC_QUEUE_PAUSED handling supposed to be handled here?
> I see that this patch lost this hunk from uvc_queue_buffer():
>
> ret |= (queue->flags & uvc_queue_paused) != 0;
> queue->flags &= ~UVC_QUEUE_PAUSED;
Good catch. This needs to be handled. The PAUSED flag needs to be set with the
irqlock held, this could be done in uvc_buffer_queue(). The return value needs
to be computed when setting the flag, so it would need to be stored in the
queue structure and read here. Adding a new UVC_QUEUE_PUMP flag should do but
will require taking the irqlock again to read and clear that flag here. A new
field is another possible solution.
Bhupesh, could implement that in v4 ? This and the poll change proposed by
Michael in his other e-mail look like the last issues that need to be fixed.
> > + return ret;
> > }
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
