Re: [PATCH 2/2] media: v4l2-mem2mem: simplify poll logic a bit

2020-08-26 Thread Hans Verkuil 
On 26/08/2020 16:32, Ezequiel Garcia wrote:
> On Wed, 26 Aug 2020 at 08:19, Alexandre Courbot  wrote:
>>
>> Hi Ezequiel, thanks for the review!
>>
>> On Wed, Aug 26, 2020 at 1:15 PM Ezequiel Garcia
>>  wrote:
>>>
>>> Hi Alexandre,
>>>
>>> On Tue, 25 Aug 2020 at 11:56, Alexandre Courbot  wrote:

 Factorize redundant checks into a single code block, remove the early
 return, and declare variables in their innermost block. Hopefully this
 makes this code a little bit easier to follow.

>>>
>>> This _definitely_ makes the poll handling more readable.
>>>
>>> Reviewed-by: Ezequiel Garcia 
>>>
>>> See below a nitpick.
>>>
 Signed-off-by: Alexandre Courbot 
 ---
  drivers/media/v4l2-core/v4l2-mem2mem.c | 35 +++---
  1 file changed, 15 insertions(+), 20 deletions(-)

 diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
 b/drivers/media/v4l2-core/v4l2-mem2mem.c
 index 0d0192119af20..aeac9707123d0 100644
 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
 +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
 @@ -841,7 +841,6 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
 *file,
