Skip to contentSkip to conversation

media: clean up unnecessary vb2_queue_release calls

PATCH Superseded Hans Verkuil 6 participants 28 messages Jul 17 – Jul 27

Subsystems (22)
Media (V4L/DVB) Staging Arm i.MX and Layerscape Broadcom BCM2711/BCM2835 Arm architecture +18 more subsystems

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Hans Verkuil

author
#0 lore raw

cover media: clean up unnecessary vb2_queue_release calls

Many drivers call vb2_queue_release() unnecessarily. There is normally no reason for drivers to call this function directly, instead call (_)vb2_fop_release(), vb2_video_unregister_device() or v4l2_m2m_ctx_release().

This series removes the vb2_queue_release() calls from most drivers.

There are still a few old drivers that call it, but let's get the majority fixed.

Regards,

Hans

Hans Verkuil (19):
media: videobuf2-v4l2.h: improve vb2_queue_release() documentation
media: pci: hws: drop call to vb2_queue_release
media: pci: ipu6: drop calls to vb2_video_unregister_device and
vb2_queue_release
media: pci: zoran: use vb2_video_unregister_device
media: platform: amd: isp4: drop vb2_queue_release calls
media: amlogic: c3: isp: drop vb2_queue_release calls
media: amphion: drop unnecessary vb2_queue_release call
media: arm: mali-c55: drop unnecessary vb2_queue_release calls
media: imagination: e5010-jpeg-enc: drop vb2_queue_release
media: nuvoton: drop unnecessary vb2_queue_release calls
media: nxp: imx7-media-csi: use
_vb2_fop_release/vb2_video_unregister_device
media: raspberrypi: pisp_be: use vb2_video_unregister_device
media: renesas: rzv2h-cru: use vb2_video_unregister_device
media: renesas: rzv2h-ivc: drop unnecessary vb2_queue_release calls
media: samsung: s3c-camif: drop unnecessary vb2_queue_release
media: ti: j721e-csi2rx: use vb2_video_unregister_device
staging: media: imx: drop vb2_queue_release call
staging: media: ipu7: drop unnecessary cleanup calls
media: renesas: rzg2l-cru: drop unnecessary vb2_queue_release

 drivers/media/pci/hws/hws_video.c                  |  5 -----
 drivers/media/pci/intel/ipu6/ipu6-isys-video.c     |  6 +-----
 drivers/media/pci/zoran/zoran.h                    |  1 -
 drivers/media/pci/zoran/zoran_card.c               |  4 +---
 drivers/media/pci/zoran/zoran_driver.c             |  5 -----
 drivers/media/platform/amd/isp4/isp4_video.c       |  8 +++-----
 .../media/platform/amlogic/c3/isp/c3-isp-capture.c |  7 ++-----
 .../media/platform/amlogic/c3/isp/c3-isp-params.c  | 11 ++++-------
 .../media/platform/amlogic/c3/isp/c3-isp-stats.c   |  7 ++-----
 drivers/media/platform/amphion/vpu_v4l2.c          |  8 +-------
 .../media/platform/arm/mali-c55/mali-c55-capture.c |  4 +---
 .../media/platform/arm/mali-c55/mali-c55-params.c  |  4 +---
 .../media/platform/arm/mali-c55/mali-c55-stats.c   |  4 +---
 .../media/platform/imagination/e5010-jpeg-enc.c    |  8 +-------
 drivers/media/platform/nuvoton/npcm-video.c        |  7 ++-----
 drivers/media/platform/nxp/imx7-media-csi.c        | 10 +++-------
 .../media/platform/raspberrypi/pisp_be/pisp_be.c   | 14 +++++---------
 .../media/platform/renesas/rzg2l-cru/rzg2l-video.c |  5 ++---
 .../platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c   |  7 ++-----
 .../platform/samsung/s3c-camif/camif-capture.c     |  7 +------
 .../media/platform/ti/j721e-csi2rx/j721e-csi2rx.c  |  4 +---
 drivers/staging/media/imx/imx-media-capture.c      | 10 +++-------
 drivers/staging/media/ipu7/ipu7-isys-video.c       |  6 +-----
 include/media/videobuf2-v4l2.h                     |  4 ++++
 24 files changed, 42 insertions(+), 114 deletions(-)
Signature · 1 line
2.53.0
author
#1 lore raw

PATCH 1/19 media: videobuf2-v4l2.h: improve vb2_queue_release() documentation

Drivers should normally never call vb2_queue_release() directly. Document this.

Signed-off-by Hans Verkuil <[email protected]>
---
 include/media/videobuf2-v4l2.h | 4 ++++
 1 file changed, 4 insertions(+)
include/media/videobuf2-v4l2.h +4 -0
index d51e10c10fc0..bbdb171df969 100644
@@ -265,6 +265,10 @@ int __must_check vb2_queue_init_name(struct vb2_queue *q, const char *name);
  * This function stops streaming and performs necessary clean ups, including
  * freeing video buffer memory. The driver is responsible for freeing
  * the vb2_queue structure itself.
+ *
+ * There is normally no reason for drivers to call this function directly,
+ * instead call (_)vb2_fop_release(), vb2_video_unregister_device() or
+ * v4l2_m2m_ctx_release().
  */
 void vb2_queue_release(struct vb2_queue *q);
Signature · 1 line
2.53.0
author
#2 lore raw

PATCH 2/19 media: pci: hws: drop call to vb2_queue_release

In reply to Hans Verkuil

The call to vb2_video_unregister_device() takes care of that, no need to call vb2_queue_release directly.

Signed-off-by Hans Verkuil <[email protected]>
Cc Ben Hoff <[email protected]>
---
 drivers/media/pci/hws/hws_video.c | 5 -----
 1 file changed, 5 deletions(-)
