Re: [PATCH RFC v3 1/3] V4L: Add driver for S3C244X/S3C64XX SoC series camera interface
Hi Sylwester,
On Sat, Nov 17, 2012 at 2:39 AM, Sylwester Nawrocki
sylvester.nawro...@gmail.com wrote:
Hi Alexey,
On 11/16/2012 03:10 PM, Alexey Klimov wrote:
+static int s3c_camif_hw_init(struct camif_dev *camif, struct camif_vp
*vp)
+{
+ unsigned int ip_rev = camif-variant-ip_revision;
+ unsigned long flags;
+
+ if (camif-sensor.sd == NULL || vp-out_fmt == NULL)
+ return -EINVAL;
+
+ spin_lock_irqsave(camif-slock, flags);
+
+ if (ip_rev == S3C244X_CAMIF_IP_REV)
+ camif_hw_clear_fifo_overflow(vp);
+ camif_hw_set_camera_bus(camif);
+ camif_hw_set_source_format(camif);
+ camif_hw_set_camera_crop(camif);
+ camif_hw_set_test_pattern(camif, camif-test_pattern-val);
+ if (ip_rev == S3C6410_CAMIF_IP_REV)
+ camif_hw_set_input_path(vp);
+ camif_cfg_video_path(vp);
+ vp-state= ~ST_VP_CONFIG;
+
+ spin_unlock_irqrestore(camif-slock, flags);
+ return 0;
+}
+
+/*
+ * Initialize the video path, only up from the scaler stage. The camera
+ * input interface set up is skipped. This is useful to enable one of
the
+ * video paths when the other is already running.
+ */
+static int s3c_camif_hw_vp_init(struct camif_dev *camif, struct
camif_vp
*vp)
+{
+ unsigned int ip_rev = camif-variant-ip_revision;
+ unsigned long flags;
+
+ if (vp-out_fmt == NULL)
+ return -EINVAL;
+
+ spin_lock_irqsave(camif-slock, flags);
+ camif_prepare_dma_offset(vp);
+ if (ip_rev == S3C244X_CAMIF_IP_REV)
+ camif_hw_clear_fifo_overflow(vp);
+ camif_cfg_video_path(vp);
+ if (ip_rev == S3C6410_CAMIF_IP_REV)
+ camif_hw_set_effect(vp, false);
+ vp-state= ~ST_VP_CONFIG;
+
+ spin_unlock_irqrestore(camif-slock, flags);
+ return 0;
+}
...
+/*
+ * Reinitialize the driver so it is ready to start streaming again.
+ * Return any buffers to vb2, perform CAMIF software reset and
+ * turn off streaming at the data pipeline (sensor) if required.
+ */
+static int camif_reinitialize(struct camif_vp *vp)
+{
+ struct camif_dev *camif = vp-camif;
+ struct camif_buffer *buf;
+ unsigned long flags;
+ bool streaming;
+
+ spin_lock_irqsave(camif-slock, flags);
+ streaming = vp-state ST_VP_SENSOR_STREAMING;
+
+ vp-state= ~(ST_VP_PENDING | ST_VP_RUNNING | ST_VP_OFF |
+ ST_VP_ABORTING | ST_VP_STREAMING |
+ ST_VP_SENSOR_STREAMING | ST_VP_LASTIRQ);
+
+ /* Release unused buffers */
+ while (!list_empty(vp-pending_buf_q)) {
+ buf = camif_pending_queue_pop(vp);
+ vb2_buffer_done(buf-vb, VB2_BUF_STATE_ERROR);
+ }
+
+ while (!list_empty(vp-active_buf_q)) {
+ buf = camif_active_queue_pop(vp);
+ vb2_buffer_done(buf-vb, VB2_BUF_STATE_ERROR);
+ }
+
+ spin_unlock_irqrestore(camif-slock, flags);
+
+ if (!streaming)
+ return 0;
+
+ return sensor_set_streaming(camif, 0);
+}
...
+static int start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+ struct camif_vp *vp = vb2_get_drv_priv(vq);
+ struct camif_dev *camif = vp-camif;
+ unsigned long flags;
+ int ret;
+
+ /*
+* We assume the codec capture path is always activated
+* first, before the preview path starts streaming.
+* This is required to avoid internal FIFO overflow and
+* a need for CAMIF software reset.
+*/
+ spin_lock_irqsave(camif-slock, flags);
Here.
+
+ if (camif-stream_count == 0) {
+ camif_hw_reset(camif);
+ spin_unlock_irqrestore(camif-slock, flags);
+ ret = s3c_camif_hw_init(camif, vp);
+ } else {
+ spin_unlock_irqrestore(camif-slock, flags);
+ ret = s3c_camif_hw_vp_init(camif, vp);
+ }
+
+ if (ret 0) {
+ camif_reinitialize(vp);
+ return ret;
+ }
+
+ spin_lock_irqsave(camif-slock, flags);
Could you please check this function? Is it ok that you have double
spin_lock_irqsave()? I don't know may be it's okay. Also when you call
camif_reinitialize() you didn't call spin_unlock_irqrestore() before and
inside camif_reinitialize() you will also call spin_lock_irqsave()..
Certainly with nested spinlock locking this code would have been useless.
I suppose this is what you mean by double spin_lock_irqsave(). Since
it is known to work there must be spin_unlock_irqrestore() somewhere,
before the second spin_lock_irqsave() above. Just look around with more
focus ;)
You are right. I'm very sorry, i need to be more focus :)
--
Best regards, Klimov Alexey
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a
Re: [PATCH RFC v3 1/3] V4L: Add driver for S3C244X/S3C64XX SoC series camera interface
Hi Hans,
On 11/16/2012 02:45 PM, Hans Verkuil wrote:
Hi Sylwester,
Just one comment, see below...
On Thu November 15 2012 23:05:13 Sylwester Nawrocki wrote:
This patch adds V4L2 driver for Samsung S3C244X/S3C64XX SoC series
camera interface. The driver exposes a subdev device node for CAMIF
pixel resolution and crop control and two video capture nodes - for
the codec and preview data paths. It has been tested on Mini2440
(s3c2440) and Mini6410 (s3c6410) board with gstreamer and mplayer.
Signed-off-by: Sylwester Nawrockisylvester.nawro...@gmail.com
Signed-off-by: Tomasz Figatomasz.f...@gmail.com
---
...
+static int s3c_camif_streamon(struct file *file, void *priv,
+ enum v4l2_buf_type type)
+{
+ struct camif_vp *vp = video_drvdata(file);
+ struct camif_dev *camif = vp-camif;
+ struct media_entity *sensor =camif-sensor.sd-entity;
+ int ret;
+
+ pr_debug([vp%d]\n, vp-id);
+
+ if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;
+
+ if (vp-owner vp-owner != priv)
+ return -EBUSY;
+
+ if (s3c_vp_active(vp))
+ return 0;
+
+ ret = media_entity_pipeline_start(sensor, camif-m_pipeline);
+ if (ret 0)
+ return ret;
+
+ ret = camif_pipeline_validate(camif);
+ if (ret 0) {
+ media_entity_pipeline_stop(sensor);
+ return ret;
+ }
+
+ return vb2_streamon(vp-vb_queue, type);
+}
+
+static int s3c_camif_streamoff(struct file *file, void *priv,
+ enum v4l2_buf_type type)
+{
+ struct camif_vp *vp = video_drvdata(file);
+ struct camif_dev *camif = vp-camif;
+ int ret;
+
+ pr_debug([vp%d]\n, vp-id);
+
+ if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;
+
+ if (vp-owner vp-owner != priv)
+ return -EBUSY;
+
+ ret = vb2_streamoff(vp-vb_queue, type);
+ if (ret == 0)
+ media_entity_pipeline_stop(camif-sensor.sd-entity);
+ return ret;
+}
+
+static int s3c_camif_reqbufs(struct file *file, void *priv,
+struct v4l2_requestbuffers *rb)
+{
+ struct camif_vp *vp = video_drvdata(file);
+ int ret;
+
+ pr_debug([vp%d] rb count: %d, owner: %p, priv: %p\n,
+vp-id, rb-count, vp-owner, priv);
+
+ if (vp-owner vp-owner != priv)
+ return -EBUSY;
+
+ if (rb-count)
+ rb-count = max_t(u32, CAMIF_REQ_BUFS_MIN, rb-count);
+ else
+ vp-owner = NULL;
+
+ ret = vb2_reqbufs(vp-vb_queue, rb);
+ if (!ret) {
+ vp-reqbufs_count = rb-count;
+ if (vp-owner == NULL rb-count 0)
+ vp-owner = priv;
+ }
+
+ return ret;
+}
+
+static int s3c_camif_querybuf(struct file *file, void *priv,
+ struct v4l2_buffer *buf)
+{
+ struct camif_vp *vp = video_drvdata(file);
+ return vb2_querybuf(vp-vb_queue, buf);
+}
+
+static int s3c_camif_qbuf(struct file *file, void *priv,
+ struct v4l2_buffer *buf)
+{
+ struct camif_vp *vp = video_drvdata(file);
+
+ pr_debug([vp%d]\n, vp-id);
+
+ if (vp-owner vp-owner != priv)
+ return -EBUSY;
+
+ return vb2_qbuf(vp-vb_queue, buf);
+}
+
+static int s3c_camif_dqbuf(struct file *file, void *priv,
+ struct v4l2_buffer *buf)
+{
+ struct camif_vp *vp = video_drvdata(file);
+
+ pr_debug([vp%d] sequence: %d\n, vp-id, vp-frame_sequence);
+
+ if (vp-owner vp-owner != priv)
+ return -EBUSY;
+
+ return vb2_dqbuf(vp-vb_queue, buf, file-f_flags O_NONBLOCK);
+}
+
+static int s3c_camif_create_bufs(struct file *file, void *priv,
+struct v4l2_create_buffers *create)
+{
+ struct camif_vp *vp = video_drvdata(file);
+ int ret;
+
+ if (vp-owner vp-owner != priv)
+ return -EBUSY;
+
+ create-count = max_t(u32, 1, create-count);
+ ret = vb2_create_bufs(vp-vb_queue, create);
+
+ if (!ret vp-owner == NULL)
+ vp-owner = priv;
+
+ return ret;
+}
+
+static int s3c_camif_prepare_buf(struct file *file, void *priv,
+struct v4l2_buffer *b)
+{
+ struct camif_vp *vp = video_drvdata(file);
+ return vb2_prepare_buf(vp-vb_queue, b);
+}
+
Are you aware of the vb2 ioctl helper functions I've added? See
videobuf2-core.h,
at the end.
Yes, I just chose not to introduce these helpers now, to make any
back-porting
of this driver to older kernels easier.
They can probably replace some of these ioctls. It's something you can do later
in a separate patch, so this isn't blocking as far as I am concerned. It's just
a hint.
Thanks, I'll see which ones can be replaced. But I would prefer to make it
a separate patch for subsequent kernel
Re: [PATCH RFC v3 1/3] V4L: Add driver for S3C244X/S3C64XX SoC series camera interface
Hi Sylwester, Just one comment, see below... On Thu November 15 2012 23:05:13 Sylwester Nawrocki wrote: This patch adds V4L2 driver for Samsung S3C244X/S3C64XX SoC series camera interface. The driver exposes a subdev device node for CAMIF pixel resolution and crop control and two video capture nodes - for the codec and preview data paths. It has been tested on Mini2440 (s3c2440) and Mini6410 (s3c6410) board with gstreamer and mplayer. Signed-off-by: Sylwester Nawrocki sylvester.nawro...@gmail.com Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- drivers/media/platform/Kconfig | 12 + drivers/media/platform/Makefile |1 + drivers/media/platform/s3c-camif/Makefile|5 + drivers/media/platform/s3c-camif/camif-capture.c | 1636 ++ drivers/media/platform/s3c-camif/camif-core.c| 661 + drivers/media/platform/s3c-camif/camif-core.h| 382 + drivers/media/platform/s3c-camif/camif-regs.c| 579 drivers/media/platform/s3c-camif/camif-regs.h| 267 include/media/s3c_camif.h| 45 + 9 files changed, 3588 insertions(+), 0 deletions(-) create mode 100644 drivers/media/platform/s3c-camif/Makefile create mode 100644 drivers/media/platform/s3c-camif/camif-capture.c create mode 100644 drivers/media/platform/s3c-camif/camif-core.c create mode 100644 drivers/media/platform/s3c-camif/camif-core.h create mode 100644 drivers/media/platform/s3c-camif/camif-regs.c create mode 100644 drivers/media/platform/s3c-camif/camif-regs.h create mode 100644 include/media/s3c_camif.h diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 181c768..3dcfea6 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -109,6 +109,18 @@ config VIDEO_OMAP3_DEBUG ---help--- Enable debug messages on OMAP 3 camera controller driver. +config VIDEO_S3C_CAMIF + tristate Samsung S3C24XX/S3C64XX SoC Camera Interface driver + depends on VIDEO_V4L2 I2C VIDEO_V4L2_SUBDEV_API + depends on (PLAT_S3C64XX || PLAT_S3C24XX) PM_RUNTIME + select VIDEOBUF2_DMA_CONTIG + ---help--- + This is a v4l2 driver for s3c24xx and s3c64xx SoC series camera + host interface (CAMIF). + + To compile this driver as a module, choose M here: the module + will be called s3c-camif. + source drivers/media/platform/soc_camera/Kconfig source drivers/media/platform/s5p-fimc/Kconfig source drivers/media/platform/s5p-tv/Kconfig diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index baaa550..4817d28 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_VIDEO_CODA)+= coda.o obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE) += m2m-deinterlace.o +obj-$(CONFIG_VIDEO_S3C_CAMIF)+= s3c-camif/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_FIMC) += s5p-fimc/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) += s5p-mfc/ diff --git a/drivers/media/platform/s3c-camif/Makefile b/drivers/media/platform/s3c-camif/Makefile new file mode 100644 index 000..50bf8c5 --- /dev/null +++ b/drivers/media/platform/s3c-camif/Makefile @@ -0,0 +1,5 @@ +# Makefile for s3c244x/s3c64xx CAMIF driver + +s3c-camif-objs := camif-core.o camif-capture.o camif-regs.o + +obj-$(CONFIG_VIDEO_S3C_CAMIF) += s3c-camif.o diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c new file mode 100644 index 000..8daf684 --- /dev/null +++ b/drivers/media/platform/s3c-camif/camif-capture.c @@ -0,0 +1,1636 @@ +/* + * s3c24xx/s3c64xx SoC series Camera Interface (CAMIF) driver + * + * Copyright (C) 2012 Sylwester Nawrocki sylvester.nawro...@gmail.com + * Copyright (C) 2012 Tomasz Figa tomasz.f...@gmail.com + * + * Based on drivers/media/platform/s5p-fimc, + * Copyright (C) 2010 - 2012 Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ +#define pr_fmt(fmt) %s:%d fmt, __func__, __LINE__ + +#include linux/bug.h +#include linux/clk.h +#include linux/device.h +#include linux/errno.h +#include linux/i2c.h +#include linux/interrupt.h +#include linux/io.h +#include linux/kernel.h +#include linux/list.h +#include linux/module.h +#include linux/platform_device.h +#include linux/pm_runtime.h +#include linux/ratelimit.h +#include linux/slab.h +#include linux/types.h +#include linux/videodev2.h + +#include media/media-device.h +#include media/v4l2-ctrls.h +#include media/v4l2-event.h +#include media/v4l2-ioctl.h +#include media/videobuf2-core.h +#include media/videobuf2-dma-contig.h +
Re: [PATCH RFC v3 1/3] V4L: Add driver for S3C244X/S3C64XX SoC series camera interface
Hi Sylwester, On Fri, Nov 16, 2012 at 2:05 AM, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: This patch adds V4L2 driver for Samsung S3C244X/S3C64XX SoC series camera interface. The driver exposes a subdev device node for CAMIF pixel resolution and crop control and two video capture nodes - for the codec and preview data paths. It has been tested on Mini2440 (s3c2440) and Mini6410 (s3c6410) board with gstreamer and mplayer. Signed-off-by: Sylwester Nawrocki sylvester.nawro...@gmail.com Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- drivers/media/platform/Kconfig | 12 + drivers/media/platform/Makefile |1 + drivers/media/platform/s3c-camif/Makefile|5 + drivers/media/platform/s3c-camif/camif-capture.c | 1636 ++ drivers/media/platform/s3c-camif/camif-core.c| 661 + drivers/media/platform/s3c-camif/camif-core.h| 382 + drivers/media/platform/s3c-camif/camif-regs.c| 579 drivers/media/platform/s3c-camif/camif-regs.h| 267 include/media/s3c_camif.h| 45 + 9 files changed, 3588 insertions(+), 0 deletions(-) create mode 100644 drivers/media/platform/s3c-camif/Makefile create mode 100644 drivers/media/platform/s3c-camif/camif-capture.c create mode 100644 drivers/media/platform/s3c-camif/camif-core.c create mode 100644 drivers/media/platform/s3c-camif/camif-core.h create mode 100644 drivers/media/platform/s3c-camif/camif-regs.c create mode 100644 drivers/media/platform/s3c-camif/camif-regs.h create mode 100644 include/media/s3c_camif.h diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 181c768..3dcfea6 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -109,6 +109,18 @@ config VIDEO_OMAP3_DEBUG ---help--- Enable debug messages on OMAP 3 camera controller driver. +config VIDEO_S3C_CAMIF + tristate Samsung S3C24XX/S3C64XX SoC Camera Interface driver + depends on VIDEO_V4L2 I2C VIDEO_V4L2_SUBDEV_API + depends on (PLAT_S3C64XX || PLAT_S3C24XX) PM_RUNTIME + select VIDEOBUF2_DMA_CONTIG + ---help--- + This is a v4l2 driver for s3c24xx and s3c64xx SoC series camera + host interface (CAMIF). + + To compile this driver as a module, choose M here: the module + will be called s3c-camif. + source drivers/media/platform/soc_camera/Kconfig source drivers/media/platform/s5p-fimc/Kconfig source drivers/media/platform/s5p-tv/Kconfig diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index baaa550..4817d28 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_VIDEO_CODA) += coda.o obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)+= m2m-deinterlace.o +obj-$(CONFIG_VIDEO_S3C_CAMIF) += s3c-camif/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_FIMC) += s5p-fimc/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)+= s5p-mfc/ diff --git a/drivers/media/platform/s3c-camif/Makefile b/drivers/media/platform/s3c-camif/Makefile new file mode 100644 index 000..50bf8c5 --- /dev/null +++ b/drivers/media/platform/s3c-camif/Makefile @@ -0,0 +1,5 @@ +# Makefile for s3c244x/s3c64xx CAMIF driver + +s3c-camif-objs := camif-core.o camif-capture.o camif-regs.o + +obj-$(CONFIG_VIDEO_S3C_CAMIF) += s3c-camif.o diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c new file mode 100644 index 000..8daf684 --- /dev/null +++ b/drivers/media/platform/s3c-camif/camif-capture.c @@ -0,0 +1,1636 @@ +/* + * s3c24xx/s3c64xx SoC series Camera Interface (CAMIF) driver + * + * Copyright (C) 2012 Sylwester Nawrocki sylvester.nawro...@gmail.com + * Copyright (C) 2012 Tomasz Figa tomasz.f...@gmail.com + * + * Based on drivers/media/platform/s5p-fimc, + * Copyright (C) 2010 - 2012 Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ +#define pr_fmt(fmt) %s:%d fmt, __func__, __LINE__ + +#include linux/bug.h +#include linux/clk.h +#include linux/device.h +#include linux/errno.h +#include linux/i2c.h +#include linux/interrupt.h +#include linux/io.h +#include linux/kernel.h +#include linux/list.h +#include linux/module.h +#include linux/platform_device.h +#include linux/pm_runtime.h +#include linux/ratelimit.h +#include linux/slab.h +#include linux/types.h +#include linux/videodev2.h + +#include media/media-device.h +#include media/v4l2-ctrls.h +#include media/v4l2-event.h +#include media/v4l2-ioctl.h +#include media/videobuf2-core.h +#include
Re: [PATCH RFC v3 1/3] V4L: Add driver for S3C244X/S3C64XX SoC series camera interface
Hi Alexey,
On 11/16/2012 03:10 PM, Alexey Klimov wrote:
+static int s3c_camif_hw_init(struct camif_dev *camif, struct camif_vp
*vp)
+{
+ unsigned int ip_rev = camif-variant-ip_revision;
+ unsigned long flags;
+
+ if (camif-sensor.sd == NULL || vp-out_fmt == NULL)
+ return -EINVAL;
+
+ spin_lock_irqsave(camif-slock, flags);
+
+ if (ip_rev == S3C244X_CAMIF_IP_REV)
+ camif_hw_clear_fifo_overflow(vp);
+ camif_hw_set_camera_bus(camif);
+ camif_hw_set_source_format(camif);
+ camif_hw_set_camera_crop(camif);
+ camif_hw_set_test_pattern(camif, camif-test_pattern-val);
+ if (ip_rev == S3C6410_CAMIF_IP_REV)
+ camif_hw_set_input_path(vp);
+ camif_cfg_video_path(vp);
+ vp-state= ~ST_VP_CONFIG;
+
+ spin_unlock_irqrestore(camif-slock, flags);
+ return 0;
+}
+
+/*
+ * Initialize the video path, only up from the scaler stage. The camera
+ * input interface set up is skipped. This is useful to enable one of
the
+ * video paths when the other is already running.
+ */
+static int s3c_camif_hw_vp_init(struct camif_dev *camif, struct camif_vp
*vp)
+{
+ unsigned int ip_rev = camif-variant-ip_revision;
+ unsigned long flags;
+
+ if (vp-out_fmt == NULL)
+ return -EINVAL;
+
+ spin_lock_irqsave(camif-slock, flags);
+ camif_prepare_dma_offset(vp);
+ if (ip_rev == S3C244X_CAMIF_IP_REV)
+ camif_hw_clear_fifo_overflow(vp);
+ camif_cfg_video_path(vp);
+ if (ip_rev == S3C6410_CAMIF_IP_REV)
+ camif_hw_set_effect(vp, false);
+ vp-state= ~ST_VP_CONFIG;
+
+ spin_unlock_irqrestore(camif-slock, flags);
+ return 0;
+}
...
+/*
+ * Reinitialize the driver so it is ready to start streaming again.
+ * Return any buffers to vb2, perform CAMIF software reset and
+ * turn off streaming at the data pipeline (sensor) if required.
+ */
+static int camif_reinitialize(struct camif_vp *vp)
+{
+ struct camif_dev *camif = vp-camif;
+ struct camif_buffer *buf;
+ unsigned long flags;
+ bool streaming;
+
+ spin_lock_irqsave(camif-slock, flags);
+ streaming = vp-state ST_VP_SENSOR_STREAMING;
+
+ vp-state= ~(ST_VP_PENDING | ST_VP_RUNNING | ST_VP_OFF |
+ ST_VP_ABORTING | ST_VP_STREAMING |
+ ST_VP_SENSOR_STREAMING | ST_VP_LASTIRQ);
+
+ /* Release unused buffers */
+ while (!list_empty(vp-pending_buf_q)) {
+ buf = camif_pending_queue_pop(vp);
+ vb2_buffer_done(buf-vb, VB2_BUF_STATE_ERROR);
+ }
+
+ while (!list_empty(vp-active_buf_q)) {
+ buf = camif_active_queue_pop(vp);
+ vb2_buffer_done(buf-vb, VB2_BUF_STATE_ERROR);
+ }
+
+ spin_unlock_irqrestore(camif-slock, flags);
+
+ if (!streaming)
+ return 0;
+
+ return sensor_set_streaming(camif, 0);
+}
...
+static int start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+ struct camif_vp *vp = vb2_get_drv_priv(vq);
+ struct camif_dev *camif = vp-camif;
+ unsigned long flags;
+ int ret;
+
+ /*
+* We assume the codec capture path is always activated
+* first, before the preview path starts streaming.
+* This is required to avoid internal FIFO overflow and
+* a need for CAMIF software reset.
+*/
+ spin_lock_irqsave(camif-slock, flags);
Here.
+
+ if (camif-stream_count == 0) {
+ camif_hw_reset(camif);
+ spin_unlock_irqrestore(camif-slock, flags);
+ ret = s3c_camif_hw_init(camif, vp);
+ } else {
+ spin_unlock_irqrestore(camif-slock, flags);
+ ret = s3c_camif_hw_vp_init(camif, vp);
+ }
+
+ if (ret 0) {
+ camif_reinitialize(vp);
+ return ret;
+ }
+
+ spin_lock_irqsave(camif-slock, flags);
Could you please check this function? Is it ok that you have double
spin_lock_irqsave()? I don't know may be it's okay. Also when you call
camif_reinitialize() you didn't call spin_unlock_irqrestore() before and
inside camif_reinitialize() you will also call spin_lock_irqsave()..
Certainly with nested spinlock locking this code would have been useless.
I suppose this is what you mean by double spin_lock_irqsave(). Since
it is known to work there must be spin_unlock_irqrestore() somewhere,
before the second spin_lock_irqsave() above. Just look around with more
focus ;)
Nevertheless, it looks locking can be removed from functions
s3c_camif_hw_init() and s3c_camif_vp_init(), those are called only from
one place, where in addition the spinlock is already held. I'm going
to squash following patch into that one:
8--
diff --git a/drivers/media/platform/s3c-camif/camif-capture.c
b/drivers/media/platform/s3c-camif/camif-capture.c
index c2ecdcc..6401fdb 100644
---
