[PATCH 0/3] media: videobuf2: Expose vb2_queue_is_busy() to drivers

[PATCH 0/3] media: videobuf2: Expose vb2_queue_is_busy() to drivers

[Laurent Pinchart]

Hello,

This small patch series exposes the vb2_queue_is_busy() function,
currently internal to videobuf2-v4l2.c, to drivers. The rationale is
explained in patch 1/3, and the first use case shown in 3/3: it allows
implementing additional checks at streamon time while keeping the owner
check first (as it's cheap, compared to other checks that can be more
expensive).

Patch 2/3 is a driver-by cleanup.

Laurent Pinchart (3):
  media: videobuf2-v4l2: Expose vb2_queue_is_busy() to drivers
  media: vsp1: Don't open-code vb2_fop_release()
  media: vsp1: Use vb2_queue_is_busy()

 .../media/common/videobuf2/videobuf2-v4l2.c   | 26 +++++++------------
 .../media/platform/renesas/vsp1/vsp1_video.c  | 12 ++-------
 include/media/videobuf2-v4l2.h                | 23 ++++++++++++++--
 3 files changed, 33 insertions(+), 28 deletions(-)


base-commit: 71e6d0608e4d1b79069990c7dacb3600ced28a3b
-- 
Regards,

Laurent Pinchart

[PATCH 1/3] media: videobuf2-v4l2: Expose vb2_queue_is_busy() to drivers

[Laurent Pinchart]

vb2 queue ownership is managed by the ioctl handler helpers
(vb2_ioctl_*). There are however use cases where drivers can benefit
from checking queue ownership, for instance when open-coding an ioctl
handler that needs to perform additional checks before calling the
corresponding vb2 operation.

Expose the vb2_queue_is_busy() function in the videobuf2-v4l2.h header,
and change its first argument to a struct vb2_queue pointer as the
function name implies it operates on a queue, not a video_device.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../media/common/videobuf2/videobuf2-v4l2.c   | 26 +++++++------------
 include/media/videobuf2-v4l2.h                | 23 ++++++++++++++--
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 6edf4508c636..075d24ebf44c 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -977,12 +977,6 @@ EXPORT_SYMBOL_GPL(vb2_poll);
  * and so they simplify the driver code.
  */
 
