Enable multiple opens of a video(4) device as described in the V4L2

specification:

https://www.kernel.org/doc/html/v5.10/userspace-api/media/v4l/open.html#f1

The discussion has been started by jca@, who has implemented this
behavior recently, but limited to the same process.  This diff extends
this behavior to any process.  The first process which opens a stream
will become the device owner.  Other processes are still allowed to call
certain ioctls, but none which are related to the start/stop of a
stream, or manipulation of the streaming buffers.  At the moment only
VIDIOC_G_CTRL and VIDIOC_S_CTRL are supported to be called by non-
device owner processes, which should be extended further in the future.

There is no additional kernel locking implemented at the moment, since
video(4) already runs under the KERNEL_LOCK(), which we expect to be
sufficient for now (as discussed with claudio@).

A lot of improvement input received from anton@.

ok anton@

1 files changed, 109 insertions, 30 deletions

diff --git a/sys/dev/video.c b/sys/dev/video.c
index 49ac5b9ad4c..d1fde67d132 100644
--- a/sys/dev/video.c
+++ b/sys/dev/video.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: video.c,v 1.48 2021/01/31 19:32:01 mglocker Exp $	*/
+/*	$OpenBSD: video.c,v 1.49 2021/02/16 13:50:46 mglocker Exp $	*/
 
 /*
  * Copyright (c) 2008 Robert Nagy <robert@openbsd.org>
@@ -47,7 +47,8 @@ struct video_softc {
 	struct device		*sc_dev;	/* hardware device struct */
 	struct video_hw_if	*hw_if;		/* hardware interface */
 	char			 sc_dying;	/* device detached */
-	struct process		 *sc_owner;	/* owner process */
+	struct process		*sc_owner;	/* owner process */
+	uint8_t			 sc_open;	/* device opened */
 
 	int			 sc_fsize;
 	uint8_t			*sc_fbuffer;
@@ -69,6 +70,8 @@ int	videoactivate(struct device *, int);
 int	videoprint(void *, const char *);
 
 void	video_intr(void *);
+int	video_stop(struct video_softc *);
+int	video_claim(struct video_softc *, struct process *);
 
 struct cfattach video_ca = {
 	sizeof(struct video_softc), videoprobe, videoattach,
@@ -122,6 +125,9 @@ videoopen(dev_t dev, int flags, int fmt, struct proc *p)
 {
 	int	unit;
 	struct video_softc *sc;
+	int error = 0;
+
+	KERNEL_ASSERT_LOCKED();
 
 	unit = VIDEOUNIT(dev);
 	if (unit >= video_cd.cd_ndevs ||
@@ -129,38 +135,40 @@ videoopen(dev_t dev, int flags, int fmt, struct proc *p)
 	     sc->hw_if == NULL)
 		return (ENXIO);
 
-	if (sc->sc_owner != NULL) {
-		if (sc->sc_owner == p->p_p)
-			return (0);
-		else
-			return (EBUSY);
-	} else
-		sc->sc_owner = p->p_p;
+	if (sc->sc_open) {
+		DPRINTF(("%s: device already open\n", __func__));
+		return (0);
+	} else {
+		sc->sc_open = 1;
+		DPRINTF(("%s: set device to open\n", __func__));
+	}
 
 	sc->sc_vidmode = VIDMODE_NONE;
 	sc->sc_frames_ready = 0;
 
 	if (sc->hw_if->open != NULL)
-		return (sc->hw_if->open(sc->hw_hdl, flags, &sc->sc_fsize,
-		    sc->sc_fbuffer, video_intr, sc));
-	else
-		return (0);
+		error = sc->hw_if->open(sc->hw_hdl, flags, &sc->sc_fsize,
+		    sc->sc_fbuffer, video_intr, sc);
+
+	return (error);
 }
 
 int
 videoclose(dev_t dev, int flags, int fmt, struct proc *p)
 {
 	struct video_softc *sc;
-	int r = 0;
+	int error = 0;
 
-	sc = video_cd.cd_devs[VIDEOUNIT(dev)];
+	KERNEL_ASSERT_LOCKED();
 
-	if (sc->hw_if->close != NULL)
-		r = sc->hw_if->close(sc->hw_hdl);
+	DPRINTF(("%s: last close\n", __func__));
 
-	sc->sc_owner = NULL;
+	sc = video_cd.cd_devs[VIDEOUNIT(dev)];
+
+	error = video_stop(sc);
+	sc->sc_open = 0;
 
-	return (r);
+	return (error);
 }
 
 int
@@ -170,6 +178,8 @@ videoread(dev_t dev, struct uio *uio, int ioflag)
 	int unit, error;
 	size_t size;
 
+	KERNEL_ASSERT_LOCKED();
+
 	unit = VIDEOUNIT(dev);
 	if (unit >= video_cd.cd_ndevs ||
 	    (sc = video_cd.cd_devs[unit]) == NULL)
@@ -181,6 +191,9 @@ videoread(dev_t dev, struct uio *uio, int ioflag)
 	if (sc->sc_vidmode == VIDMODE_MMAP)
 		return (EBUSY);
 
+	if ((error = video_claim(sc, curproc->p_p)))
+		return (error);
+
 	/* start the stream if not already started */
 	if (sc->sc_vidmode == VIDMODE_NONE && sc->hw_if->start_read) {
  		error = sc->hw_if->start_read(sc->hw_hdl);
@@ -221,6 +234,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p)
 	struct v4l2_buffer *vb = (struct v4l2_buffer *)data;
 	int unit, error;
 
+	KERNEL_ASSERT_LOCKED();
+
 	unit = VIDEOUNIT(dev);
 	if (unit >= video_cd.cd_ndevs ||
 	    (sc = video_cd.cd_devs[unit]) == NULL || sc->hw_if == NULL)
@@ -231,6 +246,31 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p)
 
 	error = EOPNOTSUPP;
 	switch (cmd) {
+	case VIDIOC_G_CTRL:
+		if (sc->hw_if->g_ctrl)
+			error = (sc->hw_if->g_ctrl)(sc->hw_hdl,
+			    (struct v4l2_control *)data);
+		break;
+	case VIDIOC_S_CTRL:
+		if (sc->hw_if->s_ctrl)
+			error = (sc->hw_if->s_ctrl)(sc->hw_hdl,
+			    (struct v4l2_control *)data);
+		break;
+	default:
+		error = (ENOTTY);
+	}
+	if (error != ENOTTY)
+		return (error);
+
+	if ((error = video_claim(sc, p->p_p)))
+		return (error);
+
+	/*
+	 * The following IOCTLs can only be called by the device owner.
+	 * For further shared IOCTLs please move it up.
+	 */
+	error = EOPNOTSUPP;
+	switch (cmd) {
 	case VIDIOC_QUERYCAP:
 		if (sc->hw_if->querycap)
 			error = (sc->hw_if->querycap)(sc->hw_hdl,
@@ -326,6 +366,10 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p)
 		if (sc->hw_if->streamoff)
 			error = (sc->hw_if->streamoff)(sc->hw_hdl,
 			    (int)*data);
+		if (!error) {
+			/* Release device ownership and streaming buffers. */
+			video_stop(sc);
+		}
 		break;
 	case VIDIOC_TRY_FMT:
 		if (sc->hw_if->try_fmt)
@@ -337,16 +381,6 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p)
 			error = (sc->hw_if->queryctrl)(sc->hw_hdl,
 			    (struct v4l2_queryctrl *)data);
 		break;
