Re: [PATCH 9/9] media: vb2: use lock if wait_prepare/finish are NULL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

Thank you for the patch.

On Mon, Sep 02, 2024 at 04:04:55PM +0200, Hans Verkuil wrote:
> If the wait_prepare or wait_finish callback is set, then call it.
> If it is NULL and the queue lock pointer is not NULL, then just
> unlock/lock that mutex.
> 
> This allows simplifying drivers by dropping the wait_prepare and
> wait_finish ops (and eventually the vb2_ops_wait_prepare/finish helpers).
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 6335ac7b771a..d064e0664851 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -2035,7 +2035,10 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>  		 * become ready or for streamoff. Driver's lock is released to
>  		 * allow streamoff or qbuf to be called while waiting.
>  		 */
> -		call_void_qop(q, wait_prepare, q);
> +		if (q->ops->wait_prepare)
> +			call_void_qop(q, wait_prepare, q);
> +		else if (q->lock)
> +			mutex_unlock(q->lock);

How about calling vb2_ops_wait_prepare() here ? I think that would be
more explicit. You would likely need to move the function to
videobuf2-core.c. Same below for wait_finish.

>  
>  		/*
>  		 * All locks have been released, it is safe to sleep now.
> @@ -2045,12 +2048,16 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
>  				!list_empty(&q->done_list) || !q->streaming ||
>  				q->error);
>  
> +		if (q->ops->wait_finish)
> +			call_void_qop(q, wait_finish, q);
> +		else if (q->lock)
> +			mutex_lock(q->lock);
> +
> +		q->waiting_in_dqbuf = 0;
>  		/*
>  		 * We need to reevaluate both conditions again after reacquiring
>  		 * the locks or return an error if one occurred.
>  		 */
> -		call_void_qop(q, wait_finish, q);
> -		q->waiting_in_dqbuf = 0;
>  		if (ret) {
>  			dprintk(q, 1, "sleep was interrupted\n");
>  			return ret;

-- 
Regards,

Laurent Pinchart




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux