Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
On 06/06/2014 03:42 PM, Laurent Pinchart wrote: > On Friday 06 June 2014 11:55:49 Hans Verkuil wrote: >> On 06/06/2014 11:46 AM, Laurent Pinchart wrote: >>> On Friday 06 June 2014 11:31:55 Hans Verkuil wrote: On 06/06/2014 11:19 AM, Laurent Pinchart wrote: > On Friday 06 June 2014 14:31:15 Pawel Osciak wrote: >> Hi Laurent, >> Thanks for the patch. Did you test this to work in fileio mode? Looks >> like it should, but would like to make sure. > > No, I haven't tested it. The OMAP4 ISS driver, which is my test target > for this patch, doesn't support fileio mode. Adding VB2_READ would be > easy, but the driver requires configuring the format on the file handle > used for streaming, so I can't just run cat /dev/video*. Just test with vivi. >>> >>> But vivi doesn't call the new vb2_queue_error() function. I understand >>> that your vivi rework would make that easier as you now have an error >>> control. Should I hack something similar in the existing vivi driver ? Any >>> pointer ? >> >> Just hack it in for testing. E.g. call it when the button control is pressed >> (see vivi_s_ctrl). > > Tested-by: Laurent Pinchart > > "cat /dev/video0" outputs data until vivi calls vb2_queue_error(), at which > points cat prints > > cat: /dev/video0: Input/output error > > Restarting capture works as expected. > Nice. Once this is merged I plan on adding support for this to my vivi rewrite. Finishing the vivi rewrite (mostly cleaning things up and documentation where appropriate) is a high-prio for me. I'd like to get this in for 3.17. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
On Friday 06 June 2014 11:55:49 Hans Verkuil wrote: > On 06/06/2014 11:46 AM, Laurent Pinchart wrote: > > On Friday 06 June 2014 11:31:55 Hans Verkuil wrote: > >> On 06/06/2014 11:19 AM, Laurent Pinchart wrote: > >>> On Friday 06 June 2014 14:31:15 Pawel Osciak wrote: > Hi Laurent, > Thanks for the patch. Did you test this to work in fileio mode? Looks > like it should, but would like to make sure. > >>> > >>> No, I haven't tested it. The OMAP4 ISS driver, which is my test target > >>> for this patch, doesn't support fileio mode. Adding VB2_READ would be > >>> easy, but the driver requires configuring the format on the file handle > >>> used for streaming, so I can't just run cat /dev/video*. > >> > >> Just test with vivi. > > > > But vivi doesn't call the new vb2_queue_error() function. I understand > > that your vivi rework would make that easier as you now have an error > > control. Should I hack something similar in the existing vivi driver ? Any > > pointer ? > > Just hack it in for testing. E.g. call it when the button control is pressed > (see vivi_s_ctrl). Tested-by: Laurent Pinchart "cat /dev/video0" outputs data until vivi calls vb2_queue_error(), at which points cat prints cat: /dev/video0: Input/output error Restarting capture works as expected. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
On 06/06/2014 11:46 AM, Laurent Pinchart wrote: > Hi Hans, > > On Friday 06 June 2014 11:31:55 Hans Verkuil wrote: >> On 06/06/2014 11:19 AM, Laurent Pinchart wrote: >>> Hi Pawel, >>> >>> On Friday 06 June 2014 14:31:15 Pawel Osciak wrote: Hi Laurent, Thanks for the patch. Did you test this to work in fileio mode? Looks like it should, but would like to make sure. >>> >>> No, I haven't tested it. The OMAP4 ISS driver, which is my test target for >>> this patch, doesn't support fileio mode. Adding VB2_READ would be easy, >>> but the driver requires configuring the format on the file handle used for >>> streaming, so I can't just run cat /dev/video*. >> >> Just test with vivi. > > But vivi doesn't call the new vb2_queue_error() function. I understand that > your vivi rework would make that easier as you now have an error control. > Should I hack something similar in the existing vivi driver ? Any pointer ? > Just hack it in for testing. E.g. call it when the button control is pressed (see vivi_s_ctrl). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
Hi Hans, On Friday 06 June 2014 11:31:55 Hans Verkuil wrote: > On 06/06/2014 11:19 AM, Laurent Pinchart wrote: > > Hi Pawel, > > > > On Friday 06 June 2014 14:31:15 Pawel Osciak wrote: > >> Hi Laurent, > >> Thanks for the patch. Did you test this to work in fileio mode? Looks > >> like it should, but would like to make sure. > > > > No, I haven't tested it. The OMAP4 ISS driver, which is my test target for > > this patch, doesn't support fileio mode. Adding VB2_READ would be easy, > > but the driver requires configuring the format on the file handle used for > > streaming, so I can't just run cat /dev/video*. > > Just test with vivi. But vivi doesn't call the new vb2_queue_error() function. I understand that your vivi rework would make that easier as you now have an error control. Should I hack something similar in the existing vivi driver ? Any pointer ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
On 06/06/2014 11:19 AM, Laurent Pinchart wrote: > Hi Pawel, > > On Friday 06 June 2014 14:31:15 Pawel Osciak wrote: >> Hi Laurent, >> Thanks for the patch. Did you test this to work in fileio mode? Looks >> like it should, but would like to make sure. > > No, I haven't tested it. The OMAP4 ISS driver, which is my test target for > this patch, doesn't support fileio mode. Adding VB2_READ would be easy, but > the driver requires configuring the format on the file handle used for > streaming, so I can't just run cat /dev/video*. Just test with vivi. Regards, Hans > >> On Thu, Jun 5, 2014 at 9:23 PM, Laurent Pinchart wrote: >>> When a fatal error occurs that render the device unusable, the only >>> options for a driver to signal the error condition to userspace is to >>> set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an >>> error from the buffer prepare handler when queuing buffers. >>> >>> The buffer error flag indicates a transient error and can't be used by >>> applications to detect fatal errors. Returning an error from vb2_qbuf() >>> is thus the only real indication that a fatal error occurred. However, >>> this is difficult to handle for multithreaded applications that requeue >>> buffers from a thread other than the control thread. In particular the >>> poll() call in the control thread will not notify userspace of the >>> error. >>> >>> This patch adds an explicit mechanism to report fatal errors to >>> userspace. Applications can call the vb2_queue_error() function to >>> signal a fatal error. From this moment on, buffer preparation will >>> return -EIO to userspace, and vb2_poll() will set the POLLERR flag and >>> return immediately. The error flag is cleared when cancelling the queue, >>> either at stream off time (through vb2_streamoff) or when releasing the >>> queue with vb2_queue_release(). >>> >>> Signed-off-by: Laurent Pinchart > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
Hi Pawel, On Friday 06 June 2014 14:31:15 Pawel Osciak wrote: > Hi Laurent, > Thanks for the patch. Did you test this to work in fileio mode? Looks > like it should, but would like to make sure. No, I haven't tested it. The OMAP4 ISS driver, which is my test target for this patch, doesn't support fileio mode. Adding VB2_READ would be easy, but the driver requires configuring the format on the file handle used for streaming, so I can't just run cat /dev/video*. > On Thu, Jun 5, 2014 at 9:23 PM, Laurent Pinchart wrote: > > When a fatal error occurs that render the device unusable, the only > > options for a driver to signal the error condition to userspace is to > > set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an > > error from the buffer prepare handler when queuing buffers. > > > > The buffer error flag indicates a transient error and can't be used by > > applications to detect fatal errors. Returning an error from vb2_qbuf() > > is thus the only real indication that a fatal error occurred. However, > > this is difficult to handle for multithreaded applications that requeue > > buffers from a thread other than the control thread. In particular the > > poll() call in the control thread will not notify userspace of the > > error. > > > > This patch adds an explicit mechanism to report fatal errors to > > userspace. Applications can call the vb2_queue_error() function to > > signal a fatal error. From this moment on, buffer preparation will > > return -EIO to userspace, and vb2_poll() will set the POLLERR flag and > > return immediately. The error flag is cleared when cancelling the queue, > > either at stream off time (through vb2_streamoff) or when releasing the > > queue with vb2_queue_release(). > > > > Signed-off-by: Laurent Pinchart -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
Hi Laurent,
Thanks for the patch. Did you test this to work in fileio mode? Looks
like it should, but would like to make sure.
Thanks,
Pawel
On Thu, Jun 5, 2014 at 9:23 PM, Laurent Pinchart
wrote:
> When a fatal error occurs that render the device unusable, the only
> options for a driver to signal the error condition to userspace is to
> set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an
> error from the buffer prepare handler when queuing buffers.
>
> The buffer error flag indicates a transient error and can't be used by
> applications to detect fatal errors. Returning an error from vb2_qbuf()
> is thus the only real indication that a fatal error occurred. However,
> this is difficult to handle for multithreaded applications that requeue
> buffers from a thread other than the control thread. In particular the
> poll() call in the control thread will not notify userspace of the
> error.
>
> This patch adds an explicit mechanism to report fatal errors to
> userspace. Applications can call the vb2_queue_error() function to
> signal a fatal error. From this moment on, buffer preparation will
> return -EIO to userspace, and vb2_poll() will set the POLLERR flag and
> return immediately. The error flag is cleared when cancelling the queue,
> either at stream off time (through vb2_streamoff) or when releasing the
> queue with vb2_queue_release().
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 40
> +---
> include/media/videobuf2-core.h | 3 +++
> 2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index fd428e0..c7aa07d 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1582,6 +1582,11 @@ static int __buf_prepare(struct vb2_buffer *vb, const
> struct v4l2_buffer *b)
> return -EINVAL;
> }
>
> + if (q->error) {
> + dprintk(1, "fatal error occurred on queue\n");
> + return -EIO;
> + }
> +
> vb->state = VB2_BUF_STATE_PREPARING;
> vb->v4l2_buf.timestamp.tv_sec = 0;
> vb->v4l2_buf.timestamp.tv_usec = 0;
> @@ -1877,6 +1882,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q,
> int nonblocking)
> return -EINVAL;
> }
>
> + if (q->error) {
> + dprintk(1, "Queue in error state, will not wait for
> buffers\n");
> + return -EIO;
> + }
> +
> if (!list_empty(&q->done_list)) {
> /*
> * Found a buffer that we were waiting for.
> @@ -1902,7 +1912,8 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q,
> int nonblocking)
> */
> dprintk(3, "will sleep waiting for buffers\n");
> ret = wait_event_interruptible(q->done_wq,
> - !list_empty(&q->done_list) || !q->streaming);
> + !list_empty(&q->done_list) || !q->streaming ||
> + q->error);
>
> /*
> * We need to reevaluate both conditions again after
> reacquiring
> @@ -2099,6 +2110,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> q->streaming = 0;
> q->start_streaming_called = 0;
> q->queued_count = 0;
> + q->error = 0;
>
> /*
> * Remove all buffers from videobuf's list...
> @@ -2176,6 +2188,27 @@ static int vb2_internal_streamon(struct vb2_queue *q,
> enum v4l2_buf_type type)
> }
>
> /**
> + * vb2_queue_error() - signal a fatal error on the queue
> + * @q: videobuf2 queue
> + *
> + * Flag that a fatal unrecoverable error has occurred and wake up all
> processes
> + * waiting on the queue. Polling will now set POLLERR and queuing and
> dequeuing
> + * buffers will return -EIO.
> + *
> + * The error flag will be cleared when cancelling the queue, either from
> + * vb2_streamoff or vb2_queue_release. Drivers should thus not call this
> + * function before starting the stream, otherwise the error flag will remain
> set
> + * until the queue is released when closing the device node.
> + */
> +void vb2_queue_error(struct vb2_queue *q)
> +{
> + q->error = 1;
> +
> + wake_up_all(&q->done_wq);
> +}
> +EXPORT_SYMBOL_GPL(vb2_queue_error);
> +
> +/**
> * vb2_streamon - start streaming
> * @q: videobuf2 queue
> * @type: type argument passed from userspace to vidioc_streamon handler
> @@ -2533,9 +2566,10 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file
> *file, poll_table *wait)
> }
>
> /*
> -* There is nothing to wait for if the queue isn't streaming.
> +* There is nothing to wait for if the queue isn't streaming or if the
> +* error flag i
[PATCH/RFC v2 2/2] v4l: vb2: Add fatal error condition flag
When a fatal error occurs that render the device unusable, the only
options for a driver to signal the error condition to userspace is to
set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an
error from the buffer prepare handler when queuing buffers.
The buffer error flag indicates a transient error and can't be used by
applications to detect fatal errors. Returning an error from vb2_qbuf()
is thus the only real indication that a fatal error occurred. However,
this is difficult to handle for multithreaded applications that requeue
buffers from a thread other than the control thread. In particular the
poll() call in the control thread will not notify userspace of the
error.
This patch adds an explicit mechanism to report fatal errors to
userspace. Applications can call the vb2_queue_error() function to
signal a fatal error. From this moment on, buffer preparation will
return -EIO to userspace, and vb2_poll() will set the POLLERR flag and
return immediately. The error flag is cleared when cancelling the queue,
either at stream off time (through vb2_streamoff) or when releasing the
queue with vb2_queue_release().
Signed-off-by: Laurent Pinchart
---
drivers/media/v4l2-core/videobuf2-core.c | 40 +---
include/media/videobuf2-core.h | 3 +++
2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index fd428e0..c7aa07d 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1582,6 +1582,11 @@ static int __buf_prepare(struct vb2_buffer *vb, const
struct v4l2_buffer *b)
return -EINVAL;
}
+ if (q->error) {
+ dprintk(1, "fatal error occurred on queue\n");
+ return -EIO;
+ }
+
vb->state = VB2_BUF_STATE_PREPARING;
vb->v4l2_buf.timestamp.tv_sec = 0;
vb->v4l2_buf.timestamp.tv_usec = 0;
@@ -1877,6 +1882,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q,
int nonblocking)
return -EINVAL;
}
+ if (q->error) {
+ dprintk(1, "Queue in error state, will not wait for
buffers\n");
+ return -EIO;
+ }
+
if (!list_empty(&q->done_list)) {
/*
* Found a buffer that we were waiting for.
@@ -1902,7 +1912,8 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q,
int nonblocking)
*/
dprintk(3, "will sleep waiting for buffers\n");
ret = wait_event_interruptible(q->done_wq,
- !list_empty(&q->done_list) || !q->streaming);
+ !list_empty(&q->done_list) || !q->streaming ||
+ q->error);
/*
* We need to reevaluate both conditions again after reacquiring
@@ -2099,6 +2110,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
q->streaming = 0;
q->start_streaming_called = 0;
q->queued_count = 0;
+ q->error = 0;
/*
* Remove all buffers from videobuf's list...
@@ -2176,6 +2188,27 @@ static int vb2_internal_streamon(struct vb2_queue *q,
enum v4l2_buf_type type)
}
/**
+ * vb2_queue_error() - signal a fatal error on the queue
+ * @q: videobuf2 queue
+ *
+ * Flag that a fatal unrecoverable error has occurred and wake up all processes
+ * waiting on the queue. Polling will now set POLLERR and queuing and dequeuing
+ * buffers will return -EIO.
+ *
+ * The error flag will be cleared when cancelling the queue, either from
+ * vb2_streamoff or vb2_queue_release. Drivers should thus not call this
+ * function before starting the stream, otherwise the error flag will remain
set
+ * until the queue is released when closing the device node.
+ */
+void vb2_queue_error(struct vb2_queue *q)
+{
+ q->error = 1;
+
+ wake_up_all(&q->done_wq);
+}
+EXPORT_SYMBOL_GPL(vb2_queue_error);
+
+/**
* vb2_streamon - start streaming
* @q: videobuf2 queue
* @type: type argument passed from userspace to vidioc_streamon handler
@@ -2533,9 +2566,10 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file
*file, poll_table *wait)
}
/*
-* There is nothing to wait for if the queue isn't streaming.
+* There is nothing to wait for if the queue isn't streaming or if the
+* error flag is set.
*/
- if (!vb2_is_streaming(q))
+ if (!vb2_is_streaming(q) || q->error)
return res | POLLERR;
if (list_empty(&q->done_list))
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index bca25dc..5a67f31 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -375,6 +375,7 @@ struct v4l2_fh;
* @streaming: current streamin
