[REVIEW PATCH v2 00/11] Media link_notify behaviour change and exynos4-is updates

[REVIEW PATCH v2 00/11] Media link_notify behaviour change and exynos4-is updates

[Sylwester Nawrocki]

Hi All,

This is an updated version of a patch set [1], main changes include:
 - In patch "media: Change media device link_notify behaviour"
     - fixed link flags handling in __media_entity_setup_link(),
     - swapped MEDIA_DEV_NOTIFY_PRE_LINK_CH and MEDIA_DEV_NOTIFY_POST_LINK_CH
       at the omap3isp chunk to retain original behaviour,
 - In patch "exynos4-is: Media graph/video device locking rework"
   fixed use_count handling in fimc_lite_release() function;
 - removed patches:
   exynos4-is: Remove platform_device_id table at fimc-lite driver
   exynos4-is: Correct querycap ioctl handling at fimc-lite driver
   as those were quite obvious and I've already added them to my
   tree for 3.11;

The changes are listed at each patch if any. Below is an original cover
letter of v1.


This patch set includes change of the link_notify callback semantics.
This callback will now be invoked always before _and_ after link's state
modification by the core.

Currently this callback is only used by the omap3isp and exynos4-is
drivers, thus those drivers are also modified in patch
[07/11] media: Change media device link_notify behaviour

Any comments/suggestions on this patch are welcome.

The rest of the series includes improvements, bug fixes and preprequsite
patches for the exynos4-is driver to make some modules easier to reuse
in the upcoming exynos5-is driver and to prepare it for addition of
remaining subdevs and video nodes.

This series depends on "[RFC PATCH 0/2] Media entity links handling"
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/64214

Thanks,
Sylwester

[1] http://www.spinics.net/lists/linux-media/msg63423.html

Sylwester Nawrocki (11):
  exynos4-is: Move common functions to a separate module
  exynos4-is: Add struct exynos_video_entity
  exynos4-is: Preserve state of controls between /dev/video open/close
  exynos4-is: Media graph/video device locking rework
  exynos4-is: Do not use asynchronous runtime PM in release fop
  exynos4-is: Use common exynos_media_pipeline data structure
  exynos4-is: Remove WARN_ON() from __fimc_pipeline_close()
  exynos4-is: Fix sensor subdev -> FIMC notification setup
  exynos4-is: Add locking at fimc(-lite) subdev unregistered handler
  media: Change media device link_notify behaviour
  exynos4-is: Extend link_notify handler to support fimc-is/lite
    pipelines

 drivers/media/media-entity.c                      |   18 +-
 drivers/media/platform/exynos4-is/Kconfig         |    7 +-
 drivers/media/platform/exynos4-is/Makefile        |    5 +-
 drivers/media/platform/exynos4-is/common.c        |   41 ++++
 drivers/media/platform/exynos4-is/common.h        |   12 +
 drivers/media/platform/exynos4-is/fimc-capture.c  |  255 +++++++++++---------
 drivers/media/platform/exynos4-is/fimc-core.h     |   11 +-
 drivers/media/platform/exynos4-is/fimc-lite-reg.c |    2 +-
 drivers/media/platform/exynos4-is/fimc-lite.c     |  103 ++++----
 drivers/media/platform/exynos4-is/fimc-lite.h     |    8 +-
 drivers/media/platform/exynos4-is/fimc-reg.c      |    7 +-
 drivers/media/platform/exynos4-is/media-dev.c     |  268 ++++++++++++++-------
 drivers/media/platform/exynos4-is/media-dev.h     |   46 +++-
 drivers/media/platform/omap3isp/isp.c             |   41 ++--
 include/media/media-device.h                      |    9 +-
 include/media/s5p_fimc.h                          |   56 +++--
 16 files changed, 543 insertions(+), 346 deletions(-)
 create mode 100644 drivers/media/platform/exynos4-is/common.c
 create mode 100644 drivers/media/platform/exynos4-is/common.h

--
1.7.9.5

[REVIEW PATCH v2 01/11] exynos4-is: Move common functions to a separate module

[Sylwester Nawrocki]

Create a common module (exynos4-is-common.ko) which will hold
the common functions used across video device and subdev drivers
contained in .../exynos4-is directory.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes since v1:
 - renamed exynos4-common.ko to exynos4-is-common.ko as the
   original name seemed too generic.
---
 drivers/media/platform/exynos4-is/Kconfig     |    7 ++++-
 drivers/media/platform/exynos4-is/Makefile    |    5 ++-
 drivers/media/platform/exynos4-is/common.c    |   41 +++++++++++++++++++++++++
 drivers/media/platform/exynos4-is/common.h    |   12 ++++++++
 drivers/media/platform/exynos4-is/fimc-lite.c |   29 ++---------------
 5 files changed, 66 insertions(+), 28 deletions(-)
 create mode 100644 drivers/media/platform/exynos4-is/common.c
 create mode 100644 drivers/media/platform/exynos4-is/common.h

diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
index 6ff99b5..c622532 100644
--- a/drivers/media/platform/exynos4-is/Kconfig
+++ b/drivers/media/platform/exynos4-is/Kconfig
@@ -8,12 +8,16 @@ config VIDEO_SAMSUNG_EXYNOS4_IS
 
 if VIDEO_SAMSUNG_EXYNOS4_IS
 
+config VIDEO_EXYNOS4_IS_COMMON
+       tristate
+
 config VIDEO_S5P_FIMC
 	tristate "S5P/EXYNOS4 FIMC/CAMIF camera interface driver"
 	depends on I2C
 	select VIDEOBUF2_DMA_CONTIG
 	select V4L2_MEM2MEM_DEV
 	select MFD_SYSCON if OF
+	select VIDEO_EXYNOS4_IS_COMMON
 	help
 	  This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC camera host
 	  interface and video postprocessor (FIMC) devices.
@@ -38,6 +42,7 @@ config VIDEO_EXYNOS_FIMC_LITE
 	tristate "EXYNOS FIMC-LITE camera interface driver"
 	depends on I2C
 	select VIDEOBUF2_DMA_CONTIG
+	select VIDEO_EXYNOS4_IS_COMMON
 	help
 	  This is a V4L2 driver for Samsung EXYNOS4/5 SoC FIMC-LITE camera
 	  host interface.
@@ -58,4 +63,4 @@ config VIDEO_EXYNOS4_FIMC_IS
 	  To compile this driver as a module, choose M here: the
 	  module will be called exynos4-fimc-is.
 
-endif # VIDEO_SAMSUNG_S5P_FIMC
+endif # VIDEO_SAMSUNG_EXYNOS4_IS
diff --git a/drivers/media/platform/exynos4-is/Makefile b/drivers/media/platform/exynos4-is/Makefile
index f25f463..c2ff29b 100644
--- a/drivers/media/platform/exynos4-is/Makefile
+++ b/drivers/media/platform/exynos4-is/Makefile
@@ -1,10 +1,13 @@
 s5p-fimc-objs := fimc-core.o fimc-reg.o fimc-m2m.o fimc-capture.o media-dev.o
 exynos-fimc-lite-objs += fimc-lite-reg.o fimc-lite.o
+s5p-csis-objs := mipi-csis.o
+exynos4-is-common-objs := common.o
+
 exynos-fimc-is-objs := fimc-is.o fimc-isp.o fimc-is-sensor.o fimc-is-regs.o
 exynos-fimc-is-objs += fimc-is-param.o fimc-is-errno.o fimc-is-i2c.o
-s5p-csis-objs := mipi-csis.o
 
 obj-$(CONFIG_VIDEO_S5P_MIPI_CSIS)	+= s5p-csis.o
 obj-$(CONFIG_VIDEO_EXYNOS_FIMC_LITE)	+= exynos-fimc-lite.o
 obj-$(CONFIG_VIDEO_EXYNOS4_FIMC_IS)	+= exynos-fimc-is.o
 obj-$(CONFIG_VIDEO_S5P_FIMC)		+= s5p-fimc.o
+obj-$(CONFIG_VIDEO_EXYNOS4_IS_COMMON)	+= exynos4-is-common.o
diff --git a/drivers/media/platform/exynos4-is/common.c b/drivers/media/platform/exynos4-is/common.c
new file mode 100644
index 0000000..6720520
--- /dev/null
+++ b/drivers/media/platform/exynos4-is/common.c
@@ -0,0 +1,41 @@
+/*
+ * Samsung S5P/EXYNOS4 SoC Camera Subsystem driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <media/s5p_fimc.h>
+#include "common.h"
+
+/* Called with the media graph mutex held or entity->stream_count > 0. */
+struct v4l2_subdev *fimc_find_remote_sensor(struct media_entity *entity)
+{
+	struct media_pad *pad = &entity->pads[0];
+	struct v4l2_subdev *sd;
+
+	while (pad->flags & MEDIA_PAD_FL_SINK) {
+		/* source pad */
+		pad = media_entity_remote_source(pad);
+		if (pad == NULL ||
+		    media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
+			break;
+
+		sd = media_entity_to_v4l2_subdev(pad->entity);
+
+		if (sd->grp_id == GRP_ID_FIMC_IS_SENSOR ||
+		    sd->grp_id == GRP_ID_SENSOR)
+			return sd;
+		/* sink pad */
+		pad = &sd->entity.pads[0];
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(fimc_find_remote_sensor);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/exynos4-is/common.h b/drivers/media/platform/exynos4-is/common.h
new file mode 100644
index 0000000..3c39803
--- /dev/null
+++ b/drivers/media/platform/exynos4-is/common.h
@@ -0,0 +1,12 @@
+/*
+ * Copyright (C) 2013 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.
+ */
+
+#include <media/media-entity.h>
+#include <media/v4l2-subdev.h>
+
+struct v4l2_subdev *fimc_find_remote_sensor(struct media_entity *entity);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index c57fa65..66480e7 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -32,6 +32,7 @@
 #include <media/videobuf2-dma-contig.h>
 #include <media/s5p_fimc.h>
 
+#include "common.h"
 #include "fimc-core.h"
 #include "fimc-lite.h"
 #include "fimc-lite-reg.h"
@@ -131,30 +132,6 @@ static const struct fimc_fmt *fimc_lite_find_format(const u32 *pixelformat,
 	return def_fmt;
 }
 
-/* Called with the media graph mutex held or @me stream_count > 0. */
-static struct v4l2_subdev *__find_remote_sensor(struct media_entity *me)
-{
-	struct media_pad *pad = &me->pads[0];
-	struct v4l2_subdev *sd;
-
-	while (pad->flags & MEDIA_PAD_FL_SINK) {
-		/* source pad */
-		pad = media_entity_remote_source(pad);
-		if (pad == NULL ||
-		    media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
-			break;
-
-		sd = media_entity_to_v4l2_subdev(pad->entity);
-
-		if (sd->grp_id == GRP_ID_FIMC_IS_SENSOR ||
-		    sd->grp_id == GRP_ID_SENSOR)
-			return sd;
-		/* sink pad */
-		pad = &sd->entity.pads[0];
-	}
-	return NULL;
-}
-
 static int fimc_lite_hw_init(struct fimc_lite *fimc, bool isp_output)
 {
 	struct fimc_source_info *si;
@@ -830,7 +807,7 @@ static int fimc_lite_streamon(struct file *file, void *priv,
 	if (ret < 0)
 		goto err_p_stop;
 
-	fimc->sensor = __find_remote_sensor(&fimc->subdev.entity);
+	fimc->sensor = fimc_find_remote_sensor(&fimc->subdev.entity);
 
 	ret = vb2_ioctl_streamon(file, priv, type);
 	if (!ret) {
@@ -1212,7 +1189,7 @@ static int fimc_lite_subdev_s_stream(struct v4l2_subdev *sd, int on)
 	 * The pipeline links are protected through entity.stream_count
 	 * so there is no need to take the media graph mutex here.
 	 */
-	fimc->sensor = __find_remote_sensor(&sd->entity);
+	fimc->sensor = fimc_find_remote_sensor(&sd->entity);
 
 	if (atomic_read(&fimc->out_path) != FIMC_IO_ISP)
 		return -ENOIOCTLCMD;
-- 
1.7.9.5

[REVIEW PATCH v2 02/11] exynos4-is: Add struct exynos_video_entity

[Sylwester Nawrocki]

This patch introduces common structure for the video entities
to handle all video nodes and media pipelines associated with
them in more generic way.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-capture.c  |   43 ++++++++++++---------
 drivers/media/platform/exynos4-is/fimc-core.h     |    4 +-
 drivers/media/platform/exynos4-is/fimc-lite-reg.c |    2 +-
 drivers/media/platform/exynos4-is/fimc-lite.c     |   20 +++++-----
 drivers/media/platform/exynos4-is/fimc-lite.h     |    4 +-
 drivers/media/platform/exynos4-is/fimc-reg.c      |    7 ++--
 drivers/media/platform/exynos4-is/media-dev.c     |    4 +-
 drivers/media/platform/exynos4-is/media-dev.h     |    8 ++--
 include/media/s5p_fimc.h                          |    5 +++
 9 files changed, 55 insertions(+), 42 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 528f413..be4387b 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -320,6 +320,7 @@ static void buffer_queue(struct vb2_buffer *vb);
 int fimc_capture_resume(struct fimc_dev *fimc)
 {
 	struct fimc_vid_cap *vid_cap = &fimc->vid_cap;
+	struct exynos_video_entity *ve = &vid_cap->ve;
 	struct fimc_vid_buffer *buf;
 	int i;
 
@@ -329,7 +330,7 @@ int fimc_capture_resume(struct fimc_dev *fimc)
 	INIT_LIST_HEAD(&fimc->vid_cap.active_buf_q);
 	vid_cap->buf_index = 0;
 	fimc_pipeline_call(fimc, open, &fimc->pipeline,
-			   &vid_cap->vfd.entity, false);
+			   &ve->vdev.entity, false);
 	fimc_capture_hw_init(fimc);
 
 	clear_bit(ST_CAPT_SUSPENDED, &fimc->state);
@@ -397,7 +398,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		unsigned long size = ctx->d_frame.payload[i];
 
 		if (vb2_plane_size(vb, i) < size) {
-			v4l2_err(&ctx->fimc_dev->vid_cap.vfd,
+			v4l2_err(&ctx->fimc_dev->vid_cap.ve.vdev,
 				 "User buffer too small (%ld < %ld)\n",
 				 vb2_plane_size(vb, i), size);
 			return -EINVAL;
@@ -415,6 +416,7 @@ static void buffer_queue(struct vb2_buffer *vb)
 	struct fimc_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
 	struct fimc_dev *fimc = ctx->fimc_dev;
 	struct fimc_vid_cap *vid_cap = &fimc->vid_cap;
+	struct exynos_video_entity *ve = &vid_cap->ve;
 	unsigned long flags;
 	int min_bufs;
 
@@ -454,7 +456,7 @@ static void buffer_queue(struct vb2_buffer *vb)
 
 		ret = fimc_pipeline_call(fimc, set_stream, &fimc->pipeline, 1);
 		if (ret < 0)
-			v4l2_err(&vid_cap->vfd, "stream on failed: %d\n", ret);
+			v4l2_err(&ve->vdev, "stream on failed: %d\n", ret);
 		return;
 	}
 	spin_unlock_irqrestore(&fimc->slock, flags);
@@ -503,11 +505,12 @@ static int fimc_capture_set_default_format(struct fimc_dev *fimc);
 static int fimc_capture_open(struct file *file)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
+	struct exynos_video_entity *ve = &fimc->vid_cap.ve;
 	int ret = -EBUSY;
 
 	dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
 
-	fimc_md_graph_lock(fimc);
+	fimc_md_graph_lock(ve);
 	mutex_lock(&fimc->lock);
 
 	if (fimc_m2m_active(fimc))
@@ -526,7 +529,7 @@ static int fimc_capture_open(struct file *file)
 
 	if (v4l2_fh_is_singular_file(file)) {
 		ret = fimc_pipeline_call(fimc, open, &fimc->pipeline,
-					 &fimc->vid_cap.vfd.entity, true);
+					 &fimc->vid_cap.ve.vdev.entity, true);
 
 		if (!ret && !fimc->vid_cap.user_subdev_api)
 			ret = fimc_capture_set_default_format(fimc);
@@ -544,7 +547,7 @@ static int fimc_capture_open(struct file *file)
 	}
 unlock:
 	mutex_unlock(&fimc->lock);
-	fimc_md_graph_unlock(fimc);
+	fimc_md_graph_unlock(ve);
 	return ret;
 }
 
@@ -560,7 +563,7 @@ static int fimc_capture_release(struct file *file)
 
 	if (v4l2_fh_is_singular_file(file)) {
 		if (vc->streaming) {
-			media_entity_pipeline_stop(&vc->vfd.entity);
+			media_entity_pipeline_stop(&vc->ve.vdev.entity);
 			vc->streaming = false;
 		}
 		clear_bit(ST_CAPT_BUSY, &fimc->state);
@@ -935,11 +938,12 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 	struct v4l2_pix_format_mplane *pix = &f->fmt.pix_mp;
 	struct fimc_dev *fimc = video_drvdata(file);
 	struct fimc_ctx *ctx = fimc->vid_cap.ctx;
+	struct exynos_video_entity *ve = &fimc->vid_cap.ve;
 	struct v4l2_mbus_framefmt mf;
 	struct fimc_fmt *ffmt = NULL;
 	int ret = 0;
 
-	fimc_md_graph_lock(fimc);
+	fimc_md_graph_lock(ve);
 	mutex_lock(&fimc->lock);
 
 	if (fimc_jpeg_fourcc(pix->pixelformat)) {
@@ -975,7 +979,7 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 					pix->plane_fmt, ffmt->memplanes, true);
 unlock:
 	mutex_unlock(&fimc->lock);
-	fimc_md_graph_unlock(fimc);
+	fimc_md_graph_unlock(ve);
 
 	return ret;
 }
@@ -1076,7 +1080,7 @@ static int fimc_cap_s_fmt_mplane(struct file *file, void *priv,
 	struct fimc_dev *fimc = video_drvdata(file);
 	int ret;
 
-	fimc_md_graph_lock(fimc);
+	fimc_md_graph_lock(&fimc->vid_cap.ve);
 	mutex_lock(&fimc->lock);
 	/*
 	 * The graph is walked within __fimc_capture_set_format() to set
@@ -1088,8 +1092,8 @@ static int fimc_cap_s_fmt_mplane(struct file *file, void *priv,
 	 */
 	ret = __fimc_capture_set_format(fimc, f);
 
+	fimc_md_graph_unlock(&fimc->vid_cap.ve);
 	mutex_unlock(&fimc->lock);
-	fimc_md_graph_unlock(fimc);
 	return ret;
 }
 
@@ -1209,7 +1213,7 @@ static int fimc_cap_streamon(struct file *file, void *priv,
 	struct fimc_dev *fimc = video_drvdata(file);
 	struct fimc_pipeline *p = &fimc->pipeline;
 	struct fimc_vid_cap *vc = &fimc->vid_cap;
-	struct media_entity *entity = &vc->vfd.entity;
+	struct media_entity *entity = &vc->ve.vdev.entity;
 	struct fimc_source_info *si = NULL;
 	struct v4l2_subdev *sd;
 	int ret;
@@ -1259,14 +1263,15 @@ static int fimc_cap_streamoff(struct file *file, void *priv,
 			    enum v4l2_buf_type type)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
+	struct fimc_vid_cap *vc = &fimc->vid_cap;
 	int ret;
 
 	ret = vb2_ioctl_streamoff(file, priv, type);
 	if (ret < 0)
 		return ret;
 
-	media_entity_pipeline_stop(&fimc->vid_cap.vfd.entity);
-	fimc->vid_cap.streaming = false;
+	media_entity_pipeline_stop(&vc->ve.vdev.entity);
+	vc->streaming = false;
 	return 0;
 }
 
@@ -1735,7 +1740,7 @@ static int fimc_capture_set_default_format(struct fimc_dev *fimc)
 static int fimc_register_capture_device(struct fimc_dev *fimc,
 				 struct v4l2_device *v4l2_dev)
 {
-	struct video_device *vfd = &fimc->vid_cap.vfd;
+	struct video_device *vfd = &fimc->vid_cap.ve.vdev;
 	struct vb2_queue *q = &fimc->vid_cap.vbq;
 	struct fimc_ctx *ctx;
 	struct fimc_vid_cap *vid_cap;
@@ -1840,15 +1845,17 @@ static int fimc_capture_subdev_registered(struct v4l2_subdev *sd)
 static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd)
 {
 	struct fimc_dev *fimc = v4l2_get_subdevdata(sd);
+	struct video_device *vdev;
 
 	if (fimc == NULL)
 		return;
 
 	fimc_unregister_m2m_device(fimc);
+	vdev = &fimc->vid_cap.ve.vdev;
 
-	if (video_is_registered(&fimc->vid_cap.vfd)) {
-		video_unregister_device(&fimc->vid_cap.vfd);
-		media_entity_cleanup(&fimc->vid_cap.vfd.entity);
+	if (video_is_registered(vdev)) {
+		video_unregister_device(vdev);
+		media_entity_cleanup(&vdev->entity);
 		fimc->pipeline_ops = NULL;
 	}
 	kfree(fimc->vid_cap.ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-core.h b/drivers/media/platform/exynos4-is/fimc-core.h
index 8666e4b..401b746 100644
--- a/drivers/media/platform/exynos4-is/fimc-core.h
+++ b/drivers/media/platform/exynos4-is/fimc-core.h
@@ -285,8 +285,8 @@ struct fimc_m2m_device {
 /**
  * struct fimc_vid_cap - camera capture device information
  * @ctx: hardware context data
- * @vfd: video device node for camera capture mode
  * @subdev: subdev exposing the FIMC processing block
+ * @ve: exynos video device entity structure
  * @vd_pad: fimc video capture node pad
  * @sd_pads: fimc video processing block pads
  * @ci_fmt: image format at the FIMC camera input (and the scaler output)
@@ -307,8 +307,8 @@ struct fimc_m2m_device {
 struct fimc_vid_cap {
 	struct fimc_ctx			*ctx;
 	struct vb2_alloc_ctx		*alloc_ctx;
-	struct video_device		vfd;
 	struct v4l2_subdev		subdev;
+	struct exynos_video_entity	ve;
 	struct media_pad		vd_pad;
 	struct media_pad		sd_pads[FIMC_SD_PADS_NUM];
 	struct v4l2_mbus_framefmt	ci_fmt;
diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.c b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
index 8cc0d39..eb4f763 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite-reg.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
@@ -137,7 +137,7 @@ void flite_hw_set_source_format(struct fimc_lite *dev, struct flite_frame *f)
 	}
 
 	if (i == 0 && src_pixfmt_map[i][0] != pixelcode) {
-		v4l2_err(&dev->vfd,
+		v4l2_err(&dev->ve.vdev,
 			 "Unsupported pixel code, falling back to %#08x\n",
 			 src_pixfmt_map[i][0]);
 	}
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 66480e7..a68c9c6 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -392,7 +392,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		unsigned long size = fimc->payload[i];
 
 		if (vb2_plane_size(vb, i) < size) {
-			v4l2_err(&fimc->vfd,
+			v4l2_err(&fimc->ve.vdev,
 				 "User buffer too small (%ld < %ld)\n",
 				 vb2_plane_size(vb, i), size);
 			return -EINVAL;
@@ -458,7 +458,7 @@ static void fimc_lite_clear_event_counters(struct fimc_lite *fimc)
 static int fimc_lite_open(struct file *file)
 {
 	struct fimc_lite *fimc = video_drvdata(file);
-	struct media_entity *me = &fimc->vfd.entity;
+	struct media_entity *me = &fimc->ve.vdev.entity;
 	int ret;
 
 	mutex_lock(&me->parent->graph_mutex);
@@ -509,7 +509,7 @@ static int fimc_lite_release(struct file *file)
 	if (v4l2_fh_is_singular_file(file) &&
 	    atomic_read(&fimc->out_path) == FIMC_IO_DMA) {
 		if (fimc->streaming) {
-			media_entity_pipeline_stop(&fimc->vfd.entity);
+			media_entity_pipeline_stop(&fimc->ve.vdev.entity);
 			fimc->streaming = false;
 		}
 		clear_bit(ST_FLITE_IN_USE, &fimc->state);
@@ -792,7 +792,7 @@ static int fimc_lite_streamon(struct file *file, void *priv,
 			      enum v4l2_buf_type type)
 {
 	struct fimc_lite *fimc = video_drvdata(file);
-	struct media_entity *entity = &fimc->vfd.entity;
+	struct media_entity *entity = &fimc->ve.vdev.entity;
 	struct fimc_pipeline *p = &fimc->pipeline;
 	int ret;
 
@@ -830,7 +830,7 @@ static int fimc_lite_streamoff(struct file *file, void *priv,
 	if (ret < 0)
 		return ret;
 
-	media_entity_pipeline_stop(&fimc->vfd.entity);
+	media_entity_pipeline_stop(&fimc->ve.vdev.entity);
 	fimc->streaming = false;
 	return 0;
 }
@@ -1234,7 +1234,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
 {
 	struct fimc_lite *fimc = v4l2_get_subdevdata(sd);
 	struct vb2_queue *q = &fimc->vb_queue;
-	struct video_device *vfd = &fimc->vfd;
+	struct video_device *vfd = &fimc->ve.vdev;
 	int ret;
 
 	memset(vfd, 0, sizeof(*vfd));
@@ -1298,9 +1298,9 @@ static void fimc_lite_subdev_unregistered(struct v4l2_subdev *sd)
 	if (fimc == NULL)
 		return;
 
-	if (video_is_registered(&fimc->vfd)) {
-		video_unregister_device(&fimc->vfd);
-		media_entity_cleanup(&fimc->vfd.entity);
+	if (video_is_registered(&fimc->ve.vdev)) {
+		video_unregister_device(&fimc->ve.vdev);
+		media_entity_cleanup(&fimc->ve.vdev.entity);
 		fimc->pipeline_ops = NULL;
 	}
 }
@@ -1548,7 +1548,7 @@ static int fimc_lite_resume(struct device *dev)
 
 	INIT_LIST_HEAD(&fimc->active_buf_q);
 	fimc_pipeline_call(fimc, open, &fimc->pipeline,
-			   &fimc->vfd.entity, false);
+			   &fimc->ve.vdev.entity, false);
 	fimc_lite_hw_init(fimc, atomic_read(&fimc->out_path) == FIMC_IO_ISP);
 	clear_bit(ST_FLITE_SUSPENDED, &fimc->state);
 
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.h b/drivers/media/platform/exynos4-is/fimc-lite.h
index 47da5e0..fa3886a 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.h
+++ b/drivers/media/platform/exynos4-is/fimc-lite.h
@@ -95,8 +95,8 @@ struct flite_buffer {
  * struct fimc_lite - fimc lite structure
  * @pdev: pointer to FIMC-LITE platform device
  * @dd: SoC specific driver data structure
+ * @ve: exynos video device entity structure
  * @v4l2_dev: pointer to top the level v4l2_device
- * @vfd: video device node
  * @fh: v4l2 file handle
  * @alloc_ctx: videobuf2 memory allocator context
  * @subdev: FIMC-LITE subdev
@@ -130,8 +130,8 @@ struct flite_buffer {
 struct fimc_lite {
 	struct platform_device	*pdev;
 	struct flite_drvdata	*dd;
+	struct exynos_video_entity ve;
 	struct v4l2_device	*v4l2_dev;
-	struct video_device	vfd;
 	struct v4l2_fh		fh;
 	struct vb2_alloc_ctx	*alloc_ctx;
 	struct v4l2_subdev	subdev;
diff --git a/drivers/media/platform/exynos4-is/fimc-reg.c b/drivers/media/platform/exynos4-is/fimc-reg.c
index f079f36..1db8cb4 100644
--- a/drivers/media/platform/exynos4-is/fimc-reg.c
+++ b/drivers/media/platform/exynos4-is/fimc-reg.c
@@ -618,7 +618,7 @@ int fimc_hw_set_camera_source(struct fimc_dev *fimc,
 		}
 
 		if (i == ARRAY_SIZE(pix_desc)) {
-			v4l2_err(&vc->vfd,
+			v4l2_err(&vc->ve.vdev,
 				 "Camera color format not supported: %d\n",
 				 vc->ci_fmt.code);
 			return -EINVAL;
@@ -698,7 +698,7 @@ int fimc_hw_set_camera_type(struct fimc_dev *fimc,
 			cfg |= FIMC_REG_CIGCTRL_CAM_JPEG;
 			break;
 		default:
-			v4l2_err(&vid_cap->vfd,
+			v4l2_err(&vid_cap->ve.vdev,
 				 "Not supported camera pixel format: %#x\n",
 				 vid_cap->ci_fmt.code);
 			return -EINVAL;
@@ -721,7 +721,8 @@ int fimc_hw_set_camera_type(struct fimc_dev *fimc,
 			WARN_ONCE(1, "ISP Writeback input is not supported\n");
 		break;
 	default:
-		v4l2_err(&vid_cap->vfd, "Invalid FIMC bus type selected: %d\n",
+		v4l2_err(&vid_cap->ve.vdev,
+			 "Invalid FIMC bus type selected: %d\n",
 			 source->fimc_bus_type);
 		return -EINVAL;
 	}
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 15ef8f2..032d2b7 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -929,7 +929,7 @@ static int __fimc_md_create_flite_source_links(struct fimc_md *fmd)
 			continue;
 
 		source = &fimc->subdev.entity;
-		sink = &fimc->vfd.entity;
+		sink = &fimc->ve.vdev.entity;
 		/* FIMC-LITE's subdev and video node */
 		ret = media_entity_create_link(source, FLITE_SD_PAD_SOURCE_DMA,
 					       sink, 0, 0);
@@ -1066,7 +1066,7 @@ static int fimc_md_create_links(struct fimc_md *fmd)
 			continue;
 
 		source = &fmd->fimc[i]->vid_cap.subdev.entity;
-		sink = &fmd->fimc[i]->vid_cap.vfd.entity;
+		sink = &fmd->fimc[i]->vid_cap.ve.vdev.entity;
 
 		ret = media_entity_create_link(source, FIMC_SD_PAD_SOURCE,
 					      sink, 0, flags);
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index 44d86b6..3e9680c 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -127,14 +127,14 @@ static inline struct fimc_md *entity_to_fimc_mdev(struct media_entity *me)
 		container_of(me->parent, struct fimc_md, media_dev);
 }
 
-static inline void fimc_md_graph_lock(struct fimc_dev *fimc)
+static inline void fimc_md_graph_lock(struct exynos_video_entity *ve)
 {
-	mutex_lock(&fimc->vid_cap.vfd.entity.parent->graph_mutex);
+	mutex_lock(&ve->vdev.entity.parent->graph_mutex);
 }
 
-static inline void fimc_md_graph_unlock(struct fimc_dev *fimc)
+static inline void fimc_md_graph_unlock(struct exynos_video_entity *ve)
 {
-	mutex_unlock(&fimc->vid_cap.vfd.entity.parent->graph_mutex);
+	mutex_unlock(&ve->vdev.entity.parent->graph_mutex);
 }
 
 int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on);
diff --git a/include/media/s5p_fimc.h b/include/media/s5p_fimc.h
index f509690..f5313b4 100644
--- a/include/media/s5p_fimc.h
+++ b/include/media/s5p_fimc.h
@@ -13,6 +13,7 @@
 #define S5P_FIMC_H_
 
 #include <media/media-entity.h>
+#include <media/v4l2-dev.h>
 #include <media/v4l2-mediabus.h>
 
 /*
@@ -157,6 +158,10 @@ struct fimc_pipeline {
 	struct media_pipeline *m_pipeline;
 };
 
+struct exynos_video_entity {
+	struct video_device vdev;
+};
+
 /*
  * Media pipeline operations to be called from within the fimc(-lite)
  * video node when it is the last entity of the pipeline. Implemented
-- 
1.7.9.5

[REVIEW PATCH v2 03/11] exynos4-is: Preserve state of controls between /dev/video open/close

[Sylwester Nawrocki]

This patch moves the code for inheriting subdev v4l2 controls on the
FIMC video capture nodes from open()/close() fops to the link setup
notification callback. This allows for the state of the FIMC controls
to be always kept, in opposite to the current situation when it is
lost when last process closes video device.

There is no visible change for the original V4L2 compliant interface.
For the MC aware applications (user_subdev_api == true) inheriting
of the controls is dropped, since there can be same controls on the
subdevs withing single pipeline, now when the ISP (FIMC-IS) is also
used.

This patch is a prerequisite to allow /dev/video device to be opened
without errors even if there is no media links connecting it to an
image source (sensor) subdev. This is required for a libv4l2 plugin
to be initialized while a video node is opened and it also should be
possible to always open the device to query the capabilities.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-capture.c |   96 +++++++++++-----------
 drivers/media/platform/exynos4-is/fimc-core.h    |    3 +
 drivers/media/platform/exynos4-is/media-dev.c    |    4 -
 3 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index be4387b..762fc7b9 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -27,9 +27,10 @@
 #include <media/videobuf2-core.h>
 #include <media/videobuf2-dma-contig.h>
 
-#include "media-dev.h"
+#include "common.h"
 #include "fimc-core.h"
 #include "fimc-reg.h"
+#include "media-dev.h"
 
 static int fimc_capture_hw_init(struct fimc_dev *fimc)
 {
@@ -472,40 +473,13 @@ static struct vb2_ops fimc_capture_qops = {
 	.stop_streaming		= stop_streaming,
 };
 
-/**
- * fimc_capture_ctrls_create - initialize the control handler
- * Initialize the capture video node control handler and fill it
- * with the FIMC controls. Inherit any sensor's controls if the
- * 'user_subdev_api' flag is false (default behaviour).
- * This function need to be called with the graph mutex held.
- */
-int fimc_capture_ctrls_create(struct fimc_dev *fimc)
-{
-	struct fimc_vid_cap *vid_cap = &fimc->vid_cap;
-	struct v4l2_subdev *sensor = fimc->pipeline.subdevs[IDX_SENSOR];
-	int ret;
-
-	if (WARN_ON(vid_cap->ctx == NULL))
-		return -ENXIO;
-	if (vid_cap->ctx->ctrls.ready)
-		return 0;
-
-	ret = fimc_ctrls_create(vid_cap->ctx);
-
-	if (ret || vid_cap->user_subdev_api || !sensor ||
-	    !vid_cap->ctx->ctrls.ready)
-		return ret;
-
-	return v4l2_ctrl_add_handler(&vid_cap->ctx->ctrls.handler,
-				     sensor->ctrl_handler, NULL);
-}
-
 static int fimc_capture_set_default_format(struct fimc_dev *fimc);
 
 static int fimc_capture_open(struct file *file)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
-	struct exynos_video_entity *ve = &fimc->vid_cap.ve;
+	struct fimc_vid_cap *vc = &fimc->vid_cap;
+	struct exynos_video_entity *ve = &vc->ve;
 	int ret = -EBUSY;
 
 	dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
@@ -530,12 +504,20 @@ static int fimc_capture_open(struct file *file)
 	if (v4l2_fh_is_singular_file(file)) {
 		ret = fimc_pipeline_call(fimc, open, &fimc->pipeline,
 					 &fimc->vid_cap.ve.vdev.entity, true);
-
-		if (!ret && !fimc->vid_cap.user_subdev_api)
+		if (ret == 0)
 			ret = fimc_capture_set_default_format(fimc);
 
-		if (!ret)
-			ret = fimc_capture_ctrls_create(fimc);
+		if (ret == 0 && vc->user_subdev_api && vc->inh_sensor_ctrls) {
+			/*
+			 * Recreate controls of the the video node to drop
+			 * any controls inherited from the sensor subdev.
+			 */
+			fimc_ctrls_delete(vc->ctx);
+
+			ret = fimc_ctrls_create(vc->ctx);
+			if (ret == 0)
+				vc->inh_sensor_ctrls = false;
+		}
 
 		if (ret < 0) {
 			clear_bit(ST_CAPT_BUSY, &fimc->state);
@@ -574,10 +556,6 @@ static int fimc_capture_release(struct file *file)
 	}
 
 	pm_runtime_put(&fimc->pdev->dev);
-
-	if (v4l2_fh_is_singular_file(file))
-		fimc_ctrls_delete(fimc->vid_cap.ctx);
-
 	ret = vb2_fop_release(file);
 	mutex_unlock(&fimc->lock);
 
@@ -1410,6 +1388,8 @@ static int fimc_link_setup(struct media_entity *entity,
 {
 	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
 	struct fimc_dev *fimc = v4l2_get_subdevdata(sd);
+	struct fimc_vid_cap *vc = &fimc->vid_cap;
+	struct v4l2_subdev *sensor;
 
 	if (media_entity_type(remote->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
 		return -EINVAL;
@@ -1421,15 +1401,26 @@ static int fimc_link_setup(struct media_entity *entity,
 	    local->entity->name, remote->entity->name, flags,
 	    fimc->vid_cap.input);
 
-	if (flags & MEDIA_LNK_FL_ENABLED) {
-		if (fimc->vid_cap.input != 0)
-			return -EBUSY;
-		fimc->vid_cap.input = sd->grp_id;
+	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
+		fimc->vid_cap.input = 0;
 		return 0;
 	}
 
-	fimc->vid_cap.input = 0;
-	return 0;
+	if (vc->input != 0)
+		return -EBUSY;
+
+	vc->input = sd->grp_id;
+
+	if (vc->user_subdev_api || vc->inh_sensor_ctrls)
+		return 0;
+
+	/* Inherit V4L2 controls from the image sensor subdev. */
+	sensor = fimc_find_remote_sensor(&vc->subdev.entity);
+	if (sensor == NULL)
+		return 0;
+
+	return v4l2_ctrl_add_handler(&vc->ctx->ctrls.handler,
+				     sensor->ctrl_handler, NULL);
 }
 
 static const struct media_entity_operations fimc_sd_media_ops = {
@@ -1789,12 +1780,16 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
 
 	ret = vb2_queue_init(q);
 	if (ret)
-		goto err_ent;
+		goto err_free_ctx;
 
 	vid_cap->vd_pad.flags = MEDIA_PAD_FL_SINK;
 	ret = media_entity_init(&vfd->entity, 1, &vid_cap->vd_pad, 0);
 	if (ret)
-		goto err_ent;
+		goto err_free_ctx;
+
+	ret = fimc_ctrls_create(ctx);
+	if (ret)
+		goto err_me_cleanup;
 	/*
 	 * For proper order of acquiring/releasing the video
 	 * and the graph mutex.
@@ -1804,7 +1799,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
 
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
 	if (ret)
-		goto err_vd;
+		goto err_ctrl_free;
 
 	v4l2_info(v4l2_dev, "Registered %s as /dev/%s\n",
 		  vfd->name, video_device_node_name(vfd));
@@ -1812,9 +1807,11 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
 	vfd->ctrl_handler = &ctx->ctrls.handler;
 	return 0;
 
-err_vd:
+err_ctrl_free:
+	fimc_ctrls_delete(ctx);
+err_me_cleanup:
 	media_entity_cleanup(&vfd->entity);
-err_ent:
+err_free_ctx:
 	kfree(ctx);
 	return ret;
 }
@@ -1856,6 +1853,7 @@ static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd)
 	if (video_is_registered(vdev)) {
 		video_unregister_device(vdev);
 		media_entity_cleanup(&vdev->entity);
+		fimc_ctrls_delete(fimc->vid_cap.ctx);
 		fimc->pipeline_ops = NULL;
 	}
 	kfree(fimc->vid_cap.ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-core.h b/drivers/media/platform/exynos4-is/fimc-core.h
index 401b746..86bb8e6 100644
--- a/drivers/media/platform/exynos4-is/fimc-core.h
+++ b/drivers/media/platform/exynos4-is/fimc-core.h
@@ -303,6 +303,8 @@ struct fimc_m2m_device {
  * @refcnt: driver's private reference counter
  * @input: capture input type, grp_id of the attached subdev
  * @user_subdev_api: true if subdevs are not configured by the host driver
+ * @inh_sensor_ctrls: a flag indicating v4l2 controls are inherited from
+ * 		      an image sensor subdev
  */
 struct fimc_vid_cap {
 	struct fimc_ctx			*ctx;
@@ -326,6 +328,7 @@ struct fimc_vid_cap {
 	int				refcnt;
 	u32				input;
 	bool				user_subdev_api;
+	bool				inh_sensor_ctrls;
 };
 
 /**
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 032d2b7..122a6ba 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1272,8 +1272,6 @@ static int fimc_md_link_notify(struct media_pad *source,
 	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
 		if (ref_count > 0) {
 			ret = __fimc_pipeline_close(pipeline);
-			if (!ret && fimc)
-				fimc_ctrls_delete(fimc->vid_cap.ctx);
 		}
 		for (i = 0; i < IDX_MAX; i++)
 			pipeline->subdevs[i] = NULL;
@@ -1285,8 +1283,6 @@ static int fimc_md_link_notify(struct media_pad *source,
 		 */
 		ret = __fimc_pipeline_open(pipeline,
 					   source->entity, true);
-		if (!ret && fimc)
-			ret = fimc_capture_ctrls_create(fimc);
 	}
 
 	mutex_unlock(lock);
-- 
1.7.9.5

[REVIEW PATCH v2 04/11] exynos4-is: Media graph/video device locking rework

[Sylwester Nawrocki]

Remove driver private video node reference counters and use entity->use_count
instead. This makes the video pipelines power handling more similar to the
method used in omap3isp driver.

Now the graph mutex is taken always after the video mutex, as it is not
possible to ensure apposite order at the all modules.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes since v1:
 - fixed use_count handling in fimc_lite_release() function.
---
 drivers/media/platform/exynos4-is/fimc-capture.c |   59 ++++++++++------------
 drivers/media/platform/exynos4-is/fimc-core.h    |    2 -
 drivers/media/platform/exynos4-is/fimc-lite.c    |   23 +++++----
 drivers/media/platform/exynos4-is/fimc-lite.h    |    2 -
 drivers/media/platform/exynos4-is/media-dev.c    |   12 +----
 5 files changed, 42 insertions(+), 56 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 762fc7b9..3b24f29 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -484,7 +484,6 @@ static int fimc_capture_open(struct file *file)
 
 	dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
 
-	fimc_md_graph_lock(ve);
 	mutex_lock(&fimc->lock);
 
 	if (fimc_m2m_active(fimc))
@@ -502,6 +501,8 @@ static int fimc_capture_open(struct file *file)
 	}
 
 	if (v4l2_fh_is_singular_file(file)) {
+		fimc_md_graph_lock(ve);
+
 		ret = fimc_pipeline_call(fimc, open, &fimc->pipeline,
 					 &fimc->vid_cap.ve.vdev.entity, true);
 		if (ret == 0)
@@ -518,18 +519,19 @@ static int fimc_capture_open(struct file *file)
 			if (ret == 0)
 				vc->inh_sensor_ctrls = false;
 		}
+		if (ret == 0)
+			ve->vdev.entity.use_count++;
+
+		fimc_md_graph_unlock(ve);
 
 		if (ret < 0) {
 			clear_bit(ST_CAPT_BUSY, &fimc->state);
 			pm_runtime_put_sync(&fimc->pdev->dev);
 			v4l2_fh_release(file);
-		} else {
-			fimc->vid_cap.refcnt++;
 		}
 	}
 unlock:
 	mutex_unlock(&fimc->lock);
-	fimc_md_graph_unlock(ve);
 	return ret;
 }
 
@@ -537,26 +539,32 @@ static int fimc_capture_release(struct file *file)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
 	struct fimc_vid_cap *vc = &fimc->vid_cap;
+	bool close = v4l2_fh_is_singular_file(file);
 	int ret;
 
 	dbg("pid: %d, state: 0x%lx", task_pid_nr(current), fimc->state);
 
 	mutex_lock(&fimc->lock);
 
-	if (v4l2_fh_is_singular_file(file)) {
-		if (vc->streaming) {
-			media_entity_pipeline_stop(&vc->ve.vdev.entity);
-			vc->streaming = false;
-		}
+	if (close && vc->streaming) {
+		media_entity_pipeline_stop(&vc->ve.vdev.entity);
+		vc->streaming = false;
+	}
+
+	ret = vb2_fop_release(file);
+
+	if (close) {
 		clear_bit(ST_CAPT_BUSY, &fimc->state);
 		fimc_stop_capture(fimc, false);
 		fimc_pipeline_call(fimc, close, &fimc->pipeline);
 		clear_bit(ST_CAPT_SUSPENDED, &fimc->state);
-		fimc->vid_cap.refcnt--;
+
+		fimc_md_graph_lock(&vc->ve);
+		vc->ve.vdev.entity.use_count--;
+		fimc_md_graph_unlock(&vc->ve);
 	}
 
 	pm_runtime_put(&fimc->pdev->dev);
-	ret = vb2_fop_release(file);
 	mutex_unlock(&fimc->lock);
 
 	return ret;
@@ -921,9 +929,6 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 	struct fimc_fmt *ffmt = NULL;
 	int ret = 0;
 
-	fimc_md_graph_lock(ve);
-	mutex_lock(&fimc->lock);
-
 	if (fimc_jpeg_fourcc(pix->pixelformat)) {
 		fimc_capture_try_format(ctx, &pix->width, &pix->height,
 					NULL, &pix->pixelformat,
@@ -934,16 +939,18 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 	ffmt = fimc_capture_try_format(ctx, &pix->width, &pix->height,
 				       NULL, &pix->pixelformat,
 				       FIMC_SD_PAD_SOURCE);
-	if (!ffmt) {
-		ret = -EINVAL;
-		goto unlock;
-	}
+	if (!ffmt)
+		return -EINVAL;
 
 	if (!fimc->vid_cap.user_subdev_api) {
 		mf.width = pix->width;
 		mf.height = pix->height;
 		mf.code = ffmt->mbus_code;
+
+		fimc_md_graph_lock(ve);
 		fimc_pipeline_try_format(ctx, &mf, &ffmt, false);
+		fimc_md_graph_unlock(ve);
+
 		pix->width = mf.width;
 		pix->height = mf.height;
 		if (ffmt)
@@ -955,9 +962,6 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 	if (ffmt->flags & FMT_FLAGS_COMPRESSED)
 		fimc_get_sensor_frame_desc(fimc->pipeline.subdevs[IDX_SENSOR],
 					pix->plane_fmt, ffmt->memplanes, true);
-unlock:
-	mutex_unlock(&fimc->lock);
-	fimc_md_graph_unlock(ve);
 
 	return ret;
 }
@@ -1059,19 +1063,14 @@ static int fimc_cap_s_fmt_mplane(struct file *file, void *priv,
 	int ret;
 
 	fimc_md_graph_lock(&fimc->vid_cap.ve);
-	mutex_lock(&fimc->lock);
 	/*
 	 * The graph is walked within __fimc_capture_set_format() to set
 	 * the format at subdevs thus the graph mutex needs to be held at
-	 * this point and acquired before the video mutex, to avoid  AB-BA
-	 * deadlock when fimc_md_link_notify() is called by other thread.
-	 * Ideally the graph walking and setting format at the whole pipeline
-	 * should be removed from this driver and handled in userspace only.
+	 * this point.
 	 */
 	ret = __fimc_capture_set_format(fimc, f);
 
 	fimc_md_graph_unlock(&fimc->vid_cap.ve);
-	mutex_unlock(&fimc->lock);
 	return ret;
 }
 
@@ -1790,12 +1789,6 @@ static int fimc_register_capture_device(struct fimc_dev *fimc,
 	ret = fimc_ctrls_create(ctx);
 	if (ret)
 		goto err_me_cleanup;
-	/*
-	 * For proper order of acquiring/releasing the video
-	 * and the graph mutex.
-	 */
-	v4l2_disable_ioctl_locking(vfd, VIDIOC_TRY_FMT);
-	v4l2_disable_ioctl_locking(vfd, VIDIOC_S_FMT);
 
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
 	if (ret)
diff --git a/drivers/media/platform/exynos4-is/fimc-core.h b/drivers/media/platform/exynos4-is/fimc-core.h
index 86bb8e6..09c061d 100644
--- a/drivers/media/platform/exynos4-is/fimc-core.h
+++ b/drivers/media/platform/exynos4-is/fimc-core.h
@@ -300,7 +300,6 @@ struct fimc_m2m_device {
  * @frame_count: the frame counter for statistics
  * @reqbufs_count: the number of buffers requested in REQBUFS ioctl
  * @input_index: input (camera sensor) index
- * @refcnt: driver's private reference counter
  * @input: capture input type, grp_id of the attached subdev
  * @user_subdev_api: true if subdevs are not configured by the host driver
  * @inh_sensor_ctrls: a flag indicating v4l2 controls are inherited from
@@ -325,7 +324,6 @@ struct fimc_vid_cap {
 	unsigned int			reqbufs_count;
 	bool				streaming;
 	int				input_index;
-	int				refcnt;
 	u32				input;
 	bool				user_subdev_api;
 	bool				inh_sensor_ctrls;
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index a68c9c6..1da3ed1 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -461,8 +461,6 @@ static int fimc_lite_open(struct file *file)
 	struct media_entity *me = &fimc->ve.vdev.entity;
 	int ret;
 
-	mutex_lock(&me->parent->graph_mutex);
-
 	mutex_lock(&fimc->lock);
 	if (atomic_read(&fimc->out_path) != FIMC_IO_DMA) {
 		ret = -EBUSY;
@@ -482,11 +480,19 @@ static int fimc_lite_open(struct file *file)
 	    atomic_read(&fimc->out_path) != FIMC_IO_DMA)
 		goto unlock;
 
+	mutex_lock(&me->parent->graph_mutex);
+
 	ret = fimc_pipeline_call(fimc, open, &fimc->pipeline,
 						me, true);
+
+	/* Mark video pipeline ending at this video node as in use. */
+	if (ret == 0)
+		me->use_count++;
+
+	mutex_unlock(&me->parent->graph_mutex);
+
 	if (!ret) {
 		fimc_lite_clear_event_counters(fimc);
-		fimc->ref_count++;
 		goto unlock;
 	}
 
@@ -496,26 +502,28 @@ err_pm:
 	clear_bit(ST_FLITE_IN_USE, &fimc->state);
 unlock:
 	mutex_unlock(&fimc->lock);
-	mutex_unlock(&me->parent->graph_mutex);
 	return ret;
 }
 
 static int fimc_lite_release(struct file *file)
 {
 	struct fimc_lite *fimc = video_drvdata(file);
+	struct media_entity *entity = &fimc->ve.vdev.entity;
 
 	mutex_lock(&fimc->lock);
 
 	if (v4l2_fh_is_singular_file(file) &&
 	    atomic_read(&fimc->out_path) == FIMC_IO_DMA) {
 		if (fimc->streaming) {
-			media_entity_pipeline_stop(&fimc->ve.vdev.entity);
+			media_entity_pipeline_stop(entity);
 			fimc->streaming = false;
 		}
 		clear_bit(ST_FLITE_IN_USE, &fimc->state);
 		fimc_lite_stop_capture(fimc, false);
 		fimc_pipeline_call(fimc, close, &fimc->pipeline);
-		fimc->ref_count--;
+		mutex_lock(&entity->parent->graph_mutex);
+		entity->use_count--;
+		mutex_unlock(&entity->parent->graph_mutex);
 	}
 
 	vb2_fop_release(file);
@@ -954,8 +962,6 @@ static int fimc_lite_link_setup(struct media_entity *entity,
 		 __func__, remote->entity->name, local->entity->name,
 		 flags, fimc->source_subdev_grp_id);
 
-	mutex_lock(&fimc->lock);
-
 	switch (local->index) {
 	case FLITE_SD_PAD_SINK:
 		if (remote_ent_type != MEDIA_ENT_T_V4L2_SUBDEV) {
@@ -997,7 +1003,6 @@ static int fimc_lite_link_setup(struct media_entity *entity,
 	}
 	mb();
 
-	mutex_unlock(&fimc->lock);
 	return ret;
 }
 
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.h b/drivers/media/platform/exynos4-is/fimc-lite.h
index fa3886a..25abba9 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.h
+++ b/drivers/media/platform/exynos4-is/fimc-lite.h
@@ -125,7 +125,6 @@ struct flite_buffer {
  * @active_buf_count: number of video buffers scheduled in hardware
  * @frame_count: the captured frames counter
  * @reqbufs_count: the number of buffers requested with REQBUFS ioctl
- * @ref_count: driver's private reference counter
  */
 struct fimc_lite {
 	struct platform_device	*pdev;
@@ -163,7 +162,6 @@ struct fimc_lite {
 	struct vb2_queue	vb_queue;
 	unsigned int		frame_count;
 	unsigned int		reqbufs_count;
-	int			ref_count;
 
 	struct fimc_lite_events	events;
 	bool			streaming;
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 122a6ba..416dbcb 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1238,9 +1238,7 @@ static int fimc_md_link_notify(struct media_pad *source,
 	struct fimc_dev *fimc = NULL;
 	struct fimc_pipeline *pipeline;
 	struct v4l2_subdev *sd;
-	struct mutex *lock;
 	int i, ret = 0;
-	int ref_count;
 
 	if (media_entity_type(sink->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
 		return 0;
@@ -1253,29 +1251,24 @@ static int fimc_md_link_notify(struct media_pad *source,
 		if (WARN_ON(fimc_lite == NULL))
 			return 0;
 		pipeline = &fimc_lite->pipeline;
-		lock = &fimc_lite->lock;
 		break;
 	case GRP_ID_FIMC:
 		fimc = v4l2_get_subdevdata(sd);
 		if (WARN_ON(fimc == NULL))
 			return 0;
 		pipeline = &fimc->pipeline;
-		lock = &fimc->lock;
 		break;
 	default:
 		return 0;
 	}
 
-	mutex_lock(lock);
-	ref_count = fimc ? fimc->vid_cap.refcnt : fimc_lite->ref_count;
-
 	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
-		if (ref_count > 0) {
+		if (sink->entity->use_count > 0) {
 			ret = __fimc_pipeline_close(pipeline);
 		}
 		for (i = 0; i < IDX_MAX; i++)
 			pipeline->subdevs[i] = NULL;
-	} else if (ref_count > 0) {
+	} else if (sink->entity->use_count > 0) {
 		/*
 		 * Link activation. Enable power of pipeline elements only if
 		 * the pipeline is already in use, i.e. its video node is open.
@@ -1285,7 +1278,6 @@ static int fimc_md_link_notify(struct media_pad *source,
 					   source->entity, true);
 	}
 
-	mutex_unlock(lock);
 	return ret ? -EPIPE : ret;
 }
 
-- 
1.7.9.5

[REVIEW PATCH v2 05/11] exynos4-is: Do not use asynchronous runtime PM in release fop

[Sylwester Nawrocki]

Use pm_runtime_put_sync() instead of pm_runtime_put() to avoid races
in handling the 'state' bit flags when the fimc-capture drivers'
runtime_resume callback is called from the PM workqueue.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-capture.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 3b24f29..534ea68 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -496,7 +496,7 @@ static int fimc_capture_open(struct file *file)
 
 	ret = v4l2_fh_open(file);
 	if (ret) {
-		pm_runtime_put(&fimc->pdev->dev);
+		pm_runtime_put_sync(&fimc->pdev->dev);
 		goto unlock;
 	}
 
@@ -564,7 +564,7 @@ static int fimc_capture_release(struct file *file)
 		fimc_md_graph_unlock(&vc->ve);
 	}
 
-	pm_runtime_put(&fimc->pdev->dev);
+	pm_runtime_put_sync(&fimc->pdev->dev);
 	mutex_unlock(&fimc->lock);
 
 	return ret;
-- 
1.7.9.5

[REVIEW PATCH v2 06/11] exynos4-is: Use common exynos_media_pipeline data structure

[Sylwester Nawrocki]

This enumeration is now private to exynos4-is and the exynos5 camera
subsystem driver may have the subdevs handling designed differently.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes since v1:
 - corrected description of struct fimc_pipeline at media-dev.h
---
 drivers/media/platform/exynos4-is/fimc-capture.c |   67 ++++++++-----
 drivers/media/platform/exynos4-is/fimc-core.h    |    2 -
 drivers/media/platform/exynos4-is/fimc-lite.c    |   29 +++---
 drivers/media/platform/exynos4-is/fimc-lite.h    |    2 -
 drivers/media/platform/exynos4-is/media-dev.c    |  110 ++++++++++++++--------
 drivers/media/platform/exynos4-is/media-dev.h    |   38 ++++++++
 include/media/s5p_fimc.h                         |   53 +++++------
 7 files changed, 187 insertions(+), 114 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 534ea68..c2586a2 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -120,8 +120,7 @@ static int fimc_capture_state_cleanup(struct fimc_dev *fimc, bool suspend)
 	spin_unlock_irqrestore(&fimc->slock, flags);
 
 	if (streaming)
-		return fimc_pipeline_call(fimc, set_stream,
-					  &fimc->pipeline, 0);
+		return fimc_pipeline_call(&cap->ve, set_stream, 0);
 	else
 		return 0;
 }
@@ -179,8 +178,9 @@ static int fimc_capture_config_update(struct fimc_ctx *ctx)
 
 void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf)
 {
-	struct v4l2_subdev *csis = fimc->pipeline.subdevs[IDX_CSIS];
 	struct fimc_vid_cap *cap = &fimc->vid_cap;
+	struct fimc_pipeline *p = to_fimc_pipeline(cap->ve.pipe);
+	struct v4l2_subdev *csis = p->subdevs[IDX_CSIS];
 	struct fimc_frame *f = &cap->ctx->d_frame;
 	struct fimc_vid_buffer *v_buf;
 	struct timeval *tv;
@@ -288,8 +288,7 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
 		fimc_activate_capture(ctx);
 
 		if (!test_and_set_bit(ST_CAPT_ISP_STREAM, &fimc->state))
-			return fimc_pipeline_call(fimc, set_stream,
-						  &fimc->pipeline, 1);
+			return fimc_pipeline_call(&vid_cap->ve, set_stream, 1);
 	}
 
 	return 0;
@@ -313,7 +312,7 @@ int fimc_capture_suspend(struct fimc_dev *fimc)
 	int ret = fimc_stop_capture(fimc, suspend);
 	if (ret)
 		return ret;
-	return fimc_pipeline_call(fimc, close, &fimc->pipeline);
+	return fimc_pipeline_call(&fimc->vid_cap.ve, close);
 }
 
 static void buffer_queue(struct vb2_buffer *vb);
@@ -330,8 +329,7 @@ int fimc_capture_resume(struct fimc_dev *fimc)
 
 	INIT_LIST_HEAD(&fimc->vid_cap.active_buf_q);
 	vid_cap->buf_index = 0;
-	fimc_pipeline_call(fimc, open, &fimc->pipeline,
-			   &ve->vdev.entity, false);
+	fimc_pipeline_call(ve, open, &ve->vdev.entity, false);
 	fimc_capture_hw_init(fimc);
 
 	clear_bit(ST_CAPT_SUSPENDED, &fimc->state);
@@ -455,7 +453,7 @@ static void buffer_queue(struct vb2_buffer *vb)
 		if (test_and_set_bit(ST_CAPT_ISP_STREAM, &fimc->state))
 			return;
 
-		ret = fimc_pipeline_call(fimc, set_stream, &fimc->pipeline, 1);
+		ret = fimc_pipeline_call(ve, set_stream, 1);
 		if (ret < 0)
 			v4l2_err(&ve->vdev, "stream on failed: %d\n", ret);
 		return;
@@ -503,8 +501,8 @@ static int fimc_capture_open(struct file *file)
 	if (v4l2_fh_is_singular_file(file)) {
 		fimc_md_graph_lock(ve);
 
-		ret = fimc_pipeline_call(fimc, open, &fimc->pipeline,
-					 &fimc->vid_cap.ve.vdev.entity, true);
+		ret = fimc_pipeline_call(ve, open, &ve->vdev.entity, true);
+
 		if (ret == 0)
 			ret = fimc_capture_set_default_format(fimc);
 
@@ -555,8 +553,7 @@ static int fimc_capture_release(struct file *file)
 
 	if (close) {
 		clear_bit(ST_CAPT_BUSY, &fimc->state);
-		fimc_stop_capture(fimc, false);
-		fimc_pipeline_call(fimc, close, &fimc->pipeline);
+		fimc_pipeline_call(&vc->ve, close);
 		clear_bit(ST_CAPT_SUSPENDED, &fimc->state);
 
 		fimc_md_graph_lock(&vc->ve);
@@ -786,7 +783,8 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
 				    bool set)
 {
 	struct fimc_dev *fimc = ctx->fimc_dev;
-	struct v4l2_subdev *sd = fimc->pipeline.subdevs[IDX_SENSOR];
+	struct fimc_pipeline *p = to_fimc_pipeline(fimc->vid_cap.ve.pipe);
+	struct v4l2_subdev *sd = p->subdevs[IDX_SENSOR];
 	struct v4l2_subdev_format sfmt;
 	struct v4l2_mbus_framefmt *mf = &sfmt.format;
 	struct media_entity *me;
@@ -926,6 +924,7 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 	struct fimc_ctx *ctx = fimc->vid_cap.ctx;
 	struct exynos_video_entity *ve = &fimc->vid_cap.ve;
 	struct v4l2_mbus_framefmt mf;
+	struct v4l2_subdev *sensor;
 	struct fimc_fmt *ffmt = NULL;
 	int ret = 0;
 
@@ -959,9 +958,18 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 
 	fimc_adjust_mplane_format(ffmt, pix->width, pix->height, pix);
 
-	if (ffmt->flags & FMT_FLAGS_COMPRESSED)
-		fimc_get_sensor_frame_desc(fimc->pipeline.subdevs[IDX_SENSOR],
-					pix->plane_fmt, ffmt->memplanes, true);
+	if (ffmt->flags & FMT_FLAGS_COMPRESSED) {
+		fimc_md_graph_lock(ve);
+
+		sensor = __fimc_md_get_subdev(ve->pipe, IDX_SENSOR);
+		if (sensor)
+			fimc_get_sensor_frame_desc(sensor, pix->plane_fmt,
+						   ffmt->memplanes, true);
+		else
+			ret = -EPIPE;
+
+		fimc_md_graph_unlock(ve);
+	}
 
 	return ret;
 }
@@ -986,6 +994,7 @@ static int __fimc_capture_set_format(struct fimc_dev *fimc,
 	struct fimc_ctx *ctx = fimc->vid_cap.ctx;
 	struct v4l2_pix_format_mplane *pix = &f->fmt.pix_mp;
 	struct v4l2_mbus_framefmt *mf = &fimc->vid_cap.ci_fmt;
+	struct fimc_pipeline *p = to_fimc_pipeline(fimc->vid_cap.ve.pipe);
 	struct fimc_frame *ff = &ctx->d_frame;
 	struct fimc_fmt *s_fmt = NULL;
 	int ret, i;
@@ -1027,7 +1036,7 @@ static int __fimc_capture_set_format(struct fimc_dev *fimc,
 	fimc_adjust_mplane_format(ff->fmt, pix->width, pix->height, pix);
 
 	if (ff->fmt->flags & FMT_FLAGS_COMPRESSED) {
-		ret = fimc_get_sensor_frame_desc(fimc->pipeline.subdevs[IDX_SENSOR],
+		ret = fimc_get_sensor_frame_desc(p->subdevs[IDX_SENSOR],
 					pix->plane_fmt, ff->fmt->memplanes,
 					true);
 		if (ret < 0)
@@ -1078,14 +1087,20 @@ static int fimc_cap_enum_input(struct file *file, void *priv,
 			       struct v4l2_input *i)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
-	struct v4l2_subdev *sd = fimc->pipeline.subdevs[IDX_SENSOR];
+	struct exynos_video_entity *ve = &fimc->vid_cap.ve;
+	struct v4l2_subdev *sd;
 
 	if (i->index != 0)
 		return -EINVAL;
 
 	i->type = V4L2_INPUT_TYPE_CAMERA;
+	fimc_md_graph_lock(ve);
+	sd = __fimc_md_get_subdev(ve->pipe, IDX_SENSOR);
+	fimc_md_graph_unlock(ve);
+
 	if (sd)
 		strlcpy(i->name, sd->name, sizeof(i->name));
+
 	return 0;
 }
 
@@ -1111,6 +1126,7 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc)
 	struct v4l2_subdev_format sink_fmt, src_fmt;
 	struct fimc_vid_cap *vc = &fimc->vid_cap;
 	struct v4l2_subdev *sd = &vc->subdev;
+	struct fimc_pipeline *p = to_fimc_pipeline(vc->ve.pipe);
 	struct media_pad *sink_pad, *src_pad;
 	int i, ret;
 
@@ -1164,7 +1180,7 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc)
 		    src_fmt.format.code != sink_fmt.format.code)
 			return -EPIPE;
 
-		if (sd == fimc->pipeline.subdevs[IDX_SENSOR] &&
+		if (sd == p->subdevs[IDX_SENSOR] &&
 		    fimc_user_defined_mbus_fmt(src_fmt.format.code)) {
 			struct v4l2_plane_pix_format plane_fmt[FIMC_MAX_PLANES];
 			struct fimc_frame *frame = &vc->ctx->d_frame;
@@ -1188,7 +1204,6 @@ static int fimc_cap_streamon(struct file *file, void *priv,
 			     enum v4l2_buf_type type)
 {
 	struct fimc_dev *fimc = video_drvdata(file);
-	struct fimc_pipeline *p = &fimc->pipeline;
 	struct fimc_vid_cap *vc = &fimc->vid_cap;
 	struct media_entity *entity = &vc->ve.vdev.entity;
 	struct fimc_source_info *si = NULL;
@@ -1198,11 +1213,11 @@ static int fimc_cap_streamon(struct file *file, void *priv,
 	if (fimc_capture_active(fimc))
 		return -EBUSY;
 
-	ret = media_entity_pipeline_start(entity, p->m_pipeline);
+	ret = media_entity_pipeline_start(entity, &vc->ve.pipe->mp);
 	if (ret < 0)
 		return ret;
 
-	sd = p->subdevs[IDX_SENSOR];
+	sd = __fimc_md_get_subdev(vc->ve.pipe, IDX_SENSOR);
 	if (sd)
 		si = v4l2_get_subdev_hostdata(sd);
 
@@ -1821,12 +1836,12 @@ static int fimc_capture_subdev_registered(struct v4l2_subdev *sd)
 	if (ret)
 		return ret;
 
-	fimc->pipeline_ops = v4l2_get_subdev_hostdata(sd);
+	fimc->vid_cap.ve.pipe = v4l2_get_subdev_hostdata(sd);
 
 	ret = fimc_register_capture_device(fimc, sd->v4l2_dev);
 	if (ret) {
 		fimc_unregister_m2m_device(fimc);
-		fimc->pipeline_ops = NULL;
+		fimc->vid_cap.ve.pipe = NULL;
 	}
 
 	return ret;
@@ -1847,7 +1862,7 @@ static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd)
 		video_unregister_device(vdev);
 		media_entity_cleanup(&vdev->entity);
 		fimc_ctrls_delete(fimc->vid_cap.ctx);
-		fimc->pipeline_ops = NULL;
+		fimc->vid_cap.ve.pipe = NULL;
 	}
 	kfree(fimc->vid_cap.ctx);
 	fimc->vid_cap.ctx = NULL;
diff --git a/drivers/media/platform/exynos4-is/fimc-core.h b/drivers/media/platform/exynos4-is/fimc-core.h
index 09c061d..bba6b25 100644
--- a/drivers/media/platform/exynos4-is/fimc-core.h
+++ b/drivers/media/platform/exynos4-is/fimc-core.h
@@ -437,8 +437,6 @@ struct fimc_dev {
 	struct fimc_vid_cap		vid_cap;
 	unsigned long			state;
 	struct vb2_alloc_ctx		*alloc_ctx;
-	struct fimc_pipeline		pipeline;
-	const struct fimc_pipeline_ops	*pipeline_ops;
 };
 
 /**
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 1da3ed1..27232cd 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -210,7 +210,7 @@ static int fimc_lite_reinit(struct fimc_lite *fimc, bool suspend)
 	if (!streaming)
 		return 0;
 
-	return fimc_pipeline_call(fimc, set_stream, &fimc->pipeline, 0);
+	return fimc_pipeline_call(&fimc->ve, set_stream, 0);
 }
 
 static int fimc_lite_stop_capture(struct fimc_lite *fimc, bool suspend)
@@ -324,8 +324,7 @@ static int start_streaming(struct vb2_queue *q, unsigned int count)
 		flite_hw_capture_start(fimc);
 
 		if (!test_and_set_bit(ST_SENSOR_STREAM, &fimc->state))
-			fimc_pipeline_call(fimc, set_stream,
-					   &fimc->pipeline, 1);
+			fimc_pipeline_call(&fimc->ve, set_stream, 1);
 	}
 	if (debug > 0)
 		flite_hw_dump_regs(fimc, __func__);
@@ -429,8 +428,7 @@ static void buffer_queue(struct vb2_buffer *vb)
 		spin_unlock_irqrestore(&fimc->slock, flags);
 
 		if (!test_and_set_bit(ST_SENSOR_STREAM, &fimc->state))
-			fimc_pipeline_call(fimc, set_stream,
-					   &fimc->pipeline, 1);
+			fimc_pipeline_call(&fimc->ve, set_stream, 1);
 		return;
 	}
 	spin_unlock_irqrestore(&fimc->slock, flags);
@@ -482,8 +480,7 @@ static int fimc_lite_open(struct file *file)
 
 	mutex_lock(&me->parent->graph_mutex);
 
-	ret = fimc_pipeline_call(fimc, open, &fimc->pipeline,
-						me, true);
+	ret = fimc_pipeline_call(&fimc->ve, open, me, true);
 
 	/* Mark video pipeline ending at this video node as in use. */
 	if (ret == 0)
@@ -518,9 +515,10 @@ static int fimc_lite_release(struct file *file)
 			media_entity_pipeline_stop(entity);
 			fimc->streaming = false;
 		}
-		clear_bit(ST_FLITE_IN_USE, &fimc->state);
 		fimc_lite_stop_capture(fimc, false);
-		fimc_pipeline_call(fimc, close, &fimc->pipeline);
+		fimc_pipeline_call(&fimc->ve, close);
+		clear_bit(ST_FLITE_IN_USE, &fimc->state);
+
 		mutex_lock(&entity->parent->graph_mutex);
 		entity->use_count--;
 		mutex_unlock(&entity->parent->graph_mutex);
@@ -801,13 +799,12 @@ static int fimc_lite_streamon(struct file *file, void *priv,
 {
 	struct fimc_lite *fimc = video_drvdata(file);
 	struct media_entity *entity = &fimc->ve.vdev.entity;
-	struct fimc_pipeline *p = &fimc->pipeline;
 	int ret;
 
 	if (fimc_lite_active(fimc))
 		return -EBUSY;
 
-	ret = media_entity_pipeline_start(entity, p->m_pipeline);
+	ret = media_entity_pipeline_start(entity, &fimc->ve.pipe->mp);
 	if (ret < 0)
 		return ret;
 
@@ -1282,12 +1279,12 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
 		return ret;
 
 	video_set_drvdata(vfd, fimc);
-	fimc->pipeline_ops = v4l2_get_subdev_hostdata(sd);
+	fimc->ve.pipe = v4l2_get_subdev_hostdata(sd);
 
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
 	if (ret < 0) {
 		media_entity_cleanup(&vfd->entity);
-		fimc->pipeline_ops = NULL;
+		fimc->ve.pipe = NULL;
 		return ret;
 	}
 
@@ -1306,7 +1303,7 @@ static void fimc_lite_subdev_unregistered(struct v4l2_subdev *sd)
 	if (video_is_registered(&fimc->ve.vdev)) {
 		video_unregister_device(&fimc->ve.vdev);
 		media_entity_cleanup(&fimc->ve.vdev.entity);
-		fimc->pipeline_ops = NULL;
+		fimc->ve.pipe = NULL;
 	}
 }
 
@@ -1552,7 +1549,7 @@ static int fimc_lite_resume(struct device *dev)
 		return 0;
 
 	INIT_LIST_HEAD(&fimc->active_buf_q);
-	fimc_pipeline_call(fimc, open, &fimc->pipeline,
+	fimc_pipeline_call(&fimc->ve, open,
 			   &fimc->ve.vdev.entity, false);
 	fimc_lite_hw_init(fimc, atomic_read(&fimc->out_path) == FIMC_IO_ISP);
 	clear_bit(ST_FLITE_SUSPENDED, &fimc->state);
@@ -1579,7 +1576,7 @@ static int fimc_lite_suspend(struct device *dev)
 	if (ret < 0 || !fimc_lite_active(fimc))
 		return ret;
 
-	return fimc_pipeline_call(fimc, close, &fimc->pipeline);
+	return fimc_pipeline_call(&fimc->ve, close);
 }
 #endif /* CONFIG_PM_SLEEP */
 
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.h b/drivers/media/platform/exynos4-is/fimc-lite.h
index 25abba9..c7aa084 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.h
+++ b/drivers/media/platform/exynos4-is/fimc-lite.h
@@ -140,8 +140,6 @@ struct fimc_lite {
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_ctrl	*test_pattern;
 	int			index;
-	struct fimc_pipeline	pipeline;
-	const struct fimc_pipeline_ops *pipeline_ops;
 
 	struct mutex		lock;
 	spinlock_t		slock;
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 416dbcb..e95a6d5 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -46,7 +46,7 @@ static int __fimc_md_set_camclk(struct fimc_md *fmd,
  * Caller holds the graph mutex.
  */
 static void fimc_pipeline_prepare(struct fimc_pipeline *p,
-				  struct media_entity *me)
+					struct media_entity *me)
 {
 	struct v4l2_subdev *sd;
 	int i;
@@ -168,10 +168,11 @@ error:
  *
  * Called with the graph mutex held.
  */
-static int __fimc_pipeline_open(struct fimc_pipeline *p,
+static int __fimc_pipeline_open(struct exynos_media_pipeline *ep,
 				struct media_entity *me, bool prepare)
 {
 	struct fimc_md *fmd = entity_to_fimc_mdev(me);
+	struct fimc_pipeline *p = to_fimc_pipeline(ep);
 	struct v4l2_subdev *sd;
 	int ret;
 
@@ -214,8 +215,9 @@ err_wbclk:
  *
  * Disable power of all subdevs and turn the external sensor clock off.
  */
-static int __fimc_pipeline_close(struct fimc_pipeline *p)
+static int __fimc_pipeline_close(struct exynos_media_pipeline *ep)
 {
+	struct fimc_pipeline *p = to_fimc_pipeline(ep);
 	struct v4l2_subdev *sd = p ? p->subdevs[IDX_SENSOR] : NULL;
 	struct fimc_md *fmd;
 	int ret = 0;
@@ -242,12 +244,13 @@ static int __fimc_pipeline_close(struct fimc_pipeline *p)
  * @pipeline: video pipeline structure
  * @on: passed as the s_stream() callback argument
  */
-static int __fimc_pipeline_s_stream(struct fimc_pipeline *p, bool on)
+static int __fimc_pipeline_s_stream(struct exynos_media_pipeline *ep, bool on)
 {
 	static const u8 seq[2][IDX_MAX] = {
 		{ IDX_FIMC, IDX_SENSOR, IDX_IS_ISP, IDX_CSIS, IDX_FLITE },
 		{ IDX_CSIS, IDX_FLITE, IDX_FIMC, IDX_SENSOR, IDX_IS_ISP },
 	};
+	struct fimc_pipeline *p = to_fimc_pipeline(ep);
 	int i, ret = 0;
 
 	if (p->subdevs[IDX_SENSOR] == NULL)
@@ -271,12 +274,38 @@ error:
 }
 
 /* Media pipeline operations for the FIMC/FIMC-LITE video device driver */
-static const struct fimc_pipeline_ops fimc_pipeline_ops = {
+static const struct exynos_media_pipeline_ops fimc_pipeline_ops = {
 	.open		= __fimc_pipeline_open,
 	.close		= __fimc_pipeline_close,
 	.set_stream	= __fimc_pipeline_s_stream,
 };
 
+static struct exynos_media_pipeline *fimc_md_pipeline_create(
+						struct fimc_md *fmd)
+{
+	struct fimc_pipeline *p;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return NULL;
+
+	list_add_tail(&p->list, &fmd->pipelines);
+
+	p->ep.ops = &fimc_pipeline_ops;
+	return &p->ep;
+}
+
+static void fimc_md_pipelines_free(struct fimc_md *fmd)
+{
+	while (!list_empty(&fmd->pipelines)) {
+		struct fimc_pipeline *p;
+
+		p = list_entry(fmd->pipelines.next, typeof(*p), list);
+		list_del(&p->list);
+		kfree(p);
+	}
+}
+
 /*
  * Sensor subdevice helper functions
  */
@@ -592,6 +621,7 @@ static int register_fimc_lite_entity(struct fimc_md *fmd,
 				     struct fimc_lite *fimc_lite)
 {
 	struct v4l2_subdev *sd;
+	struct exynos_media_pipeline *ep;
 	int ret;
 
 	if (WARN_ON(fimc_lite->index >= FIMC_LITE_MAX_DEVS ||
@@ -600,7 +630,12 @@ static int register_fimc_lite_entity(struct fimc_md *fmd,
 
 	sd = &fimc_lite->subdev;
 	sd->grp_id = GRP_ID_FLITE;
-	v4l2_set_subdev_hostdata(sd, (void *)&fimc_pipeline_ops);
+
+	ep = fimc_md_pipeline_create(fmd);
+	if (!ep)
+		return -ENOMEM;
+
+	v4l2_set_subdev_hostdata(sd, ep);
 
 	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
 	if (!ret)
@@ -614,6 +649,7 @@ static int register_fimc_lite_entity(struct fimc_md *fmd,
 static int register_fimc_entity(struct fimc_md *fmd, struct fimc_dev *fimc)
 {
 	struct v4l2_subdev *sd;
+	struct exynos_media_pipeline *ep;
 	int ret;
 
 	if (WARN_ON(fimc->id >= FIMC_MAX_DEVS || fmd->fimc[fimc->id]))
@@ -621,7 +657,12 @@ static int register_fimc_entity(struct fimc_md *fmd, struct fimc_dev *fimc)
 
 	sd = &fimc->vid_cap.subdev;
 	sd->grp_id = GRP_ID_FIMC;
-	v4l2_set_subdev_hostdata(sd, (void *)&fimc_pipeline_ops);
+
+	ep = fimc_md_pipeline_create(fmd);
+	if (!ep)
+		return -ENOMEM;
+
+	v4l2_set_subdev_hostdata(sd, ep);
 
 	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
 	if (!ret) {
@@ -797,17 +838,19 @@ static void fimc_md_unregister_entities(struct fimc_md *fmd)
 	int i;
 
 	for (i = 0; i < FIMC_MAX_DEVS; i++) {
-		if (fmd->fimc[i] == NULL)
+		struct fimc_dev *dev = fmd->fimc[i];
+		if (dev == NULL)
 			continue;
-		v4l2_device_unregister_subdev(&fmd->fimc[i]->vid_cap.subdev);
-		fmd->fimc[i]->pipeline_ops = NULL;
+		v4l2_device_unregister_subdev(&dev->vid_cap.subdev);
+		dev->vid_cap.ve.pipe = NULL;
 		fmd->fimc[i] = NULL;
 	}
 	for (i = 0; i < FIMC_LITE_MAX_DEVS; i++) {
-		if (fmd->fimc_lite[i] == NULL)
+		struct fimc_lite *dev = fmd->fimc_lite[i];
+		if (dev == NULL)
 			continue;
-		v4l2_device_unregister_subdev(&fmd->fimc_lite[i]->subdev);
-		fmd->fimc_lite[i]->pipeline_ops = NULL;
+		v4l2_device_unregister_subdev(&dev->subdev);
+		dev->ve.pipe = NULL;
 		fmd->fimc_lite[i] = NULL;
 	}
 	for (i = 0; i < CSIS_MAX_ENTITIES; i++) {
@@ -1234,38 +1277,22 @@ int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on)
 static int fimc_md_link_notify(struct media_pad *source,
 			       struct media_pad *sink, u32 flags)
 {
-	struct fimc_lite *fimc_lite = NULL;
-	struct fimc_dev *fimc = NULL;
+	struct exynos_video_entity *ve;
+	struct video_device *vdev;
 	struct fimc_pipeline *pipeline;
-	struct v4l2_subdev *sd;
 	int i, ret = 0;
 
-	if (media_entity_type(sink->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
+	if (media_entity_type(sink->entity) != MEDIA_ENT_T_DEVNODE_V4L)
 		return 0;
 
-	sd = media_entity_to_v4l2_subdev(sink->entity);
-
-	switch (sd->grp_id) {
-	case GRP_ID_FLITE:
-		fimc_lite = v4l2_get_subdevdata(sd);
-		if (WARN_ON(fimc_lite == NULL))
-			return 0;
-		pipeline = &fimc_lite->pipeline;
-		break;
-	case GRP_ID_FIMC:
-		fimc = v4l2_get_subdevdata(sd);
-		if (WARN_ON(fimc == NULL))
-			return 0;
-		pipeline = &fimc->pipeline;
-		break;
-	default:
-		return 0;
-	}
+	vdev = media_entity_to_video_device(sink->entity);
+	ve = vdev_to_exynos_video_entity(vdev);
+	pipeline = to_fimc_pipeline(ve->pipe);
+
+	if (!(flags & MEDIA_LNK_FL_ENABLED) && pipeline->subdevs[IDX_SENSOR]) {
+		if (sink->entity->use_count > 0)
+			ret = __fimc_pipeline_close(ve->pipe);
 
-	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
-		if (sink->entity->use_count > 0) {
-			ret = __fimc_pipeline_close(pipeline);
-		}
 		for (i = 0; i < IDX_MAX; i++)
 			pipeline->subdevs[i] = NULL;
 	} else if (sink->entity->use_count > 0) {
@@ -1274,8 +1301,7 @@ static int fimc_md_link_notify(struct media_pad *source,
 		 * the pipeline is already in use, i.e. its video node is open.
 		 * Recreate the controls destroyed during the link deactivation.
 		 */
-		ret = __fimc_pipeline_open(pipeline,
-					   source->entity, true);
+		ret = __fimc_pipeline_open(ve->pipe, sink->entity, true);
 	}
 
 	return ret ? -EPIPE : ret;
@@ -1358,6 +1384,7 @@ static int fimc_md_probe(struct platform_device *pdev)
 
 	spin_lock_init(&fmd->slock);
 	fmd->pdev = pdev;
+	INIT_LIST_HEAD(&fmd->pipelines);
 
 	strlcpy(fmd->media_dev.model, "SAMSUNG S5P FIMC",
 		sizeof(fmd->media_dev.model));
@@ -1445,6 +1472,7 @@ static int fimc_md_remove(struct platform_device *pdev)
 		return 0;
 	device_remove_file(&pdev->dev, &dev_attr_subdev_conf_mode);
 	fimc_md_unregister_entities(fmd);
+	fimc_md_pipelines_free(fmd);
 	media_device_unregister(&fmd->media_dev);
 	fimc_md_put_clocks(fmd);
 	return 0;
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index 3e9680c..a704eea 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -18,6 +18,7 @@
 #include <media/media-entity.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-subdev.h>
+#include <media/s5p_fimc.h>
 
 #include "fimc-core.h"
 #include "fimc-lite.h"
@@ -40,6 +41,29 @@ enum {
 	FIMC_MAX_WBCLKS
 };
 
+enum fimc_subdev_index {
+	IDX_SENSOR,
+	IDX_CSIS,
+	IDX_FLITE,
+	IDX_IS_ISP,
+	IDX_FIMC,
+	IDX_MAX,
+};
+
+/*
+ * This structure represents a chain of media entities, including a data
+ * source entity (e.g. an image sensor subdevice), a data capture entity
+ * - a video capture device node and any remaining entities.
+ */
+struct fimc_pipeline {
+	struct exynos_media_pipeline ep;
+	struct list_head list;
+	struct media_entity *vdev_entity;
+	struct v4l2_subdev *subdevs[IDX_MAX];
+};
+
+#define to_fimc_pipeline(_ep) container_of(_ep, struct fimc_pipeline, ep)
+
 struct fimc_csis_info {
 	struct v4l2_subdev *sd;
 	int id;
@@ -104,7 +128,9 @@ struct fimc_md {
 		struct pinctrl_state *state_idle;
 	} pinctl;
 	bool user_subdev_api;
+
 	spinlock_t slock;
+	struct list_head pipelines;
 };
 
 #define is_subdev_pad(pad) (pad == NULL || \
@@ -149,4 +175,16 @@ static inline bool fimc_md_is_isp_available(struct device_node *node)
 #define fimc_md_is_isp_available(node) (false)
 #endif /* CONFIG_OF */
 
+static inline struct v4l2_subdev *__fimc_md_get_subdev(
+				struct exynos_media_pipeline *ep,
+				unsigned int index)
+{
+	struct fimc_pipeline *p = to_fimc_pipeline(ep);
+
+	if (!p || index >= IDX_MAX)
+		return NULL;
+	else
+		return p->subdevs[index];
+}
+
 #endif
diff --git a/include/media/s5p_fimc.h b/include/media/s5p_fimc.h
index f5313b4..0afadb6 100644
--- a/include/media/s5p_fimc.h
+++ b/include/media/s5p_fimc.h
@@ -141,41 +141,40 @@ struct fimc_fmt {
 #define FMT_FLAGS_YUV		(1 << 7)
 };
 
-enum fimc_subdev_index {
-	IDX_SENSOR,
-	IDX_CSIS,
-	IDX_FLITE,
-	IDX_IS_ISP,
-	IDX_FIMC,
-	IDX_MAX,
-};
-
-struct media_pipeline;
-struct v4l2_subdev;
+struct exynos_media_pipeline;
 
-struct fimc_pipeline {
-	struct v4l2_subdev *subdevs[IDX_MAX];
-	struct media_pipeline *m_pipeline;
+/*
+ * Media pipeline operations to be called from within a video node,  i.e. the
+ * last entity within the pipeline. Implemented by related media device driver.
+ */
+struct exynos_media_pipeline_ops {
+	int (*prepare)(struct exynos_media_pipeline *p,
+						struct media_entity *me);
+	int (*unprepare)(struct exynos_media_pipeline *p);
+	int (*open)(struct exynos_media_pipeline *p, struct media_entity *me,
+							bool resume);
+	int (*close)(struct exynos_media_pipeline *p);
+	int (*set_stream)(struct exynos_media_pipeline *p, bool state);
 };
 
 struct exynos_video_entity {
 	struct video_device vdev;
+	struct exynos_media_pipeline *pipe;
 };
 
-/*
- * Media pipeline operations to be called from within the fimc(-lite)
- * video node when it is the last entity of the pipeline. Implemented
- * by corresponding media device driver.
- */
-struct fimc_pipeline_ops {
-	int (*open)(struct fimc_pipeline *p, struct media_entity *me,
-			  bool resume);
-	int (*close)(struct fimc_pipeline *p);
-	int (*set_stream)(struct fimc_pipeline *p, bool state);
+struct exynos_media_pipeline {
+	struct media_pipeline mp;
+	const struct exynos_media_pipeline_ops *ops;
 };
 
-#define fimc_pipeline_call(f, op, p, args...)				\
-	(!(f) ? -ENODEV : (((f)->pipeline_ops && (f)->pipeline_ops->op) ? \
-			    (f)->pipeline_ops->op((p), ##args) : -ENOIOCTLCMD))
+static inline struct exynos_video_entity *vdev_to_exynos_video_entity(
+					struct video_device *vdev)
+{
+	return container_of(vdev, struct exynos_video_entity, vdev);
+}
+
+#define fimc_pipeline_call(ent, op, args...)				  \
+	(!(ent) ? -ENOENT : (((ent)->pipe->ops && (ent)->pipe->ops->op) ? \
+	(ent)->pipe->ops->op(((ent)->pipe), ##args) : -ENOIOCTLCMD))	  \
 
 #endif /* S5P_FIMC_H_ */
-- 
1.7.9.5

[REVIEW PATCH v2 07/11] exynos4-is: Remove WARN_ON() from __fimc_pipeline_close()

[Sylwester Nawrocki]

It's not a critical error to call __fimc_pipeline_close() with missing
sensor subdev entity. Replace WARN_ON() with pr_warn() and return 0
instead of -EINVAL to fix control flow in some conditions.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index e95a6d5..973f8a9 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -220,16 +220,16 @@ static int __fimc_pipeline_close(struct exynos_media_pipeline *ep)
 	struct fimc_pipeline *p = to_fimc_pipeline(ep);
 	struct v4l2_subdev *sd = p ? p->subdevs[IDX_SENSOR] : NULL;
 	struct fimc_md *fmd;
-	int ret = 0;
-
-	if (WARN_ON(sd == NULL))
-		return -EINVAL;
+	int ret;
 
-	if (p->subdevs[IDX_SENSOR]) {
-		ret = fimc_pipeline_s_power(p, 0);
-		fimc_md_set_camclk(sd, false);
+	if (sd == NULL) {
+		pr_warn("%s(): No sensor subdev\n", __func__);
+		return 0;
 	}
 
+	ret = fimc_pipeline_s_power(p, 0);
+	fimc_md_set_camclk(sd, false);
+
 	fmd = entity_to_fimc_mdev(&sd->entity);
 
 	/* Disable PXLASYNC clock if this pipeline includes FIMC-IS */
-- 
1.7.9.5

[REVIEW PATCH v2 08/11] exynos4-is: Fix sensor subdev -> FIMC notification setup

[Sylwester Nawrocki]

Ensure the v4l2_device notifications from sensor subdev works
also after the media links reconfiguration.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c |   47 ++++++++++++++++---------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 973f8a9..d7f7342 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1,8 +1,8 @@
 /*
  * S5P/EXYNOS4 SoC series camera host interface media device driver
  *
- * Copyright (C) 2011 - 2012 Samsung Electronics Co., Ltd.
- * Sylwester Nawrocki <s.nawrocki@samsung.com>
+ * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published
@@ -39,6 +39,26 @@
 static int __fimc_md_set_camclk(struct fimc_md *fmd,
 				struct fimc_source_info *si,
 				bool on);
+
+/* Set up image sensor subdev -> FIMC capture node notifications. */
+static void __setup_sensor_notification(struct fimc_md *fmd,
+					struct v4l2_subdev *sensor,
+					struct v4l2_subdev *fimc_sd)
+{
+	struct fimc_source_info *src_inf;
+	struct fimc_sensor_info *md_si;
+	unsigned long flags;
+
+	src_inf = v4l2_get_subdev_hostdata(sensor);
+	if (!src_inf || WARN_ON(fmd == NULL))
+		return;
+
+	md_si = source_to_sensor_info(src_inf);
+	spin_lock_irqsave(&fmd->slock, flags);
+	md_si->host = v4l2_get_subdevdata(fimc_sd);
+	spin_unlock_irqrestore(&fmd->slock, flags);
+}
+
 /**
  * fimc_pipeline_prepare - update pipeline information with subdevice pointers
  * @me: media entity terminating the pipeline
@@ -48,7 +68,9 @@ static int __fimc_md_set_camclk(struct fimc_md *fmd,
 static void fimc_pipeline_prepare(struct fimc_pipeline *p,
 					struct media_entity *me)
 {
+	struct fimc_md *fmd = entity_to_fimc_mdev(me);
 	struct v4l2_subdev *sd;
+	struct v4l2_subdev *sensor = NULL;
 	int i;
 
 	for (i = 0; i < IDX_MAX; i++)
@@ -73,8 +95,10 @@ static void fimc_pipeline_prepare(struct fimc_pipeline *p,
 		sd = media_entity_to_v4l2_subdev(pad->entity);
 
 		switch (sd->grp_id) {
-		case GRP_ID_FIMC_IS_SENSOR:
 		case GRP_ID_SENSOR:
+			sensor = sd;
+			/* fall through */
+		case GRP_ID_FIMC_IS_SENSOR:
 			p->subdevs[IDX_SENSOR] = sd;
 			break;
 		case GRP_ID_CSIS:
@@ -84,7 +108,7 @@ static void fimc_pipeline_prepare(struct fimc_pipeline *p,
 			p->subdevs[IDX_FLITE] = sd;
 			break;
 		case GRP_ID_FIMC:
-			/* No need to control FIMC subdev through subdev ops */
+			p->subdevs[IDX_FIMC] = sd;
 			break;
 		case GRP_ID_FIMC_IS:
 			p->subdevs[IDX_IS_ISP] = sd;
@@ -96,6 +120,9 @@ static void fimc_pipeline_prepare(struct fimc_pipeline *p,
 		if (me->num_pads == 1)
 			break;
 	}
+
+	if (sensor && p->subdevs[IDX_FIMC])
+		__setup_sensor_notification(fmd, sensor, p->subdevs[IDX_FIMC]);
 }
 
 /**
@@ -923,18 +950,6 @@ static int __fimc_md_create_fimc_sink_links(struct fimc_md *fmd,
 
 		v4l2_info(&fmd->v4l2_dev, "created link [%s] %c> [%s]\n",
 			  source->name, flags ? '=' : '-', sink->name);
-
-		if (flags == 0 || sensor == NULL)
-			continue;
-
-		if (!WARN_ON(si == NULL)) {
-			unsigned long irq_flags;
-			struct fimc_sensor_info *inf = source_to_sensor_info(si);
-
-			spin_lock_irqsave(&fmd->slock, irq_flags);
-			inf->host = fmd->fimc[i];
-			spin_unlock_irqrestore(&fmd->slock, irq_flags);
-		}
 	}
 
 	for (i = 0; i < FIMC_LITE_MAX_DEVS; i++) {
-- 
1.7.9.5

[REVIEW PATCH v2 09/11] exynos4-is: Add locking at fimc(-lite) subdev unregistered handler

[Sylwester Nawrocki]

Protect the fimc/fimc-lite video nodes unregistration with their video
lock. This prevents a kernel crash when e.g. udev opens a video node
right after the driver registers it and then the driver tries to
unregister it and defers its probing. Using video_is_unregistered()
together with the video mutex allows safe unregistration of the video
nodes at any time.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-capture.c |    4 ++++
 drivers/media/platform/exynos4-is/fimc-lite.c    |    4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index c2586a2..25f8a1e 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -1855,6 +1855,8 @@ static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd)
 	if (fimc == NULL)
 		return;
 
+	mutex_lock(&fimc->lock);
+
 	fimc_unregister_m2m_device(fimc);
 	vdev = &fimc->vid_cap.ve.vdev;
 
@@ -1866,6 +1868,8 @@ static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd)
 	}
 	kfree(fimc->vid_cap.ctx);
 	fimc->vid_cap.ctx = NULL;
+
+	mutex_unlock(&fimc->lock);
 }
 
 static const struct v4l2_subdev_internal_ops fimc_capture_sd_internal_ops = {
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 27232cd..142788a 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1300,11 +1300,15 @@ static void fimc_lite_subdev_unregistered(struct v4l2_subdev *sd)
 	if (fimc == NULL)
 		return;
 
+	mutex_lock(&fimc->lock);
+
 	if (video_is_registered(&fimc->ve.vdev)) {
 		video_unregister_device(&fimc->ve.vdev);
 		media_entity_cleanup(&fimc->ve.vdev.entity);
 		fimc->ve.pipe = NULL;
 	}
+
+	mutex_unlock(&fimc->lock);
 }
 
 static const struct v4l2_subdev_internal_ops fimc_lite_subdev_internal_ops = {
-- 
1.7.9.5

[REVIEW PATCH v2 10/11] media: Change media device link_notify behaviour

[Sylwester Nawrocki]

Currently the media device link_notify callback is invoked before the
actual change of state of a link when the link is being enabled, and
after the actual change of state when the link is being disabled.

This doesn't allow a media device driver to perform any operations
on a full graph before a link is disabled, as well as performing
any tasks on a modified graph right after a link's state is changed.

This patch modifies signature of the link_notify callback. This
callback is now called always before and after a link's state change.
To distinguish the notifications a 'notification' argument is added
to the link_notify callback: MEDIA_DEV_NOTIFY_PRE_LINK_CH indicates
notification before link's state change and
MEDIA_DEV_NOTIFY_POST_LINK_CH corresponds to a notification after
link flags change.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes since v1:

 - fixed link flags handling in __media_entity_setup_link()
 - swapped MEDIA_DEV_NOTIFY_PRE_LINK_CH and MEDIA_DEV_NOTIFY_POST_LINK_CH
   at the omap3isp chunk to retain original behaviour.
---
 drivers/media/media-entity.c                  |   18 +++--------
 drivers/media/platform/exynos4-is/media-dev.c |   16 +++++-----
 drivers/media/platform/omap3isp/isp.c         |   41 +++++++++++++++----------
 include/media/media-device.h                  |    9 ++++--
 4 files changed, 46 insertions(+), 38 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index bd85dc3..139fd27 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -547,25 +547,17 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
 
 	mdev = source->parent;
 
-	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify) {
-		ret = mdev->link_notify(link->source, link->sink,
-					MEDIA_LNK_FL_ENABLED);
+	if (mdev->link_notify) {
+		ret = mdev->link_notify(link, flags,
+					MEDIA_DEV_NOTIFY_PRE_LINK_CH);
 		if (ret < 0)
 			return ret;
 	}
 
 	ret = __media_entity_setup_link_notify(link, flags);
-	if (ret < 0)
-		goto err;
 
-	if (!(flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
-		mdev->link_notify(link->source, link->sink, 0);
-
-	return 0;
-
-err:
-	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
-		mdev->link_notify(link->source, link->sink, 0);
+	if (mdev->link_notify)
+		mdev->link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);
 
 	return ret;
 }
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index d7f7342..04abf6f 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1289,34 +1289,36 @@ int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on)
 	return __fimc_md_set_camclk(fmd, si, on);
 }
 
-static int fimc_md_link_notify(struct media_pad *source,
-			       struct media_pad *sink, u32 flags)
+static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
+						unsigned int notification)
 {
+	struct media_entity *sink = link->sink->entity;
 	struct exynos_video_entity *ve;
 	struct video_device *vdev;
 	struct fimc_pipeline *pipeline;
 	int i, ret = 0;
 
-	if (media_entity_type(sink->entity) != MEDIA_ENT_T_DEVNODE_V4L)
+	if (media_entity_type(sink) != MEDIA_ENT_T_DEVNODE_V4L ||
+	    notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH)
 		return 0;
 
-	vdev = media_entity_to_video_device(sink->entity);
+	vdev = media_entity_to_video_device(sink);
 	ve = vdev_to_exynos_video_entity(vdev);
 	pipeline = to_fimc_pipeline(ve->pipe);
 
 	if (!(flags & MEDIA_LNK_FL_ENABLED) && pipeline->subdevs[IDX_SENSOR]) {
-		if (sink->entity->use_count > 0)
+		if (sink->use_count > 0)
 			ret = __fimc_pipeline_close(ve->pipe);
 
 		for (i = 0; i < IDX_MAX; i++)
 			pipeline->subdevs[i] = NULL;
-	} else if (sink->entity->use_count > 0) {
+	} else if (sink->use_count > 0) {
 		/*
 		 * Link activation. Enable power of pipeline elements only if
 		 * the pipeline is already in use, i.e. its video node is open.
 		 * Recreate the controls destroyed during the link deactivation.
 		 */
-		ret = __fimc_pipeline_open(ve->pipe, sink->entity, true);
+		ret = __fimc_pipeline_open(ve->pipe, sink, true);
 	}
 
 	return ret ? -EPIPE : ret;
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 1d7dbd5..ef830c7 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -792,9 +792,9 @@ int omap3isp_pipeline_pm_use(struct media_entity *entity, int use)
 
 /*
  * isp_pipeline_link_notify - Link management notification callback
- * @source: Pad at the start of the link
- * @sink: Pad at the end of the link
+ * @link: The link
  * @flags: New link flags that will be applied
+ * @notification: The link's state change notification type (MEDIA_DEV_NOTIFY_*)
  *
  * React to link management on powered pipelines by updating the use count of
  * all entities in the source and sink sides of the link. Entities are powered
@@ -804,29 +804,38 @@ int omap3isp_pipeline_pm_use(struct media_entity *entity, int use)
  * off is assumed to never fail. This function will not fail for disconnection
  * events.
  */
-static int isp_pipeline_link_notify(struct media_pad *source,
-				    struct media_pad *sink, u32 flags)
+static int isp_pipeline_link_notify(struct media_link *link, unsigned int flags,
+						unsigned int notification)
 {
-	int source_use = isp_pipeline_pm_use_count(source->entity);
-	int sink_use = isp_pipeline_pm_use_count(sink->entity);
+	struct media_entity *source = link->source->entity;
+	struct media_entity *sink = link->sink->entity;
+	int source_use = isp_pipeline_pm_use_count(source);
+	int sink_use = isp_pipeline_pm_use_count(sink);
 	int ret;
 
-	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
+	if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
+	    !(flags & MEDIA_LNK_FL_ENABLED)) {
 		/* Powering off entities is assumed to never fail. */
-		isp_pipeline_pm_power(source->entity, -sink_use);
-		isp_pipeline_pm_power(sink->entity, -source_use);
+		isp_pipeline_pm_power(source, -sink_use);
+		isp_pipeline_pm_power(sink, -source_use);
 		return 0;
 	}
 
-	ret = isp_pipeline_pm_power(source->entity, sink_use);
-	if (ret < 0)
-		return ret;
+	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
+	        (flags & MEDIA_LNK_FL_ENABLED)) {
 
-	ret = isp_pipeline_pm_power(sink->entity, source_use);
-	if (ret < 0)
-		isp_pipeline_pm_power(source->entity, -sink_use);
+		ret = isp_pipeline_pm_power(source, sink_use);
+		if (ret < 0)
+			return ret;
 
-	return ret;
+		ret = isp_pipeline_pm_power(sink, source_use);
+		if (ret < 0)
+			isp_pipeline_pm_power(source, -sink_use);
+
+		return ret;
+	}
+
+	return 0;
 }
 
 /* -----------------------------------------------------------------------------
diff --git a/include/media/media-device.h b/include/media/media-device.h
index eaade98..353c4ee 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -45,6 +45,7 @@ struct device;
  * @entities:	List of registered entities
  * @lock:	Entities list lock
  * @graph_mutex: Entities graph operation lock
+ * @link_notify: Link state change notification callback
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
@@ -75,10 +76,14 @@ struct media_device {
 	/* Serializes graph operations. */
 	struct mutex graph_mutex;
 
-	int (*link_notify)(struct media_pad *source,
-			   struct media_pad *sink, u32 flags);
+	int (*link_notify)(struct media_link *link, unsigned int flags,
+			   unsigned int notification);
 };
 
+/* Supported link_notify @notification values. */
+#define MEDIA_DEV_NOTIFY_PRE_LINK_CH	0
+#define MEDIA_DEV_NOTIFY_POST_LINK_CH	1
+
 /* media_devnode to media_device */
 #define to_media_device(node) container_of(node, struct media_device, devnode)
 
-- 
1.7.9.5

[REVIEW PATCH v2 11/11] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines

[Sylwester Nawrocki]

This patch corrects the link_notify handler to support more complex
pipelines, including fimc-lite and fimc-is entities.

After the FIMC-IS driver addition the assumptions made in the link_notify
callback are no longer valid, e.g. the link between fimc-lite subdev and
its video node is not immutable any more and there is more subdevs than
just sensor, MIPI-CSIS and FIMC(-LITE).

The graph is now walked and for each video node found a media pipeline
which ends at this node is disabled/enabled (the subdevs are powered
on/off).

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c |  103 +++++++++++++++++++------
 1 file changed, 81 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 04abf6f..f8ada95 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1289,39 +1289,98 @@ int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on)
 	return __fimc_md_set_camclk(fmd, si, on);
 }
 
-static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
-						unsigned int notification)
+static int __fimc_md_modify_pipeline(struct media_entity *entity, bool enable)
 {
-	struct media_entity *sink = link->sink->entity;
 	struct exynos_video_entity *ve;
+	struct fimc_pipeline *p;
 	struct video_device *vdev;
-	struct fimc_pipeline *pipeline;
-	int i, ret = 0;
+	int ret;
 
-	if (media_entity_type(sink) != MEDIA_ENT_T_DEVNODE_V4L ||
-	    notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH)
+	vdev = media_entity_to_video_device(entity);
+	if (vdev->entity.use_count == 0)
 		return 0;
 
-	vdev = media_entity_to_video_device(sink);
 	ve = vdev_to_exynos_video_entity(vdev);
-	pipeline = to_fimc_pipeline(ve->pipe);
+	p = to_fimc_pipeline(ve->pipe);
+	/*
+	 * Nothing to do if we are disabling the pipeline, some link
+	 * has been disconnected and p->subdevs array is cleared now.
+	 */
+	if (!enable && p->subdevs[IDX_SENSOR] == NULL)
+		return 0;
 
-	if (!(flags & MEDIA_LNK_FL_ENABLED) && pipeline->subdevs[IDX_SENSOR]) {
-		if (sink->use_count > 0)
-			ret = __fimc_pipeline_close(ve->pipe);
+	if (enable)
+		ret = __fimc_pipeline_open(ve->pipe, entity, true);
+	else
+		ret = __fimc_pipeline_close(ve->pipe);
 
-		for (i = 0; i < IDX_MAX; i++)
-			pipeline->subdevs[i] = NULL;
-	} else if (sink->use_count > 0) {
-		/*
-		 * Link activation. Enable power of pipeline elements only if
-		 * the pipeline is already in use, i.e. its video node is open.
-		 * Recreate the controls destroyed during the link deactivation.
-		 */
-		ret = __fimc_pipeline_open(ve->pipe, sink, true);
+	if (ret == 0 && !enable)
+		memset(p->subdevs, 0, sizeof(p->subdevs));
+
+	return ret;
+}
+
+/* Locking: called with entity->parent->graph_mutex mutex held. */
+static int __fimc_md_modify_pipelines(struct media_entity *entity, bool enable)
+{
+	struct media_entity *entity_err = entity;
+	struct media_entity_graph graph;
+	int ret;
+
+	/*
+	 * Walk current graph and call the pipeline open/close routine for each
+	 * opened video node that belongs to the graph of entities connected
+	 * through active links. This is needed as we cannot power on/off the
+	 * subdevs in random order.
+	 */
+	media_entity_graph_walk_start(&graph, entity);
+
+	while ((entity = media_entity_graph_walk_next(&graph))) {
+		if (media_entity_type(entity) != MEDIA_ENT_T_DEVNODE)
+			continue;
+
+		ret  = __fimc_md_modify_pipeline(entity, enable);
+
+		if (ret < 0)
+			goto err;
+	}
+
+	return 0;
+ err:
+	media_entity_graph_walk_start(&graph, entity_err);
+
+	while ((entity_err = media_entity_graph_walk_next(&graph))) {
+		if (media_entity_type(entity_err) != MEDIA_ENT_T_DEVNODE)
+			continue;
+
+		__fimc_md_modify_pipeline(entity_err, !enable);
+
+		if (entity_err == entity)
+			break;
+	}
+
+	return ret;
+}
+
+static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
+				unsigned int notification)
+{
+	struct media_entity *sink = link->sink->entity;
+	int ret = 0;
+
+	/* Before link disconnection */
+	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH) {
+		if (!(flags & MEDIA_LNK_FL_ENABLED))
+			ret = __fimc_md_modify_pipelines(sink, false);
+		else
+			; /* TODO: Link state change validation */
+	/* After link activation */
+	} else if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
+		   (flags & MEDIA_LNK_FL_ENABLED)) {
+		ret = __fimc_md_modify_pipelines(sink, true);
 	}
 
-	return ret ? -EPIPE : ret;
+	return ret ? -EPIPE : 0;
 }
 
 static ssize_t fimc_md_sysfs_show(struct device *dev,
-- 
1.7.9.5

Re: [REVIEW PATCH v2 10/11] media: Change media device link_notify behaviour

[Sakari Ailus]

Hi Sylwester,

Thanks for the patch!

On Fri, May 31, 2013 at 04:37:26PM +0200, Sylwester Nawrocki wrote:
...
> @@ -547,25 +547,17 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
>  
>  	mdev = source->parent;
>  
> -	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify) {
> -		ret = mdev->link_notify(link->source, link->sink,
> -					MEDIA_LNK_FL_ENABLED);
> +	if (mdev->link_notify) {
> +		ret = mdev->link_notify(link, flags,
> +					MEDIA_DEV_NOTIFY_PRE_LINK_CH);
>  		if (ret < 0)
>  			return ret;
>  	}
>  
>  	ret = __media_entity_setup_link_notify(link, flags);
> -	if (ret < 0)
> -		goto err;
>  
> -	if (!(flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
> -		mdev->link_notify(link->source, link->sink, 0);
> -
> -	return 0;
> -
> -err:
> -	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
> -		mdev->link_notify(link->source, link->sink, 0);
> +	if (mdev->link_notify)
> +		mdev->link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);

This changes the behaviour of link_notify() so that the flags will be the
same independently of whether there was an error. I wonder if that's
intentional.

I'd think that in the case of error the flags wouldn't change from what they
were, i.e. the flags argument would be "link->flags" instead of "flags".

>  	return ret;
>  }

...

> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index eaade98..353c4ee 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -45,6 +45,7 @@ struct device;
>   * @entities:	List of registered entities
>   * @lock:	Entities list lock
>   * @graph_mutex: Entities graph operation lock
> + * @link_notify: Link state change notification callback
>   *
>   * This structure represents an abstract high-level media device. It allows easy
>   * access to entities and provides basic media device-level support. The
> @@ -75,10 +76,14 @@ struct media_device {
>  	/* Serializes graph operations. */
>  	struct mutex graph_mutex;
>  
> -	int (*link_notify)(struct media_pad *source,
> -			   struct media_pad *sink, u32 flags);
> +	int (*link_notify)(struct media_link *link, unsigned int flags,
> +			   unsigned int notification);

media_link->flags is unsigned long. The patch doesn't break anything, but it
switches from u32/unsigned long to unsigned int/unsigned long for the field.

How about making media_link->flags unsigned int (or unsigned long) at the
same time, or not changing it? This could be fixed in a separate patch as
well (which I'm not necessarily expect from you now). There are probably a
number of places that would need to be changed.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [REVIEW PATCH v2 10/11] media: Change media device link_notify behaviour

[Sylwester Nawrocki]

Hi,

On 06/06/2013 02:11 AM, Sakari Ailus wrote:
> Hi Sylwester,
>
> Thanks for the patch!

And thanks for taking time to review!

> On Fri, May 31, 2013 at 04:37:26PM +0200, Sylwester Nawrocki wrote:
> ...
>> @@ -547,25 +547,17 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
>>
>>   	mdev = source->parent;
>>
>> -	if ((flags&  MEDIA_LNK_FL_ENABLED)&&  mdev->link_notify) {
>> -		ret = mdev->link_notify(link->source, link->sink,
>> -					MEDIA_LNK_FL_ENABLED);
>> +	if (mdev->link_notify) {
>> +		ret = mdev->link_notify(link, flags,
>> +					MEDIA_DEV_NOTIFY_PRE_LINK_CH);
>>   		if (ret<  0)
>>   			return ret;
>>   	}
>>
>>   	ret = __media_entity_setup_link_notify(link, flags);
>> -	if (ret<  0)
>> -		goto err;
>>
>> -	if (!(flags&  MEDIA_LNK_FL_ENABLED)&&  mdev->link_notify)
>> -		mdev->link_notify(link->source, link->sink, 0);
>> -
>> -	return 0;
>> -
>> -err:
>> -	if ((flags&  MEDIA_LNK_FL_ENABLED)&&  mdev->link_notify)
>> -		mdev->link_notify(link->source, link->sink, 0);
>> +	if (mdev->link_notify)
>> +		mdev->link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);
>
> This changes the behaviour of link_notify() so that the flags will be the
> same independently of whether there was an error. I wonder if that's
> intentional.

Yes, that's intentional. However I failed to update the omap3isp driver
link_notify handler properly to handle the link set up error case. I'll
correct that in next iteration.

> I'd think that in the case of error the flags wouldn't change from what they
> were, i.e. the flags argument would be "link->flags" instead of "flags".

The idea was to allow the link_notify handler to determine when link state
change succeeded, and when not. So the 'flags' link_notify() argument
indicates what was the intended state, while the actual state can be
determined by checking link->flags.

I considered not introducing the 'notification' argument and just comparing
'flags' and 'link->flags', but those look same before and after a call to
__media_entity_setup_link_notify() in error case.

>>   	return ret;
>>   }
>
> ...
>
>> diff --git a/include/media/media-device.h b/include/media/media-device.h
>> index eaade98..353c4ee 100644
>> --- a/include/media/media-device.h
>> +++ b/include/media/media-device.h
>> @@ -45,6 +45,7 @@ struct device;
>>    * @entities:	List of registered entities
>>    * @lock:	Entities list lock
>>    * @graph_mutex: Entities graph operation lock
>> + * @link_notify: Link state change notification callback
>>    *
>>    * This structure represents an abstract high-level media device. It allows easy
>>    * access to entities and provides basic media device-level support. The
>> @@ -75,10 +76,14 @@ struct media_device {
>>   	/* Serializes graph operations. */
>>   	struct mutex graph_mutex;
>>
>> -	int (*link_notify)(struct media_pad *source,
>> -			   struct media_pad *sink, u32 flags);
>> +	int (*link_notify)(struct media_link *link, unsigned int flags,
>> +			   unsigned int notification);
>
> media_link->flags is unsigned long. The patch doesn't break anything, but it
> switches from u32/unsigned long to unsigned int/unsigned long for the field.
>
> How about making media_link->flags unsigned int (or unsigned long) at the
> same time, or not changing it? This could be fixed in a separate patch as
> well (which I'm not necessarily expect from you now). There are probably a
> number of places that would need to be changed.

Hmm, OK, I'll revert this 'flags' type change. I guess it's best to use
'unsigned int' all over, but I'll leave it out for a separate patch, for
someone to write. :)


Thanks,
Sylwester