[RFC][PATCH 0/2] media: fix polling not to wait if a buffer is available

[RFC][PATCH 0/2] media: fix polling not to wait if a buffer is available

[Seung-Woo Kim]

As poll behavior described in following link, polling needs to just return if
already some buffer is in done list.
Link: http://www.spinics.net/lists/linux-media/msg34759.html

But in current vb2 and v4l2_m2m, poll function always calls poll_wait(), so it
needs to wait until next vb2_buffer_done() or queue is cancelled.

So I add check routine for done_list before calling poll_wait().
But I'm not sure that locking for done_lock of queue is also needed in this
case or not because done_list of queue is checked without locking in some
other parts of vb2.

Seung-Woo Kim (2):
  media: vb2: return for polling if a buffer is available
  media: v4l2-mem2mem: return for polling if a buffer is available

 drivers/media/v4l2-core/v4l2-mem2mem.c   |    6 ++++--
 drivers/media/v4l2-core/videobuf2-core.c |    3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

-- 
1.7.4.1

[RFC][PATCH 1/2] media: vb2: return for polling if a buffer is available

[Seung-Woo Kim]

The vb2_poll() does not need to wait next vb_buffer_done() if there is already
a buffer in done_list of queue, but current vb2_poll() always waits.
So done_list is checked before calling poll_wait().

Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
---
 drivers/media/v4l2-core/videobuf2-core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index db1235d..e941d2b 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1996,7 +1996,8 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
 	if (list_empty(&q->queued_list))
 		return res | POLLERR;
 
-	poll_wait(file, &q->done_wq, wait);
+	if (list_empty(&q->done_list))
+		poll_wait(file, &q->done_wq, wait);
 
 	/*
 	 * Take first buffer available for dequeuing.
-- 
1.7.4.1

[RFC][PATCH 2/2] media: v4l2-mem2mem: return for polling if a buffer is available

[Seung-Woo Kim]

The v4l2_m2m_poll() does not need to wait if there is already a buffer in
done_list of source and destination queues, but current v4l2_m2m_poll() always
waits. So done_list of each queue is checked before calling poll_wait().

Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index da99cf7..b6f0316 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -458,8 +458,10 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 	if (m2m_ctx->m2m_dev->m2m_ops->unlock)
 		m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
 
-	poll_wait(file, &src_q->done_wq, wait);
-	poll_wait(file, &dst_q->done_wq, wait);
+	if (list_empty(&src_q->done_list))
+		poll_wait(file, &src_q->done_wq, wait);
+	if (list_empty(&dst_q->done_list))
+		poll_wait(file, &dst_q->done_wq, wait);
 
 	if (m2m_ctx->m2m_dev->m2m_ops->lock)
 		m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
-- 
1.7.4.1

Re: [RFC][PATCH 1/2] media: vb2: return for polling if a buffer is available

[Marek Szyprowski]

Hello,

On 4/1/2013 8:40 AM, Seung-Woo Kim wrote:
> The vb2_poll() does not need to wait next vb_buffer_done() if there is already
> a buffer in done_list of queue, but current vb2_poll() always waits.
> So done_list is checked before calling poll_wait().
>
> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>

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

> ---
>   drivers/media/v4l2-core/videobuf2-core.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index db1235d..e941d2b 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1996,7 +1996,8 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait)
>   	if (list_empty(&q->queued_list))
>   		return res | POLLERR;
>   
> -	poll_wait(file, &q->done_wq, wait);
> +	if (list_empty(&q->done_list))
> +		poll_wait(file, &q->done_wq, wait);
>   
>   	/*
>   	 * Take first buffer available for dequeuing.

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

Re: [RFC][PATCH 0/2] media: fix polling not to wait if a buffer is available

[김승우]

Ping this patch-set.

The first patch for vb2 was acked by Marek.

any suggestion for the second patch?

Regards,
- Seung-Woo Kim

On 2013년 04월 01일 15:40, Seung-Woo Kim wrote:
> As poll behavior described in following link, polling needs to just return if
> already some buffer is in done list.
> Link: http://www.spinics.net/lists/linux-media/msg34759.html
> 
> But in current vb2 and v4l2_m2m, poll function always calls poll_wait(), so it
> needs to wait until next vb2_buffer_done() or queue is cancelled.
> 
> So I add check routine for done_list before calling poll_wait().
> But I'm not sure that locking for done_lock of queue is also needed in this
> case or not because done_list of queue is checked without locking in some
> other parts of vb2.
> 
> Seung-Woo Kim (2):
>   media: vb2: return for polling if a buffer is available
>   media: v4l2-mem2mem: return for polling if a buffer is available
> 
>  drivers/media/v4l2-core/v4l2-mem2mem.c   |    6 ++++--
>  drivers/media/v4l2-core/videobuf2-core.c |    3 ++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

Re: [RFC][PATCH 2/2] media: v4l2-mem2mem: return for polling if a buffer is available

[Marek Szyprowski]

Hello,

On 4/1/2013 8:40 AM, Seung-Woo Kim wrote:
> The v4l2_m2m_poll() does not need to wait if there is already a buffer in
> done_list of source and destination queues, but current v4l2_m2m_poll() always
> waits. So done_list of each queue is checked before calling poll_wait().
>
> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>

Right now I have no idea how to fix this better than it has been 
proposed in your patch. I wonder what will happen if the device doesn't 
release both source and destination buffers at the same time, but this 
situation is purely hypothetical as there is no driver which does it 
such way, therefore:

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

Sorry for a long delay, I had to find some time to analyze the code.

> ---
>   drivers/media/v4l2-core/v4l2-mem2mem.c |    6 ++++--
>   1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index da99cf7..b6f0316 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -458,8 +458,10 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>   	if (m2m_ctx->m2m_dev->m2m_ops->unlock)
>   		m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
>   
> -	poll_wait(file, &src_q->done_wq, wait);
> -	poll_wait(file, &dst_q->done_wq, wait);
> +	if (list_empty(&src_q->done_list))
> +		poll_wait(file, &src_q->done_wq, wait);
> +	if (list_empty(&dst_q->done_list))
> +		poll_wait(file, &dst_q->done_wq, wait);
>   
>   	if (m2m_ctx->m2m_dev->m2m_ops->lock)
>   		m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);

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