Skip to content

Commit

Permalink
V4L/DVB (12744): em28xx: restructure fh/dev locking to handle both vi…
Browse files Browse the repository at this point in the history
…deo 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>
  • Loading branch information
Devin Heitmueller authored and Mauro Carvalho Chehab committed Sep 19, 2009
1 parent 91f6dce commit 8c873d3
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 90 deletions.
182 changes: 94 additions & 88 deletions drivers/media/video/em28xx/em28xx-video.c
Original file line number Diff line number Diff line change
Expand Up @@ -833,34 +833,63 @@ static void video_mux(struct em28xx *dev, int index)
}

/* Usage lock check functions */
static int res_get(struct em28xx_fh *fh)
static int res_get(struct em28xx_fh *fh, unsigned int bit)
{
struct em28xx *dev = fh->dev;
int rc = 0;

/* This instance already has stream_on */
if (fh->stream_on)
return rc;
if (fh->resources & bit)
/* have it already allocated */
return 1;

if (dev->stream_on)
return -EBUSY;
/* 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;
}

dev->stream_on = 1;
fh->stream_on = 1;
return rc;
static int res_check(struct em28xx_fh *fh, unsigned int bit)
{
return (fh->resources & bit);
}

static int res_check(struct em28xx_fh *fh)
static int res_locked(struct em28xx *dev, unsigned int bit)
{
return fh->stream_on;
return (dev->resources & bit);
}

static void res_free(struct em28xx_fh *fh)
static void res_free(struct em28xx_fh *fh, unsigned int bits)
{
struct em28xx *dev = fh->dev;

fh->stream_on = 0;
dev->stream_on = 0;
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;
}
}

/*
Expand Down Expand Up @@ -1103,12 +1132,6 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
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);

Expand Down Expand Up @@ -1668,24 +1691,25 @@ static int vidioc_streamon(struct file *file, void *priv,
{
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;
}
Expand All @@ -1707,16 +1731,16 @@ static int vidioc_streamoff(struct file *file, void *priv,
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;
}
Expand Down Expand Up @@ -2095,17 +2119,16 @@ static int em28xx_v4l2_open(struct file *filp)
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);

Expand Down Expand Up @@ -2162,20 +2185,21 @@ static int em28xx_v4l2_close(struct file *filp)

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;
}
Expand All @@ -2197,15 +2221,11 @@ static int em28xx_v4l2_close(struct file *filp)
}
}

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;
}

Expand All @@ -2230,22 +2250,17 @@ em28xx_v4l2_read(struct file *filp, char __user *buf, size_t count,
*/

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);
}


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);
Expand All @@ -2268,19 +2283,17 @@ static unsigned int em28xx_v4l2_poll(struct file *filp, poll_table *wait)
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;
}
}

/*
Expand All @@ -2296,13 +2309,6 @@ static int em28xx_v4l2_mmap(struct file *filp, struct vm_area_struct *vma)
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)
Expand Down
10 changes: 8 additions & 2 deletions drivers/media/video/em28xx/em28xx.h
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,10 @@ enum em28xx_dev_state {
#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];
Expand All @@ -464,8 +468,8 @@ struct em28xx;

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;
Expand Down Expand Up @@ -495,7 +499,6 @@ struct em28xx {
/* 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;

Expand Down Expand Up @@ -563,6 +566,9 @@ struct em28xx {
struct video_device *vbi_dev;
struct video_device *radio_dev;

/* resources in use */
unsigned int resources;

unsigned char eedata[256];

/* Isoc control struct */
Expand Down

0 comments on commit 8c873d3

Please sign in to comment.