drivers/media/pci/hws/hws_video.c +0 -5
index 18e4bc6901d3..27f12bbec00a 100644
@@ -384,11 +384,6 @@ static void hws_video_drain_queue_locked(struct hws_video *vid)
 
 static void hws_video_release_registration(struct hws_video *vid)
 {
-	if (vid->buffer_queue.ops) {
-		vb2_queue_release(&vid->buffer_queue);
-		vid->buffer_queue.ops = NULL;
-	}
-
 	if (!vid->video_device)
 		return;
Signature · 1 line
2.53.0
maintainer Reviewed-by on v1 2/19
#3 lore raw

Re: [PATCH 02/19] media: pci: hws: drop call to vb2_queue_release

Looks good to me.

Reviewed-by Ben Hoff <[email protected]>
Quoted 29 lines · Hans Verkuil

The call to vb2_video_unregister_device() takes care of that, no need to call vb2_queue_release directly.

Signed-off-by Hans Verkuil <[email protected]>
Cc Ben Hoff <[email protected]>
---
 drivers/media/pci/hws/hws_video.c | 5 -----
 1 file changed, 5 deletions(-)
drivers/media/pci/hws/hws_video.c +0 -6
index 18e4bc6901d3..27f12bbec00a 100644
@@ -384,11 +384,6 @@ static void hws_video_drain_queue_locked(struct hws_video *vid)
 
 static void hws_video_release_registration(struct hws_video *vid)
 {
-       if (vid->buffer_queue.ops) {
-               vb2_queue_release(&vid->buffer_queue);
-               vid->buffer_queue.ops = NULL;
-       }
-
        if (!vid->video_device)
                return;
 
--

2.53.0

author
#4 lore raw

PATCH 3/19 media: pci: ipu6: drop calls to vb2_video_unregister_device and vb2_queue_release

In reply to Hans Verkuil

Both the calls to vb2_video_unregister_device and vb2_queue_release are not needed: drop these. The video device was never registered, so there is also no need for any cleanup.

Signed-off-by Hans Verkuil <[email protected]>
Cc Sakari Ailus <[email protected]>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/media/pci/intel/ipu6/ipu6-isys-video.c +1 -5
index 3ac48d2076da..b8dab3a29ab0 100644
@@ -1267,7 +1267,7 @@ int ipu6_isys_video_init(struct ipu6_isys_video *av)
 	av->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
 	ret = media_entity_pads_init(&av->vdev.entity, 1, &av->pad);
 	if (ret)
-		goto out_vb2_queue_release;
+		goto out_free_watermark;
 
 	av->vdev.entity.ops = &entity_ops;
 	av->vdev.release = video_device_release_empty;

@@ -1293,12 +1293,8 @@ int ipu6_isys_video_init(struct ipu6_isys_video *av)
 	return ret;
 
 out_media_entity_cleanup:
-	vb2_video_unregister_device(&av->vdev);
 	media_entity_cleanup(&av->vdev.entity);
 
-out_vb2_queue_release:
-	vb2_queue_release(&av->aq.vbq);
-
 out_free_watermark:
 	mutex_destroy(&av->mutex);
Signature · 1 line
2.53.0
maintainer Acked-by on v1 3/19
#5 lore raw

Re: [PATCH 03/19] media: pci: ipu6: drop calls to vb2_video_unregister_device and vb2_queue_release

Quoted 6 lines · Hans Verkuil

Both the calls to vb2_video_unregister_device and vb2_queue_release are not needed: drop these. The video device was never registered, so there is also no need for any cleanup.

Signed-off-by Hans Verkuil <[email protected]>
Cc Sakari Ailus <[email protected]>
Acked-by Sakari Ailus <[email protected]>
Signature · 1 line
Sakari Ailus
author
#6 lore raw

PATCH 4/19 media: pci: zoran: use vb2_video_unregister_device

In reply to Hans Verkuil

Drop call to vb2_queue_release, use vb2_video_unregister_device instead.

Signed-off-by Hans Verkuil <[email protected]>
---
 drivers/media/pci/zoran/zoran.h        | 1 -
 drivers/media/pci/zoran/zoran_card.c   | 4 +---
 drivers/media/pci/zoran/zoran_driver.c | 5 -----
 3 files changed, 1 insertion(+), 9 deletions(-)
drivers/media/pci/zoran/zoran.h +0 -1
index d05e222b3921..da2bb01f84d5 100644
@@ -318,5 +318,4 @@ static inline struct zoran *to_zoran(struct v4l2_device *v4l2_dev)
 	pci_info((zr)->pci_dev, format, ##args) \
 
 int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq, int dir);
-void zoran_queue_exit(struct zoran *zr);
 int zr_set_buf(struct zoran *zr);
drivers/media/pci/zoran/zoran_card.c +1 -3
index 8bcf355a5418..5f6f550904dc 100644
@@ -885,7 +885,7 @@ static int zoran_init_video_device(struct zoran *zr, struct video_device *video_
 
 static void zoran_exit_video_devices(struct zoran *zr)
 {
-	video_unregister_device(zr->video_dev);
+	vb2_video_unregister_device(zr->video_dev);
 	kfree(zr->video_dev);
 }
 

@@ -1072,8 +1072,6 @@ static void zoran_remove(struct pci_dev *pdev)
 
 	debugfs_remove_recursive(zr->dbgfs_dir);
 
-	zoran_queue_exit(zr);
-
 	/* unregister videocodec bus */
 	if (zr->codec)
 		videocodec_detach(zr->codec);
drivers/media/pci/zoran/zoran_driver.c +0 -5
index 5b4d5dd06edb..2e7e3fbe490c 100644
@@ -976,8 +976,3 @@ int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq, int dir)
 	zr->video_dev->queue = vq;
 	return 0;
 }
-
-void zoran_queue_exit(struct zoran *zr)
-{
-	vb2_queue_release(zr->video_dev->queue);
-}
Signature · 1 line
2.53.0
author
#7 lore raw

PATCH 5/19 media: platform: amd: isp4: drop vb2_queue_release calls

In reply to Hans Verkuil

These calls are not needed since the video device was never registered.

Signed-off-by Hans Verkuil <[email protected]>
Cc Bin Du <[email protected]>
Cc Nirujogi Pratap <[email protected]>
Cc Laurent Pinchart <[email protected]>
---
 drivers/media/platform/amd/isp4/isp4_video.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/media/platform/amd/isp4/isp4_video.c +3 -5
index 0cebb39f98e1..0e702add6144 100644
@@ -742,13 +742,13 @@ int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd)
 	ret = isp4vid_fill_buffer_size(&isp_vdev->format);
 	if (ret) {
 		dev_err(v4l2_dev->dev, "fail to fill buffer size: %d\n", ret);
-		goto err_release_vb2_queue;
+		return ret;
 	}
 
 	ret = isp4vid_set_fmt_2_isp(isp_sd, &isp_vdev->format);
 	if (ret) {
 		dev_err(v4l2_dev->dev, "fail init format :%d\n", ret);
-		goto err_release_vb2_queue;
+		return ret;
 	}
 
 	/* Initialize the video_device struct */

@@ -760,7 +760,7 @@ int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd)
 
 	if (ret) {
 		dev_err(v4l2_dev->dev, "init media entity pad fail:%d\n", ret);
-		goto err_release_vb2_queue;
+		return ret;
 	}
 
 	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE |

@@ -786,8 +786,6 @@ int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd)
 
 err_entity_cleanup:
 	media_entity_cleanup(&isp_vdev->vdev.entity);
-err_release_vb2_queue:
-	vb2_queue_release(q);
 	return ret;
 }
Signature · 1 line
2.53.0
maintainer Acked-by on v1 5/19
#8 lore raw

Re: [PATCH 05/19] media: platform: amd: isp4: drop vb2_queue_release calls

Quoted 5 lines · Hans Verkuil

