Commit 2468083f authored by Hans de Goede's avatar Hans de Goede Committed by Mauro Carvalho Chehab
Browse files

media: atomisp: Fix locking around asd->streaming read/write 



For reading / writing the asd->streaming enum the following rules
should be followed:

1. Writers of streaming must hold both isp->mutex and isp->lock.
2. Readers of streaming need to hold only one of the two locks.

Not all writers where properly taking both locks this fixes this.

In the case of the readers, many readers depend on their caller
to hold isp->mutex, add asserts for this

And in the case of atomisp_css_get_dis_stat() it is called with
isp->mutex held, so there is no need to take the spinlock just
for reading the streaming value.

Reviewed-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
parent d33a6d32

drivers/staging/media/atomisp/pci/atomisp_cmd.c

+30 −2
Original line number Diff line number Diff line
@@ -899,6 +899,8 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error, 
	struct v4l2_control ctrl;

	bool reset_wdt_timer = false;



	lockdep_assert_held(&isp->mutex);



	if (

	    buf_type != IA_CSS_BUFFER_TYPE_METADATA &&

	    buf_type != IA_CSS_BUFFER_TYPE_3A_STATISTICS &&
@@ -1298,6 +1300,9 @@ static void __atomisp_css_recover(struct atomisp_device *isp, bool isp_timeout) 
	bool stream_restart[MAX_STREAM_NUM] = {0};

	bool depth_mode = false;

	int i, ret, depth_cnt = 0;

	unsigned long flags;



	lockdep_assert_held(&isp->mutex);



	atomisp_css_irq_enable(isp, IA_CSS_IRQ_INFO_CSS_RECEIVER_SOF, false);
@@ -1320,7 +1325,9 @@ static void __atomisp_css_recover(struct atomisp_device *isp, bool isp_timeout) 


		stream_restart[asd->index] = true;



		spin_lock_irqsave(&isp->lock, flags);

		asd->streaming = ATOMISP_DEVICE_STREAMING_STOPPING;

		spin_unlock_irqrestore(&isp->lock, flags);



		/* stream off sensor */

		ret = v4l2_subdev_call(
@@ -1335,7 +1342,9 @@ static void __atomisp_css_recover(struct atomisp_device *isp, bool isp_timeout) 
		css_pipe_id = atomisp_get_css_pipe_id(asd);

		atomisp_css_stop(asd, css_pipe_id, true);



		spin_lock_irqsave(&isp->lock, flags);

		asd->streaming = ATOMISP_DEVICE_STREAMING_DISABLED;

		spin_unlock_irqrestore(&isp->lock, flags);



		asd->preview_exp_id = 1;

		asd->postview_exp_id = 1;
@@ -1376,11 +1385,14 @@ static void __atomisp_css_recover(struct atomisp_device *isp, bool isp_timeout) 
						   IA_CSS_INPUT_MODE_BUFFERED_SENSOR);



		css_pipe_id = atomisp_get_css_pipe_id(asd);

		if (atomisp_css_start(asd, css_pipe_id, true))

		if (atomisp_css_start(asd, css_pipe_id, true)) {

			dev_warn(isp->dev,

				 "start SP failed, so do not set streaming to be enable!\n");

		else

		} else {

			spin_lock_irqsave(&isp->lock, flags);

			asd->streaming = ATOMISP_DEVICE_STREAMING_ENABLED;

			spin_unlock_irqrestore(&isp->lock, flags);

		}



		atomisp_csi2_configure(asd);

	}
