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@

diff --git a/sys/dev/video.c b/sys/dev/video.c
index 49ac5b9..d1fde67 100644 (file)
--- 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()
@@ -539,6 +585,39 @@ video_intr(void *addr)
        selwakeup(&sc->sc_rsel);
 }
 
+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)
 {