[RFCv1 PATCH 0/6] Don't start streaming unless requested by the poll mask.

[RFCv1 PATCH 0/6] Don't start streaming unless requested by the poll mask.

[Hans Verkuil]

The patch adding core support for poll_requested_events() looks ready for v3.1,
so this patch series builds on it to fix the vivi and ivtv drivers.

It also uses it in videobuf. I think it makes sense to add it there as well,
even though no videobuf-drivers use events (yet).

If there are no comments, then I'd like to make a pull request for this by
the end of the week.

Regards,

	Hans

PS: Note that I'm having vacation until July 25th, so I won't be very active on
the mailinglist. These poll patches are the only thing that I'm working on since
I really want to get these merged for v3.1.

[RFCv1 PATCH 1/6] poll: add poll_requested_events() function

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

In some cases the poll() implementation in a driver has to do different
things depending on the events the caller wants to poll for. An example is
when a driver needs to start a DMA engine if the caller polls for POLLIN,
but doesn't want to do that if POLLIN is not requested but instead only
POLLOUT or POLLPRI is requested. This is something that can happen in the
video4linux subsystem.

Unfortunately, the current epoll/poll/select implementation doesn't provide
that information reliably. The poll_table_struct does have it: it has a key
field with the event mask. But once a poll() call matches one or more bits
of that mask any following poll() calls are passed a NULL poll_table_struct
pointer.

The solution is to set the qproc field to NULL in poll_table_struct once
poll() matches the events, not the poll_table_struct pointer itself. That
way drivers can obtain the mask through a new poll_requested_events inline.

The poll_table_struct can still be NULL since some kernel code calls it
internally (netfs_state_poll() in ./drivers/staging/pohmelfs/netfs.h). In
that case poll_requested_events() returns ~0 (i.e. all events).

