On 04/25/2016 09:59 AM, Ricardo Ribalda Delgado wrote: > When using a device is read/write mode, vb2 does not handle properly the > first select/poll operation. > > The reason for this, is that when this code has been refactored, some of > the operations have changed their order, and now fileio emulator is not > started. > > The reintroduced check to the core is enabled by a quirk flag, that > avoids this check by other subsystems like DVB. > > Reported-by: Dimitrios Katsaros <patcherwork@xxxxxxxxx> > Cc: Junghak Sung <jh1009.sung@xxxxxxxxxxx> > Fixes: 49d8ab9feaf2 ("media] media: videobuf2: Separate vb2_poll()") > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@xxxxxxxxx> > --- > > v2: By Hans Verkuil <hverkuil@xxxxxxxxx> and > Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>: > > Add a quirk bit to enable this behaviour on the core. > drivers/media/v4l2-core/videobuf2-core.c | 9 +++++++++ > drivers/media/v4l2-core/videobuf2-v4l2.c | 9 +-------- > include/media/videobuf2-core.h | 4 ++++ > 3 files changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c > index 5d016f496e0e..58eb9be13510 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -2298,6 +2298,15 @@ unsigned int vb2_core_poll(struct vb2_queue *q, struct file *file, > return POLLERR; > > /* > + * For compatibility with vb1: if QBUF hasn't been called yet, then > + * return POLLERR as well. This only affects capture queues, output > + * queues will always initialize waiting_for_buffers to false. > + */ This comment should be moved to where the quirk is set in v4l2. This comment shouldn't refer to vb2 at all. How about: "If this quirk is set and if QBUF hasn't been called..." and perhaps at the end of the comment: "This quirk is set by V4L2 for backwards compatibility reasons." although I am not completely certain we need that at all here. > + if (q->quirk_poll_must_check_waiting_for_buffers && > + q->waiting_for_buffers && (req_events & (POLLIN | POLLRDNORM))) > + return POLLERR; > + > + /* > * For output streams you can call write() as long as there are fewer > * buffers queued than there are buffers available. > */ > diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c > index 91f552124050..3e649adf85ef 100644 > --- a/drivers/media/v4l2-core/videobuf2-v4l2.c > +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c > @@ -765,6 +765,7 @@ int vb2_queue_init(struct vb2_queue *q) > q->is_output = V4L2_TYPE_IS_OUTPUT(q->type); > q->copy_timestamp = (q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) > == V4L2_BUF_FLAG_TIMESTAMP_COPY; > + q->quirk_poll_must_check_waiting_for_buffers = true; > > return vb2_core_queue_init(q); > } > @@ -818,14 +819,6 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait) > poll_wait(file, &fh->wait, wait); > } > > - /* > - * For compatibility with vb1: if QBUF hasn't been called yet, then > - * return POLLERR as well. This only affects capture queues, output > - * queues will always initialize waiting_for_buffers to false. > - */ > - if (q->waiting_for_buffers && (req_events & (POLLIN | POLLRDNORM))) > - return POLLERR; > - > return res | vb2_core_poll(q, file, wait); > } > EXPORT_SYMBOL_GPL(vb2_poll); > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 8a0f55b6c2ba..3a1620946068 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -400,6 +400,9 @@ struct vb2_buf_ops { > * @fileio_read_once: report EOF after reading the first buffer > * @fileio_write_immediately: queue buffer after each write() call > * @allow_zero_bytesused: allow bytesused == 0 to be passed to the driver > + * @quirk_poll_must_check_waiting_for_buffers: Return POLLERR at poll when QBUF > + * has not been called. This is a vb1 idiom that has been adopted > + * also by vb2. > * @lock: pointer to a mutex that protects the vb2_queue struct. The > * driver can set this to a mutex to let the v4l2 core serialize > * the queuing ioctls. If the driver wants to handle locking > @@ -463,6 +466,7 @@ struct vb2_queue { > unsigned fileio_read_once:1; > unsigned fileio_write_immediately:1; > unsigned allow_zero_bytesused:1; > + unsigned quirk_poll_must_check_waiting_for_buffers:1; > > struct mutex *lock; > void *owner; > Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html