[You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

These calls are not needed since the video device was never registered.

Would it be better to say:

"Drop the unnecessary vb2_queue_release() calls from failure paths before the video device is registered."

Quoted 47 lines
Signed-off-by Hans Verkuil <[email protected]>
Cc Bin Du <[email protected]>
Cc Nirujogi Pratap <[email protected]>
Cc Laurent Pinchart <[email protected]>
---
 drivers/media/platform/amd/isp4/isp4_video.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/media/platform/amd/isp4/isp4_video.c +3 -6
index 0cebb39f98e1..0e702add6144 100644
@@ -742,13 +742,13 @@ int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd)
        ret = isp4vid_fill_buffer_size(&isp_vdev->format);
        if (ret) {
                dev_err(v4l2_dev->dev, "fail to fill buffer size: %d\n", ret);
-               goto err_release_vb2_queue;
+               return ret;
        }
 
        ret = isp4vid_set_fmt_2_isp(isp_sd, &isp_vdev->format);
        if (ret) {
                dev_err(v4l2_dev->dev, "fail init format :%d\n", ret);
-               goto err_release_vb2_queue;
+               return ret;
        }
 
        /* Initialize the video_device struct */

@@ -760,7 +760,7 @@ int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd)
 
        if (ret) {
                dev_err(v4l2_dev->dev, "init media entity pad fail:%d\n", ret);
-               goto err_release_vb2_queue;
+               return ret;
        }
 
        vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE |

@@ -786,8 +786,6 @@ int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd)
 
 err_entity_cleanup:
        media_entity_cleanup(&isp_vdev->vdev.entity);
-err_release_vb2_queue:
-       vb2_queue_release(q);
        return ret;
 }
 
--
Quoted 2 lines

2.53.0

Acked-by Bin Du <[email protected]>
author
#9 lore raw

PATCH 6/19 media: amlogic: c3: isp: drop vb2_queue_release calls

In reply to Hans Verkuil

Either that function was called when the video device was not yet registered (so nothing to release), or use vb2_video_unregister_device instead of the video_unregister_device/vb2_queue_release pair.

Signed-off-by Hans Verkuil <[email protected]>
Cc Keke Li <[email protected]>
Cc Laurent Pinchart <[email protected]>
---
 .../media/platform/amlogic/c3/isp/c3-isp-capture.c    |  7 ++-----
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 11 ++++-------
 drivers/media/platform/amlogic/c3/isp/c3-isp-stats.c  |  7 ++-----
 3 files changed, 8 insertions(+), 17 deletions(-)
drivers/media/platform/amlogic/c3/isp/c3-isp-capture.c +2 -5
index 11d85f5342f0..a0d92ac1b78c 100644
@@ -720,7 +720,7 @@ static int c3_isp_register_capture(struct c3_isp_capture *cap)
 	cap->pad.flags = MEDIA_PAD_FL_SINK;
 	ret = media_entity_pads_init(&vdev->entity, 1, &cap->pad);
 	if (ret)
-		goto err_queue_release;
+		goto err_destroy;
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
 	if (ret) {

@@ -733,8 +733,6 @@ static int c3_isp_register_capture(struct c3_isp_capture *cap)
 
 err_entity_cleanup:
 	media_entity_cleanup(&vdev->entity);
-err_queue_release:
-	vb2_queue_release(vb2_q);
 err_destroy:
 	mutex_destroy(&cap->lock);
 	return ret;

@@ -789,9 +787,8 @@ void c3_isp_captures_unregister(struct c3_isp_device *isp)
 
 		if (!cap->isp)
 			continue;
-		vb2_queue_release(&cap->vb2_q);
 		media_entity_cleanup(&cap->vdev.entity);
-		video_unregister_device(&cap->vdev);
+		vb2_video_unregister_device(&cap->vdev);
 		mutex_destroy(&cap->lock);
 	}
 }
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c +4 -7
index aec3eed0e443..782f18c90995 100644
@@ -876,12 +876,12 @@ int c3_isp_params_register(struct c3_isp_device *isp)
 
 	ret = vb2_queue_init(vb2_q);
 	if (ret)
-		goto err_detroy;
+		goto err_destroy;
 
 	params->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&vdev->entity, 1, &params->pad);
 	if (ret)
-		goto err_queue_release;
+		goto err_destroy;
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
 	if (ret < 0) {

@@ -894,9 +894,7 @@ int c3_isp_params_register(struct c3_isp_device *isp)
 
 err_entity_cleanup:
 	media_entity_cleanup(&vdev->entity);
-err_queue_release:
-	vb2_queue_release(vb2_q);
-err_detroy:
+err_destroy:
 	mutex_destroy(&params->lock);
 	return ret;
 }

@@ -905,9 +903,8 @@ void c3_isp_params_unregister(struct c3_isp_device *isp)
 {
 	struct c3_isp_params *params = &isp->params;
 
-	vb2_queue_release(&params->vb2_q);
 	media_entity_cleanup(&params->vdev.entity);
-	video_unregister_device(&params->vdev);
+	vb2_video_unregister_device(&params->vdev);
 	mutex_destroy(&params->lock);
 }
drivers/media/platform/amlogic/c3/isp/c3-isp-stats.c +2 -5
index 8a5d7e1a30c9..78722cbe44b4 100644
@@ -279,7 +279,7 @@ int c3_isp_stats_register(struct c3_isp_device *isp)
 	stats->pad.flags = MEDIA_PAD_FL_SINK;
 	ret = media_entity_pads_init(&vdev->entity, 1, &stats->pad);
 	if (ret)
