Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1c3bb743 authored by Guennadi Liakhovetski's avatar Guennadi Liakhovetski Committed by Mauro Carvalho Chehab
Browse files

V4L/DVB (10083): soc-camera: unify locking, play nicer with videobuf locking



Move mutex from host drivers to camera device object, take into account
videobuf locking.

Signed-off-by: default avatarGuennadi Liakhovetski <lg@denx.de>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent bf507158
Loading
Loading
Loading
Loading
+6 −9
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@
#include <linux/version.h>
#include <linux/device.h>
#include <linux/platform_device.h>
#include <linux/mutex.h>
#include <linux/clk.h>

#include <media/v4l2-common.h>
@@ -164,8 +163,6 @@
			CICR0_PERRM | CICR0_QDM | CICR0_CDM | CICR0_SOFM | \
			CICR0_EOFM | CICR0_FOM)

static DEFINE_MUTEX(camera_lock);

/*
 * Structures
 */
@@ -813,16 +810,17 @@ static irqreturn_t pxa_camera_irq(int irq, void *data)
	return IRQ_HANDLED;
}

/* The following two functions absolutely depend on the fact, that
 * there can be only one camera on PXA quick capture interface */
/*
 * The following two functions absolutely depend on the fact, that
 * there can be only one camera on PXA quick capture interface
 * Called with .video_lock held
 */
static int pxa_camera_add_device(struct soc_camera_device *icd)
{
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
	struct pxa_camera_dev *pcdev = ici->priv;
	int ret;

	mutex_lock(&camera_lock);

	if (pcdev->icd) {
		ret = -EBUSY;
		goto ebusy;
@@ -838,11 +836,10 @@ static int pxa_camera_add_device(struct soc_camera_device *icd)
		pcdev->icd = icd;

ebusy:
	mutex_unlock(&camera_lock);

	return ret;
}

/* Called with .video_lock held */
static void pxa_camera_remove_device(struct soc_camera_device *icd)
{
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+2 −7
Original line number Diff line number Diff line
@@ -29,7 +29,6 @@
#include <linux/version.h>
#include <linux/device.h>
#include <linux/platform_device.h>
#include <linux/mutex.h>
#include <linux/videodev2.h>
#include <linux/clk.h>

@@ -75,8 +74,6 @@
#define CDBYR2 0x98 /* Capture data bottom-field address Y register 2 */
#define CDBCR2 0x9c /* Capture data bottom-field address C register 2 */

static DEFINE_MUTEX(camera_lock);

/* per video frame buffer */
struct sh_mobile_ceu_buffer {
	struct videobuf_buffer vb; /* v4l buffer must be first */
@@ -292,14 +289,13 @@ static irqreturn_t sh_mobile_ceu_irq(int irq, void *data)
	return IRQ_HANDLED;
}

/* Called with .video_lock held */
static int sh_mobile_ceu_add_device(struct soc_camera_device *icd)
{
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
	struct sh_mobile_ceu_dev *pcdev = ici->priv;
	int ret = -EBUSY;

	mutex_lock(&camera_lock);

	if (pcdev->icd)
		goto err;

@@ -319,11 +315,10 @@ static int sh_mobile_ceu_add_device(struct soc_camera_device *icd)

	pcdev->icd = icd;
err:
	mutex_unlock(&camera_lock);

	return ret;
}

/* Called with .video_lock held */
static void sh_mobile_ceu_remove_device(struct soc_camera_device *icd)
{
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+83 −16
Original line number Diff line number Diff line
@@ -33,7 +33,6 @@
static LIST_HEAD(hosts);
static LIST_HEAD(devices);
static DEFINE_MUTEX(list_lock);
static DEFINE_MUTEX(video_lock);

const struct soc_camera_data_format *soc_camera_format_by_fourcc(
	struct soc_camera_device *icd, unsigned int fourcc)
@@ -270,8 +269,10 @@ static int soc_camera_open(struct inode *inode, struct file *file)
	if (!icf)
		return -ENOMEM;

	/* Protect against icd->remove() until we module_get() both drivers. */
	mutex_lock(&video_lock);
	/*
	 * It is safe to dereference these pointers now as long as a user has
	 * the video device open - we are protected by the held cdev reference.
	 */

	vdev = video_devdata(file);
	icd = container_of(vdev->parent, struct soc_camera_device, dev);
@@ -289,6 +290,9 @@ static int soc_camera_open(struct inode *inode, struct file *file)
		goto emgi;
	}

	/* Protect against icd->remove() until we module_get() both drivers. */
	mutex_lock(&icd->video_lock);

	icf->icd = icd;
	icd->use_count++;

@@ -304,7 +308,7 @@ static int soc_camera_open(struct inode *inode, struct file *file)
		}
	}

	mutex_unlock(&video_lock);
	mutex_unlock(&icd->video_lock);

	file->private_data = icf;
	dev_dbg(&icd->dev, "camera device open\n");
@@ -313,16 +317,16 @@ static int soc_camera_open(struct inode *inode, struct file *file)

	return 0;

	/* All errors are entered with the video_lock held */
	/* First two errors are entered with the .video_lock held */
eiciadd:
	soc_camera_free_user_formats(icd);
eiufmt:
	icd->use_count--;
	mutex_unlock(&icd->video_lock);
	module_put(ici->ops->owner);
emgi:
	module_put(icd->ops->owner);
emgd:
	mutex_unlock(&video_lock);
	vfree(icf);
	return ret;
}
@@ -334,15 +338,16 @@ static int soc_camera_close(struct inode *inode, struct file *file)
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
	struct video_device *vdev = icd->vdev;

	mutex_lock(&video_lock);
	mutex_lock(&icd->video_lock);
	icd->use_count--;
	if (!icd->use_count) {
		ici->ops->remove(icd);
		soc_camera_free_user_formats(icd);
	}
	mutex_unlock(&icd->video_lock);

	module_put(icd->ops->owner);
	module_put(ici->ops->owner);
	mutex_unlock(&video_lock);

	vfree(icf);

@@ -424,18 +429,27 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
	if (ret < 0)
		return ret;

	mutex_lock(&icf->vb_vidq.vb_lock);

	if (videobuf_queue_is_busy(&icf->vb_vidq)) {
		dev_err(&icd->dev, "S_FMT denied: queue busy\n");
		ret = -EBUSY;
		goto unlock;
	}

	rect.left	= icd->x_current;
	rect.top	= icd->y_current;
	rect.width	= pix->width;
	rect.height	= pix->height;
	ret = ici->ops->set_fmt(icd, pix->pixelformat, &rect);
	if (ret < 0) {
		return ret;
		goto unlock;
	} else if (!icd->current_fmt ||
		   icd->current_fmt->fourcc != pixfmt) {
		dev_err(&ici->dev,
			"Host driver hasn't set up current format correctly!\n");
		return -EINVAL;
		ret = -EINVAL;
		goto unlock;
	}

	icd->width		= rect.width;
@@ -449,7 +463,12 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
		icd->width, icd->height);

	/* set physical bus parameters */
	return ici->ops->set_bus_param(icd, pixfmt);
	ret = ici->ops->set_bus_param(icd, pixfmt);

unlock:
	mutex_unlock(&icf->vb_vidq.vb_lock);

	return ret;
}