-	case VIDIOC_G_CTRL:
-		if (sc->hw_if->g_ctrl)
-			error = (sc->hw_if->g_ctrl)(sc->hw_hdl,
-			    (struct v4l2_control *)data);
-		break;
-	case VIDIOC_S_CTRL:
-		if (sc->hw_if->s_ctrl)
-			error = (sc->hw_if->s_ctrl)(sc->hw_hdl,
-			    (struct v4l2_control *)data);
-		break;
 	default:
 		error = (ENOTTY);
 	}
@@ -361,6 +395,8 @@ videopoll(dev_t dev, int events, struct proc *p)
 	struct video_softc *sc;
 	int error, revents = 0;
 
+	KERNEL_ASSERT_LOCKED();
+
 	if (unit >= video_cd.cd_ndevs ||
 	    (sc = video_cd.cd_devs[unit]) == NULL)
 		return (POLLERR);
@@ -368,6 +404,9 @@ videopoll(dev_t dev, int events, struct proc *p)
 	if (sc->sc_dying)
 		return (POLLERR);
 
+	if ((error = video_claim(sc, p->p_p)))
+		return (error);
+
 	DPRINTF(("%s: events=0x%x\n", __func__, events));
 
 	if (events & (POLLIN | POLLRDNORM)) {
@@ -405,6 +444,8 @@ videommap(dev_t dev, off_t off, int prot)
 	caddr_t p;
 	paddr_t pa;
 
+	KERNEL_ASSERT_LOCKED();
+
 	DPRINTF(("%s: off=%lld, prot=%d\n", __func__, off, prot));
 
 	unit = VIDEOUNIT(dev);
@@ -466,7 +507,9 @@ videokqfilter(dev_t dev, struct knote *kn)
 {
 	int unit = VIDEOUNIT(dev);
 	struct video_softc *sc;
-	int s;
+	int s, error;
+
+	KERNEL_ASSERT_LOCKED();
 
 	if (unit >= video_cd.cd_ndevs ||
 	    (sc = video_cd.cd_devs[unit]) == NULL)
@@ -484,6 +527,9 @@ videokqfilter(dev_t dev, struct knote *kn)
 		return (EINVAL);
 	}
 
+	if ((error = video_claim(sc, curproc->p_p)))
+		return (error);
+
 	/*
 	 * Start the stream in read() mode if not already started.  If
 	 * the user wanted mmap() mode, he should have called mmap()
@@ -540,6 +586,39 @@ video_intr(void *addr)
 }
 
 int
+video_stop(struct video_softc *sc)
+{
+	int error = 0;
+
+	DPRINTF(("%s: stream close\n", __func__));
+
+	if (sc->hw_if->close != NULL)
+		error = sc->hw_if->close(sc->hw_hdl);
+
+	sc->sc_vidmode = VIDMODE_NONE;
+	sc->sc_frames_ready = 0;
+	sc->sc_owner = NULL;
+
+	return (error);
+}
+
+int
+video_claim(struct video_softc *sc, struct process *p)
+{
+	if (sc->sc_owner != NULL && sc->sc_owner != p) {
+		DPRINTF(("%s: already owned=%p\n", __func__, sc->sc_owner));
+		return (EBUSY);
+	}
+
+	if (sc->sc_owner == NULL) {
+		sc->sc_owner = p;
+		DPRINTF(("%s: new owner=%p\n", __func__, sc->sc_owner));
+	}
+
+	return (0);
+}
+
+int
 videoprint(void *aux, const char *pnp)
 {
 	if (pnp != NULL)