-		goto err_queue_release;
+		goto err_destroy;
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
 	if (ret) {

@@ -292,8 +292,6 @@ int c3_isp_stats_register(struct c3_isp_device *isp)
 
 err_entity_cleanup:
 	media_entity_cleanup(&vdev->entity);
-err_queue_release:
-	vb2_queue_release(vb2_q);
 err_destroy:
 	mutex_destroy(&stats->lock);
 	return ret;

@@ -303,9 +301,8 @@ void c3_isp_stats_unregister(struct c3_isp_device *isp)
 {
 	struct c3_isp_stats *stats = &isp->stats;
 
-	vb2_queue_release(&stats->vb2_q);
 	media_entity_cleanup(&stats->vdev.entity);
-	video_unregister_device(&stats->vdev);
+	vb2_video_unregister_device(&stats->vdev);
 	mutex_destroy(&stats->lock);
 }
Signature · 1 line
2.53.0
maintainer
#10 lore raw

Re: [PATCH 06/19] media: amlogic: c3: isp: drop vb2_queue_release calls

Hi Hans,

Thank you for the patch.

Quoted 36 lines · Hans Verkuil

Either that function was called when the video device was not yet registered (so nothing to release), or use vb2_video_unregister_device instead of the video_unregister_device/vb2_queue_release pair.

Signed-off-by Hans Verkuil <[email protected]>
Cc Keke Li <[email protected]>
Cc Laurent Pinchart <[email protected]>
---
 .../media/platform/amlogic/c3/isp/c3-isp-capture.c    |  7 ++-----
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 11 ++++-------
 drivers/media/platform/amlogic/c3/isp/c3-isp-stats.c  |  7 ++-----
 3 files changed, 8 insertions(+), 17 deletions(-)
drivers/media/platform/amlogic/c3/isp/c3-isp-capture.c +1 -3
index 11d85f5342f0..a0d92ac1b78c 100644
@@ -720,7 +720,7 @@ static int c3_isp_register_capture(struct c3_isp_capture *cap)
 	cap->pad.flags = MEDIA_PAD_FL_SINK;
 	ret = media_entity_pads_init(&vdev->entity, 1, &cap->pad);
 	if (ret)
-		goto err_queue_release;
+		goto err_destroy;
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
 	if (ret) {

@@ -733,8 +733,6 @@ static int c3_isp_register_capture(struct c3_isp_capture *cap)
 
 err_entity_cleanup:
 	media_entity_cleanup(&vdev->entity);
-err_queue_release:
-	vb2_queue_release(vb2_q);
 err_destroy:
 	mutex_destroy(&cap->lock);
 	return ret;

drivers/media/platform/amlogic/c3/isp/c3-isp-capture.c void c3_isp_captures_unregister(struct c3_isp_device *isp) · line 791

Quoted 7 lines
drivers/media/platform/amlogic/c3/isp/c3-isp-capture.c +1 -2
@@ -789,9 +787,8 @@ void c3_isp_captures_unregister(struct c3_isp_device *isp)
 
 		if (!cap->isp)
 			continue;
-		vb2_queue_release(&cap->vb2_q);
 		media_entity_cleanup(&cap->vdev.entity);
-		video_unregister_device(&cap->vdev);
+		vb2_video_unregister_device(&cap->vdev);

Is it safe to change the order of operations here, can't driver code called from queue release access the media entity ?

Same below.

Quoted 77 lines
drivers/media/platform/amlogic/c3/isp/c3-isp-capture.c +0 -0
 		mutex_destroy(&cap->lock);
 	}
 }
drivers/media/platform/amlogic/c3/isp/c3-isp-params.c +4 -7
index aec3eed0e443..782f18c90995 100644
@@ -876,12 +876,12 @@ int c3_isp_params_register(struct c3_isp_device *isp)
 
 	ret = vb2_queue_init(vb2_q);
 	if (ret)
-		goto err_detroy;
+		goto err_destroy;
 
 	params->pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&vdev->entity, 1, &params->pad);
 	if (ret)
-		goto err_queue_release;
+		goto err_destroy;
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
 	if (ret < 0) {

@@ -894,9 +894,7 @@ int c3_isp_params_register(struct c3_isp_device *isp)
 
 err_entity_cleanup:
 	media_entity_cleanup(&vdev->entity);
-err_queue_release:
-	vb2_queue_release(vb2_q);
-err_detroy:
+err_destroy:
 	mutex_destroy(&params->lock);
 	return ret;
 }

@@ -905,9 +903,8 @@ void c3_isp_params_unregister(struct c3_isp_device *isp)
 {
 	struct c3_isp_params *params = &isp->params;
 
-	vb2_queue_release(&params->vb2_q);
 	media_entity_cleanup(&params->vdev.entity);
-	video_unregister_device(&params->vdev);
+	vb2_video_unregister_device(&params->vdev);
 	mutex_destroy(&params->lock);
 }
drivers/media/platform/amlogic/c3/isp/c3-isp-stats.c +2 -5
index 8a5d7e1a30c9..78722cbe44b4 100644
@@ -279,7 +279,7 @@ int c3_isp_stats_register(struct c3_isp_device *isp)
 	stats->pad.flags = MEDIA_PAD_FL_SINK;
 	ret = media_entity_pads_init(&vdev->entity, 1, &stats->pad);
 	if (ret)
-		goto err_queue_release;
+		goto err_destroy;
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
 	if (ret) {

@@ -292,8 +292,6 @@ int c3_isp_stats_register(struct c3_isp_device *isp)
 
 err_entity_cleanup:
 	media_entity_cleanup(&vdev->entity);
-err_queue_release:
-	vb2_queue_release(vb2_q);
 err_destroy:
 	mutex_destroy(&stats->lock);
 	return ret;

@@ -303,9 +301,8 @@ void c3_isp_stats_unregister(struct c3_isp_device *isp)
 {
 	struct c3_isp_stats *stats = &isp->stats;
 
-	vb2_queue_release(&stats->vb2_q);
 	media_entity_cleanup(&stats->vdev.entity);
-	video_unregister_device(&stats->vdev);
+	vb2_video_unregister_device(&stats->vdev);
 	mutex_destroy(&stats->lock);
 }
Signature · 3 lines
Regards,

Laurent Pinchart
author
#11 lore raw

PATCH 7/19 media: amphion: drop unnecessary vb2_queue_release call

In reply to Hans Verkuil

If vb2_queue_init fails, then there is no need to call vb2_queue_release.

Signed-off-by Hans Verkuil <[email protected]>
Cc Ming Qian <[email protected]>
---
 drivers/media/platform/amphion/vpu_v4l2.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/media/platform/amphion/vpu_v4l2.c +1 -7
index 7cccc994fc50..af03cb82e9d5 100644
@@ -684,13 +684,7 @@ static int vpu_m2m_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_q
 	dst_vq->buf_struct_size = sizeof(struct vpu_vb2_buffer);
 	dst_vq->dev = inst->vpu->dev;
 	dst_vq->lock = &inst->lock;
-	ret = vb2_queue_init(dst_vq);
-	if (ret) {
-		vb2_queue_release(src_vq);
-		return ret;
-	}
-
-	return 0;
+	return vb2_queue_init(dst_vq);
 }
 
 static int vpu_v4l2_release(struct vpu_inst *inst)
Signature · 1 line
2.53.0
author
#12 lore raw

PATCH 8/19 media: arm: mali-c55: drop unnecessary vb2_queue_release calls

In reply to Hans Verkuil

There is no need to call vb2_queue_release if the video device has not been registered, or if vb2_video_unregister_device was called since that already calls vb2_queue_release internally.

Signed-off-by Hans Verkuil <[email protected]>
Cc Daniel Scally <[email protected]>
Cc Jacopo Mondi <[email protected]>
---
 drivers/media/platform/arm/mali-c55/mali-c55-capture.c | 4 +---
 drivers/media/platform/arm/mali-c55/mali-c55-params.c  | 4 +---
 drivers/media/platform/arm/mali-c55/mali-c55-stats.c   | 4 +---
 3 files changed, 3 insertions(+), 9 deletions(-)
