Re: [PATCH v1 12/19] uvcvideo: Reorganize next buffer handling.
Hi Pawel,
Thank you for the patch.
On Friday 30 August 2013 11:17:11 Pawel Osciak wrote:
> Move getting the first buffer from the current queue to a uvc_queue function
> and out of the USB completion handler.
Could you please add a sentence explaining why the patch is needed ?
> Signed-off-by: Pawel Osciak
> ---
> drivers/media/usb/uvc/uvc_isight.c | 6 --
> drivers/media/usb/uvc/uvc_queue.c | 14 ++
> drivers/media/usb/uvc/uvc_video.c | 29 -
> drivers/media/usb/uvc/uvcvideo.h | 7 +++
> 4 files changed, 33 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_isight.c
> b/drivers/media/usb/uvc/uvc_isight.c index 8510e72..ab01286 100644
> --- a/drivers/media/usb/uvc/uvc_isight.c
> +++ b/drivers/media/usb/uvc/uvc_isight.c
> @@ -99,10 +99,12 @@ static int isight_decode(struct uvc_video_queue *queue,
> struct uvc_buffer *buf, return 0;
> }
>
> -void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream,
> - struct uvc_buffer *buf)
> +void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream)
> {
> int ret, i;
> + struct uvc_buffer *buf;
Could you please move this on top of the previous line ?
> +
> + buf = uvc_queue_get_first_buf(&stream->queue);
>
> for (i = 0; i < urb->number_of_packets; ++i) {
> if (urb->iso_frame_desc[i].status < 0) {
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index cd962be..55d2670 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -352,6 +352,20 @@ void uvc_queue_cancel(struct uvc_video_queue *queue,
> int disconnect) spin_unlock_irqrestore(&queue->irqlock, flags);
> }
>
> +struct uvc_buffer *uvc_queue_get_first_buf(struct uvc_video_queue *queue)
> +{
> + struct uvc_buffer *buf = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&queue->irqlock, flags);
> + if (!list_empty(&queue->irqqueue))
> + buf = list_first_entry(&queue->irqqueue, struct uvc_buffer,
> + queue);
> + spin_unlock_irqrestore(&queue->irqlock, flags);
> +
> + return buf;
> +}
> +
> struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
> struct uvc_buffer *buf)
> {
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index b4ebccd..2f9a5fa 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1193,11 +1193,11 @@ static int uvc_video_encode_data(struct
> uvc_streaming *stream, /*
> * Completion handler for video URBs.
> */
> -static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming
> *stream, -struct uvc_buffer *buf)
> +static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming
> *stream) {
> u8 *mem;
> int ret, i;
> + struct uvc_buffer *buf = NULL;
Same here (and below).
> for (i = 0; i < urb->number_of_packets; ++i) {
> if (urb->iso_frame_desc[i].status < 0) {
> @@ -1211,6 +1211,7 @@ static void uvc_video_decode_isoc(struct urb *urb,
> struct uvc_streaming *stream,
>
> /* Decode the payload header. */
> mem = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
> + buf = uvc_queue_get_first_buf(&stream->queue);
Can't this call be moved outside of the loop ?
> do {
> ret = uvc_video_decode_start(stream, buf, mem,
> urb->iso_frame_desc[i].actual_length);
> @@ -1241,11 +1242,11 @@ static void uvc_video_decode_isoc(struct urb *urb,
> struct uvc_streaming *stream, }
> }
>
> -static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming
> *stream, -struct uvc_buffer *buf)
> +static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming
> *stream) {
> u8 *mem;
> int len, ret;
> + struct uvc_buffer *buf;
>
> /*
>* Ignore ZLPs if they're not part of a frame, otherwise process them
> @@ -1258,6 +1259,8 @@ static void uvc_video_decode_bulk(struct urb *urb,
> struct uvc_streaming *stream, len = urb->actual_length;
> stream->bulk.payload_size += len;
>
> + buf = uvc_queue_get_first_buf(&stream->queue);
> +
> /* If the URB is the first of its payload, decode and save the
>* header.
>*/
> @@ -1309,12 +1312,13 @@ static void uvc_video_decode_bulk(struct urb *urb,
> struct uvc_streaming *stream, }
> }
>
> -static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming
> *stream, -struct uvc_buffer *buf)
> +static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming
> *stream) {
> u8 *mem = urb->transfer_buffer;
> int len = stream->urb_size, ret;
> + struct uvc_buffer *buf;
>
> + buf = uvc_queue_get_first_buf(&stream->queue);
> if (buf == NULL) {
> urb->transfer_bu
[PATCH v1 12/19] uvcvideo: Reorganize next buffer handling.
Move getting the first buffer from the current queue to a uvc_queue function
and out of the USB completion handler.
Signed-off-by: Pawel Osciak
---
drivers/media/usb/uvc/uvc_isight.c | 6 --
drivers/media/usb/uvc/uvc_queue.c | 14 ++
drivers/media/usb/uvc/uvc_video.c | 29 -
drivers/media/usb/uvc/uvcvideo.h | 7 +++
4 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_isight.c
b/drivers/media/usb/uvc/uvc_isight.c
index 8510e72..ab01286 100644
--- a/drivers/media/usb/uvc/uvc_isight.c
+++ b/drivers/media/usb/uvc/uvc_isight.c
@@ -99,10 +99,12 @@ static int isight_decode(struct uvc_video_queue *queue,
struct uvc_buffer *buf,
return 0;
}
-void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream,
- struct uvc_buffer *buf)
+void uvc_video_decode_isight(struct urb *urb, struct uvc_streaming *stream)
{
int ret, i;
+ struct uvc_buffer *buf;
+
+ buf = uvc_queue_get_first_buf(&stream->queue);
for (i = 0; i < urb->number_of_packets; ++i) {
if (urb->iso_frame_desc[i].status < 0) {
diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index cd962be..55d2670 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -352,6 +352,20 @@ void uvc_queue_cancel(struct uvc_video_queue *queue, int
disconnect)
spin_unlock_irqrestore(&queue->irqlock, flags);
}
+struct uvc_buffer *uvc_queue_get_first_buf(struct uvc_video_queue *queue)
+{
+ struct uvc_buffer *buf = NULL;
+ unsigned long flags;
+
+ spin_lock_irqsave(&queue->irqlock, flags);
+ if (!list_empty(&queue->irqqueue))
+ buf = list_first_entry(&queue->irqqueue, struct uvc_buffer,
+ queue);
+ spin_unlock_irqrestore(&queue->irqlock, flags);
+
+ return buf;
+}
+
struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
struct uvc_buffer *buf)
{
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c
index b4ebccd..2f9a5fa 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1193,11 +1193,11 @@ static int uvc_video_encode_data(struct uvc_streaming
*stream,
/*
* Completion handler for video URBs.
*/
-static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming
*stream,
- struct uvc_buffer *buf)
+static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming
*stream)
{
u8 *mem;
int ret, i;
+ struct uvc_buffer *buf = NULL;
for (i = 0; i < urb->number_of_packets; ++i) {
if (urb->iso_frame_desc[i].status < 0) {
@@ -1211,6 +1211,7 @@ static void uvc_video_decode_isoc(struct urb *urb, struct
uvc_streaming *stream,
/* Decode the payload header. */
mem = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
+ buf = uvc_queue_get_first_buf(&stream->queue);
do {
ret = uvc_video_decode_start(stream, buf, mem,
urb->iso_frame_desc[i].actual_length);
@@ -1241,11 +1242,11 @@ static void uvc_video_decode_isoc(struct urb *urb,
struct uvc_streaming *stream,
}
}
-static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming
*stream,
- struct uvc_buffer *buf)
+static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming
*stream)
{
u8 *mem;
int len, ret;
+ struct uvc_buffer *buf;
/*
* Ignore ZLPs if they're not part of a frame, otherwise process them
@@ -1258,6 +1259,8 @@ static void uvc_video_decode_bulk(struct urb *urb, struct
uvc_streaming *stream,
len = urb->actual_length;
stream->bulk.payload_size += len;
+ buf = uvc_queue_get_first_buf(&stream->queue);
+
/* If the URB is the first of its payload, decode and save the
* header.
*/
@@ -1309,12 +1312,13 @@ static void uvc_video_decode_bulk(struct urb *urb,
struct uvc_streaming *stream,
}
}
-static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming
*stream,
- struct uvc_buffer *buf)
+static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming
*stream)
{
u8 *mem = urb->transfer_buffer;
int len = stream->urb_size, ret;
+ struct uvc_buffer *buf;
+ buf = uvc_queue_get_first_buf(&stream->queue);
if (buf == NULL) {
urb->transfer_buffer_length = 0;
return;
@@ -1355,9 +1359,6 @@ static void uvc_video_encode_bulk(struct urb *urb, struct
uvc_streaming *stream,
static void uvc_video_complete(struct urb *urb)
{
struct uvc_streaming *stream = urb->context;
- struct uvc_video_queue *queue = &stream->queue;
- struct uvc_buffer *buf = NULL;
- unsigned lon