@@ -1608,6 +1620,8 @@ void atomisp_css_flush(struct atomisp_device *isp) 
{

	int i;



	lockdep_assert_held(&isp->mutex);



	if (!atomisp_streaming_count(isp))

		return;
@@ -4046,6 +4060,8 @@ void atomisp_handle_parameter_and_buffer(struct atomisp_video_pipe *pipe) 
	unsigned long irqflags;

	bool need_to_enqueue_buffer = false;



	lockdep_assert_held(&asd->isp->mutex);



	if (!asd) {

		dev_err(pipe->isp->dev, "%s(): asd is NULL, device is %s\n",

			__func__, pipe->vdev.name);
@@ -4139,6 +4155,8 @@ int atomisp_set_parameters(struct video_device *vdev, 
	struct atomisp_css_params *css_param = &asd->params.css_param;

	int ret;



	lockdep_assert_held(&asd->isp->mutex);



	if (!asd) {

		dev_err(pipe->isp->dev, "%s(): asd is NULL, device is %s\n",

			__func__, vdev->name);
@@ -5537,6 +5555,8 @@ int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f) 
	struct v4l2_subdev_fh fh;

	int ret;



	lockdep_assert_held(&isp->mutex);



	if (!asd) {

		dev_err(isp->dev, "%s(): asd is NULL, device is %s\n",

			__func__, vdev->name);
@@ -6159,6 +6179,8 @@ int atomisp_offline_capture_configure(struct atomisp_sub_device *asd, 
{

	struct v4l2_ctrl *c;



	lockdep_assert_held(&asd->isp->mutex);



	/*

	* In case of M10MO ZSL capture case, we need to issue a separate

	* capture request to M10MO which will output captured jpeg image
@@ -6433,6 +6455,8 @@ int atomisp_exp_id_capture(struct atomisp_sub_device *asd, int *exp_id) 
	int value = *exp_id;

	int ret;



	lockdep_assert_held(&isp->mutex);



	ret = __is_raw_buffer_locked(asd, value);

	if (ret) {

		dev_err(isp->dev, "%s exp_id %d invalid %d.\n", __func__, value, ret);
@@ -6454,6 +6478,8 @@ int atomisp_exp_id_unlock(struct atomisp_sub_device *asd, int *exp_id) 
	int value = *exp_id;

	int ret;



	lockdep_assert_held(&isp->mutex);



	ret = __clear_raw_buffer_bitmap(asd, value);

	if (ret) {

		dev_err(isp->dev, "%s exp_id %d invalid %d.\n", __func__, value, ret);
@@ -6489,6 +6515,8 @@ int atomisp_inject_a_fake_event(struct atomisp_sub_device *asd, int *event) 
	if (!event || asd->streaming != ATOMISP_DEVICE_STREAMING_ENABLED)

		return -EINVAL;



	lockdep_assert_held(&asd->isp->mutex);



	dev_dbg(asd->isp->dev, "%s: trying to inject a fake event 0x%x\n",

		__func__, *event);

drivers/staging/media/atomisp/pci/atomisp_compat_css20.c

+5 −5
Original line number Diff line number Diff line
@@ -3626,6 +3626,8 @@ int atomisp_css_get_dis_stat(struct atomisp_sub_device *asd, 
	struct atomisp_dis_buf *dis_buf;

	unsigned long flags;



	lockdep_assert_held(&isp->mutex);



	if (!asd->params.dvs_stat->hor_prod.odd_real ||

	    !asd->params.dvs_stat->hor_prod.odd_imag ||

	    !asd->params.dvs_stat->hor_prod.even_real ||
@@ -3637,12 +3639,8 @@ int atomisp_css_get_dis_stat(struct atomisp_sub_device *asd, 
		return -EINVAL;



	/* isp needs to be streaming to get DIS statistics */

	spin_lock_irqsave(&isp->lock, flags);

	if (asd->streaming != ATOMISP_DEVICE_STREAMING_ENABLED) {

		spin_unlock_irqrestore(&isp->lock, flags);

	if (asd->streaming != ATOMISP_DEVICE_STREAMING_ENABLED)

		return -EINVAL;

	}

	spin_unlock_irqrestore(&isp->lock, flags);



	if (atomisp_compare_dvs_grid(asd, &stats->dvs2_stat.grid_info) != 0)

		/* If the grid info in the argument differs from the current
@@ -3801,6 +3799,8 @@ int atomisp_css_isr_thread(struct atomisp_device *isp, 
	bool reset_wdt_timer[MAX_STREAM_NUM] = {false};

	int i;



	lockdep_assert_held(&isp->mutex);



	while (!ia_css_dequeue_psys_event(&current_event.event)) {

		if (current_event.event.type ==

		    IA_CSS_EVENT_TYPE_FW_ASSERT) {

drivers/staging/media/atomisp/pci/atomisp_fops.c

+3 −0
Original line number Diff line number Diff line
@@ -813,6 +813,7 @@ static int atomisp_release(struct file *file) 
	struct v4l2_requestbuffers req;

	struct v4l2_subdev_fh fh;

	struct v4l2_rect clear_compose = {0};

	unsigned long flags;

	int ret = 0;



	v4l2_fh_init(&fh.vfh, vdev);
@@ -878,7 +879,9 @@ static int atomisp_release(struct file *file) 


	/* clear the asd field to show this camera is not used */

	isp->inputs[asd->input_curr].asd = NULL;

	spin_lock_irqsave(&isp->lock, flags);

	asd->streaming = ATOMISP_DEVICE_STREAMING_DISABLED;

	spin_unlock_irqrestore(&isp->lock, flags);



	if (atomisp_dev_users(isp))

		goto done;

drivers/staging/media/atomisp/pci/atomisp_internal.h

+1 −1
Original line number Diff line number Diff line
@@ -266,7 +266,7 @@ struct atomisp_device { 


	atomic_t wdt_work_queued;



	spinlock_t lock; /* Just for streaming below */

	spinlock_t lock; /* Protects asd[i].streaming */



	bool need_gfx_throttle;

drivers/staging/media/atomisp/pci/atomisp_ioctl.c

+4 −0
Original line number Diff line number Diff line
@@ -1837,7 +1837,9 @@ static int atomisp_streamon(struct file *file, void *fh, 
	if (ret)

		goto out;



	spin_lock_irqsave(&isp->lock, irqflags);

	asd->streaming = ATOMISP_DEVICE_STREAMING_ENABLED;

	spin_unlock_irqrestore(&isp->lock, irqflags);

	atomic_set(&asd->sof_count, -1);

	atomic_set(&asd->sequence, -1);

	atomic_set(&asd->sequence_temp, -1);
@@ -1910,7 +1912,9 @@ static int atomisp_streamon(struct file *file, void *fh, 
	ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,

			       video, s_stream, 1);

	if (ret) {

		spin_lock_irqsave(&isp->lock, irqflags);

		asd->streaming = ATOMISP_DEVICE_STREAMING_DISABLED;

		spin_unlock_irqrestore(&isp->lock, irqflags);

		ret = -EINVAL;

		goto out;

	}