drivers/media/platform/arm/mali-c55/mali-c55-capture.c +1 -3
index 7aaa5c3f7354..37412e33f825 100644
@@ -904,13 +904,11 @@ static int mali_c55_register_cap_dev(struct mali_c55 *mali_c55,
 		dev_err(mali_c55->dev,
 			"%s failed to register video device\n",
 			cap_dev->vdev.name);
-		goto err_release_vb2q;
+		goto err_cleanup_media_entity;
 	}
 
 	return 0;
 
-err_release_vb2q:
-	vb2_queue_release(vb2q);
 err_cleanup_media_entity:
 	media_entity_cleanup(&cap_dev->vdev.entity);
 err_destroy_mutex:
drivers/media/platform/arm/mali-c55/mali-c55-params.c +1 -3
index de0e9d898db7..93664791e2c9 100644
@@ -922,15 +922,13 @@ int mali_c55_register_params(struct mali_c55 *mali_c55)
 	if (ret) {
 		dev_err(mali_c55->dev,
 			"failed to register params video device\n");
-		goto err_release_vb2q;
+		goto err_cleanup_entity;
 	}
 
 	params->mali_c55 = mali_c55;
 
 	return 0;
 
-err_release_vb2q:
-	vb2_queue_release(vb2q);
 err_cleanup_entity:
 	media_entity_cleanup(&params->vdev.entity);
 err_destroy_mutex:
drivers/media/platform/arm/mali-c55/mali-c55-stats.c +1 -3
index 655e52a5f288..aaeb361b2be2 100644
@@ -304,15 +304,13 @@ int mali_c55_register_stats(struct mali_c55 *mali_c55)
 	if (ret) {
 		dev_err(mali_c55->dev,
 			"failed to register stats video device\n");
-		goto err_release_vb2q;
+		goto err_cleanup_entity;
 	}
 
 	stats->mali_c55 = mali_c55;
 
 	return 0;
 
-err_release_vb2q:
-	vb2_queue_release(vb2q);
 err_cleanup_entity:
 	media_entity_cleanup(&stats->vdev.entity);
 err_destroy_mutex:
Signature · 1 line
2.53.0
author
#13 lore raw

PATCH 9/19 media: imagination: e5010-jpeg-enc: drop vb2_queue_release

In reply to Hans Verkuil

No need to call vb2_queue_release after a failed vb2_queue_init. Drop that call and simplify the code.

Signed-off-by Hans Verkuil <[email protected]>
Cc Devarsh Thakkar <[email protected]>
---
 drivers/media/platform/imagination/e5010-jpeg-enc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/media/platform/imagination/e5010-jpeg-enc.c +1 -7
index 42ad9ee3993b..2676bb25b5e2 100644
@@ -606,13 +606,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
 	dst_vq->lock = &e5010->mutex;
 	dst_vq->dev = e5010->v4l2_dev.dev;
 
-	ret = vb2_queue_init(dst_vq);
-	if (ret) {
-		vb2_queue_release(src_vq);
-		return ret;
-	}
-
-	return 0;
+	return vb2_queue_init(dst_vq);
 }
 
 static int e5010_s_ctrl(struct v4l2_ctrl *ctrl)
Signature · 1 line
2.53.0
maintainer Reviewed-by on v1 9/19
#14 lore raw

Re: [PATCH 09/19] media: imagination: e5010-jpeg-enc: drop vb2_queue_release

Quoted 5 lines · Hans Verkuil

No need to call vb2_queue_release after a failed vb2_queue_init. Drop that call and simplify the code.

Signed-off-by Hans Verkuil <[email protected]>
Cc Devarsh Thakkar <[email protected]>
Reviewed-by Devarsh Thakkar <[email protected]>

Regards
Devarsh

Quoted 23 lines
---
 drivers/media/platform/imagination/e5010-jpeg-enc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/media/platform/imagination/e5010-jpeg-enc.c +1 -7
index 42ad9ee3993b..2676bb25b5e2 100644
@@ -606,13 +606,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
 	dst_vq->lock = &e5010->mutex;
 	dst_vq->dev = e5010->v4l2_dev.dev;
 
-	ret = vb2_queue_init(dst_vq);
-	if (ret) {
-		vb2_queue_release(src_vq);
-		return ret;
-	}
-
-	return 0;
+	return vb2_queue_init(dst_vq);
 }
 
 static int e5010_s_ctrl(struct v4l2_ctrl *ctrl)
author
#15 lore raw

PATCH 10/19 media: nuvoton: drop unnecessary vb2_queue_release calls

In reply to Hans Verkuil

There is no need to call vb2_queue_release if video device registration fails.

And use vb2_video_unregister_device instead of the video_unregister_device/vb2_queue_release pair since that will call vb2_queue_release internally.

Signed-off-by Hans Verkuil <[email protected]>
Cc Joseph Liu <[email protected]>
Cc Marvin Lin <[email protected]>
---
 drivers/media/platform/nuvoton/npcm-video.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/media/platform/nuvoton/npcm-video.c +2 -5
index 52505af35c08..088dec771b67 100644
@@ -1635,13 +1635,11 @@ static int npcm_video_setup_video(struct npcm_video *video)
 	rc = video_register_device(vdev, VFL_TYPE_VIDEO, 0);
 	if (rc) {
 		dev_err(video->dev, "Failed to register video device\n");
-		goto rel_vb_queue;
+		goto rel_ctrl_handler;
 	}
 
 	return 0;
 
-rel_vb_queue:
-	vb2_queue_release(vbq);
 rel_ctrl_handler:
 	v4l2_ctrl_handler_free(&video->ctrl_handler);
 	v4l2_device_unregister(v4l2_dev);

@@ -1807,8 +1805,7 @@ static void npcm_video_remove(struct platform_device *pdev)
 	struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
 	struct npcm_video *video = to_npcm_video(v4l2_dev);
 
-	video_unregister_device(&video->vdev);
-	vb2_queue_release(&video->queue);
+	vb2_video_unregister_device(&video->vdev);
 	v4l2_ctrl_handler_free(&video->ctrl_handler);
 	v4l2_device_unregister(v4l2_dev);
 	if (video->ece.enable)
Signature · 1 line
2.53.0
author
#16 lore raw

PATCH 11/19 media: nxp: imx7-media-csi: use _vb2_fop_release/vb2_video_unregister_device

In reply to Hans Verkuil

Use _vb2_fop_release instead of manually calling vb2_queue_release and v4l2_fh_release.

Use vb2_video_unregister_device instead of video_unregister_device to ensure vb2_queue_release is called inside vb2_video_unregister_device if needed.

Signed-off-by Hans Verkuil <[email protected]>
Cc Laurent Pinchart <[email protected]>
Cc Frank Li <[email protected]>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/media/platform/nxp/imx7-media-csi.c +3 -7
index 7ddc7ba06e3d..eb2b54944ed1 100644
@@ -1545,14 +1545,10 @@ static int imx7_csi_video_release(struct file *file)
 
 	mutex_lock(&csi->vdev_mutex);
 
