V4L/DVB (12744): em28xx: restructure fh/dev locking to handle both video and vbi
The current locking infrastructure didn't support having multiple fds accessing
the device (such as video and vbi). Rework the locking infrastructure,
borrowing the design from cx88.
This work was sponsored by EyeMagnet Limited.
Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c
index 55bc8ea..dda4a76 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -833,34 +833,63 @@
}
/* Usage lock check functions */
-static int res_get(struct em28xx_fh *fh)
-{
- struct em28xx *dev = fh->dev;
- int rc = 0;
-
- /* This instance already has stream_on */
- if (fh->stream_on)
- return rc;
-
- if (dev->stream_on)
- return -EBUSY;
-
- dev->stream_on = 1;
- fh->stream_on = 1;
- return rc;
-}
-
-static int res_check(struct em28xx_fh *fh)
-{
- return fh->stream_on;
-}
-
-static void res_free(struct em28xx_fh *fh)
+static int res_get(struct em28xx_fh *fh, unsigned int bit)
{
struct em28xx *dev = fh->dev;
- fh->stream_on = 0;
- dev->stream_on = 0;
+ if (fh->resources & bit)
+ /* have it already allocated */
+ return 1;
+
+ /* is it free? */
+ mutex_lock(&dev->lock);
+ if (dev->resources & bit) {
+ /* no, someone else uses it */
+ mutex_unlock(&dev->lock);
+ return 0;
+ }
+ /* it's free, grab it */
+ fh->resources |= bit;
+ dev->resources |= bit;
+ em28xx_videodbg("res: get %d\n", bit);
+ mutex_unlock(&dev->lock);
+ return 1;
+}
+
+static int res_check(struct em28xx_fh *fh, unsigned int bit)
+{
+ return (fh->resources & bit);
+}
+
+static int res_locked(struct em28xx *dev, unsigned int bit)
+{
+ return (dev->resources & bit);
+}
+
+static void res_free(struct em28xx_fh *fh, unsigned int bits)
+{
+ struct em28xx *dev = fh->dev;
+
+ BUG_ON((fh->resources & bits) != bits);
+
+ mutex_lock(&dev->lock);
+ fh->resources &= ~bits;
+ dev->resources &= ~bits;
+ em28xx_videodbg("res: put %d\n", bits);
+ mutex_unlock(&dev->lock);
+}
+
+static int get_ressource(struct em28xx_fh *fh)
+{
+ switch (fh->type) {
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+ return EM28XX_RESOURCE_VIDEO;
+ case V4L2_BUF_TYPE_VBI_CAPTURE:
+ return EM28XX_RESOURCE_VBI;
+ default:
+ BUG();
+ return 0;
+ }
}
/*
@@ -1103,12 +1132,6 @@
goto out;
}
- if (dev->stream_on && !fh->stream_on) {
- em28xx_errdev("%s device in use by another fh\n", __func__);
- rc = -EBUSY;
- goto out;
- }
-
rc = em28xx_set_video_format(dev, f->fmt.pix.pixelformat,
f->fmt.pix.width, f->fmt.pix.height);
@@ -1668,24 +1691,25 @@
{
struct em28xx_fh *fh = priv;
struct em28xx *dev = fh->dev;
- int rc;
+ int rc = -EINVAL;
rc = check_dev(dev);
if (rc < 0)
return rc;
+ if (unlikely(type != fh->type))
+ return -EINVAL;
- mutex_lock(&dev->lock);
- rc = res_get(fh);
+ em28xx_videodbg("vidioc_streamon fh=%p t=%d fh->res=%d dev->res=%d\n",
+ fh, type, fh->resources, dev->resources);
- if (likely(rc >= 0)) {
- if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
- rc = videobuf_streamon(&fh->vb_vidq);
- else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
- rc = videobuf_streamon(&fh->vb_vbiq);
- }
+ if (unlikely(!res_get(fh,get_ressource(fh))))
+ return -EBUSY;
- mutex_unlock(&dev->lock);
+ if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ rc = videobuf_streamon(&fh->vb_vidq);
+ else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
+ rc = videobuf_streamon(&fh->vb_vbiq);
return rc;
}
@@ -1707,16 +1731,16 @@
if (type != fh->type)
return -EINVAL;
- mutex_lock(&dev->lock);
+ em28xx_videodbg("vidioc_streamoff fh=%p t=%d fh->res=%d dev->res=%d\n",
+ fh, type, fh->resources, dev->resources);
- if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
videobuf_streamoff(&fh->vb_vidq);
- else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
+ res_free(fh, EM28XX_RESOURCE_VIDEO);
+ } else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
videobuf_streamoff(&fh->vb_vbiq);
-
- res_free(fh);
-
- mutex_unlock(&dev->lock);
+ res_free(fh, EM28XX_RESOURCE_VBI);
+ }
return 0;
}
@@ -2095,17 +2119,16 @@
else
field = V4L2_FIELD_INTERLACED;
- if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
- videobuf_queue_vmalloc_init(&fh->vb_vidq, &em28xx_video_qops,
- NULL, &dev->slock, fh->type, field,
- sizeof(struct em28xx_buffer), fh);
+ videobuf_queue_vmalloc_init(&fh->vb_vidq, &em28xx_video_qops,
+ NULL, &dev->slock,
+ V4L2_BUF_TYPE_VIDEO_CAPTURE, field,
+ sizeof(struct em28xx_buffer), fh);
- if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
- videobuf_queue_vmalloc_init(&fh->vb_vbiq, &em28xx_vbi_qops,
- NULL, &dev->slock,
- V4L2_BUF_TYPE_VBI_CAPTURE,
- V4L2_FIELD_SEQ_TB,
- sizeof(struct em28xx_buffer), fh);
+ videobuf_queue_vmalloc_init(&fh->vb_vbiq, &em28xx_vbi_qops,
+ NULL, &dev->slock,
+ V4L2_BUF_TYPE_VBI_CAPTURE,
+ V4L2_FIELD_SEQ_TB,
+ sizeof(struct em28xx_buffer), fh);
mutex_unlock(&dev->lock);
@@ -2162,20 +2185,21 @@
em28xx_videodbg("users=%d\n", dev->users);
-
- mutex_lock(&dev->lock);
- if (res_check(fh))
- res_free(fh);
-
- if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && dev->users == 1) {
+ if (res_check(fh, EM28XX_RESOURCE_VIDEO)) {
videobuf_stop(&fh->vb_vidq);
- videobuf_mmap_free(&fh->vb_vidq);
+ res_free(fh, EM28XX_RESOURCE_VIDEO);
+ }
+ if (res_check(fh, EM28XX_RESOURCE_VBI)) {
+ videobuf_stop(&fh->vb_vbiq);
+ res_free(fh, EM28XX_RESOURCE_VBI);
+ }
+
+ if(dev->users == 1) {
/* the device is already disconnect,
free the remaining resources */
if (dev->state & DEV_DISCONNECTED) {
em28xx_release_resources(dev);
- mutex_unlock(&dev->lock);
kfree(dev);
return 0;
}
@@ -2197,15 +2221,11 @@
}
}
- if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
- videobuf_stop(&fh->vb_vbiq);
- videobuf_mmap_free(&fh->vb_vbiq);
- }
-
+ videobuf_mmap_free(&fh->vb_vidq);
+ videobuf_mmap_free(&fh->vb_vbiq);
kfree(fh);
dev->users--;
wake_up_interruptible_nr(&dev->open, 1);
- mutex_unlock(&dev->lock);
return 0;
}
@@ -2230,12 +2250,8 @@
*/
if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
- mutex_lock(&dev->lock);
- rc = res_get(fh);
- mutex_unlock(&dev->lock);
-
- if (unlikely(rc < 0))
- return rc;
+ if (res_locked(dev, EM28XX_RESOURCE_VIDEO))
+ return -EBUSY;
return videobuf_read_stream(&fh->vb_vidq, buf, count, pos, 0,
filp->f_flags & O_NONBLOCK);
@@ -2243,9 +2259,8 @@
if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
- mutex_lock(&dev->lock);
- rc = res_get(fh);
- mutex_unlock(&dev->lock);
+ if (!res_get(fh, EM28XX_RESOURCE_VBI))
+ return -EBUSY;
return videobuf_read_stream(&fh->vb_vbiq, buf, count, pos, 0,
filp->f_flags & O_NONBLOCK);
@@ -2268,19 +2283,17 @@
if (rc < 0)
return rc;
- mutex_lock(&dev->lock);
- rc = res_get(fh);
- mutex_unlock(&dev->lock);
-
- if (unlikely(rc < 0))
- return POLLERR;
-
- if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+ if (!res_get(fh, EM28XX_RESOURCE_VIDEO))
+ return POLLERR;
return videobuf_poll_stream(filp, &fh->vb_vidq, wait);
- else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
+ } else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
+ if (!res_get(fh, EM28XX_RESOURCE_VBI))
+ return POLLERR;
return videobuf_poll_stream(filp, &fh->vb_vbiq, wait);
- else
+ } else {
return POLLERR;
+ }
}
/*
@@ -2296,13 +2309,6 @@
if (rc < 0)
return rc;
- mutex_lock(&dev->lock);
- rc = res_get(fh);
- mutex_unlock(&dev->lock);
-
- if (unlikely(rc < 0))
- return rc;
-
if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
rc = videobuf_mmap_mapper(&fh->vb_vidq, vma);
else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h
index 8dac50b..b4a6e072 100644
--- a/drivers/media/video/em28xx/em28xx.h
+++ b/drivers/media/video/em28xx/em28xx.h
@@ -444,6 +444,10 @@
#define EM28XX_AUDIO 0x10
#define EM28XX_DVB 0x20
+/* em28xx resource types (used for res_get/res_lock etc */
+#define EM28XX_RESOURCE_VIDEO 0x01
+#define EM28XX_RESOURCE_VBI 0x02
+
struct em28xx_audio {
char name[50];
char *transfer_buffer[EM28XX_AUDIO_BUFS];
@@ -464,8 +468,8 @@
struct em28xx_fh {
struct em28xx *dev;
- unsigned int stream_on:1; /* Locks streams */
int radio;
+ unsigned int resources;
struct videobuf_queue vb_vidq;
struct videobuf_queue vb_vbiq;
@@ -495,7 +499,6 @@
/* Vinmode/Vinctl used at the driver */
int vinmode, vinctl;
- unsigned int stream_on:1; /* Locks streams */
unsigned int has_audio_class:1;
unsigned int has_alsa_audio:1;
@@ -563,6 +566,9 @@
struct video_device *vbi_dev;
struct video_device *radio_dev;
+ /* resources in use */
+ unsigned int resources;
+
unsigned char eedata[256];
/* Isoc control struct */