Since eventpoll always leaves the key field at ~0 instead of using the
requested events mask, that source was changed as well to properly fill in
the key field.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Jonathan Corbet <corbet@lwn.net>
---
 fs/eventpoll.c       |   19 +++++++++++++++----
 fs/select.c          |   38 +++++++++++++++++---------------------
 include/linux/poll.h |    7 ++++++-
 3 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index f9cfd16..6a54a69 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -650,9 +650,12 @@ static int ep_read_events_proc(struct eventpoll *ep, struct list_head *head,
 			       void *priv)
 {
 	struct epitem *epi, *tmp;
+	poll_table pt;
 
+	init_poll_funcptr(&pt, NULL);
 	list_for_each_entry_safe(epi, tmp, head, rdllink) {
-		if (epi->ffd.file->f_op->poll(epi->ffd.file, NULL) &
+		pt.key = epi->event.events;
+		if (epi->ffd.file->f_op->poll(epi->ffd.file, &pt) &
 		    epi->event.events)
 			return POLLIN | POLLRDNORM;
 		else {
@@ -946,6 +949,7 @@ static int ep_insert(struct eventpoll *ep, struct epoll_event *event,
 	/* Initialize the poll table using the queue callback */
 	epq.epi = epi;
 	init_poll_funcptr(&epq.pt, ep_ptable_queue_proc);
+	epq.pt.key = event->events;
 
 	/*
 	 * Attach the item to the poll hooks and get current event bits.
@@ -1027,20 +1031,23 @@ static int ep_modify(struct eventpoll *ep, struct epitem *epi, struct epoll_even
 {
 	int pwake = 0;
 	unsigned int revents;
+	poll_table pt;
+
+	init_poll_funcptr(&pt, NULL);
 
 	/*
 	 * Set the new event interest mask before calling f_op->poll();
 	 * otherwise we might miss an event that happens between the
 	 * f_op->poll() call and the new event set registering.
 	 */
-	epi->event.events = event->events;
+	epi->event.events = pt.key = event->events;
 	epi->event.data = event->data; /* protected by mtx */
 
 	/*
 	 * Get current event bits. We can safely use the file* here because
 	 * its usage count has been increased by the caller of this function.
 	 */
-	revents = epi->ffd.file->f_op->poll(epi->ffd.file, NULL);
+	revents = epi->ffd.file->f_op->poll(epi->ffd.file, &pt);
 
 	/*
 	 * If the item is "hot" and it is not registered inside the ready
@@ -1075,6 +1082,9 @@ static int ep_send_events_proc(struct eventpoll *ep, struct list_head *head,
 	unsigned int revents;
 	struct epitem *epi;
 	struct epoll_event __user *uevent;
+	poll_table pt;
+
+	init_poll_funcptr(&pt, NULL);
 
 	/*
 	 * We can loop without lock because we are passed a task private list.
@@ -1087,7 +1097,8 @@ static int ep_send_events_proc(struct eventpoll *ep, struct list_head *head,
 
 		list_del_init(&epi->rdllink);
 
-		revents = epi->ffd.file->f_op->poll(epi->ffd.file, NULL) &
+		pt.key = epi->event.events;
+		revents = epi->ffd.file->f_op->poll(epi->ffd.file, &pt) &
 			epi->event.events;
 
 		/*
diff --git a/fs/select.c b/fs/select.c
index d33418f..b6765cf 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -386,13 +386,11 @@ get_max:
 static inline void wait_key_set(poll_table *wait, unsigned long in,
 				unsigned long out, unsigned long bit)
 {
-	if (wait) {
-		wait->key = POLLEX_SET;
-		if (in & bit)
-			wait->key |= POLLIN_SET;
-		if (out & bit)
-			wait->key |= POLLOUT_SET;
-	}
+	wait->key = POLLEX_SET;
+	if (in & bit)
+		wait->key |= POLLIN_SET;
+	if (out & bit)
+		wait->key |= POLLOUT_SET;
 }
 
 int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
@@ -414,7 +412,7 @@ int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
 	poll_initwait(&table);
 	wait = &table.pt;
 	if (end_time && !end_time->tv_sec && !end_time->tv_nsec) {
-		wait = NULL;
+		wait->qproc = NULL;
 		timed_out = 1;
 	}
 
@@ -459,17 +457,17 @@ int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
 					if ((mask & POLLIN_SET) && (in & bit)) {
 						res_in |= bit;
 						retval++;
-						wait = NULL;
+						wait->qproc = NULL;
 					}
 					if ((mask & POLLOUT_SET) && (out & bit)) {
 						res_out |= bit;
 						retval++;
-						wait = NULL;
+						wait->qproc = NULL;
 					}
 					if ((mask & POLLEX_SET) && (ex & bit)) {
 						res_ex |= bit;
 						retval++;
-						wait = NULL;
+						wait->qproc = NULL;
 					}
 				}
 			}
@@ -481,7 +479,7 @@ int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
 				*rexp = res_ex;
 			cond_resched();
 		}
-		wait = NULL;
+		wait->qproc = NULL;
 		if (retval || timed_out || signal_pending(current))
 			break;
 		if (table.error) {
@@ -720,7 +718,7 @@ struct poll_list {
  * interested in events matching the pollfd->events mask, and the result
  * matching that mask is both recorded in pollfd->revents and returned. The
  * pwait poll_table will be used by the fd-provided poll handler for waiting,
- * if non-NULL.
+ * if pwait->qproc is non-NULL.
  */
 static inline unsigned int do_pollfd(struct pollfd *pollfd, poll_table *pwait)
 {
@@ -738,9 +736,7 @@ static inline unsigned int do_pollfd(struct pollfd *pollfd, poll_table *pwait)
 		if (file != NULL) {
 			mask = DEFAULT_POLLMASK;
 			if (file->f_op && file->f_op->poll) {
-				if (pwait)
-					pwait->key = pollfd->events |
-							POLLERR | POLLHUP;
+				pwait->key = pollfd->events | POLLERR | POLLHUP;
 				mask = file->f_op->poll(file, pwait);
 			}
 			/* Mask out unneeded events. */
@@ -763,7 +759,7 @@ static int do_poll(unsigned int nfds,  struct poll_list *list,
 
 	/* Optimise the no-wait case */
 	if (end_time && !end_time->tv_sec && !end_time->tv_nsec) {
-		pt = NULL;
+		pt->qproc = NULL;
 		timed_out = 1;
 	}
 
@@ -781,22 +777,22 @@ static int do_poll(unsigned int nfds,  struct poll_list *list,
 			for (; pfd != pfd_end; pfd++) {
 				/*
 				 * Fish for events. If we found one, record it
-				 * and kill the poll_table, so we don't
+				 * and kill poll_table->qproc, so we don't
 				 * needlessly register any other waiters after
 				 * this. They'll get immediately deregistered
 				 * when we break out and return.
 				 */
 				if (do_pollfd(pfd, pt)) {
 					count++;
-					pt = NULL;
+					pt->qproc = NULL;
 				}
 			}
 		}
 		/*
 		 * All waiters have already been registered, so don't provide
-		 * a poll_table to them on the next loop iteration.
+		 * a poll_table->qproc to them on the next loop iteration.
 		 */
-		pt = NULL;
+		pt->qproc = NULL;
 		if (!count) {
 			count = wait->error;
 			if (signal_pending(current))
diff --git a/include/linux/poll.h b/include/linux/poll.h
index cf40010..fe1e360 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -39,10 +39,15 @@ typedef struct poll_table_struct {
 
 static inline void poll_wait(struct file * filp, wait_queue_head_t * wait_address, poll_table *p)
 {
-	if (p && wait_address)
+	if (p && p->qproc && wait_address)
 		p->qproc(filp, wait_address, p);
 }
 
+static inline unsigned long poll_requested_events(const poll_table *p)
+{
+	return p ? p->key : ~0UL;
+}
+
 static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
 {
 	pt->qproc = qproc;
-- 
1.7.1

[RFCv1 PATCH 2/6] ivtv: only start streaming in poll() if polling for input.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/ivtv/ivtv-fileops.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/ivtv/ivtv-fileops.c b/drivers/media/video/ivtv/ivtv-fileops.c
index 38f0522..a931ecf 100644
--- a/drivers/media/video/ivtv/ivtv-fileops.c
+++ b/drivers/media/video/ivtv/ivtv-fileops.c
@@ -744,8 +744,9 @@ unsigned int ivtv_v4l2_dec_poll(struct file *filp, poll_table *wait)
 	return res;
 }
 
-unsigned int ivtv_v4l2_enc_poll(struct file *filp, poll_table * wait)
+unsigned int ivtv_v4l2_enc_poll(struct file *filp, poll_table *wait)
 {
+	unsigned long req_events = poll_requested_events(wait);
 	struct ivtv_open_id *id = fh2id(filp->private_data);
 	struct ivtv *itv = id->itv;
 	struct ivtv_stream *s = &itv->streams[id->type];
@@ -753,7 +754,8 @@ unsigned int ivtv_v4l2_enc_poll(struct file *filp, poll_table * wait)
 	unsigned res = 0;
 
 	/* Start a capture if there is none */
-	if (!eof && !test_bit(IVTV_F_S_STREAMING, &s->s_flags)) {
+	if (!eof && !test_bit(IVTV_F_S_STREAMING, &s->s_flags) &&
+			(req_events & (POLLIN | POLLRDNORM))) {
 		int rc;
 
 		mutex_lock(&itv->serialize_lock);
-- 
1.7.1

[RFCv1 PATCH 3/6] videobuf2: only start streaming in poll() if so requested by the poll mask.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/videobuf2-core.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 3015e60..1892bb8 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -1365,6 +1365,7 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q);
  */
 unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 {
+	unsigned long req_events = poll_requested_events(wait);
 	unsigned long flags;
 	unsigned int ret;
 	struct vb2_buffer *vb = NULL;
@@ -1373,12 +1374,14 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 	 * Start file I/O emulator only if streaming API has not been used yet.
 	 */
 	if (q->num_buffers == 0 && q->fileio == NULL) {
-		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ)) {
+		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
+				(req_events & (POLLIN | POLLRDNORM))) {
 			ret = __vb2_init_fileio(q, 1);
 			if (ret)
 				return POLLERR;
 		}
-		if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE)) {
+		if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE) &&
+				(req_events & (POLLOUT | POLLWRNORM))) {
 			ret = __vb2_init_fileio(q, 0);
 			if (ret)
 				return POLLERR;
-- 
1.7.1

[RFCv1 PATCH 4/6] videobuf: only start streaming in poll() if so requested by the poll mask.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/videobuf-core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index de4fa4e..ffdf59c 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -1129,6 +1129,7 @@ unsigned int videobuf_poll_stream(struct file *file,
 				  struct videobuf_queue *q,
 				  poll_table *wait)
 {
+	unsigned long req_events = poll_requested_events(wait);
 	struct videobuf_buffer *buf = NULL;
 	unsigned int rc = 0;
 
@@ -1137,7 +1138,7 @@ unsigned int videobuf_poll_stream(struct file *file,
 		if (!list_empty(&q->stream))
 			buf = list_entry(q->stream.next,
 					 struct videobuf_buffer, stream);
-	} else {
+	} else if (req_events & (POLLIN | POLLRDNORM)) {
 		if (!q->reading)
 			__videobuf_read_start(q);
 		if (!q->reading) {
-- 
1.7.1

[RFCv1 PATCH 5/6] videobuf2-core: also test for pending events.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/videobuf2-core.c |   41 +++++++++++++++++++++++----------
 1 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 1892bb8..1922bf8 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -19,6 +19,9 @@
 #include <linux/slab.h>
 #include <linux/sched.h>
 
+#include <media/v4l2-dev.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 #include <media/videobuf2-core.h>
 
 static int debug;
@@ -1360,15 +1363,28 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q);
  * For OUTPUT queues, if a buffer is ready to be dequeued, the file descriptor
  * will be reported as available for writing.
  *
+ * If the driver uses struct v4l2_fh, then vb2_poll() will also check for any
+ * pending events.
+ *
  * The return values from this function are intended to be directly returned
  * from poll handler in driver.
  */
 unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 {
+	struct video_device *vfd = video_devdata(file);
 	unsigned long req_events = poll_requested_events(wait);
-	unsigned long flags;
-	unsigned int ret;
 	struct vb2_buffer *vb = NULL;
+	unsigned int res = 0;
+	unsigned long flags;
+
+	if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) {
+		struct v4l2_fh *fh = file->private_data;
+
+		if (v4l2_event_pending(fh))
+			res = POLLPRI;
+		else if (req_events & POLLPRI)
+			poll_wait(file, &fh->wait, wait);
+	}
 
 	/*
 	 * Start file I/O emulator only if streaming API has not been used yet.
@@ -1376,19 +1392,17 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 	if (q->num_buffers == 0 && q->fileio == NULL) {
 		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ) &&
 				(req_events & (POLLIN | POLLRDNORM))) {
-			ret = __vb2_init_fileio(q, 1);
-			if (ret)
-				return POLLERR;
+			if (__vb2_init_fileio(q, 1))
+				return res | POLLERR;
 		}
 		if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE) &&
 				(req_events & (POLLOUT | POLLWRNORM))) {
-			ret = __vb2_init_fileio(q, 0);
-			if (ret)
-				return POLLERR;
+			if (__vb2_init_fileio(q, 0))
+				return res | POLLERR;
 			/*
 			 * Write to OUTPUT queue can be done immediately.
 			 */
-			return POLLOUT | POLLWRNORM;
+			return res | POLLOUT | POLLWRNORM;
 		}
 	}
 
@@ -1396,7 +1410,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 	 * There is nothing to wait for if no buffers have already been queued.
 	 */
 	if (list_empty(&q->queued_list))
-		return POLLERR;
+		return res | POLLERR;
 
 	poll_wait(file, &q->done_wq, wait);
 
@@ -1411,10 +1425,11 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 
 	if (vb && (vb->state == VB2_BUF_STATE_DONE
 			|| vb->state == VB2_BUF_STATE_ERROR)) {
-		return (V4L2_TYPE_IS_OUTPUT(q->type)) ? POLLOUT | POLLWRNORM :
-			POLLIN | POLLRDNORM;
+		return (V4L2_TYPE_IS_OUTPUT(q->type)) ?
+				res | POLLOUT | POLLWRNORM :
+				res | POLLIN | POLLRDNORM;
 	}
-	return 0;
+	return res;
 }
 EXPORT_SYMBOL_GPL(vb2_poll);
 
-- 
1.7.1

[RFCv1 PATCH 6/6] vivi: let vb2_poll handle events.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The vb2_poll function now tests for events and sets POLLPRI accordingly.
So there it is no longer necessary to test for it in the vivi driver.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/vivi.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
index a848bd2..fc3c88f 100644
--- a/drivers/media/video/vivi.c
+++ b/drivers/media/video/vivi.c
@@ -1039,17 +1039,10 @@ static unsigned int
 vivi_poll(struct file *file, struct poll_table_struct *wait)
 {
 	struct vivi_dev *dev = video_drvdata(file);
-	struct v4l2_fh *fh = file->private_data;
 	struct vb2_queue *q = &dev->vb_vidq;
-	unsigned int res;
 
 	dprintk(dev, 1, "%s\n", __func__);
-	res = vb2_poll(q, file, wait);
-	if (v4l2_event_pending(fh))
-		res |= POLLPRI;
-	else
-		poll_wait(file, &fh->wait, wait);
-	return res;
+	return vb2_poll(q, file, wait);
 }
 
 static int vivi_close(struct file *file)
-- 
1.7.1

RE: [RFCv1 PATCH 5/6] videobuf2-core: also test for pending events.

[Marek Szyprowski]

Hello,

On Wednesday, July 13, 2011 11:39 AM Hans Verkuil wrote:

> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>  drivers/media/video/videobuf2-core.c |   41 +++++++++++++++++++++++-------
> ---
>  1 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index 1892bb8..1922bf8 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -19,6 +19,9 @@
>  #include <linux/slab.h>
>  #include <linux/sched.h>
> 
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-fh.h>
> +#include <media/v4l2-event.h>
>  #include <media/videobuf2-core.h>
> 
>  static int debug;
> @@ -1360,15 +1363,28 @@ static int __vb2_cleanup_fileio(struct vb2_queue
> *q);
>   * For OUTPUT queues, if a buffer is ready to be dequeued, the file
> descriptor
>   * will be reported as available for writing.
>   *
> + * If the driver uses struct v4l2_fh, then vb2_poll() will also check for
> any
> + * pending events.
> + *
>   * The return values from this function are intended to be directly
> returned
>   * from poll handler in driver.
>   */
>  unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table
> *wait)
>  {
> +	struct video_device *vfd = video_devdata(file);
>  	unsigned long req_events = poll_requested_events(wait);
> -	unsigned long flags;
> -	unsigned int ret;
>  	struct vb2_buffer *vb = NULL;
> +	unsigned int res = 0;
> +	unsigned long flags;
> +
> +	if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) {
> +		struct v4l2_fh *fh = file->private_data;
> +
> +		if (v4l2_event_pending(fh))
> +			res = POLLPRI;
> +		else if (req_events & POLLPRI)
> +			poll_wait(file, &fh->wait, wait);
> +	}
> 
>  	/*
>  	 * Start file I/O emulator only if streaming API has not been used
> yet.
> @@ -1376,19 +1392,17 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
> file *file, poll_table *wait)
>  	if (q->num_buffers == 0 && q->fileio == NULL) {
>  		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ)
> &&
>  				(req_events & (POLLIN | POLLRDNORM))) {
> -			ret = __vb2_init_fileio(q, 1);
> -			if (ret)
> -				return POLLERR;
> +			if (__vb2_init_fileio(q, 1))
> +				return res | POLLERR;
>  		}
>  		if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE)
> &&
>  				(req_events & (POLLOUT | POLLWRNORM))) {
> -			ret = __vb2_init_fileio(q, 0);
> -			if (ret)
> -				return POLLERR;
> +			if (__vb2_init_fileio(q, 0))
> +				return res | POLLERR;
>  			/*
>  			 * Write to OUTPUT queue can be done immediately.
>  			 */
> -			return POLLOUT | POLLWRNORM;
> +			return res | POLLOUT | POLLWRNORM;
>  		}
>  	}
> 
> @@ -1396,7 +1410,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
> file *file, poll_table *wait)
>  	 * There is nothing to wait for if no buffers have already been
> queued.
>  	 */
>  	if (list_empty(&q->queued_list))
> -		return POLLERR;
> +		return res | POLLERR;
> 
>  	poll_wait(file, &q->done_wq, wait);
> 
> @@ -1411,10 +1425,11 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
> file *file, poll_table *wait)
> 
>  	if (vb && (vb->state == VB2_BUF_STATE_DONE
>  			|| vb->state == VB2_BUF_STATE_ERROR)) {
> -		return (V4L2_TYPE_IS_OUTPUT(q->type)) ? POLLOUT | POLLWRNORM :
> -			POLLIN | POLLRDNORM;
> +		return (V4L2_TYPE_IS_OUTPUT(q->type)) ?
> +				res | POLLOUT | POLLWRNORM :
> +				res | POLLIN | POLLRDNORM;
>  	}
> -	return 0;
> +	return res;
>  }
>  EXPORT_SYMBOL_GPL(vb2_poll);
> 
> --

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

RE: [RFCv1 PATCH 3/6] videobuf2: only start streaming in poll() if so requested by the poll mask.

[Marek Szyprowski]

Hello,

On Wednesday, July 13, 2011 11:39 AM Hans Verkuil wrote:

> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>  drivers/media/video/videobuf2-core.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index 3015e60..1892bb8 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1365,6 +1365,7 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q);
>   */
>  unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table
> *wait)
>  {
> +	unsigned long req_events = poll_requested_events(wait);
>  	unsigned long flags;
>  	unsigned int ret;
>  	struct vb2_buffer *vb = NULL;
> @@ -1373,12 +1374,14 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
> file *file, poll_table *wait)
>  	 * Start file I/O emulator only if streaming API has not been used
> yet.
>  	 */
>  	if (q->num_buffers == 0 && q->fileio == NULL) {
> -		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ))
> {
> +		if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ)
> &&
> +				(req_events & (POLLIN | POLLRDNORM))) {
>  			ret = __vb2_init_fileio(q, 1);
>  			if (ret)
>  				return POLLERR;
>  		}
> -		if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE))
> {
> +		if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE)
> &&
> +				(req_events & (POLLOUT | POLLWRNORM))) {
>  			ret = __vb2_init_fileio(q, 0);
>  			if (ret)
>  				return POLLERR;
> --

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [RFCv1 PATCH 5/6] videobuf2-core: also test for pending events.

[Pawel Osciak]

On Sun, Jul 17, 2011 at 23:30, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
> On Wednesday, July 13, 2011 11:39 AM Hans Verkuil wrote:
>
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>

Acked-by: Pawel Osciak <pawel@osciak.com>

>> ---
>>  drivers/media/video/videobuf2-core.c |   41 +++++++++++++++++++++++-------
>> ---
>>  1 files changed, 28 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/media/video/videobuf2-core.c
>> b/drivers/media/video/videobuf2-core.c
>> index 1892bb8..1922bf8 100644
>> --- a/drivers/media/video/videobuf2-core.c
>> +++ b/drivers/media/video/videobuf2-core.c
>> @@ -19,6 +19,9 @@
>>  #include <linux/slab.h>
>>  #include <linux/sched.h>
>>
>> +#include <media/v4l2-dev.h>
>> +#include <media/v4l2-fh.h>
>> +#include <media/v4l2-event.h>
>>  #include <media/videobuf2-core.h>
>>
>>  static int debug;
>> @@ -1360,15 +1363,28 @@ static int __vb2_cleanup_fileio(struct vb2_queue
>> *q);
>>   * For OUTPUT queues, if a buffer is ready to be dequeued, the file
>> descriptor
>>   * will be reported as available for writing.
>>   *
>> + * If the driver uses struct v4l2_fh, then vb2_poll() will also check for
>> any
>> + * pending events.
>> + *
>>   * The return values from this function are intended to be directly
>> returned
>>   * from poll handler in driver.
>>   */
>>  unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table
>> *wait)
>>  {
>> +     struct video_device *vfd = video_devdata(file);
>>       unsigned long req_events = poll_requested_events(wait);
>> -     unsigned long flags;
>> -     unsigned int ret;
>>       struct vb2_buffer *vb = NULL;
>> +     unsigned int res = 0;
>> +     unsigned long flags;
>> +
>> +     if (test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags)) {
>> +             struct v4l2_fh *fh = file->private_data;
>> +
>> +             if (v4l2_event_pending(fh))
>> +                     res = POLLPRI;
>> +             else if (req_events & POLLPRI)
>> +                     poll_wait(file, &fh->wait, wait);
>> +     }
>>
>>       /*
>>        * Start file I/O emulator only if streaming API has not been used
>> yet.
>> @@ -1376,19 +1392,17 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
>> file *file, poll_table *wait)
>>       if (q->num_buffers == 0 && q->fileio == NULL) {
>>               if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ)
>> &&
>>                               (req_events & (POLLIN | POLLRDNORM))) {
>> -                     ret = __vb2_init_fileio(q, 1);
>> -                     if (ret)
>> -                             return POLLERR;
>> +                     if (__vb2_init_fileio(q, 1))
>> +                             return res | POLLERR;
>>               }
>>               if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE)
>> &&
>>                               (req_events & (POLLOUT | POLLWRNORM))) {
>> -                     ret = __vb2_init_fileio(q, 0);
>> -                     if (ret)
>> -                             return POLLERR;
>> +                     if (__vb2_init_fileio(q, 0))
>> +                             return res | POLLERR;
>>                       /*
>>                        * Write to OUTPUT queue can be done immediately.
>>                        */
>> -                     return POLLOUT | POLLWRNORM;
>> +                     return res | POLLOUT | POLLWRNORM;
>>               }
>>       }
>>
>> @@ -1396,7 +1410,7 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
>> file *file, poll_table *wait)
>>        * There is nothing to wait for if no buffers have already been
>> queued.
>>        */
>>       if (list_empty(&q->queued_list))
>> -             return POLLERR;
>> +             return res | POLLERR;
>>
>>       poll_wait(file, &q->done_wq, wait);
>>
>> @@ -1411,10 +1425,11 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
>> file *file, poll_table *wait)
>>
>>       if (vb && (vb->state == VB2_BUF_STATE_DONE
>>                       || vb->state == VB2_BUF_STATE_ERROR)) {
>> -             return (V4L2_TYPE_IS_OUTPUT(q->type)) ? POLLOUT | POLLWRNORM :
>> -                     POLLIN | POLLRDNORM;
>> +             return (V4L2_TYPE_IS_OUTPUT(q->type)) ?
>> +                             res | POLLOUT | POLLWRNORM :
>> +                             res | POLLIN | POLLRDNORM;
>>       }
>> -     return 0;
>> +     return res;
>>  }
>>  EXPORT_SYMBOL_GPL(vb2_poll);
>>
>> --
>
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
>
>
>
>



-- 
Best regards,
Pawel Osciak

Re: [RFCv1 PATCH 3/6] videobuf2: only start streaming in poll() if so requested by the poll mask.

[Pawel Osciak]

Hi Hans,

On Sun, Jul 17, 2011 at 23:30, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
> On Wednesday, July 13, 2011 11:39 AM Hans Verkuil wrote:
>
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>

Acked-by: Pawel Osciak <pawel@osciak.com>

I have to say, this is cool stuff!
Pawel

>> ---
>>  drivers/media/video/videobuf2-core.c |    7 +++++--
>>  1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/video/videobuf2-core.c
>> b/drivers/media/video/videobuf2-core.c
>> index 3015e60..1892bb8 100644
>> --- a/drivers/media/video/videobuf2-core.c
>> +++ b/drivers/media/video/videobuf2-core.c
>> @@ -1365,6 +1365,7 @@ static int __vb2_cleanup_fileio(struct vb2_queue *q);
>>   */
>>  unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table
>> *wait)
>>  {
>> +     unsigned long req_events = poll_requested_events(wait);
>>       unsigned long flags;
>>       unsigned int ret;
>>       struct vb2_buffer *vb = NULL;
>> @@ -1373,12 +1374,14 @@ unsigned int vb2_poll(struct vb2_queue *q, struct
>> file *file, poll_table *wait)
>>        * Start file I/O emulator only if streaming API has not been used
>> yet.
>>        */
>>       if (q->num_buffers == 0 && q->fileio == NULL) {
>> -             if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ))
>> {
>> +             if (!V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_READ)
>> &&
>> +                             (req_events & (POLLIN | POLLRDNORM))) {
>>                       ret = __vb2_init_fileio(q, 1);
>>                       if (ret)
>>                               return POLLERR;
>>               }
>> -             if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE))
>> {
>> +             if (V4L2_TYPE_IS_OUTPUT(q->type) && (q->io_modes & VB2_WRITE)
>> &&
>> +                             (req_events & (POLLOUT | POLLWRNORM))) {
>>                       ret = __vb2_init_fileio(q, 0);
>>                       if (ret)
>>                               return POLLERR;
>> --
>
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
>
>
>
>



-- 
Best regards,
Pawel Osciak