All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: linux-media@vger.kernel.org
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Robert Foss <robert.foss@linaro.org>,
	Andrey Konovalov <andrey.konovalov@linaro.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Helen Koike <helen.koike@collabora.com>,
	Sowjanya Komatineni <skomatineni@nvidia.com>,
	Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Eddie James <eajames@linux.vnet.ibm.com>,
	Maxime Ripard <maxime@cerno.tech>,
	Alexandre Courbot <acourbot@chromium.org>,
	Tiffany Lin <tiffany.lin@mediatek.com>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>
Subject: [PATCHv3 1/7] videobuf2-v4l2.c: add vb2_video_unregister_device helper function
Date: Mon, 13 Jul 2020 13:30:42 +0200	[thread overview]
Message-ID: <20200713113048.1150542-2-hverkuil-cisco@xs4all.nl> (raw)
In-Reply-To: <20200713113048.1150542-1-hverkuil-cisco@xs4all.nl>

If a driver calls (_)vb2_fop_release(), then such a driver should also
call vb2_video_unregister_device() instead of video_unregister_device().
This helper will all vb2_queue_release() if a filehandle is marked as
owner of the queue. This ensures that at unregister time any streaming
is cancelled and all buffers are returned to userspace.

This is very useful for complex drivers since this stops all streaming
in all subdevs in the pipeline controlled by this video device. Otherwise
this would happen until the owner filehandle is closed, which can be quite
some time later.

Bonus points for ordering the includes :-)

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Andrey Konovalov <andrey.konovalov@linaro.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Helen Koike <helen.koike@collabora.com>
Cc: Sowjanya Komatineni <skomatineni@nvidia.com>
Cc: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Eddie James <eajames@linux.vnet.ibm.com>
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Alexandre Courbot <acourbot@chromium.org>
Cc: Tiffany Lin <tiffany.lin@mediatek.com>
Cc: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 .../media/common/videobuf2/videobuf2-v4l2.c   | 51 ++++++++++++++++---
 include/media/videobuf2-v4l2.h                | 17 +++++++
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 57aa183bd198..576057132d3e 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -14,21 +14,22 @@
  * the Free Software Foundation.
  */
 
+#include <linux/device.h>
 #include <linux/err.h>
+#include <linux/freezer.h>
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/kthread.h>
 #include <linux/mm.h>
+#include <linux/module.h>
 #include <linux/poll.h>
-#include <linux/slab.h>
 #include <linux/sched.h>
-#include <linux/freezer.h>
-#include <linux/kthread.h>
+#include <linux/slab.h>
 
+#include <media/v4l2-common.h>
 #include <media/v4l2-dev.h>
 #include <media/v4l2-device.h>
-#include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
-#include <media/v4l2-common.h>
+#include <media/v4l2-fh.h>
 
 #include <media/videobuf2-v4l2.h>
 