static int soc_camera_enum_fmt_vid_cap(struct file *file, void  *priv,
@@ -510,6 +529,7 @@ static int soc_camera_streamon(struct file *file, void *priv,
{
	struct soc_camera_file *icf = file->private_data;
	struct soc_camera_device *icd = icf->icd;
	int ret;

	WARN_ON(priv != file->private_data);

@@ -518,10 +538,16 @@ static int soc_camera_streamon(struct file *file, void *priv,
	if (i != V4L2_BUF_TYPE_VIDEO_CAPTURE)
		return -EINVAL;

	mutex_lock(&icd->video_lock);

	icd->ops->start_capture(icd);

	/* This calls buf_queue from host driver's videobuf_queue_ops */
	return videobuf_streamon(&icf->vb_vidq);
	ret = videobuf_streamon(&icf->vb_vidq);

	mutex_unlock(&icd->video_lock);

	return ret;
}

static int soc_camera_streamoff(struct file *file, void *priv,
@@ -537,12 +563,16 @@ static int soc_camera_streamoff(struct file *file, void *priv,
	if (i != V4L2_BUF_TYPE_VIDEO_CAPTURE)
		return -EINVAL;

	mutex_lock(&icd->video_lock);

	/* This calls buf_release from host driver's videobuf_queue_ops for all
	 * remaining buffers. When the last buffer is freed, stop capture */
	videobuf_streamoff(&icf->vb_vidq);

	icd->ops->stop_capture(icd);

	mutex_unlock(&icd->video_lock);

	return 0;
}

@@ -654,6 +684,9 @@ static int soc_camera_s_crop(struct file *file, void *fh,
	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
		return -EINVAL;

	/* Cropping is allowed during a running capture, guard consistency */
	mutex_lock(&icf->vb_vidq.vb_lock);

	ret = ici->ops->set_fmt(icd, 0, &a->c);
	if (!ret) {
		icd->width	= a->c.width;
@@ -662,6 +695,8 @@ static int soc_camera_s_crop(struct file *file, void *fh,
		icd->y_current	= a->c.top;
	}

	mutex_unlock(&icf->vb_vidq.vb_lock);

	return ret;
}

@@ -775,11 +810,32 @@ static int soc_camera_probe(struct device *dev)
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
	int ret;

	/*
	 * Possible race scenario:
	 * modprobe <camera-host-driver> triggers __func__
	 * at this moment respective <camera-sensor-driver> gets rmmod'ed
	 * to protect take module references.
	 */

	if (!try_module_get(icd->ops->owner)) {
		dev_err(&icd->dev, "Couldn't lock sensor driver.\n");
		ret = -EINVAL;
		goto emgd;
	}

	if (!try_module_get(ici->ops->owner)) {
		dev_err(&icd->dev, "Couldn't lock capture bus driver.\n");
		ret = -EINVAL;
		goto emgi;
	}

	mutex_lock(&icd->video_lock);

	/* We only call ->add() here to activate and probe the camera.
	 * We shall ->remove() and deactivate it immediately afterwards. */
	ret = ici->ops->add(icd);
	if (ret < 0)
		return ret;
		goto eiadd;

	ret = icd->ops->probe(icd);
	if (ret >= 0) {
@@ -793,6 +849,12 @@ static int soc_camera_probe(struct device *dev)
	}
	ici->ops->remove(icd);

eiadd:
	mutex_unlock(&icd->video_lock);
	module_put(ici->ops->owner);
emgi:
	module_put(icd->ops->owner);
emgd:
	return ret;
}

@@ -966,6 +1028,7 @@ int soc_camera_device_register(struct soc_camera_device *icd)
	icd->dev.release	= dummy_release;
	icd->use_count		= 0;
	icd->host_priv		= NULL;
	mutex_init(&icd->video_lock);

	return scan_add_device(icd);
}
@@ -1012,6 +1075,10 @@ static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = {
#endif
};

/*
 * Usually called from the struct soc_camera_ops .probe() method, i.e., from
 * soc_camera_probe() above with .video_lock held
 */
int soc_camera_video_start(struct soc_camera_device *icd)
{
	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
@@ -1027,7 +1094,7 @@ int soc_camera_video_start(struct soc_camera_device *icd)
	dev_dbg(&ici->dev, "Allocated video_device %p\n", vdev);

	strlcpy(vdev->name, ici->drv_name, sizeof(vdev->name));
	/* Maybe better &ici->dev */

	vdev->parent		= &icd->dev;
	vdev->current_norm	= V4L2_STD_UNKNOWN;
	vdev->fops		= &soc_camera_fops;
@@ -1061,10 +1128,10 @@ void soc_camera_video_stop(struct soc_camera_device *icd)
	if (!icd->dev.parent || !vdev)
		return;

	mutex_lock(&video_lock);
	mutex_lock(&icd->video_lock);
	video_unregister_device(vdev);
	icd->vdev = NULL;
	mutex_unlock(&video_lock);
	mutex_unlock(&icd->video_lock);
}
EXPORT_SYMBOL(soc_camera_video_stop);

+5 −3
Original line number Diff line number Diff line
@@ -12,9 +12,10 @@
#ifndef SOC_CAMERA_H
#define SOC_CAMERA_H

#include <linux/mutex.h>
#include <linux/pm.h>
#include <linux/videodev2.h>
#include <media/videobuf-core.h>
#include <linux/pm.h>

struct soc_camera_device {
	struct list_head list;
@@ -45,9 +46,10 @@ struct soc_camera_device {
	struct soc_camera_format_xlate *user_formats;
	int num_user_formats;
	struct module *owner;
	void *host_priv;		/* per-device host private data */
	/* soc_camera.c private count. Only accessed with video_lock held */
	void *host_priv;		/* Per-device host private data */
	/* soc_camera.c private count. Only accessed with .video_lock held */
	int use_count;
	struct mutex video_lock;	/* Protects device data */
};

struct soc_camera_file {