-	if (file->private_data == vq->owner) {
-		vb2_queue_release(vq);
-		vq->owner = NULL;
-	}
+	_vb2_fop_release(file, NULL);
 
 	v4l2_pipeline_pm_put(&csi->vdev->entity);
 
-	v4l2_fh_release(file);
 	mutex_unlock(&csi->vdev_mutex);
 	return 0;
 }

@@ -1627,7 +1623,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
 				    MEDIA_LNK_FL_ENABLED);
 	if (ret) {
 		dev_err(csi->dev, "failed to create link to device node\n");
-		video_unregister_device(vdev);
+		vb2_video_unregister_device(vdev);
 		return ret;
 	}
 

@@ -1637,7 +1633,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
 static void imx7_csi_video_unregister(struct imx7_csi *csi)
 {
 	media_entity_cleanup(&csi->vdev->entity);
-	video_unregister_device(csi->vdev);
+	vb2_video_unregister_device(csi->vdev);
 }
 
 static int imx7_csi_video_init(struct imx7_csi *csi)
Signature · 1 line
2.53.0
maintainer
#17 lore raw

Re: [PATCH 11/19] media: nxp: imx7-media-csi: use _vb2_fop_release/vb2_video_unregister_device

Quoted 6 lines · Hans Verkuil

Use _vb2_fop_release instead of manually calling vb2_queue_release and v4l2_fh_release.

Use vb2_video_unregister_device instead of video_unregister_device to ensure vb2_queue_release is called inside vb2_video_unregister_device if needed.

Please expand on the "if needed" to explain why this is needed.

Quoted 14 lines
Signed-off-by Hans Verkuil <[email protected]>
Cc Laurent Pinchart <[email protected]>
Cc Frank Li <[email protected]>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/media/platform/nxp/imx7-media-csi.c +0 -0
index 7ddc7ba06e3d..eb2b54944ed1 100644
@@ -1545,14 +1545,10 @@ static int imx7_csi_video_release(struct file *file)
 
 	mutex_lock(&csi->vdev_mutex);

drivers/media/platform/nxp/imx7-media-csi.c static int imx7_csi_video_release(struct file *file) · line 1555

Quoted 10 lines
drivers/media/platform/nxp/imx7-media-csi.c +1 -5
 
-	if (file->private_data == vq->owner) {
-		vb2_queue_release(vq);
-		vq->owner = NULL;
-	}
+	_vb2_fop_release(file, NULL);
 
 	v4l2_pipeline_pm_put(&csi->vdev->entity);
 
-	v4l2_fh_release(file);

You're effectively inverting the order of v4l2_pipeline_pm_put() and v4l2_fh_release(). Please explain in the commit message why this is safe.

Commit messages need first and foremost to explain *why* a change is needed. The *what* and *how* are secondary.

Quoted 21 lines
drivers/media/platform/nxp/imx7-media-csi.c +2 -2
 	mutex_unlock(&csi->vdev_mutex);
 	return 0;
 }

@@ -1627,7 +1623,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
 				    MEDIA_LNK_FL_ENABLED);
 	if (ret) {
 		dev_err(csi->dev, "failed to create link to device node\n");
-		video_unregister_device(vdev);
+		vb2_video_unregister_device(vdev);
 		return ret;
 	}
 

@@ -1637,7 +1633,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
 static void imx7_csi_video_unregister(struct imx7_csi *csi)
 {
 	media_entity_cleanup(&csi->vdev->entity);
-	video_unregister_device(csi->vdev);
+	vb2_video_unregister_device(csi->vdev);
 }
 
 static int imx7_csi_video_init(struct imx7_csi *csi)
Signature · 3 lines
Regards,

Laurent Pinchart
author
#18 lore raw

Re: [PATCH 11/19] media: nxp: imx7-media-csi: use _vb2_fop_release/vb2_video_unregister_device

Quoted 9 lines · Laurent Pinchart
Quoted 6 lines · Hans Verkuil

Use _vb2_fop_release instead of manually calling vb2_queue_release and v4l2_fh_release.

Use vb2_video_unregister_device instead of video_unregister_device to ensure vb2_queue_release is called inside vb2_video_unregister_device if needed.

Please expand on the "if needed" to explain why this is needed.

Replaced this with:

"Use vb2_video_unregister_device instead of video_unregister_device to ensure vb2_queue_release is called inside vb2_video_unregister_device when streaming. That way streaming is guaranteed to be stopped when the device is unregistered."

Quoted 24 lines
Quoted 24 lines
Signed-off-by Hans Verkuil <[email protected]>
Cc Laurent Pinchart <[email protected]>
Cc Frank Li <[email protected]>
---
 drivers/media/platform/nxp/imx7-media-csi.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/media/platform/nxp/imx7-media-csi.c +1 -4
index 7ddc7ba06e3d..eb2b54944ed1 100644
@@ -1545,14 +1545,10 @@ static int imx7_csi_video_release(struct file *file)
 
 	mutex_lock(&csi->vdev_mutex);
 
-	if (file->private_data == vq->owner) {
-		vb2_queue_release(vq);
-		vq->owner = NULL;
-	}
+	_vb2_fop_release(file, NULL);
 
 	v4l2_pipeline_pm_put(&csi->vdev->entity);
Quoted 8 lines

drivers/media/platform/nxp/imx7-media-csi.c static int imx7_csi_video_release(struct file *file) · line 1555

Quoted 1 line
drivers/media/platform/nxp/imx7-media-csi.c +0 -1
-	v4l2_fh_release(file);

You're effectively inverting the order of v4l2_pipeline_pm_put() and v4l2_fh_release(). Please explain in the commit message why this is safe.

Commit messages need first and foremost to explain *why* a change is needed. The *what* and *how* are secondary.

Updated commit log. v4l2_pipeline_pm_put doesn't access v4l2_fh at all, so it is fine to do it in this order.

Quoted 13 lines
Quoted 13 lines
drivers/media/platform/nxp/imx7-media-csi.c +1 -1
 	mutex_unlock(&csi->vdev_mutex);
 	return 0;
 }

@@ -1627,7 +1623,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
 				    MEDIA_LNK_FL_ENABLED);
 	if (ret) {
 		dev_err(csi->dev, "failed to create link to device node\n");
-		video_unregister_device(vdev);
+		vb2_video_unregister_device(vdev);
 		return ret;
 	}
Quoted 6 lines

drivers/media/platform/nxp/imx7-media-csi.c static int imx7_csi_video_register(struct imx7_csi *csi) · line 1636