@@ -1234,6 +1235,44 @@ unsigned long vb2_fop_get_unmapped_area(struct file *file, unsigned long addr,
 EXPORT_SYMBOL_GPL(vb2_fop_get_unmapped_area);
 #endif
 
+void vb2_video_unregister_device(struct video_device *vdev)
+{
+	/* Check if vdev was ever registered at all */
+	if (!vdev || !video_is_registered(vdev))
+		return;
+
+	/*
+	 * Calling this function only makes sense if vdev->queue is set.
+	 * If it is NULL, then just call video_unregister_device() instead.
+	 */
+	WARN_ON(!vdev->queue);
+
+	/*
+	 * Take a reference to the device since video_unregister_device()
+	 * calls device_unregister(), but we don't want that to release
+	 * the device since we want to clean up the queue first.
+	 */
+	get_device(&vdev->dev);
+	video_unregister_device(vdev);
+	if (vdev->queue && vdev->queue->owner) {
+		struct mutex *lock = vdev->queue->lock ?
+			vdev->queue->lock : vdev->lock;
+
+		if (lock)
+			mutex_lock(lock);
+		vb2_queue_release(vdev->queue);
+		vdev->queue->owner = NULL;
+		if (lock)
+			mutex_unlock(lock);
+	}
+	/*
+	 * Now we put the device, and in most cases this will release
+	 * everything.
+	 */
+	put_device(&vdev->dev);
+}
+EXPORT_SYMBOL_GPL(vb2_video_unregister_device);
+
 /* vb2_ops helpers. Only use if vq->lock is non-NULL. */
 
 void vb2_ops_wait_prepare(struct vb2_queue *vq)
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index b7b5a9cb5a28..c203047eb834 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -23,6 +23,8 @@
 #error VB2_MAX_PLANES != VIDEO_MAX_PLANES
 #endif
 
+struct video_device;
+
 /**
  * struct vb2_v4l2_buffer - video buffer information for v4l2.
  *
@@ -319,6 +321,21 @@ unsigned long vb2_fop_get_unmapped_area(struct file *file, unsigned long addr,
 		unsigned long len, unsigned long pgoff, unsigned long flags);
 #endif
 
+/**
+ * vb2_video_unregister_device - unregister the video device and release queue
+ *
+ * @vdev: pointer to &struct video_device
+ *
+ * If the driver uses vb2_fop_release()/_vb2_fop_release(), then it should use
+ * vb2_video_unregister_device() instead of video_unregister_device().
+ *
+ * This function will call video_unregister_device() and then release the
+ * vb2_queue if streaming is in progress. This will stop streaming and
+ * this will simplify the unbind sequence since after this call all subdevs
+ * will have stopped streaming as well.
+ */
+void vb2_video_unregister_device(struct video_device *vdev);
+
 /**
  * vb2_ops_wait_prepare - helper function to lock a struct &vb2_queue
  *
-- 
2.27.0


  reply	other threads:[~2020-07-13 11:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 11:30 [PATCHv3 0/7] media: use vb2_video_unregister_device() Hans Verkuil
2020-07-13 11:30 ` Hans Verkuil [this message]
2020-07-13 15:55   ` [PATCHv3 1/7] videobuf2-v4l2.c: add vb2_video_unregister_device helper function Robert Foss
2020-07-16  8:00     ` Hans Verkuil
2020-07-13 11:30 ` [PATCHv3 2/7] qcom/camss: use vb2_video_unregister_device() Hans Verkuil
2020-07-13 16:05   ` Robert Foss
2020-07-16  8:12     ` Hans Verkuil
2020-07-16  8:28       ` Andrey Konovalov
2020-07-17 16:21   ` Andrey Konovalov
2020-07-17 17:23     ` Hans Verkuil
2020-07-13 11:30 ` [PATCHv3 3/7] media/pci: " Hans Verkuil
2020-07-13 11:30 ` [PATCHv3 4/7] media/platform: drop vb2_queue_release() Hans Verkuil
2020-07-13 11:30 ` [PATCHv3 5/7] media/usb: use vb2_video_unregister_device() Hans Verkuil
2020-07-13 11:30 ` [PATCHv3 6/7] vimc: " Hans Verkuil
2020-07-13 11:30 ` [PATCHv3 7/7] staging/media: drop vb2_queue_release() Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200713113048.1150542-2-hverkuil-cisco@xs4all.nl \
    --to=hverkuil-cisco@xs4all.nl \
    --cc=acourbot@chromium.org \
    --cc=andrey.konovalov@linaro.org \
    --cc=dafna.hirschfeld@collabora.com \
    --cc=eajames@linux.vnet.ibm.com \
    --cc=helen.koike@collabora.com \
    --cc=linux-media@vger.kernel.org \
    --cc=maxime@cerno.tech \
    --cc=narmstrong@baylibre.com \
    --cc=robert.foss@linaro.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=skomatineni@nvidia.com \
    --cc=stanimir.varbanov@linaro.org \
    --cc=tiffany.lin@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.