struct poll_table_struct *wait)
  {
 struct vb2_queue *src_q, *dst_q;
 -   struct vb2_buffer *src_vb = NULL, *dst_vb = NULL;
 __poll_t rc = 0;
 unsigned long flags;

 @@ -863,33 +862,29 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
 *file,
 return EPOLLERR;

 spin_lock_irqsave(&src_q->done_lock, flags);
 -   if (!list_empty(&src_q->done_list))
 -   src_vb = list_first_entry(&src_q->done_list, struct 
 vb2_buffer,
 -   done_entry);
 -   if (src_vb && (src_vb->state == VB2_BUF_STATE_DONE
 -   || src_vb->state == VB2_BUF_STATE_ERROR))
 -   rc |= EPOLLOUT | EPOLLWRNORM;
 +   if (!list_empty(&src_q->done_list)) {
 +   struct vb2_buffer *src_vb = list_first_entry(
 +   &src_q->done_list, struct vb2_buffer, done_entry);
 +   if (src_vb->state == VB2_BUF_STATE_DONE ||
 +   src_vb->state == VB2_BUF_STATE_ERROR)
 +   rc |= EPOLLOUT | EPOLLWRNORM;
 +   }
 spin_unlock_irqrestore(&src_q->done_lock, flags);

 spin_lock_irqsave(&dst_q->done_lock, flags);
 -   if (list_empty(&dst_q->done_list)) {
 +   if (!list_empty(&dst_q->done_list)) {
 +   struct vb2_buffer *dst_vb = list_first_entry(
 +   &dst_q->done_list, struct vb2_buffer, done_entry);
 +   if (dst_vb->state == VB2_BUF_STATE_DONE ||
 +   dst_vb->state == VB2_BUF_STATE_ERROR)
 +   rc |= EPOLLIN | EPOLLRDNORM;
 +   } else if (dst_q->last_buffer_dequeued) {
 /*
  * If the last buffer was dequeued from the capture queue,
  * return immediately. DQBUF will return -EPIPE.
  */
>>>
>>> The part about "returning immediately" doesn't make
>>> much sense now. Could we rephrase this, keeping the -EPIPE
>>> comment?
>>
>> I understood this sentence as referring to the system call and not
>> just this function, but maybe we can rephrase this as "... make
>> user-space wake up immediately"?
>>
> 
> But is this really about user-space wakeup? I am under the impression
> that past poll_wait on both queues, we are already about to return
> (and wakeup).
> 
> The way I see it, the original commit intention was to skip any
> done_list handling, returning immediately on the last buffer condition.
> 
> How about just
> 
> """
> If the last buffer was dequeued from the capture queue,
> signal userspace. DQBUF will return -EPIPE.

I'd write 'DQBUF(CAPTURE)' here to emphasize that only the capture
queue will return -EPIPE when you try to dequeue from it.

Also note that the original text was a copy-and-paste from vb2_core_poll().
The phrase 'return immediately' makes sense in that context since that
poll code deals with a single queue. In this case you have two queues,
and 'return immediately' no longer applies (in fact, that effectively is
the bug that being fixed here!).

Regards,

Hans

> """
> 
> ?
> 
>>>
>>> Thanks,
>>> Ezequiel
>>>
 -   if (dst_q->last_buffer_dequeued) {
 -   spin_unlock_irqrestore(&dst_q->done_lock, flags);
 -   rc |= EPOLLIN | EPOLLRDNORM;
 -   return rc;
 -   }
 -   }
 -
 -   if (!list_empty(&dst_q->done_list))
 -   dst_vb = list_first_entry(&dst_q->done_list, struct 
 vb2_buffer,
 -   done_entry);
 -

Re: [PATCH 2/2] media: v4l2-mem2mem: simplify poll logic a bit

2020-08-26 Thread Ezequiel Garcia 
On Wed, 26 Aug 2020 at 08:19, Alexandre Courbot  wrote:
>
> Hi Ezequiel, thanks for the review!
>
> On Wed, Aug 26, 2020 at 1:15 PM Ezequiel Garcia
>  wrote:
> >
> > Hi Alexandre,
> >
> > On Tue, 25 Aug 2020 at 11:56, Alexandre Courbot  wrote:
> > >
> > > Factorize redundant checks into a single code block, remove the early
> > > return, and declare variables in their innermost block. Hopefully this
> > > makes this code a little bit easier to follow.
> > >
> >
> > This _definitely_ makes the poll handling more readable.
> >
> > Reviewed-by: Ezequiel Garcia 
> >
> > See below a nitpick.
> >
> > > Signed-off-by: Alexandre Courbot 
> > > ---
> > >  drivers/media/v4l2-core/v4l2-mem2mem.c | 35 +++---
> > >  1 file changed, 15 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
> > > b/drivers/media/v4l2-core/v4l2-mem2mem.c
> > > index 0d0192119af20..aeac9707123d0 100644
> > > --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> > > +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> > > @@ -841,7 +841,6 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
> > > *file,
> > >struct poll_table_struct *wait)
> > >  {
> > > struct vb2_queue *src_q, *dst_q;
> > > -   struct vb2_buffer *src_vb = NULL, *dst_vb = NULL;
> > > __poll_t rc = 0;
> > > unsigned long flags;
> > >
> > > @@ -863,33 +862,29 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
> > > *file,
> > > return EPOLLERR;
> > >
> > > spin_lock_irqsave(&src_q->done_lock, flags);
> > > -   if (!list_empty(&src_q->done_list))
> > > -   src_vb = list_first_entry(&src_q->done_list, struct 
> > > vb2_buffer,
> > > -   done_entry);
> > > -   if (src_vb && (src_vb->state == VB2_BUF_STATE_DONE
> > > -   || src_vb->state == VB2_BUF_STATE_ERROR))
> > > -   rc |= EPOLLOUT | EPOLLWRNORM;
> > > +   if (!list_empty(&src_q->done_list)) {
> > > +   struct vb2_buffer *src_vb = list_first_entry(
> > > +   &src_q->done_list, struct vb2_buffer, done_entry);
> > > +   if (src_vb->state == VB2_BUF_STATE_DONE ||
> > > +   src_vb->state == VB2_BUF_STATE_ERROR)
> > > +   rc |= EPOLLOUT | EPOLLWRNORM;
> > > +   }
> > > spin_unlock_irqrestore(&src_q->done_lock, flags);
> > >
> > > spin_lock_irqsave(&dst_q->done_lock, flags);
> > > -   if (list_empty(&dst_q->done_list)) {
> > > +   if (!list_empty(&dst_q->done_list)) {
> > > +   struct vb2_buffer *dst_vb = list_first_entry(
> > > +   &dst_q->done_list, struct vb2_buffer, done_entry);
> > > +   if (dst_vb->state == VB2_BUF_STATE_DONE ||
> > > +   dst_vb->state == VB2_BUF_STATE_ERROR)
> > > +   rc |= EPOLLIN | EPOLLRDNORM;
> > > +   } else if (dst_q->last_buffer_dequeued) {
> > > /*
> > >  * If the last buffer was dequeued from the capture queue,
> > >  * return immediately. DQBUF will return -EPIPE.
> > >  */
> >
> > The part about "returning immediately" doesn't make
> > much sense now. Could we rephrase this, keeping the -EPIPE
> > comment?
>
> I understood this sentence as referring to the system call and not
> just this function, but maybe we can rephrase this as "... make
> user-space wake up immediately"?
>

But is this really about user-space wakeup? I am under the impression
that past poll_wait on both queues, we are already about to return
(and wakeup).

The way I see it, the original commit intention was to skip any
done_list handling, returning immediately on the last buffer condition.

How about just

"""
If the last buffer was dequeued from the capture queue,
signal userspace. DQBUF will return -EPIPE.
"""

?

> >
> > Thanks,
> > Ezequiel
> >
> > > -   if (dst_q->last_buffer_dequeued) {
> > > -   spin_unlock_irqrestore(&dst_q->done_lock, flags);
> > > -   rc |= EPOLLIN | EPOLLRDNORM;
> > > -   return rc;
> > > -   }
> > > -   }
> > > -
> > > -   if (!list_empty(&dst_q->done_list))
> > > -   dst_vb = list_first_entry(&dst_q->done_list, struct 
> > > vb2_buffer,
> > > -   done_entry);
> > > -   if (dst_vb && (dst_vb->state == VB2_BUF_STATE_DONE
> > > -   || dst_vb->state == VB2_BUF_STATE_ERROR))
> > > rc |= EPOLLIN | EPOLLRDNORM;
> > > +   }
> > > spin_unlock_irqrestore(&dst_q->done_lock, flags);
> > >
> > > return rc;
> > > --
> > > 2.28.0
> > >

Re: [PATCH 2/2] media: v4l2-mem2mem: simplify poll logic a bit

2020-08-26 Thread Hans Verkuil 
On 25/08/2020 16:55, Alexandre Courbot wrote:
> Factorize redundant checks into a single code block, remove the early
> return, and declare variables in their innermost block. Hopefully this
> makes this code a little bit easier to follow.
> 
> Signed-off-by: Alexandre Courbot 
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 35 +++---
>  1 file changed, 15 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
> b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 0d0192119af20..aeac9707123d0 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -841,7 +841,6 @@ static __poll_t v4l2_m2m_poll_for_data(struct file *file,
>  struct poll_table_struct *wait)
>  {
>   struct vb2_queue *src_q, *dst_q;
> - struct vb2_buffer *src_vb = NULL, *dst_vb = NULL;
>   __poll_t rc = 0;
>   unsigned long flags;
>  
> @@ -863,33 +862,29 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
> *file,
>   return EPOLLERR;
>  
>   spin_lock_irqsave(&src_q->done_lock, flags);
> - if (!list_empty(&src_q->done_list))
> - src_vb = list_first_entry(&src_q->done_list, struct vb2_buffer,
> - done_entry);
> - if (src_vb && (src_vb->state == VB2_BUF_STATE_DONE
> - || src_vb->state == VB2_BUF_STATE_ERROR))
> - rc |= EPOLLOUT | EPOLLWRNORM;
> + if (!list_empty(&src_q->done_list)) {
> + struct vb2_buffer *src_vb = list_first_entry(
> + &src_q->done_list, struct vb2_buffer, done_entry);
> + if (src_vb->state == VB2_BUF_STATE_DONE ||
> + src_vb->state == VB2_BUF_STATE_ERROR)

This test is unnecessary: only buffers in state DONE or ERROR can be on the 
done_list.

> + rc |= EPOLLOUT | EPOLLWRNORM;
> + }
>   spin_unlock_irqrestore(&src_q->done_lock, flags);
>  
>   spin_lock_irqsave(&dst_q->done_lock, flags);
> - if (list_empty(&dst_q->done_list)) {
> + if (!list_empty(&dst_q->done_list)) {
> + struct vb2_buffer *dst_vb = list_first_entry(
> + &dst_q->done_list, struct vb2_buffer, done_entry);
> + if (dst_vb->state == VB2_BUF_STATE_DONE ||
> + dst_vb->state == VB2_BUF_STATE_ERROR)

Ditto.

Regards,

Hans

> + rc |= EPOLLIN | EPOLLRDNORM;
> + } else if (dst_q->last_buffer_dequeued) {
>   /*
>* If the last buffer was dequeued from the capture queue,
>* return immediately. DQBUF will return -EPIPE.
>*/
> - if (dst_q->last_buffer_dequeued) {
> - spin_unlock_irqrestore(&dst_q->done_lock, flags);
> - rc |= EPOLLIN | EPOLLRDNORM;
> - return rc;
> - }
> - }
> -
> - if (!list_empty(&dst_q->done_list))
> - dst_vb = list_first_entry(&dst_q->done_list, struct vb2_buffer,
> - done_entry);
> - if (dst_vb && (dst_vb->state == VB2_BUF_STATE_DONE
> - || dst_vb->state == VB2_BUF_STATE_ERROR))
>   rc |= EPOLLIN | EPOLLRDNORM;
> + }
>   spin_unlock_irqrestore(&dst_q->done_lock, flags);
>  
>   return rc;
>

Re: [PATCH 2/2] media: v4l2-mem2mem: simplify poll logic a bit

2020-08-26 Thread Alexandre Courbot 
Hi Ezequiel, thanks for the review!

On Wed, Aug 26, 2020 at 1:15 PM Ezequiel Garcia
 wrote:
>
> Hi Alexandre,
>
> On Tue, 25 Aug 2020 at 11:56, Alexandre Courbot  wrote:
> >
> > Factorize redundant checks into a single code block, remove the early
> > return, and declare variables in their innermost block. Hopefully this
> > makes this code a little bit easier to follow.
> >
>
> This _definitely_ makes the poll handling more readable.
>
> Reviewed-by: Ezequiel Garcia 
>
> See below a nitpick.
>
> > Signed-off-by: Alexandre Courbot 
> > ---
> >  drivers/media/v4l2-core/v4l2-mem2mem.c | 35 +++---
> >  1 file changed, 15 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
> > b/drivers/media/v4l2-core/v4l2-mem2mem.c
> > index 0d0192119af20..aeac9707123d0 100644
> > --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> > +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> > @@ -841,7 +841,6 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
> > *file,
> >struct poll_table_struct *wait)
> >  {
> > struct vb2_queue *src_q, *dst_q;
> > -   struct vb2_buffer *src_vb = NULL, *dst_vb = NULL;
> > __poll_t rc = 0;
> > unsigned long flags;
> >
> > @@ -863,33 +862,29 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
> > *file,
> > return EPOLLERR;
> >
> > spin_lock_irqsave(&src_q->done_lock, flags);
> > -   if (!list_empty(&src_q->done_list))
> > -   src_vb = list_first_entry(&src_q->done_list, struct 
> > vb2_buffer,
> > -   done_entry);
> > -   if (src_vb && (src_vb->state == VB2_BUF_STATE_DONE
> > -   || src_vb->state == VB2_BUF_STATE_ERROR))
> > -   rc |= EPOLLOUT | EPOLLWRNORM;
> > +   if (!list_empty(&src_q->done_list)) {
> > +   struct vb2_buffer *src_vb = list_first_entry(
> > +   &src_q->done_list, struct vb2_buffer, done_entry);
> > +   if (src_vb->state == VB2_BUF_STATE_DONE ||
> > +   src_vb->state == VB2_BUF_STATE_ERROR)
> > +   rc |= EPOLLOUT | EPOLLWRNORM;
> > +   }
> > spin_unlock_irqrestore(&src_q->done_lock, flags);
> >
> > spin_lock_irqsave(&dst_q->done_lock, flags);
> > -   if (list_empty(&dst_q->done_list)) {
> > +   if (!list_empty(&dst_q->done_list)) {
> > +   struct vb2_buffer *dst_vb = list_first_entry(
> > +   &dst_q->done_list, struct vb2_buffer, done_entry);
> > +   if (dst_vb->state == VB2_BUF_STATE_DONE ||
> > +   dst_vb->state == VB2_BUF_STATE_ERROR)
> > +   rc |= EPOLLIN | EPOLLRDNORM;
> > +   } else if (dst_q->last_buffer_dequeued) {
> > /*
> >  * If the last buffer was dequeued from the capture queue,
> >  * return immediately. DQBUF will return -EPIPE.
> >  */
>
> The part about "returning immediately" doesn't make
> much sense now. Could we rephrase this, keeping the -EPIPE
> comment?

I understood this sentence as referring to the system call and not
just this function, but maybe we can rephrase this as "... make
user-space wake up immediately"?

>
> Thanks,
> Ezequiel
>
> > -   if (dst_q->last_buffer_dequeued) {
> > -   spin_unlock_irqrestore(&dst_q->done_lock, flags);
> > -   rc |= EPOLLIN | EPOLLRDNORM;
> > -   return rc;
> > -   }
> > -   }
> > -
> > -   if (!list_empty(&dst_q->done_list))
> > -   dst_vb = list_first_entry(&dst_q->done_list, struct 
> > vb2_buffer,
> > -   done_entry);
> > -   if (dst_vb && (dst_vb->state == VB2_BUF_STATE_DONE
> > -   || dst_vb->state == VB2_BUF_STATE_ERROR))
> > rc |= EPOLLIN | EPOLLRDNORM;
> > +   }
> > spin_unlock_irqrestore(&dst_q->done_lock, flags);
> >
> > return rc;
> > --
> > 2.28.0
> >

Re: [PATCH 2/2] media: v4l2-mem2mem: simplify poll logic a bit

2020-08-25 Thread Ezequiel Garcia 
Hi Alexandre,

On Tue, 25 Aug 2020 at 11:56, Alexandre Courbot  wrote:
>
> Factorize redundant checks into a single code block, remove the early
> return, and declare variables in their innermost block. Hopefully this
> makes this code a little bit easier to follow.
>

This _definitely_ makes the poll handling more readable.

Reviewed-by: Ezequiel Garcia 

See below a nitpick.

> Signed-off-by: Alexandre Courbot 
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 35 +++---
>  1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
> b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 0d0192119af20..aeac9707123d0 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -841,7 +841,6 @@ static __poll_t v4l2_m2m_poll_for_data(struct file *file,
>struct poll_table_struct *wait)
>  {
> struct vb2_queue *src_q, *dst_q;
> -   struct vb2_buffer *src_vb = NULL, *dst_vb = NULL;
> __poll_t rc = 0;
> unsigned long flags;
>
> @@ -863,33 +862,29 @@ static __poll_t v4l2_m2m_poll_for_data(struct file 
> *file,
> return EPOLLERR;
>
> spin_lock_irqsave(&src_q->done_lock, flags);
> -   if (!list_empty(&src_q->done_list))
> -   src_vb = list_first_entry(&src_q->done_list, struct 
> vb2_buffer,
> -   done_entry);
> -   if (src_vb && (src_vb->state == VB2_BUF_STATE_DONE
> -   || src_vb->state == VB2_BUF_STATE_ERROR))
> -   rc |= EPOLLOUT | EPOLLWRNORM;
> +   if (!list_empty(&src_q->done_list)) {
> +   struct vb2_buffer *src_vb = list_first_entry(
> +   &src_q->done_list, struct vb2_buffer, done_entry);
> +   if (src_vb->state == VB2_BUF_STATE_DONE ||
> +   src_vb->state == VB2_BUF_STATE_ERROR)
> +   rc |= EPOLLOUT | EPOLLWRNORM;
> +   }
> spin_unlock_irqrestore(&src_q->done_lock, flags);
>
> spin_lock_irqsave(&dst_q->done_lock, flags);
> -   if (list_empty(&dst_q->done_list)) {
> +   if (!list_empty(&dst_q->done_list)) {
> +   struct vb2_buffer *dst_vb = list_first_entry(
> +   &dst_q->done_list, struct vb2_buffer, done_entry);
> +   if (dst_vb->state == VB2_BUF_STATE_DONE ||
> +   dst_vb->state == VB2_BUF_STATE_ERROR)
> +   rc |= EPOLLIN | EPOLLRDNORM;
> +   } else if (dst_q->last_buffer_dequeued) {
> /*
>  * If the last buffer was dequeued from the capture queue,
>  * return immediately. DQBUF will return -EPIPE.
>  */

The part about "returning immediately" doesn't make
much sense now. Could we rephrase this, keeping the -EPIPE
comment?

Thanks,
Ezequiel

> -   if (dst_q->last_buffer_dequeued) {
> -   spin_unlock_irqrestore(&dst_q->done_lock, flags);
> -   rc |= EPOLLIN | EPOLLRDNORM;
> -   return rc;
> -   }
> -   }
> -
> -   if (!list_empty(&dst_q->done_list))
> -   dst_vb = list_first_entry(&dst_q->done_list, struct 
> vb2_buffer,
> -   done_entry);
> -   if (dst_vb && (dst_vb->state == VB2_BUF_STATE_DONE
> -   || dst_vb->state == VB2_BUF_STATE_ERROR))
> rc |= EPOLLIN | EPOLLRDNORM;
> +   }
> spin_unlock_irqrestore(&dst_q->done_lock, flags);
>
> return rc;
> --
> 2.28.0
>

[PATCH 2/2] media: v4l2-mem2mem: simplify poll logic a bit

2020-08-25 Thread Alexandre Courbot 
Factorize redundant checks into a single code block, remove the early
return, and declare variables in their innermost block. Hopefully this
makes this code a little bit easier to follow.

Signed-off-by: Alexandre Courbot 
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 35 +++---
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 0d0192119af20..aeac9707123d0 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -841,7 +841,6 @@ static __poll_t v4l2_m2m_poll_for_data(struct file *file,
   struct poll_table_struct *wait)
 {
struct vb2_queue *src_q, *dst_q;
-   struct vb2_buffer *src_vb = NULL, *dst_vb = NULL;
__poll_t rc = 0;
unsigned long flags;
 
@@ -863,33 +862,29 @@ static __poll_t v4l2_m2m_poll_for_data(struct file *file,
return EPOLLERR;
 
spin_lock_irqsave(&src_q->done_lock, flags);
-   if (!list_empty(&src_q->done_list))
-   src_vb = list_first_entry(&src_q->done_list, struct vb2_buffer,
-   done_entry);
-   if (src_vb && (src_vb->state == VB2_BUF_STATE_DONE
-   || src_vb->state == VB2_BUF_STATE_ERROR))
-   rc |= EPOLLOUT | EPOLLWRNORM;
+   if (!list_empty(&src_q->done_list)) {
+   struct vb2_buffer *src_vb = list_first_entry(
+   &src_q->done_list, struct vb2_buffer, done_entry);
+   if (src_vb->state == VB2_BUF_STATE_DONE ||
+   src_vb->state == VB2_BUF_STATE_ERROR)
+   rc |= EPOLLOUT | EPOLLWRNORM;
+   }
spin_unlock_irqrestore(&src_q->done_lock, flags);
 
spin_lock_irqsave(&dst_q->done_lock, flags);
-   if (list_empty(&dst_q->done_list)) {
+   if (!list_empty(&dst_q->done_list)) {
+   struct vb2_buffer *dst_vb = list_first_entry(
+   &dst_q->done_list, struct vb2_buffer, done_entry);
+   if (dst_vb->state == VB2_BUF_STATE_DONE ||
+   dst_vb->state == VB2_BUF_STATE_ERROR)
+   rc |= EPOLLIN | EPOLLRDNORM;
+   } else if (dst_q->last_buffer_dequeued) {
/*
 * If the last buffer was dequeued from the capture queue,
 * return immediately. DQBUF will return -EPIPE.
 */
-   if (dst_q->last_buffer_dequeued) {
-   spin_unlock_irqrestore(&dst_q->done_lock, flags);
-   rc |= EPOLLIN | EPOLLRDNORM;
-   return rc;
-   }
-   }
-
-   if (!list_empty(&dst_q->done_list))
-   dst_vb = list_first_entry(&dst_q->done_list, struct vb2_buffer,
-   done_entry);
-   if (dst_vb && (dst_vb->state == VB2_BUF_STATE_DONE
-   || dst_vb->state == VB2_BUF_STATE_ERROR))
rc |= EPOLLIN | EPOLLRDNORM;
+   }
spin_unlock_irqrestore(&dst_q->done_lock, flags);
 
return rc;
-- 
2.28.0