-/* The queue is busy if there is a owner and you are not that owner. */
-static inline bool vb2_queue_is_busy(struct video_device *vdev, struct file *file)
-{
-	return vdev->queue->owner && vdev->queue->owner != file->private_data;
-}
-
 /* vb2 ioctl helpers */
 
 int vb2_ioctl_reqbufs(struct file *file, void *priv,
@@ -997,7 +991,7 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
 	p->flags = flags;
 	if (res)
 		return res;
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 	res = vb2_core_reqbufs(vdev->queue, p->memory, p->flags, &p->count);
 	/* If count == 0, then the owner has released all buffers and he
@@ -1026,7 +1020,7 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
 		return res != -EBUSY ? res : 0;
 	if (res)
 		return res;
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 
 	res = vb2_create_bufs(vdev->queue, p);
@@ -1041,7 +1035,7 @@ int vb2_ioctl_prepare_buf(struct file *file, void *priv,
 {
 	struct video_device *vdev = video_devdata(file);
 
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 	return vb2_prepare_buf(vdev->queue, vdev->v4l2_dev->mdev, p);
 }
@@ -1060,7 +1054,7 @@ int vb2_ioctl_qbuf(struct file *file, void *priv, struct v4l2_buffer *p)
 {
 	struct video_device *vdev = video_devdata(file);
 
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 	return vb2_qbuf(vdev->queue, vdev->v4l2_dev->mdev, p);
 }
@@ -1070,7 +1064,7 @@ int vb2_ioctl_dqbuf(struct file *file, void *priv, struct v4l2_buffer *p)
 {
 	struct video_device *vdev = video_devdata(file);
 
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 	return vb2_dqbuf(vdev->queue, p, file->f_flags & O_NONBLOCK);
 }
@@ -1080,7 +1074,7 @@ int vb2_ioctl_streamon(struct file *file, void *priv, enum v4l2_buf_type i)
 {
 	struct video_device *vdev = video_devdata(file);
 
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 	return vb2_streamon(vdev->queue, i);
 }
@@ -1090,7 +1084,7 @@ int vb2_ioctl_streamoff(struct file *file, void *priv, enum v4l2_buf_type i)
 {
 	struct video_device *vdev = video_devdata(file);
 
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 	return vb2_streamoff(vdev->queue, i);
 }
@@ -1100,7 +1094,7 @@ int vb2_ioctl_expbuf(struct file *file, void *priv, struct v4l2_exportbuffer *p)
 {
 	struct video_device *vdev = video_devdata(file);
 
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		return -EBUSY;
 	return vb2_expbuf(vdev->queue, p);
 }
@@ -1152,7 +1146,7 @@ ssize_t vb2_fop_write(struct file *file, const char __user *buf,
 		return -EINVAL;
 	if (lock && mutex_lock_interruptible(lock))
 		return -ERESTARTSYS;
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		goto exit;
 	err = vb2_write(vdev->queue, buf, count, ppos,
 		       file->f_flags & O_NONBLOCK);
@@ -1176,7 +1170,7 @@ ssize_t vb2_fop_read(struct file *file, char __user *buf,
 		return -EINVAL;
 	if (lock && mutex_lock_interruptible(lock))
 		return -ERESTARTSYS;
-	if (vb2_queue_is_busy(vdev, file))
+	if (vb2_queue_is_busy(vdev->queue, file))
 		goto exit;
 	err = vb2_read(vdev->queue, buf, count, ppos,
 		       file->f_flags & O_NONBLOCK);
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index b66585e304e2..1fb99583cd45 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -302,10 +302,29 @@ __poll_t vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait);
  * The following functions are not part of the vb2 core API, but are simple
  * helper functions that you can use in your struct v4l2_file_operations,
  * struct v4l2_ioctl_ops and struct vb2_ops. They will serialize if vb2_queue->lock
- * or video_device->lock is set, and they will set and test vb2_queue->owner
- * to check if the calling filehandle is permitted to do the queuing operation.
+ * or video_device->lock is set, and they will set and test the queue owner
+ * (vb2_queue->owner) to check if the calling filehandle is permitted to do the
+ * queuing operation.
  */
 
+/**
+ * vb2_queue_is_busy() - check if the queue is busy
+ * @q:		pointer to &struct vb2_queue with videobuf2 queue.
+ * @file:	file through which the vb2 queue access is performed
+ *
+ * The queue is considered busy if is has an owner and the owner is not the
+ * @file.
+ *
+ * Queue ownership is acquired and checked by some of the v4l2_ioctl_ops helpers
+ * below. Drivers can also use this function directly when they need to
+ * open-code ioctl handlers, for instance to add additional checks between the
+ * queue ownership test and the call to the corresponding vb2 operation.
+ */
+static inline bool vb2_queue_is_busy(struct vb2_queue *q, struct file *file)
+{
+	return q->owner && q->owner != file->private_data;
+}
+
 /* struct v4l2_ioctl_ops helpers */
 
 int vb2_ioctl_reqbufs(struct file *file, void *priv,
-- 
Regards,

Laurent Pinchart

[PATCH 2/3] media: vsp1: Don't open-code vb2_fop_release()

[Laurent Pinchart]

Use the vb2_fop_release() helper to replace the open-coded version. The
video->lock is assigned to the queue lock, used by vb2_fop_release(), so
the only functional difference is that v4l2_fh_release() is now called
before vsp1_device_put(). This should be harmless.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_video.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 044eb5778820..8f53abc71db2 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -1129,19 +1129,11 @@ static int vsp1_video_open(struct file *file)
 static int vsp1_video_release(struct file *file)
 {
 	struct vsp1_video *video = video_drvdata(file);
-	struct v4l2_fh *vfh = file->private_data;
 
-	mutex_lock(&video->lock);
-	if (video->queue.owner == vfh) {
-		vb2_queue_release(&video->queue);
-		video->queue.owner = NULL;
-	}
-	mutex_unlock(&video->lock);
+	vb2_fop_release(file);
 
 	vsp1_device_put(video->vsp1);
 
-	v4l2_fh_release(file);
-
 	file->private_data = NULL;
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 3/3] media: vsp1: Use vb2_queue_is_busy()

[Laurent Pinchart]

Use the new vb2_queue_is_busy() helper to replace the open-coded
version.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/vsp1/vsp1_video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 8f53abc71db2..4da70b2b0869 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -1032,7 +1032,7 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 	struct vsp1_pipeline *pipe;
 	int ret;
 
-	if (video->queue.owner && video->queue.owner != file->private_data)
+	if (vb2_queue_is_busy(&video->queue, file))
 		return -EBUSY;
 
 	/*
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] media: videobuf2-v4l2: Expose vb2_queue_is_busy() to drivers

[Kieran Bingham]

Hi Laurent,

Quoting Laurent Pinchart (2022-03-18 21:14:44)
> vb2 queue ownership is managed by the ioctl handler helpers
> (vb2_ioctl_*). There are however use cases where drivers can benefit
> from checking queue ownership, for instance when open-coding an ioctl
> handler that needs to perform additional checks before calling the
> corresponding vb2 operation.
> 
> Expose the vb2_queue_is_busy() function in the videobuf2-v4l2.h header,
> and change its first argument to a struct vb2_queue pointer as the
> function name implies it operates on a queue, not a video_device.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../media/common/videobuf2/videobuf2-v4l2.c   | 26 +++++++------------
>  include/media/videobuf2-v4l2.h                | 23 ++++++++++++++--
>  2 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 6edf4508c636..075d24ebf44c 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -977,12 +977,6 @@ EXPORT_SYMBOL_GPL(vb2_poll);
>   * and so they simplify the driver code.
>   */
>  
> -/* The queue is busy if there is a owner and you are not that owner. */
> -static inline bool vb2_queue_is_busy(struct video_device *vdev, struct file *file)
> -{
> -       return vdev->queue->owner && vdev->queue->owner != file->private_data;
> -}
> -
>  /* vb2 ioctl helpers */
>  
>  int vb2_ioctl_reqbufs(struct file *file, void *priv,
> @@ -997,7 +991,7 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
>         p->flags = flags;
>         if (res)
>                 return res;
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>         res = vb2_core_reqbufs(vdev->queue, p->memory, p->flags, &p->count);
>         /* If count == 0, then the owner has released all buffers and he
> @@ -1026,7 +1020,7 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
>                 return res != -EBUSY ? res : 0;
>         if (res)
>                 return res;
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>  
>         res = vb2_create_bufs(vdev->queue, p);
> @@ -1041,7 +1035,7 @@ int vb2_ioctl_prepare_buf(struct file *file, void *priv,
>  {
>         struct video_device *vdev = video_devdata(file);
>  
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>         return vb2_prepare_buf(vdev->queue, vdev->v4l2_dev->mdev, p);
>  }
> @@ -1060,7 +1054,7 @@ int vb2_ioctl_qbuf(struct file *file, void *priv, struct v4l2_buffer *p)
>  {
>         struct video_device *vdev = video_devdata(file);
>  
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>         return vb2_qbuf(vdev->queue, vdev->v4l2_dev->mdev, p);
>  }
> @@ -1070,7 +1064,7 @@ int vb2_ioctl_dqbuf(struct file *file, void *priv, struct v4l2_buffer *p)
>  {
>         struct video_device *vdev = video_devdata(file);
>  
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>         return vb2_dqbuf(vdev->queue, p, file->f_flags & O_NONBLOCK);
>  }
> @@ -1080,7 +1074,7 @@ int vb2_ioctl_streamon(struct file *file, void *priv, enum v4l2_buf_type i)
>  {
>         struct video_device *vdev = video_devdata(file);
>  
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>         return vb2_streamon(vdev->queue, i);
>  }
> @@ -1090,7 +1084,7 @@ int vb2_ioctl_streamoff(struct file *file, void *priv, enum v4l2_buf_type i)
>  {
>         struct video_device *vdev = video_devdata(file);
>  
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>         return vb2_streamoff(vdev->queue, i);
>  }
> @@ -1100,7 +1094,7 @@ int vb2_ioctl_expbuf(struct file *file, void *priv, struct v4l2_exportbuffer *p)
>  {
>         struct video_device *vdev = video_devdata(file);
>  
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 return -EBUSY;
>         return vb2_expbuf(vdev->queue, p);
>  }
> @@ -1152,7 +1146,7 @@ ssize_t vb2_fop_write(struct file *file, const char __user *buf,
>                 return -EINVAL;
>         if (lock && mutex_lock_interruptible(lock))
>                 return -ERESTARTSYS;
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 goto exit;
>         err = vb2_write(vdev->queue, buf, count, ppos,
>                        file->f_flags & O_NONBLOCK);
> @@ -1176,7 +1170,7 @@ ssize_t vb2_fop_read(struct file *file, char __user *buf,
>                 return -EINVAL;
>         if (lock && mutex_lock_interruptible(lock))
>                 return -ERESTARTSYS;
> -       if (vb2_queue_is_busy(vdev, file))
> +       if (vb2_queue_is_busy(vdev->queue, file))
>                 goto exit;
>         err = vb2_read(vdev->queue, buf, count, ppos,
>                        file->f_flags & O_NONBLOCK);
> diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
> index b66585e304e2..1fb99583cd45 100644
> --- a/include/media/videobuf2-v4l2.h
> +++ b/include/media/videobuf2-v4l2.h
> @@ -302,10 +302,29 @@ __poll_t vb2_poll(struct vb2_queue *q, struct file *file, poll_table *wait);
>   * The following functions are not part of the vb2 core API, but are simple
>   * helper functions that you can use in your struct v4l2_file_operations,
>   * struct v4l2_ioctl_ops and struct vb2_ops. They will serialize if vb2_queue->lock
> - * or video_device->lock is set, and they will set and test vb2_queue->owner
> - * to check if the calling filehandle is permitted to do the queuing operation.
> + * or video_device->lock is set, and they will set and test the queue owner
> + * (vb2_queue->owner) to check if the calling filehandle is permitted to do the
> + * queuing operation.
>   */
>  
> +/**
> + * vb2_queue_is_busy() - check if the queue is busy
> + * @q:         pointer to &struct vb2_queue with videobuf2 queue.
> + * @file:      file through which the vb2 queue access is performed
> + *
> + * The queue is considered busy if is has an owner and the owner is not the

 if it has

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> + * @file.
> + *
> + * Queue ownership is acquired and checked by some of the v4l2_ioctl_ops helpers
> + * below. Drivers can also use this function directly when they need to
> + * open-code ioctl handlers, for instance to add additional checks between the
> + * queue ownership test and the call to the corresponding vb2 operation.
> + */
> +static inline bool vb2_queue_is_busy(struct vb2_queue *q, struct file *file)
> +{
> +       return q->owner && q->owner != file->private_data;
> +}
> +
>  /* struct v4l2_ioctl_ops helpers */
>  
>  int vb2_ioctl_reqbufs(struct file *file, void *priv,
> -- 
> Regards,
> 
> Laurent Pinchart
>

Re: [PATCH 3/3] media: vsp1: Use vb2_queue_is_busy()

[Kieran Bingham]

Quoting Laurent Pinchart (2022-03-18 21:14:46)
> Use the new vb2_queue_is_busy() helper to replace the open-coded
> version.
> 

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> index 8f53abc71db2..4da70b2b0869 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> @@ -1032,7 +1032,7 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>         struct vsp1_pipeline *pipe;
>         int ret;
>  
> -       if (video->queue.owner && video->queue.owner != file->private_data)
> +       if (vb2_queue_is_busy(&video->queue, file))
>                 return -EBUSY;
>  
>         /*
> -- 
> Regards,
> 
> Laurent Pinchart
>

Re: [PATCH 2/3] media: vsp1: Don't open-code vb2_fop_release()

[Kieran Bingham]

Quoting Laurent Pinchart (2022-03-18 21:14:45)
> Use the vb2_fop_release() helper to replace the open-coded version. The
> video->lock is assigned to the queue lock, used by vb2_fop_release(), so
> the only functional difference is that v4l2_fh_release() is now called
> before vsp1_device_put(). This should be harmless.
> 

Seems to check out.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_video.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> index 044eb5778820..8f53abc71db2 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> @@ -1129,19 +1129,11 @@ static int vsp1_video_open(struct file *file)
>  static int vsp1_video_release(struct file *file)
>  {
>         struct vsp1_video *video = video_drvdata(file);
> -       struct v4l2_fh *vfh = file->private_data;
>  
> -       mutex_lock(&video->lock);
> -       if (video->queue.owner == vfh) {
> -               vb2_queue_release(&video->queue);
> -               video->queue.owner = NULL;
> -       }
> -       mutex_unlock(&video->lock);
> +       vb2_fop_release(file);
>  
>         vsp1_device_put(video->vsp1);
>  
> -       v4l2_fh_release(file);
> -
>         file->private_data = NULL;
>  
>         return 0;
> -- 
> Regards,
> 
> Laurent Pinchart
>

[PATCH v2 2/3] media: vsp1: Don't open-code vb2_fop_release()

[Laurent Pinchart]

Use the vb2_fop_release() helper to replace the open-coded version. The
video->lock is assigned to the queue lock, used by vb2_fop_release(), so
the only functional difference is that v4l2_fh_release() is now called
before vsp1_device_put(). This should be harmless.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
Changes since v1:

- Remove redundant clear of file->private_data
---
 drivers/media/platform/renesas/vsp1/vsp1_video.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 497f352e9f8c..0180ffce35f7 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -1127,21 +1127,11 @@ static int vsp1_video_open(struct file *file)
 static int vsp1_video_release(struct file *file)
 {
 	struct vsp1_video *video = video_drvdata(file);
-	struct v4l2_fh *vfh = file->private_data;
 
-	mutex_lock(&video->lock);
-	if (video->queue.owner == vfh) {
-		vb2_queue_release(&video->queue);
-		video->queue.owner = NULL;
-	}
-	mutex_unlock(&video->lock);
+	vb2_fop_release(file);
 
 	vsp1_device_put(video->vsp1);
 
-	v4l2_fh_release(file);
-
-	file->private_data = NULL;
-
 	return 0;
 }
 
-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] media: vsp1: Use vb2_queue_is_busy()

[Tomasz Figa]

Hi Laurent,

On Fri, Mar 18, 2022 at 11:14:46PM +0200, Laurent Pinchart wrote:
> Use the new vb2_queue_is_busy() helper to replace the open-coded
> version.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_video.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> index 8f53abc71db2..4da70b2b0869 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
> @@ -1032,7 +1032,7 @@ vsp1_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
>  	struct vsp1_pipeline *pipe;
>  	int ret;
>  
> -	if (video->queue.owner && video->queue.owner != file->private_data)
> +	if (vb2_queue_is_busy(&video->queue, file))
>  		return -EBUSY;
>  
>  	/*

Thanks for the patch and really sorry for the long delay. Finally
catching up with my backlog.

An alternative would be to have all the stream start code placed under
the vb2 start_streaming callback, symmetrically to
what the driver already does with streamoff/stop_streaming. That would
eliminate the need to export the symbol from the vb2 framework.

Have you considered that option?

Best regards,
Tomasz