Quoted 6 lines
drivers/media/platform/nxp/imx7-media-csi.c +1 -1
@@ -1637,7 +1633,7 @@ static int imx7_csi_video_register(struct imx7_csi *csi)
 static void imx7_csi_video_unregister(struct imx7_csi *csi)
 {
 	media_entity_cleanup(&csi->vdev->entity);
-	video_unregister_device(csi->vdev);
+	vb2_video_unregister_device(csi->vdev);

Note: just like your comment in patch 06/19, the media_entity_cleanup should come after the device is unregistered. I've swapped this in both places.

If media_entity_cleanup is ever changed to actually free something instead of being an empty function, then I suspect several drivers will stop working since that has never been tested. :-(

Regards,

Hans

Quoted 4 lines
Quoted 3 lines
drivers/media/platform/nxp/imx7-media-csi.c +0 -0
 }
 
 static int imx7_csi_video_init(struct imx7_csi *csi)
author
#19 lore raw

PATCH 12/19 media: raspberrypi: pisp_be: use vb2_video_unregister_device

In reply to Hans Verkuil

Use vb2_video_unregister_device instead of the
video_unregister_device/vb2_queue_release pair.

This simplifies the code.

Signed-off-by Hans Verkuil <[email protected]>
Cc Jacopo Mondi <[email protected]>
Cc Raspberry Pi Kernel Maintenance <[email protected]>
---
 .../media/platform/raspberrypi/pisp_be/pisp_be.c   | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)
drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +5 -9
index d60d92d2ffa1..4689e961bb19 100644
@@ -1443,7 +1443,7 @@ static int pispbe_init_node(struct pispbe_dev *pispbe, unsigned int id)
 		dev_err(pispbe->dev,
 			"Failed to register media pads for %s device node\n",
 			NODE_NAME(node));
-		goto err_unregister_queue;
+		goto err_mutex_destroy;
 	}
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);

@@ -1451,7 +1451,7 @@ static int pispbe_init_node(struct pispbe_dev *pispbe, unsigned int id)
 		dev_err(pispbe->dev,
 			"Failed to register video %s device node\n",
 			NODE_NAME(node));
-		goto err_unregister_queue;
+		goto err_mutex_destroy;
 	}
 	video_set_drvdata(vdev, node);
 

@@ -1472,9 +1472,7 @@ static int pispbe_init_node(struct pispbe_dev *pispbe, unsigned int id)
 	return 0;
 
 err_unregister_video_dev:
-	video_unregister_device(&node->vfd);
-err_unregister_queue:
-	vb2_queue_release(&node->queue);
+	vb2_video_unregister_device(&node->vfd);
 err_mutex_destroy:
 	mutex_destroy(&node->node_lock);
 	mutex_destroy(&node->queue_lock);

@@ -1576,8 +1574,7 @@ static int pispbe_init_devices(struct pispbe_dev *pispbe)
 	media_device_unregister(mdev);
 err_unregister_nodes:
 	while (num_regist-- > 0) {
-		video_unregister_device(&pispbe->node[num_regist].vfd);
-		vb2_queue_release(&pispbe->node[num_regist].queue);
+		vb2_video_unregister_device(&pispbe->node[num_regist].vfd);
 	}
 	v4l2_device_unregister_subdev(&pispbe->sd);
 	media_entity_cleanup(&pispbe->sd.entity);

@@ -1605,8 +1602,7 @@ static void pispbe_destroy_devices(struct pispbe_dev *pispbe)
 	media_device_unregister(&pispbe->mdev);
 
 	for (int i = PISPBE_NUM_NODES - 1; i >= 0; i--) {
-		video_unregister_device(&pispbe->node[i].vfd);
-		vb2_queue_release(&pispbe->node[i].queue);
+		vb2_video_unregister_device(&pispbe->node[i].vfd);
 		mutex_destroy(&pispbe->node[i].node_lock);
 		mutex_destroy(&pispbe->node[i].queue_lock);
 	}
Signature · 1 line
2.53.0
author
#20 lore raw

PATCH 13/19 media: renesas: rzv2h-cru: use vb2_video_unregister_device

In reply to Hans Verkuil

Use vb2_video_unregister_device instead of video_unregister_device:
this ensures vb2_queue_release is called internally at the right time.

Signed-off-by Hans Verkuil <[email protected]>
Cc Lad Prabhakar <[email protected]>
Cc Jacopo Mondi <[email protected]>
---
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c +2 -2
index 5185a547461d..2f5ab1e4707e 100644
@@ -1091,7 +1091,7 @@ static void rzg2l_cru_v4l2_init(struct rzg2l_cru_dev *cru)
 void rzg2l_cru_video_unregister(struct rzg2l_cru_dev *cru)
 {
 	media_device_unregister(&cru->mdev);
-	video_unregister_device(&cru->vdev);
+	vb2_video_unregister_device(&cru->vdev);
 }
 
 int rzg2l_cru_video_register(struct rzg2l_cru_dev *cru)

@@ -1118,7 +1118,7 @@ int rzg2l_cru_video_register(struct rzg2l_cru_dev *cru)
 
 	ret = media_device_register(&cru->mdev);
 	if (ret) {
-		video_unregister_device(&cru->vdev);
+		vb2_video_unregister_device(&cru->vdev);
 		return ret;
 	}
Signature · 1 line
2.53.0
author
#21 lore raw

PATCH 14/19 media: renesas: rzv2h-ivc: drop unnecessary vb2_queue_release calls

In reply to Hans Verkuil

No need to call vb2_queue_release if the video device is never registered.

No need to call vb2_queue_release if vb2_video_unregister_device is used (it calls vb2_queue_release internally).

Replace a video_unregister_device/vb2_queue_release pair by vb2_video_unregister_device.

Signed-off-by Hans Verkuil <[email protected]>
Cc Daniel Scally <[email protected]>
Cc Jacopo Mondi <[email protected]>
---
 drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c +2 -5
index 932fed38cf3f..d37179dc8bda 100644
@@ -511,7 +511,7 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
 	ivc->vdev.pad.flags = MEDIA_PAD_FL_SOURCE;
 	ret = media_entity_pads_init(&ivc->vdev.dev.entity, 1, &ivc->vdev.pad);
 	if (ret)
-		goto err_release_vb2q;
+		return ret;
 
 	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
 	if (ret) {

@@ -531,11 +531,9 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
 	return 0;
 
 err_unregister_vdev:
-	video_unregister_device(vdev);
+	vb2_video_unregister_device(vdev);
 err_cleanup_vdev_entity:
 	media_entity_cleanup(&vdev->entity);
-err_release_vb2q:
-	vb2_queue_release(vb2q);
 
 	return ret;
 }

@@ -547,5 +545,4 @@ void rzv2h_deinit_video_dev_and_queue(struct rzv2h_ivc *ivc)
 
 	vb2_video_unregister_device(vdev);
 	media_entity_cleanup(&vdev->entity);
-	vb2_queue_release(vb2q);
 }
