Re: [PATCH v2 3/3] media: uvcvideo: Remove stream->is_streaming field

[Ricardo Ribalda]

Hi Laurent, Hi Hans
> > If this isn't fixed, then at least add a comment explaining why you test for
> > != V4L2_BUF_TYPE_META_CAPTURE before calling uvc_pm_put. It's not obvious.
>
> Agreed.

Maybe this is better than a comment?

diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index 72c5494dee9f..7f9d731df32c 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -39,8 +39,6 @@ static inline struct uvc_buffer
*uvc_vbuf_to_buffer(struct vb2_v4l2_buffer *buf)

 /*
  * Return all queued buffers to videobuf2 in the requested state.
- *
- * This function must be called with the queue spinlock held.
  */
 static void uvc_queue_return_buffers(struct uvc_video_queue *queue,
                               enum uvc_buffer_state state)
@@ -49,6 +47,8 @@ static void uvc_queue_return_buffers(struct
uvc_video_queue *queue,
                                        ? VB2_BUF_STATE_ERROR
                                        : VB2_BUF_STATE_QUEUED;

+       spin_lock_irq(&queue->irqlock);
+
        while (!list_empty(&queue->irqqueue)) {
                struct uvc_buffer *buf = list_first_entry(&queue->irqqueue,
                                                          struct uvc_buffer,
@@ -57,6 +57,8 @@ static void uvc_queue_return_buffers(struct
uvc_video_queue *queue,
                buf->state = state;
                vb2_buffer_done(&buf->buf.vb2_buf, vb2_state);
        }
+
+       spin_unlock_irq(&queue->irqlock);
 }

 /* -----------------------------------------------------------------------------
@@ -157,7 +159,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
                uvc_video_clock_update(stream, vbuf, buf);
 }

-static int uvc_start_streaming(struct vb2_queue *vq, unsigned int count)
+static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
 {
        struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
        struct uvc_streaming *stream = uvc_queue_to_stream(queue);
@@ -171,25 +173,29 @@ static int uvc_start_streaming(struct vb2_queue
*vq, unsigned int count)
        if (ret == 0)
                return 0;

-       spin_lock_irq(&queue->irqlock);
        uvc_queue_return_buffers(queue, UVC_BUF_STATE_QUEUED);
-       spin_unlock_irq(&queue->irqlock);

        return ret;
 }

-static void uvc_stop_streaming(struct vb2_queue *vq)
+static void uvc_stop_streaming_meta(struct vb2_queue *vq)
 {
        struct uvc_video_queue *queue = vb2_get_drv_priv(vq);

        lockdep_assert_irqs_enabled();

-       if (vq->type != V4L2_BUF_TYPE_META_CAPTURE)
-               uvc_video_stop_streaming(uvc_queue_to_stream(queue));
+       uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
+}
+
+static void uvc_stop_streaming_video(struct vb2_queue *vq)
+{
+       struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
+
+       lockdep_assert_irqs_enabled();
+
+       uvc_video_stop_streaming(uvc_queue_to_stream(queue));

-       spin_lock_irq(&queue->irqlock);
        uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
-       spin_unlock_irq(&queue->irqlock);
 }

 static const struct vb2_ops uvc_queue_qops = {
@@ -197,15 +203,15 @@ static const struct vb2_ops uvc_queue_qops = {
        .buf_prepare = uvc_buffer_prepare,
        .buf_queue = uvc_buffer_queue,
        .buf_finish = uvc_buffer_finish,
-       .start_streaming = uvc_start_streaming,
-       .stop_streaming = uvc_stop_streaming,
+       .start_streaming = uvc_start_streaming_video,
+       .stop_streaming = uvc_stop_streaming_video,
 };

 static const struct vb2_ops uvc_meta_queue_qops = {
        .queue_setup = uvc_queue_setup,
        .buf_prepare = uvc_buffer_prepare,
        .buf_queue = uvc_buffer_queue,
-       .stop_streaming = uvc_stop_streaming,
+       .stop_streaming = uvc_stop_streaming_meta,
 };

 int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type)



-- 
Ricardo Ribalda