media: atomisp: Add atomisp_pipe_check() helper

Several of the ioctl handlers all do the same checks
(isp->fatal_error and asd->streaming errors) add
an atomisp_pipe_check() helper for this.

Note this changes the vidioc_s_fmt_vid_cap and vidioc_s_input handlers
to now reject calls made while asd->streaming==STOPPING. This fixes
a possible race where one thread can make this ioctls while
vidioc_streamoff is running from another thread and it has
temporarily released isp->mutex to kill the watchdog timers / work.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 0870789..7945852 100644 (file)
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -5549,16 +5549,13 @@ int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
        struct v4l2_subdev_fh fh;
        int ret;
 
-       lockdep_assert_held(&isp->mutex);
+       ret = atomisp_pipe_check(pipe, true);
+       if (ret)
+               return ret;
 
        if (source_pad >= ATOMISP_SUBDEV_PADS_NUM)
                return -EINVAL;
 
-       if (asd->streaming == ATOMISP_DEVICE_STREAMING_ENABLED) {
-               dev_warn(isp->dev, "ISP does not support set format while at streaming!\n");
-               return -EBUSY;
-       }
-
        dev_dbg(isp->dev,
                "setting resolution %ux%u on pad %u for asd%d, bytesperline %u\n",
                f->fmt.pix.width, f->fmt.pix.height, source_pad,

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 9c7022b..9b50f63 100644 (file)
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -535,6 +535,32 @@ atomisp_get_format_bridge_from_mbus(u32 mbus_code)
        return NULL;
 }
 
+int atomisp_pipe_check(struct atomisp_video_pipe *pipe, bool settings_change)
+{
+       lockdep_assert_held(&pipe->isp->mutex);
+
+       if (pipe->isp->isp_fatal_error)
+               return -EIO;
+
+       switch (pipe->asd->streaming) {
+       case ATOMISP_DEVICE_STREAMING_DISABLED:
+               break;
+       case ATOMISP_DEVICE_STREAMING_ENABLED:
+               if (settings_change) {
+                       dev_err(pipe->isp->dev, "Set fmt/input IOCTL while streaming\n");
+                       return -EBUSY;
+               }
+               break;
+       case ATOMISP_DEVICE_STREAMING_STOPPING:
+               dev_err(pipe->isp->dev, "IOCTL issued while stopping\n");
+               return -EBUSY;
+       default:
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
 /*
  * v4l2 ioctls
  * return ISP capabilities
@@ -646,12 +672,18 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
 {
        struct video_device *vdev = video_devdata(file);
        struct atomisp_device *isp = video_get_drvdata(vdev);
-       struct atomisp_sub_device *asd = atomisp_to_video_pipe(vdev)->asd;
+       struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
+       struct atomisp_sub_device *asd = pipe->asd;
        struct v4l2_subdev *camera = NULL;
        struct v4l2_subdev *motor;
        int ret;
 
        mutex_lock(&isp->mutex);
+
+       ret = atomisp_pipe_check(pipe, true);
+       if (ret)
+               goto error;
+
        if (input >= ATOM_ISP_MAX_INPUTS || input >= isp->input_cnt) {
                dev_dbg(isp->dev, "input_cnt: %d\n", isp->input_cnt);
                ret = -EINVAL;
@@ -678,13 +710,6 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
                goto error;
        }
 
-       if (atomisp_subdev_streaming_count(asd)) {
-               dev_err(isp->dev,
-                       "ISP is still streaming, stop first\n");
-               ret = -EINVAL;
-               goto error;
-       }
-
        /* power off the current owned sensor, as it is not used this time */
        if (isp->inputs[asd->input_curr].asd == asd &&
            asd->input_curr != input) {
@@ -976,11 +1001,6 @@ static int atomisp_s_fmt_cap(struct file *file, void *fh,
        int ret;
 
        mutex_lock(&isp->mutex);
-       if (isp->isp_fatal_error) {
-               ret = -EIO;
-               mutex_unlock(&isp->mutex);
-               return ret;
-       }
        ret = atomisp_set_fmt(vdev, f);
        mutex_unlock(&isp->mutex);
        return ret;
@@ -1236,20 +1256,13 @@ static int atomisp_qbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
        struct ia_css_frame *handle = NULL;
        u32 length;
        u32 pgnr;
-       int ret = 0;
+       int ret;
 
        mutex_lock(&isp->mutex);
-       if (isp->isp_fatal_error) {
-               ret = -EIO;
-               goto error;
-       }
 
-       if (asd->streaming == ATOMISP_DEVICE_STREAMING_STOPPING) {
-               dev_err(isp->dev, "%s: reject, as ISP at stopping.\n",
-                       __func__);
-               ret = -EIO;
+       ret = atomisp_pipe_check(pipe, false);
+       if (ret)
                goto error;
-       }
 
        if (!buf || buf->index >= VIDEO_MAX_FRAME ||
            !pipe->capq.bufs[buf->index]) {
@@ -1418,23 +1431,13 @@ static int atomisp_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
        struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
        struct atomisp_sub_device *asd = pipe->asd;
        struct atomisp_device *isp = video_get_drvdata(vdev);
-       int ret = 0;
+       int ret;
 
        mutex_lock(&isp->mutex);
-
-       if (isp->isp_fatal_error) {
-               mutex_unlock(&isp->mutex);
-               return -EIO;
-       }
-
-       if (asd->streaming == ATOMISP_DEVICE_STREAMING_STOPPING) {
-               mutex_unlock(&isp->mutex);
-               dev_err(isp->dev, "%s: reject, as ISP at stopping.\n",
-                       __func__);
-               return -EIO;
-       }
-
+       ret = atomisp_pipe_check(pipe, false);
        mutex_unlock(&isp->mutex);
+       if (ret)
+               return ret;
 
        ret = videobuf_dqbuf(&pipe->capq, buf, file->f_flags & O_NONBLOCK);
        if (ret) {
@@ -1668,8 +1671,8 @@ static int atomisp_streamon(struct file *file, void *fh,
        enum ia_css_pipe_id css_pipe_id;
        unsigned int sensor_start_stream;
        unsigned int wdt_duration = ATOMISP_ISP_TIMEOUT_DURATION;
-       int ret = 0;
        unsigned long irqflags;
+       int ret;
 
        dev_dbg(isp->dev, "Start stream on pad %d for asd%d\n",
                atomisp_subdev_source_pad(vdev), asd->index);
@@ -1680,15 +1683,9 @@ static int atomisp_streamon(struct file *file, void *fh,
        }
 
        mutex_lock(&isp->mutex);
-       if (isp->isp_fatal_error) {
-               ret = -EIO;
-               goto out;
-       }
-
-       if (asd->streaming == ATOMISP_DEVICE_STREAMING_STOPPING) {
-               ret = -EBUSY;
+       ret = atomisp_pipe_check(pipe, false);
+       if (ret)
                goto out;
-       }
 
        if (pipe->capq.streaming)
                goto out;

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.h b/drivers/staging/media/atomisp/pci/atomisp_ioctl.h
index 382b782..61a6148 100644 (file)
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.h
@@ -34,6 +34,8 @@ atomisp_format_bridge *atomisp_get_format_bridge(unsigned int pixelformat);
 const struct
 atomisp_format_bridge *atomisp_get_format_bridge_from_mbus(u32 mbus_code);
 
+int atomisp_pipe_check(struct atomisp_video_pipe *pipe, bool streaming_ok);
+
 int atomisp_alloc_css_stat_bufs(struct atomisp_sub_device *asd,
                                uint16_t stream_id);