Signature · 1 line
2.53.0
author
#22 lore raw

PATCH 15/19 media: samsung: s3c-camif: drop unnecessary vb2_queue_release

In reply to Hans Verkuil

Use _vb2_fop_release instead of vb2_queue_release/v4l2_fh_release.

No need to call camif_stop_capture: _vb2_fop_release will call the stop_streaming op if streaming is in progress, and that op calls camif_stop_capture.

Signed-off-by Hans Verkuil <[email protected]>
Cc Sylwester Nawrocki <[email protected]>
---
 drivers/media/platform/samsung/s3c-camif/camif-capture.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)
drivers/media/platform/samsung/s3c-camif/camif-capture.c +1 -6
index ed1a1d693293..3cbf58b2128e 100644
@@ -572,16 +572,11 @@ static int s3c_camif_close(struct file *file)
 
 	mutex_lock(&camif->lock);
 
-	if (vp->owner == file_to_v4l2_fh(file)) {
-		camif_stop_capture(vp);
-		vb2_queue_release(&vp->vb_queue);
-		vp->owner = NULL;
-	}
+	ret = _vb2_fop_release(file, NULL);
 
 	sensor_set_power(camif, 0);
 
 	pm_runtime_put(camif->dev);
-	ret = v4l2_fh_release(file);
 
 	mutex_unlock(&camif->lock);
 	return ret;
Signature · 1 line
2.53.0
author
#23 lore raw

PATCH 16/19 media: ti: j721e-csi2rx: use vb2_video_unregister_device

In reply to Hans Verkuil

Replace the vb2_queue_release/video_unregister_device pair by a single vb2_video_unregister_device call.

Signed-off-by Hans Verkuil <[email protected]>
Cc Jai Luthra <[email protected]>
---
 drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c +1 -3
index 4769931b1930..7ae7d1e33ce3 100644
@@ -1268,9 +1268,7 @@ static void ti_csi2rx_cleanup_ctx(struct ti_csi2rx_ctx *ctx)
 	if (!pm_runtime_status_suspended(ctx->csi->dev))
 		dma_release_channel(ctx->dma.chan);
 
-	vb2_queue_release(&ctx->vidq);
-
-	video_unregister_device(&ctx->vdev);
+	vb2_video_unregister_device(&ctx->vdev);
 
 	mutex_destroy(&ctx->mutex);
 }
Signature · 1 line
2.53.0
author
#24 lore raw

PATCH 17/19 staging: media: imx: drop vb2_queue_release call

In reply to Hans Verkuil

Replace vb2_queue_release/v4l2_fh_release by _vb2_fop_release.

Replace video_unregister_device by vb2_video_unregister_device to ensure vb2_queue_release is called from vb2_video_unregister_device.

Signed-off-by Hans Verkuil <[email protected]>
Cc Steve Longerbeam <[email protected]>
Cc Philipp Zabel <[email protected]>
Cc Frank Li <[email protected]>
---
 drivers/staging/media/imx/imx-media-capture.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/staging/media/imx/imx-media-capture.c +3 -7
index bfd71d25facc..e95989a6f861 100644
@@ -808,14 +808,10 @@ static int capture_release(struct file *file)
 
 	mutex_lock(&priv->mutex);
 
-	if (file->private_data == vq->owner) {
-		vb2_queue_release(vq);
-		vq->owner = NULL;
-	}
+	_vb2_fop_release(file, NULL);
 
 	v4l2_pipeline_pm_put(&vfd->entity);
 
-	v4l2_fh_release(file);
 	mutex_unlock(&priv->mutex);
 	return 0;
 }

@@ -938,7 +934,7 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev,
 				    &vfd->entity, 0, link_flags);
 	if (ret) {
 		dev_err(priv->dev, "failed to create link to device node\n");
-		video_unregister_device(vfd);
+		vb2_video_unregister_device(vfd);
 		return ret;
 	}
 

@@ -955,7 +951,7 @@ void imx_media_capture_device_unregister(struct imx_media_video_dev *vdev)
 	struct video_device *vfd = priv->vdev.vfd;
 
 	media_entity_cleanup(&vfd->entity);
-	video_unregister_device(vfd);
+	vb2_video_unregister_device(vfd);
 }
 EXPORT_SYMBOL_GPL(imx_media_capture_device_unregister);
Signature · 1 line
2.53.0
author
#25 lore raw

PATCH 18/19 staging: media: ipu7: drop unnecessary cleanup calls

In reply to Hans Verkuil

If the video device isn't registered, then there is no need to call vb2_video_unregister_device or vb2_queue_release.

Signed-off-by Hans Verkuil <[email protected]>
Cc Sakari Ailus <[email protected]>
---
 drivers/staging/media/ipu7/ipu7-isys-video.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/staging/media/ipu7/ipu7-isys-video.c +1 -5
index 8c6730833f24..5f3f091aabb5 100644
@@ -1035,7 +1035,7 @@ int ipu7_isys_video_init(struct ipu7_isys_video *av)
 	av->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
 	ret = media_entity_pads_init(&av->vdev.entity, 1, &av->pad);
 	if (ret)
-		goto out_vb2_queue_cleanup;
+		goto out_mutex_destroy;
 
 	av->vdev.entity.ops = &entity_ops;
 	av->vdev.release = video_device_release_empty;

@@ -1058,12 +1058,8 @@ int ipu7_isys_video_init(struct ipu7_isys_video *av)
 	return ret;
 
 out_media_entity_cleanup:
-	vb2_video_unregister_device(&av->vdev);
 	media_entity_cleanup(&av->vdev.entity);
 
-out_vb2_queue_cleanup:
-	vb2_queue_release(&av->aq.vbq);
-
 out_mutex_destroy:
 	mutex_destroy(&av->mutex);
Signature · 1 line
2.53.0
author
#27 lore raw

PATCH 19/19 media: renesas: rzg2l-cru: drop unnecessary vb2_queue_release

In reply to Hans Verkuil

There is no need to call vb2_queue_release if the video device has not been registered, or if vb2_video_unregister_device was called since that already calls vb2_queue_release internally.

Signed-off-by Hans Verkuil <[email protected]>
Cc Lad Prabhakar <[email protected]>
Cc Jacopo Mondi <[email protected]>
---
 drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 1 -
 1 file changed, 1 deletion(-)
drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c +0 -1
index 2f5ab1e4707e..edca6cfb5839 100644
@@ -767,7 +767,6 @@ void rzg2l_cru_dma_unregister(struct rzg2l_cru_dev *cru)
 	mutex_destroy(&cru->lock);
 
 	v4l2_device_unregister(&cru->v4l2_dev);
-	vb2_queue_release(&cru->queue);
 }
 
 int rzg2l_cru_dma_register(struct rzg2l_cru_dev *cru)
Signature · 1 line
2.53.0