Commit dd669e90 authored by Guennadi Liakhovetski's avatar Guennadi Liakhovetski Committed by Mauro Carvalho Chehab

[media] soc-camera: remove struct soc_camera_device::video_lock

Currently soc-camera has a per-device node lock, used for video operations
and a per-host lock for code paths, modifying host's pipeline. Manipulating
the two locks increases complexity and doesn't bring any advantages. This
patch removes the per-device lock and uses the per-host lock for all
operations.
Signed-off-by: default avatarGuennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent 8a97d4c1
...@@ -745,7 +745,7 @@ static int isi_camera_get_formats(struct soc_camera_device *icd, ...@@ -745,7 +745,7 @@ static int isi_camera_get_formats(struct soc_camera_device *icd,
return formats; return formats;
} }
/* Called with .video_lock held */ /* Called with .host_lock held */
static int isi_camera_add_device(struct soc_camera_device *icd) static int isi_camera_add_device(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
...@@ -770,7 +770,7 @@ static int isi_camera_add_device(struct soc_camera_device *icd) ...@@ -770,7 +770,7 @@ static int isi_camera_add_device(struct soc_camera_device *icd)
icd->devnum); icd->devnum);
return 0; return 0;
} }
/* Called with .video_lock held */ /* Called with .host_lock held */
static void isi_camera_remove_device(struct soc_camera_device *icd) static void isi_camera_remove_device(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
......
...@@ -26,7 +26,6 @@ ...@@ -26,7 +26,6 @@
#include <linux/mm.h> #include <linux/mm.h>
#include <linux/module.h> #include <linux/module.h>
#include <linux/moduleparam.h> #include <linux/moduleparam.h>
#include <linux/mutex.h>
#include <linux/platform_device.h> #include <linux/platform_device.h>
#include <linux/sched.h> #include <linux/sched.h>
#include <linux/slab.h> #include <linux/slab.h>
...@@ -373,7 +372,7 @@ static void mx1_camera_init_videobuf(struct videobuf_queue *q, ...@@ -373,7 +372,7 @@ static void mx1_camera_init_videobuf(struct videobuf_queue *q,
videobuf_queue_dma_contig_init(q, &mx1_videobuf_ops, icd->parent, videobuf_queue_dma_contig_init(q, &mx1_videobuf_ops, icd->parent,
&pcdev->lock, V4L2_BUF_TYPE_VIDEO_CAPTURE, &pcdev->lock, V4L2_BUF_TYPE_VIDEO_CAPTURE,
V4L2_FIELD_NONE, V4L2_FIELD_NONE,
sizeof(struct mx1_buffer), icd, &icd->video_lock); sizeof(struct mx1_buffer), icd, &icd->host_lock);
} }
static int mclk_get_divisor(struct mx1_camera_dev *pcdev) static int mclk_get_divisor(struct mx1_camera_dev *pcdev)
......
...@@ -28,7 +28,6 @@ ...@@ -28,7 +28,6 @@
#include <linux/time.h> #include <linux/time.h>
#include <linux/device.h> #include <linux/device.h>
#include <linux/platform_device.h> #include <linux/platform_device.h>
#include <linux/mutex.h>
#include <linux/clk.h> #include <linux/clk.h>
#include <media/v4l2-common.h> #include <media/v4l2-common.h>
......
...@@ -510,7 +510,7 @@ static void mx3_camera_activate(struct mx3_camera_dev *mx3_cam, ...@@ -510,7 +510,7 @@ static void mx3_camera_activate(struct mx3_camera_dev *mx3_cam,
clk_set_rate(mx3_cam->clk, rate); clk_set_rate(mx3_cam->clk, rate);
} }
/* Called with .video_lock held */ /* Called with .host_lock held */
static int mx3_camera_add_device(struct soc_camera_device *icd) static int mx3_camera_add_device(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
...@@ -530,7 +530,7 @@ static int mx3_camera_add_device(struct soc_camera_device *icd) ...@@ -530,7 +530,7 @@ static int mx3_camera_add_device(struct soc_camera_device *icd)
return 0; return 0;
} }
/* Called with .video_lock held */ /* Called with .host_lock held */
static void mx3_camera_remove_device(struct soc_camera_device *icd) static void mx3_camera_remove_device(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
......
...@@ -1383,12 +1383,12 @@ static void omap1_cam_init_videobuf(struct videobuf_queue *q, ...@@ -1383,12 +1383,12 @@ static void omap1_cam_init_videobuf(struct videobuf_queue *q,
videobuf_queue_dma_contig_init(q, &omap1_videobuf_ops, videobuf_queue_dma_contig_init(q, &omap1_videobuf_ops,
icd->parent, &pcdev->lock, icd->parent, &pcdev->lock,
V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_NONE, V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_NONE,
sizeof(struct omap1_cam_buf), icd, &icd->video_lock); sizeof(struct omap1_cam_buf), icd, &icd->host_lock);
else else
videobuf_queue_sg_init(q, &omap1_videobuf_ops, videobuf_queue_sg_init(q, &omap1_videobuf_ops,
icd->parent, &pcdev->lock, icd->parent, &pcdev->lock,
V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_NONE, V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_NONE,
sizeof(struct omap1_cam_buf), icd, &icd->video_lock); sizeof(struct omap1_cam_buf), icd, &icd->host_lock);
/* use videobuf mode (auto)selected with the module parameter */ /* use videobuf mode (auto)selected with the module parameter */
pcdev->vb_mode = sg_mode ? OMAP1_CAM_DMA_SG : OMAP1_CAM_DMA_CONTIG; pcdev->vb_mode = sg_mode ? OMAP1_CAM_DMA_SG : OMAP1_CAM_DMA_CONTIG;
......
...@@ -842,7 +842,7 @@ static void pxa_camera_init_videobuf(struct videobuf_queue *q, ...@@ -842,7 +842,7 @@ static void pxa_camera_init_videobuf(struct videobuf_queue *q,
*/ */
videobuf_queue_sg_init(q, &pxa_videobuf_ops, NULL, &pcdev->lock, videobuf_queue_sg_init(q, &pxa_videobuf_ops, NULL, &pcdev->lock,
V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_NONE, V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_FIELD_NONE,
sizeof(struct pxa_buffer), icd, &icd->video_lock); sizeof(struct pxa_buffer), icd, &icd->host_lock);
} }
static u32 mclk_get_divisor(struct platform_device *pdev, static u32 mclk_get_divisor(struct platform_device *pdev,
...@@ -958,7 +958,7 @@ static irqreturn_t pxa_camera_irq(int irq, void *data) ...@@ -958,7 +958,7 @@ static irqreturn_t pxa_camera_irq(int irq, void *data)
/* /*
* The following two functions absolutely depend on the fact, that * The following two functions absolutely depend on the fact, that
* there can be only one camera on PXA quick capture interface * there can be only one camera on PXA quick capture interface
* Called with .video_lock held * Called with .host_lock held
*/ */
static int pxa_camera_add_device(struct soc_camera_device *icd) static int pxa_camera_add_device(struct soc_camera_device *icd)
{ {
...@@ -978,7 +978,7 @@ static int pxa_camera_add_device(struct soc_camera_device *icd) ...@@ -978,7 +978,7 @@ static int pxa_camera_add_device(struct soc_camera_device *icd)
return 0; return 0;
} }
/* Called with .video_lock held */ /* Called with .host_lock held */
static void pxa_camera_remove_device(struct soc_camera_device *icd) static void pxa_camera_remove_device(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
......
...@@ -543,7 +543,7 @@ static struct v4l2_subdev *find_csi2(struct sh_mobile_ceu_dev *pcdev) ...@@ -543,7 +543,7 @@ static struct v4l2_subdev *find_csi2(struct sh_mobile_ceu_dev *pcdev)
return NULL; return NULL;
} }
/* Called with .video_lock held */ /* Called with .host_lock held */
static int sh_mobile_ceu_add_device(struct soc_camera_device *icd) static int sh_mobile_ceu_add_device(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
...@@ -587,7 +587,7 @@ static int sh_mobile_ceu_add_device(struct soc_camera_device *icd) ...@@ -587,7 +587,7 @@ static int sh_mobile_ceu_add_device(struct soc_camera_device *icd)
return 0; return 0;
} }
/* Called with .video_lock held */ /* Called with .host_lock held */
static void sh_mobile_ceu_remove_device(struct soc_camera_device *icd) static void sh_mobile_ceu_remove_device(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
......
...@@ -383,7 +383,7 @@ static int soc_camera_prepare_buf(struct file *file, void *priv, ...@@ -383,7 +383,7 @@ static int soc_camera_prepare_buf(struct file *file, void *priv,
return vb2_prepare_buf(&icd->vb2_vidq, b); return vb2_prepare_buf(&icd->vb2_vidq, b);
} }
/* Always entered with .video_lock held */ /* Always entered with .host_lock held */
static int soc_camera_init_user_formats(struct soc_camera_device *icd) static int soc_camera_init_user_formats(struct soc_camera_device *icd)
{ {
struct v4l2_subdev *sd = soc_camera_to_subdev(icd); struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
...@@ -450,7 +450,7 @@ static int soc_camera_init_user_formats(struct soc_camera_device *icd) ...@@ -450,7 +450,7 @@ static int soc_camera_init_user_formats(struct soc_camera_device *icd)
return ret; return ret;
} }
/* Always entered with .video_lock held */ /* Always entered with .host_lock held */
static void soc_camera_free_user_formats(struct soc_camera_device *icd) static void soc_camera_free_user_formats(struct soc_camera_device *icd)
{ {
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
...@@ -526,7 +526,7 @@ static int soc_camera_open(struct file *file) ...@@ -526,7 +526,7 @@ static int soc_camera_open(struct file *file)
ici = to_soc_camera_host(icd->parent); ici = to_soc_camera_host(icd->parent);
mutex_unlock(&list_lock); mutex_unlock(&list_lock);
if (mutex_lock_interruptible(&icd->video_lock)) if (mutex_lock_interruptible(&ici->host_lock))
return -ERESTARTSYS; return -ERESTARTSYS;
if (!try_module_get(ici->ops->owner)) { if (!try_module_get(ici->ops->owner)) {
dev_err(icd->pdev, "Couldn't lock capture bus driver.\n"); dev_err(icd->pdev, "Couldn't lock capture bus driver.\n");
...@@ -555,9 +555,7 @@ static int soc_camera_open(struct file *file) ...@@ -555,9 +555,7 @@ static int soc_camera_open(struct file *file)
if (icl->reset) if (icl->reset)
icl->reset(icd->pdev); icl->reset(icd->pdev);
mutex_lock(&ici->host_lock);
ret = ici->ops->add(icd); ret = ici->ops->add(icd);
mutex_unlock(&ici->host_lock);
if (ret < 0) { if (ret < 0) {
dev_err(icd->pdev, "Couldn't activate the camera: %d\n", ret); dev_err(icd->pdev, "Couldn't activate the camera: %d\n", ret);
goto eiciadd; goto eiciadd;
...@@ -576,7 +574,7 @@ static int soc_camera_open(struct file *file) ...@@ -576,7 +574,7 @@ static int soc_camera_open(struct file *file)
* Try to configure with default parameters. Notice: this is the * Try to configure with default parameters. Notice: this is the
* very first open, so, we cannot race against other calls, * very first open, so, we cannot race against other calls,
* apart from someone else calling open() simultaneously, but * apart from someone else calling open() simultaneously, but
* .video_lock is protecting us against it. * .host_lock is protecting us against it.
*/ */
ret = soc_camera_set_fmt(icd, &f); ret = soc_camera_set_fmt(icd, &f);
if (ret < 0) if (ret < 0)
...@@ -591,7 +589,7 @@ static int soc_camera_open(struct file *file) ...@@ -591,7 +589,7 @@ static int soc_camera_open(struct file *file)
} }
v4l2_ctrl_handler_setup(&icd->ctrl_handler); v4l2_ctrl_handler_setup(&icd->ctrl_handler);
} }
mutex_unlock(&icd->video_lock); mutex_unlock(&ici->host_lock);
file->private_data = icd; file->private_data = icd;
dev_dbg(icd->pdev, "camera device open\n"); dev_dbg(icd->pdev, "camera device open\n");
...@@ -599,7 +597,7 @@ static int soc_camera_open(struct file *file) ...@@ -599,7 +597,7 @@ static int soc_camera_open(struct file *file)
return 0; return 0;
/* /*
* First four errors are entered with the .video_lock held * First four errors are entered with the .host_lock held
* and use_count == 1 * and use_count == 1
*/ */
einitvb: einitvb:
...@@ -608,14 +606,12 @@ static int soc_camera_open(struct file *file) ...@@ -608,14 +606,12 @@ static int soc_camera_open(struct file *file)
eresume: eresume:
__soc_camera_power_off(icd); __soc_camera_power_off(icd);
epower: epower:
mutex_lock(&ici->host_lock);
ici->ops->remove(icd); ici->ops->remove(icd);
mutex_unlock(&ici->host_lock);
eiciadd: eiciadd:
icd->use_count--; icd->use_count--;
module_put(ici->ops->owner); module_put(ici->ops->owner);
emodule: emodule:
mutex_unlock(&icd->video_lock); mutex_unlock(&ici->host_lock);
return ret; return ret;
} }
...@@ -625,7 +621,7 @@ static int soc_camera_close(struct file *file) ...@@ -625,7 +621,7 @@ static int soc_camera_close(struct file *file)
struct soc_camera_device *icd = file->private_data; struct soc_camera_device *icd = file->private_data;
struct soc_camera_host *ici = to_soc_camera_host(icd->parent); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
mutex_lock(&icd->video_lock); mutex_lock(&ici->host_lock);
icd->use_count--; icd->use_count--;
if (!icd->use_count) { if (!icd->use_count) {
pm_runtime_suspend(&icd->vdev->dev); pm_runtime_suspend(&icd->vdev->dev);
...@@ -633,16 +629,14 @@ static int soc_camera_close(struct file *file) ...@@ -633,16 +629,14 @@ static int soc_camera_close(struct file *file)
if (ici->ops->init_videobuf2) if (ici->ops->init_videobuf2)
vb2_queue_release(&icd->vb2_vidq); vb2_queue_release(&icd->vb2_vidq);
mutex_lock(&ici->host_lock);
ici->ops->remove(icd); ici->ops->remove(icd);
mutex_unlock(&ici->host_lock);
__soc_camera_power_off(icd); __soc_camera_power_off(icd);
} }
if (icd->streamer == file) if (icd->streamer == file)
icd->streamer = NULL; icd->streamer = NULL;
mutex_unlock(&icd->video_lock); mutex_unlock(&ici->host_lock);
module_put(ici->ops->owner); module_put(ici->ops->owner);
...@@ -679,13 +673,13 @@ static int soc_camera_mmap(struct file *file, struct vm_area_struct *vma) ...@@ -679,13 +673,13 @@ static int soc_camera_mmap(struct file *file, struct vm_area_struct *vma)
if (icd->streamer != file) if (icd->streamer != file)
return -EBUSY; return -EBUSY;
if (mutex_lock_interruptible(&icd->video_lock)) if (mutex_lock_interruptible(&ici->host_lock))
return -ERESTARTSYS; return -ERESTARTSYS;
if (ici->ops->init_videobuf) if (ici->ops->init_videobuf)
err = videobuf_mmap_mapper(&icd->vb_vidq, vma); err = videobuf_mmap_mapper(&icd->vb_vidq, vma);
else else
err = vb2_mmap(&icd->vb2_vidq, vma); err = vb2_mmap(&icd->vb2_vidq, vma);
mutex_unlock(&icd->video_lock); mutex_unlock(&ici->host_lock);
dev_dbg(icd->pdev, "vma start=0x%08lx, size=%ld, ret=%d\n", dev_dbg(icd->pdev, "vma start=0x%08lx, size=%ld, ret=%d\n",
(unsigned long)vma->vm_start, (unsigned long)vma->vm_start,
...@@ -704,26 +698,28 @@ static unsigned int soc_camera_poll(struct file *file, poll_table *pt) ...@@ -704,26 +698,28 @@ static unsigned int soc_camera_poll(struct file *file, poll_table *pt)
if (icd->streamer != file) if (icd->streamer != file)
return POLLERR; return POLLERR;
mutex_lock(&icd->video_lock); mutex_lock(&ici->host_lock);
if (ici->ops->init_videobuf && list_empty(&icd->vb_vidq.stream)) if (ici->ops->init_videobuf && list_empty(&icd->vb_vidq.stream))
dev_err(icd->pdev, "Trying to poll with no queued buffers!\n"); dev_err(icd->pdev, "Trying to poll with no queued buffers!\n");
else else
res = ici->ops->poll(file, pt); res = ici->ops->poll(file, pt);
mutex_unlock(&icd->video_lock); mutex_unlock(&ici->host_lock);
return res; return res;
} }
void soc_camera_lock(struct vb2_queue *vq) void soc_camera_lock(struct vb2_queue *vq)
{ {
struct soc_camera_device *icd = vb2_get_drv_priv(vq); struct soc_camera_device *icd = vb2_get_drv_priv(vq);
mutex_lock(&icd->video_lock); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
mutex_lock(&ici->host_lock);
} }
EXPORT_SYMBOL(soc_camera_lock); EXPORT_SYMBOL(soc_camera_lock);
void soc_camera_unlock(struct vb2_queue *vq) void soc_camera_unlock(struct vb2_queue *vq)
{ {
struct soc_camera_device *icd = vb2_get_drv_priv(vq); struct soc_camera_device *icd = vb2_get_drv_priv(vq);
mutex_unlock(&icd->video_lock); struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
mutex_unlock(&ici->host_lock);
} }
EXPORT_SYMBOL(soc_camera_unlock); EXPORT_SYMBOL(soc_camera_unlock);
...@@ -1213,7 +1209,7 @@ static int soc_camera_probe(struct soc_camera_device *icd) ...@@ -1213,7 +1209,7 @@ static int soc_camera_probe(struct soc_camera_device *icd)
* itself is protected against concurrent open() calls, but we also have * itself is protected against concurrent open() calls, but we also have
* to protect our data. * to protect our data.
*/ */
mutex_lock(&icd->video_lock); mutex_lock(&ici->host_lock);
ret = soc_camera_video_start(icd); ret = soc_camera_video_start(icd);
if (ret < 0) if (ret < 0)
...@@ -1227,16 +1223,14 @@ static int soc_camera_probe(struct soc_camera_device *icd) ...@@ -1227,16 +1223,14 @@ static int soc_camera_probe(struct soc_camera_device *icd)
icd->field = mf.field; icd->field = mf.field;
} }
mutex_lock(&ici->host_lock);
ici->ops->remove(icd); ici->ops->remove(icd);
mutex_unlock(&ici->host_lock);
mutex_unlock(&icd->video_lock); mutex_unlock(&ici->host_lock);
return 0; return 0;
evidstart: evidstart:
mutex_unlock(&icd->video_lock); mutex_unlock(&ici->host_lock);
soc_camera_free_user_formats(icd); soc_camera_free_user_formats(icd);
eiufmt: eiufmt:
ectrl: ectrl:
...@@ -1451,7 +1445,6 @@ static int soc_camera_device_register(struct soc_camera_device *icd) ...@@ -1451,7 +1445,6 @@ static int soc_camera_device_register(struct soc_camera_device *icd)
icd->devnum = num; icd->devnum = num;
icd->use_count = 0; icd->use_count = 0;
icd->host_priv = NULL; icd->host_priv = NULL;
mutex_init(&icd->video_lock);
list_add_tail(&icd->list, &devices); list_add_tail(&icd->list, &devices);
...@@ -1509,7 +1502,7 @@ static int video_dev_create(struct soc_camera_device *icd) ...@@ -1509,7 +1502,7 @@ static int video_dev_create(struct soc_camera_device *icd)
vdev->release = video_device_release; vdev->release = video_device_release;
vdev->tvnorms = V4L2_STD_UNKNOWN; vdev->tvnorms = V4L2_STD_UNKNOWN;
vdev->ctrl_handler = &icd->ctrl_handler; vdev->ctrl_handler = &icd->ctrl_handler;
vdev->lock = &icd->video_lock; vdev->lock = &ici->host_lock;
icd->vdev = vdev; icd->vdev = vdev;
...@@ -1517,7 +1510,7 @@ static int video_dev_create(struct soc_camera_device *icd) ...@@ -1517,7 +1510,7 @@ static int video_dev_create(struct soc_camera_device *icd)
} }
/* /*
* Called from soc_camera_probe() above (with .video_lock held???) * Called from soc_camera_probe() above with .host_lock held
*/ */
static int soc_camera_video_start(struct soc_camera_device *icd) static int soc_camera_video_start(struct soc_camera_device *icd)
{ {
......
...@@ -46,9 +46,8 @@ struct soc_camera_device { ...@@ -46,9 +46,8 @@ struct soc_camera_device {
int num_user_formats; int num_user_formats;
enum v4l2_field field; /* Preserve field over close() */ enum v4l2_field field; /* Preserve field over close() */
void *host_priv; /* Per-device host private data */ void *host_priv; /* Per-device host private data */
/* soc_camera.c private count. Only accessed with .video_lock held */ /* soc_camera.c private count. Only accessed with .host_lock held */
int use_count; int use_count;
struct mutex video_lock; /* Protects device data */
struct file *streamer; /* stream owner */ struct file *streamer; /* stream owner */
union { union {
struct videobuf_queue vb_vidq; struct videobuf_queue vb_vidq;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment