devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem
@ 2013-04-24  7:41 Shaik Ameer Basha
  2013-04-24  7:41 ` [RFC v2 1/6] media: exynos4-is: modify existing mdev to use common pipeline Shaik Ameer Basha
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Shaik Ameer Basha @ 2013-04-24  7:41 UTC (permalink / raw)
  To: linux-media, devicetree-discuss, linux-samsung-soc
  Cc: s.nawrocki, shaik.samsung, arunkk.samsung

The following patchset features:

1] Creating a common pipeline framework which can be used by all
Exynos series SoCs for developing media device drivers.
2] Modified the existing fimc-mdevice for exynos4 to use the common
pipeline framework.
3] Adding of media device driver for Exynos5 Imaging subsystem.
4] Upgrading mipi-csis and fimc-lite drivers for Exynos5 SoCs.

Current changes are not tested on exynos4 series SoCs. Current media
device driver only support one pipeline (pipeline0) which consists of
        Sensor --> MIPI-CSIS --> FIMC-LITE
        Sensor --> FIMC-LITE
G-Scaler support to pipeline0 will be added later.

Once the fimc-is device driver is posted, one more pipeline (pipeline1)
will be added for exynos5 media device driver for fimc-is sub-devices.

This patchset is rebased on:
git://linuxtv.org/snawrocki/samsung.git:for_v3.10_2

Shaik Ameer Basha (6):
  media: exynos4-is: modify existing mdev to use common pipeline
  fimc-lite: Adding Exynos5 compatibility to fimc-lite driver
  media: fimc-lite: Adding support for Exynos5
  media: fimc-lite: Fix for DMA output corruption
  media: s5p-csis: Adding Exynos5250 compatibility
  media: exynos5-is: Adding media device driver for exynos5

 .../devicetree/bindings/media/exynos5-mdev.txt     |  153 +++
 drivers/media/platform/Kconfig                     |    1 +
 drivers/media/platform/Makefile                    |    1 +
 drivers/media/platform/exynos4-is/fimc-capture.c   |   47 +-
 drivers/media/platform/exynos4-is/fimc-lite-reg.c  |   16 +-
 drivers/media/platform/exynos4-is/fimc-lite-reg.h  |   41 +-
 drivers/media/platform/exynos4-is/fimc-lite.c      |   45 +-
 drivers/media/platform/exynos4-is/fimc-lite.h      |    4 +-
 drivers/media/platform/exynos4-is/media-dev.c      |  179 +++-
 drivers/media/platform/exynos4-is/media-dev.h      |   16 +
 drivers/media/platform/exynos4-is/mipi-csis.c      |    3 +-
 drivers/media/platform/exynos5-is/Kconfig          |    7 +
 drivers/media/platform/exynos5-is/Makefile         |    4 +
 drivers/media/platform/exynos5-is/exynos5-mdev.c   | 1131 ++++++++++++++++++++
 drivers/media/platform/exynos5-is/exynos5-mdev.h   |  120 +++
 include/media/s5p_fimc.h                           |   46 +-
 16 files changed, 1757 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/exynos5-mdev.txt
 create mode 100644 drivers/media/platform/exynos5-is/Kconfig
 create mode 100644 drivers/media/platform/exynos5-is/Makefile
 create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.c
 create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.h

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC v2 1/6] media: exynos4-is: modify existing mdev to use common pipeline
  2013-04-24  7:41 [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem Shaik Ameer Basha
@ 2013-04-24  7:41 ` Shaik Ameer Basha
  2013-04-29 11:24   ` Sylwester Nawrocki
  2013-04-24  7:41 ` [RFC v2 2/6] fimc-lite: Adding Exynos5 compatibility to fimc-lite driver Shaik Ameer Basha
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Shaik Ameer Basha @ 2013-04-24  7:41 UTC (permalink / raw)
  To: linux-media, devicetree-discuss, linux-samsung-soc
  Cc: s.nawrocki, shaik.samsung, arunkk.samsung

Current fimc_pipeline is tightly coupled with exynos4-is media
device driver. And this will not allow to use the same pipeline
across different exynos series media device drivers.

This patch adds,
1] Changing of the existing pipeline as a common pipeline
   to be used across multiple exynos series media device drivers.
2] Modifies the existing exynos4-is media device driver to
   use the updated common pipeline implementation.

Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-capture.c |   47 ++++--
 drivers/media/platform/exynos4-is/fimc-lite.c    |    4 +-
 drivers/media/platform/exynos4-is/media-dev.c    |  179 +++++++++++++++++++---
 drivers/media/platform/exynos4-is/media-dev.h    |   16 ++
 include/media/s5p_fimc.h                         |   46 ++++--
 5 files changed, 248 insertions(+), 44 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
index 72c516a..904d725 100644
--- a/drivers/media/platform/exynos4-is/fimc-capture.c
+++ b/drivers/media/platform/exynos4-is/fimc-capture.c
@@ -178,13 +178,16 @@ 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 v4l2_subdev *csis;
 	struct fimc_vid_cap *cap = &fimc->vid_cap;
 	struct fimc_frame *f = &cap->ctx->d_frame;
 	struct fimc_vid_buffer *v_buf;
 	struct timeval *tv;
 	struct timespec ts;
 
+	csis = fimc_pipeline_get_subdev(fimc->pipeline_ops,
+				&fimc->pipeline, EXYNOS_SD_CSIS);
+
 	if (test_and_clear_bit(ST_CAPT_SHUT, &fimc->state)) {
 		wake_up(&fimc->irq_queue);
 		goto done;
@@ -480,9 +483,12 @@ static struct vb2_ops fimc_capture_qops = {
 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];
+	struct v4l2_subdev *sensor;
 	int ret;
 
+	sensor = fimc_pipeline_get_subdev(fimc->pipeline_ops,
+				&fimc->pipeline, EXYNOS_SD_SENSOR);
+
 	if (WARN_ON(vid_cap->ctx == NULL))
 		return -ENXIO;
 	if (vid_cap->ctx->ctrls.ready)
@@ -800,7 +806,7 @@ 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 v4l2_subdev *sd;
 	struct v4l2_subdev_format sfmt;
 	struct v4l2_mbus_framefmt *mf = &sfmt.format;
 	struct media_entity *me;
@@ -809,6 +815,8 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
 	int ret, i = 1;
 	u32 fcc;
 
+	sd = fimc_pipeline_get_subdev(fimc->pipeline_ops,
+				&fimc->pipeline, EXYNOS_SD_SENSOR);
 	if (WARN_ON(!sd || !tfmt))
 		return -EINVAL;
 
@@ -974,8 +982,10 @@ 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);
+		fimc_get_sensor_frame_desc(
+			fimc_pipeline_get_subdev(fimc->pipeline_ops,
+				&fimc->pipeline, EXYNOS_SD_SENSOR),
+			pix->plane_fmt, ffmt->memplanes, true);
 unlock:
 	mutex_unlock(&fimc->lock);
 	fimc_md_graph_unlock(fimc);
@@ -1044,9 +1054,10 @@ 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],
-					pix->plane_fmt, ff->fmt->memplanes,
-					true);
+		ret = fimc_get_sensor_frame_desc(
+			fimc_pipeline_get_subdev(fimc->pipeline_ops,
+				&fimc->pipeline, EXYNOS_SD_SENSOR),
+			pix->plane_fmt, ff->fmt->memplanes, true);
 		if (ret < 0)
 			return ret;
 	}
@@ -1100,7 +1111,10 @@ 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 v4l2_subdev *sd;
+
+	sd = fimc_pipeline_get_subdev(fimc->pipeline_ops,
+				&fimc->pipeline, EXYNOS_SD_SENSOR);
 
 	if (i->index != 0)
 		return -EINVAL;
@@ -1186,8 +1200,10 @@ 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] &&
-		    fimc_user_defined_mbus_fmt(src_fmt.format.code)) {
+		if (sd == fimc_pipeline_get_subdev(fimc->pipeline_ops,
+				&fimc->pipeline, EXYNOS_SD_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;
 			unsigned int i;
@@ -1220,11 +1236,12 @@ 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, &p->m_pipeline);
 	if (ret < 0)
 		return ret;
 
-	sd = p->subdevs[IDX_SENSOR];
+	sd = fimc_pipeline_get_subdev(fimc->pipeline_ops, p,
+						EXYNOS_SD_SENSOR);
 	if (sd)
 		si = v4l2_get_subdev_hostdata(sd);
 
@@ -1830,6 +1847,9 @@ static int fimc_capture_subdev_registered(struct v4l2_subdev *sd)
 		return ret;
 
 	fimc->pipeline_ops = v4l2_get_subdev_hostdata(sd);
+	fimc->pipeline_ops->init(&fimc->pipeline);
+	fimc->pipeline_ops->register_notify_cb(&fimc->pipeline,
+						fimc_sensor_notify);
 
 	ret = fimc_register_capture_device(fimc, sd->v4l2_dev);
 	if (ret) {
@@ -1852,6 +1872,7 @@ static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd)
 	if (video_is_registered(&fimc->vid_cap.vfd)) {
 		video_unregister_device(&fimc->vid_cap.vfd);
 		media_entity_cleanup(&fimc->vid_cap.vfd.entity);
+		fimc->pipeline_ops->free(&fimc->pipeline);
 		fimc->pipeline_ops = NULL;
 	}
 	kfree(fimc->vid_cap.ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 661d0d1..4878089 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -817,7 +817,7 @@ static int fimc_lite_streamon(struct file *file, void *priv,
 	if (fimc_lite_active(fimc))
 		return -EBUSY;
 
-	ret = media_entity_pipeline_start(entity, p->m_pipeline);
+	ret = media_entity_pipeline_start(entity, &p->m_pipeline);
 	if (ret < 0)
 		return ret;
 
@@ -1296,6 +1296,7 @@ static int fimc_lite_subdev_registered(struct v4l2_subdev *sd)
 
 	video_set_drvdata(vfd, fimc);
 	fimc->pipeline_ops = v4l2_get_subdev_hostdata(sd);
+	fimc->pipeline_ops->init(&fimc->pipeline);
 
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, -1);
 	if (ret < 0) {
@@ -1319,6 +1320,7 @@ static void fimc_lite_subdev_unregistered(struct v4l2_subdev *sd)
 	if (video_is_registered(&fimc->vfd)) {
 		video_unregister_device(&fimc->vfd);
 		media_entity_cleanup(&fimc->vfd.entity);
+		fimc->pipeline_ops->free(&fimc->pipeline);
 		fimc->pipeline_ops = NULL;
 	}
 }
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 1dbd554..c80633a 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -36,6 +36,8 @@
 #include "fimc-lite.h"
 #include "mipi-csis.h"
 
+static struct exynos4_pipeline exynos4_pl;
+
 static int __fimc_md_set_camclk(struct fimc_md *fmd,
 				struct fimc_source_info *si,
 				bool on);
@@ -45,7 +47,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,
+static void fimc_pipeline_prepare(struct exynos4_pipeline *p,
 				  struct media_entity *me)
 {
 	struct v4l2_subdev *sd;
@@ -131,7 +133,7 @@ static int __subdev_set_power(struct v4l2_subdev *sd, int on)
  *
  * Needs to be called with the graph mutex held.
  */
-static int fimc_pipeline_s_power(struct fimc_pipeline *p, bool on)
+static int fimc_pipeline_s_power(struct exynos4_pipeline *p, bool on)
 {
 	static const u8 seq[2][IDX_MAX - 1] = {
 		{ IDX_IS_ISP, IDX_SENSOR, IDX_CSIS, IDX_FLITE },
@@ -146,8 +148,6 @@ static int fimc_pipeline_s_power(struct fimc_pipeline *p, bool on)
 		unsigned int idx = seq[on][i];
 
 		ret = __subdev_set_power(p->subdevs[idx], on);
-
-
 		if (ret < 0 && ret != -ENXIO)
 			goto error;
 	}
@@ -160,6 +160,48 @@ error:
 	return ret;
 }
 
+
+/**
+ * __fimc_pipeline_init - allocate the fimc_pipeline structure and do the
+ *                        basic initialization.
+ * @p: Pointer to fimc_pipeline structure
+ *
+ * Initializes the 'is_init' variable to indicate the pipeline structure
+ * is valid.
+ */
+static int __fimc_pipeline_init(struct fimc_pipeline *p)
+{
+	struct exynos4_pipeline *ep = &exynos4_pl;
+
+	if (ep->is_init)
+		return 0;
+
+	ep->is_init = true;
+	p->priv = (void *)ep;
+	return 0;
+}
+
+/**
+ * __fimc_pipeline_free - free the allocated resources for fimc_pipeline
+ * @p: Pointer to fimc_pipeline structure
+ *
+ * sets the 'is_init' variable to false, to indicate the pipeline structure
+ * is not valid.
+ *
+ * RETURNS:
+ * Zero in case of success and -EINVAL in case of failure.
+ */
+static int __fimc_pipeline_free(struct fimc_pipeline *p)
+{
+	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
+
+	if (!ep || !ep->is_init)
+		return -EINVAL;
+
+	ep->is_init = false;
+	return 0;
+}
+
 /**
  * __fimc_pipeline_open - update the pipeline information, enable power
  *                        of all pipeline subdevs and the sensor clock
@@ -171,6 +213,7 @@ error:
 static int __fimc_pipeline_open(struct fimc_pipeline *p,
 				struct media_entity *me, bool prepare)
 {
+	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
 	struct fimc_md *fmd = entity_to_fimc_mdev(me);
 	struct v4l2_subdev *sd;
 	int ret;
@@ -178,15 +221,18 @@ static int __fimc_pipeline_open(struct fimc_pipeline *p,
 	if (WARN_ON(p == NULL || me == NULL))
 		return -EINVAL;
 
+	if (!ep || !ep->is_init)
+		return -EINVAL;
+
 	if (prepare)
-		fimc_pipeline_prepare(p, me);
+		fimc_pipeline_prepare(ep, me);
 
-	sd = p->subdevs[IDX_SENSOR];
+	sd = ep->subdevs[IDX_SENSOR];
 	if (sd == NULL)
 		return -EINVAL;
 
 	/* Disable PXLASYNC clock if this pipeline includes FIMC-IS */
-	if (!IS_ERR(fmd->wbclk[CLK_IDX_WB_B]) && p->subdevs[IDX_IS_ISP]) {
+	if (!IS_ERR(fmd->wbclk[CLK_IDX_WB_B]) && ep->subdevs[IDX_IS_ISP]) {
 		ret = clk_prepare_enable(fmd->wbclk[CLK_IDX_WB_B]);
 		if (ret < 0)
 			return ret;
@@ -195,14 +241,14 @@ static int __fimc_pipeline_open(struct fimc_pipeline *p,
 	if (ret < 0)
 		goto err_wbclk;
 
-	ret = fimc_pipeline_s_power(p, 1);
+	ret = fimc_pipeline_s_power(ep, 1);
 	if (!ret)
 		return 0;
 
 	fimc_md_set_camclk(sd, false);
 
 err_wbclk:
-	if (!IS_ERR(fmd->wbclk[CLK_IDX_WB_B]) && p->subdevs[IDX_IS_ISP])
+	if (!IS_ERR(fmd->wbclk[CLK_IDX_WB_B]) && ep->subdevs[IDX_IS_ISP])
 		clk_disable_unprepare(fmd->wbclk[CLK_IDX_WB_B]);
 
 	return ret;
@@ -216,22 +262,23 @@ err_wbclk:
  */
 static int __fimc_pipeline_close(struct fimc_pipeline *p)
 {
-	struct v4l2_subdev *sd = p ? p->subdevs[IDX_SENSOR] : NULL;
+	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
+	struct v4l2_subdev *sd = ep ? ep->subdevs[IDX_SENSOR] : NULL;
 	struct fimc_md *fmd;
 	int ret = 0;
 
 	if (WARN_ON(sd == NULL))
 		return -EINVAL;
 
-	if (p->subdevs[IDX_SENSOR]) {
-		ret = fimc_pipeline_s_power(p, 0);
+	if (ep->subdevs[IDX_SENSOR]) {
+		ret = fimc_pipeline_s_power(ep, 0);
 		fimc_md_set_camclk(sd, false);
 	}
 
 	fmd = entity_to_fimc_mdev(&sd->entity);
 
 	/* Disable PXLASYNC clock if this pipeline includes FIMC-IS */
-	if (!IS_ERR(fmd->wbclk[CLK_IDX_WB_B]) && p->subdevs[IDX_IS_ISP])
+	if (!IS_ERR(fmd->wbclk[CLK_IDX_WB_B]) && ep->subdevs[IDX_IS_ISP])
 		clk_disable_unprepare(fmd->wbclk[CLK_IDX_WB_B]);
 
 	return ret == -ENXIO ? 0 : ret;
@@ -244,19 +291,20 @@ static int __fimc_pipeline_close(struct fimc_pipeline *p)
  */
 static int __fimc_pipeline_s_stream(struct fimc_pipeline *p, bool on)
 {
+	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
 	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 },
 	};
 	int i, ret = 0;
 
-	if (p->subdevs[IDX_SENSOR] == NULL)
+	if (ep->subdevs[IDX_SENSOR] == NULL)
 		return -ENODEV;
 
 	for (i = 0; i < IDX_MAX; i++) {
 		unsigned int idx = seq[on][i];
 
-		ret = v4l2_subdev_call(p->subdevs[idx], video, s_stream, on);
+		ret = v4l2_subdev_call(ep->subdevs[idx], video, s_stream, on);
 
 		if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
 			goto error;
@@ -265,16 +313,74 @@ static int __fimc_pipeline_s_stream(struct fimc_pipeline *p, bool on)
 error:
 	for (; i >= 0; i--) {
 		unsigned int idx = seq[on][i];
-		v4l2_subdev_call(p->subdevs[idx], video, s_stream, !on);
+		v4l2_subdev_call(ep->subdevs[idx], video, s_stream, !on);
 	}
 	return ret;
 }
 
+/**
+ * __fimc_pipeline_get_subdev_sensor - if valid pipeline, returns the sensor
+ *                                     subdev pointer else returns NULL
+ * @p: Pointer to fimc_pipeline structure
+ * @sd_idx: Index of the subdev associated with the current pipeline
+ *
+ * RETURNS:
+ * If success, returns valid subdev pointer or NULL in case of sudbev not found
+ */
+static struct v4l2_subdev *__fimc_pipeline_get_subdev(struct fimc_pipeline *p,
+					enum exynos_subdev_index sd_idx)
+{
+	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
+	struct v4l2_subdev *sd;
+
+	if (!ep || !ep->is_init)
+		return NULL;
+
+	switch (sd_idx) {
+	case EXYNOS_SD_SENSOR:
+		sd = ep->subdevs[IDX_SENSOR];
+		break;
+	case EXYNOS_SD_CSIS:
+		sd = ep->subdevs[IDX_CSIS];
+		break;
+	case EXYNOS_SD_FLITE:
+		sd = ep->subdevs[IDX_FLITE];
+		break;
+	case EXYNOS_SD_IS_ISP:
+		sd = ep->subdevs[IDX_IS_ISP];
+		break;
+	case EXYNOS_SD_FIMC:
+		sd = ep->subdevs[IDX_FIMC];
+		break;
+	default:
+		sd = NULL;
+		break;
+	}
+	return sd;
+}
+
+static void __fimc_pipeline_register_notify_callback(
+		struct fimc_pipeline *p,
+		void (*notify_cb)(struct v4l2_subdev *sd,
+					unsigned int notification, void *arg))
+{
+	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
+
+	if (!notify_cb)
+		return;
+
+	ep->sensor_notify = notify_cb;
+}
+
 /* Media pipeline operations for the FIMC/FIMC-LITE video device driver */
 static const struct fimc_pipeline_ops fimc_pipeline_ops = {
+	.init		= __fimc_pipeline_init,
+	.free		= __fimc_pipeline_free,
 	.open		= __fimc_pipeline_open,
 	.close		= __fimc_pipeline_close,
 	.set_stream	= __fimc_pipeline_s_stream,
+	.get_subdev	= __fimc_pipeline_get_subdev,
+	.register_notify_cb     = __fimc_pipeline_register_notify_callback,
 };
 
 /*
@@ -1234,6 +1340,7 @@ static int fimc_md_link_notify(struct media_pad *source,
 	struct fimc_lite *fimc_lite = NULL;
 	struct fimc_dev *fimc = NULL;
 	struct fimc_pipeline *pipeline;
+	struct exynos4_pipeline *ep;
 	struct v4l2_subdev *sd;
 	struct mutex *lock;
 	int i, ret = 0;
@@ -1266,6 +1373,10 @@ static int fimc_md_link_notify(struct media_pad *source,
 	mutex_lock(lock);
 	ref_count = fimc ? fimc->vid_cap.refcnt : fimc_lite->ref_count;
 
+	ep = (struct exynos4_pipeline *)pipeline->priv;
+	if (!ep || !ep->is_init)
+		return -EINVAL;
+
 	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
 		if (ref_count > 0) {
 			ret = __fimc_pipeline_close(pipeline);
@@ -1273,7 +1384,7 @@ static int fimc_md_link_notify(struct media_pad *source,
 				fimc_ctrls_delete(fimc->vid_cap.ctx);
 		}
 		for (i = 0; i < IDX_MAX; i++)
-			pipeline->subdevs[i] = NULL;
+			ep->subdevs[i] = NULL;
 	} else if (ref_count > 0) {
 		/*
 		 * Link activation. Enable power of pipeline elements only if
@@ -1290,6 +1401,38 @@ static int fimc_md_link_notify(struct media_pad *source,
 	return ret ? -EPIPE : ret;
 }
 
+/**
+ * fimc_md_sensor_notify - v4l2_device notification from a sensor subdev
+ * @sd: pointer to a subdev generating the notification
+ * @notification: the notification type, must be S5P_FIMC_TX_END_NOTIFY
+ * @arg: pointer to an u32 type integer that stores the frame payload value
+ *
+ * Passes the sensor notification to the capture device
+ */
+void fimc_md_sensor_notify(struct v4l2_subdev *sd,
+				unsigned int notification, void *arg)
+{
+	struct fimc_md *fmd;
+	struct exynos4_pipeline *ep;
+	struct fimc_pipeline *p;
+	unsigned long flags;
+
+	fmd = entity_to_fimc_mdev(&sd->entity);
+
+	if (sd == NULL)
+		return;
+
+	p = media_pipe_to_fimc_pipeline(sd->entity.pipe);
+	ep = (struct exynos4_pipeline *)p->priv;
+
+	spin_lock_irqsave(&fmd->slock, flags);
+
+	if (ep->sensor_notify)
+		ep->sensor_notify(sd, notification, arg);
+
+	spin_unlock_irqrestore(&fmd->slock, flags);
+}
+
 static ssize_t fimc_md_sysfs_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
 {
@@ -1375,7 +1518,7 @@ static int fimc_md_probe(struct platform_device *pdev)
 
 	v4l2_dev = &fmd->v4l2_dev;
 	v4l2_dev->mdev = &fmd->media_dev;
-	v4l2_dev->notify = fimc_sensor_notify;
+	v4l2_dev->notify = fimc_md_sensor_notify;
 	strlcpy(v4l2_dev->name, "s5p-fimc-md", sizeof(v4l2_dev->name));
 
 	fmd->use_isp = fimc_md_is_isp_available(dev->of_node);
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index 44d86b6..7ee8dd8 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -40,6 +40,15 @@ enum {
 	FIMC_MAX_WBCLKS
 };
 
+enum exynos4_subdev_index {
+	IDX_SENSOR,
+	IDX_CSIS,
+	IDX_FLITE,
+	IDX_IS_ISP,
+	IDX_FIMC,
+	IDX_MAX,
+};
+
 struct fimc_csis_info {
 	struct v4l2_subdev *sd;
 	int id;
@@ -107,6 +116,13 @@ struct fimc_md {
 	spinlock_t slock;
 };
 
+struct exynos4_pipeline {
+	int is_init;
+	struct v4l2_subdev *subdevs[IDX_MAX];
+	void (*sensor_notify)(struct v4l2_subdev *sd,
+			unsigned int notification, void *arg);
+};
+
 #define is_subdev_pad(pad) (pad == NULL || \
 	media_entity_type(pad->entity) == MEDIA_ENT_T_V4L2_SUBDEV)
 
diff --git a/include/media/s5p_fimc.h b/include/media/s5p_fimc.h
index f509690..2908a02 100644
--- a/include/media/s5p_fimc.h
+++ b/include/media/s5p_fimc.h
@@ -105,6 +105,24 @@ struct s5p_platform_fimc {
  */
 #define S5P_FIMC_TX_END_NOTIFY _IO('e', 0)
 
+/* Group IDs of sensor, MIPI-CSIS, FIMC-LITE and the writeback subdevs. */
+#define GRP_ID_SENSOR		(1 << 8)
+#define GRP_ID_FIMC_IS_SENSOR	(1 << 9)
+#define GRP_ID_WRITEBACK	(1 << 10)
+#define GRP_ID_CSIS		(1 << 11)
+#define GRP_ID_FIMC		(1 << 12)
+#define GRP_ID_FLITE		(1 << 13)
+#define GRP_ID_FIMC_IS		(1 << 14)
+
+enum exynos_subdev_index {
+	EXYNOS_SD_SENSOR,
+	EXYNOS_SD_CSIS,
+	EXYNOS_SD_FLITE,
+	EXYNOS_SD_IS_ISP,
+	EXYNOS_SD_FIMC,
+	EXYNOS_SD_MAX,
+};
+
 #define FIMC_MAX_PLANES	3
 
 /**
@@ -140,21 +158,12 @@ 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 fimc_pipeline {
-	struct v4l2_subdev *subdevs[IDX_MAX];
-	struct media_pipeline *m_pipeline;
+	struct media_pipeline m_pipeline;
+	void *priv;
 };
 
 /*
@@ -163,14 +172,27 @@ struct fimc_pipeline {
  * by corresponding media device driver.
  */
 struct fimc_pipeline_ops {
+	int (*init) (struct fimc_pipeline *p);
+	int (*free) (struct fimc_pipeline *p);
 	int (*open)(struct fimc_pipeline *p, struct media_entity *me,
-			  bool resume);
+					bool resume);
 	int (*close)(struct fimc_pipeline *p);
 	int (*set_stream)(struct fimc_pipeline *p, bool state);
+	struct v4l2_subdev *(*get_subdev)(struct fimc_pipeline *p,
+					enum exynos_subdev_index sd_idx);
+	void (*register_notify_cb)(struct fimc_pipeline *p,
+					void (*cb)(struct v4l2_subdev *sd,
+					unsigned int notification, void *arg));
 };
 
 #define fimc_pipeline_call(f, op, p, args...)				\
 	(!(f) ? -ENODEV : (((f)->pipeline_ops && (f)->pipeline_ops->op) ? \
 			    (f)->pipeline_ops->op((p), ##args) : -ENOIOCTLCMD))
 
+#define fimc_pipeline_get_subdev(ops, p, idx_subdev)			\
+	((ops && ops->get_subdev) ? ops->get_subdev(p, idx_subdev) : NULL)
+
+#define media_pipe_to_fimc_pipeline(mp) \
+	container_of(mp, struct fimc_pipeline, m_pipeline)
+
 #endif /* S5P_FIMC_H_ */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v2 2/6] fimc-lite: Adding Exynos5 compatibility to fimc-lite driver
  2013-04-24  7:41 [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem Shaik Ameer Basha
  2013-04-24  7:41 ` [RFC v2 1/6] media: exynos4-is: modify existing mdev to use common pipeline Shaik Ameer Basha
@ 2013-04-24  7:41 ` Shaik Ameer Basha
  2013-04-29 12:08   ` Sylwester Nawrocki
  2013-04-24  7:41 ` [RFC v2 3/6] media: fimc-lite: Adding support for Exynos5 Shaik Ameer Basha
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Shaik Ameer Basha @ 2013-04-24  7:41 UTC (permalink / raw)
  To: linux-media, devicetree-discuss, linux-samsung-soc
  Cc: s.nawrocki, shaik.samsung, arunkk.samsung

This patch adds,
1] Exynos5 soc compatibility to the fimc-lite driver
2] Multiple dma output buffer support as from Exynos5 onwards,
   fimc-lite h/w ip supports multiple dma buffers.

Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-lite.c |   19 ++++++++++++++++++-
 drivers/media/platform/exynos4-is/fimc-lite.h |    4 +++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 4878089..cb173ec 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1467,7 +1467,7 @@ static int fimc_lite_probe(struct platform_device *pdev)
 		fimc->index = pdev->id;
 	}
 
-	if (!drv_data || fimc->index < 0 || fimc->index >= FIMC_LITE_MAX_DEVS)
+	if (!drv_data || fimc->index < 0 || fimc->index >= drv_data->num_devs)
 		return -EINVAL;
 
 	fimc->dd = drv_data;
@@ -1625,6 +1625,19 @@ static struct flite_drvdata fimc_lite_drvdata_exynos4 = {
 	.out_width_align	= 8,
 	.win_hor_offs_align	= 2,
 	.out_hor_offs_align	= 8,
+	.support_multi_dma_buf	= false,
+	.num_devs = 2,
+};
+
+/* EXYNOS5250 */
+static struct flite_drvdata fimc_lite_drvdata_exynos5 = {
+	.max_width		= 8192,
+	.max_height		= 8192,
+	.out_width_align	= 8,
+	.win_hor_offs_align	= 2,
+	.out_hor_offs_align	= 8,
+	.support_multi_dma_buf	= true,
+	.num_devs = 3,
 };
 
 static struct platform_device_id fimc_lite_driver_ids[] = {
@@ -1641,6 +1654,10 @@ static const struct of_device_id flite_of_match[] = {
 		.compatible = "samsung,exynos4212-fimc-lite",
 		.data = &fimc_lite_drvdata_exynos4,
 	},
+	{
+		.compatible = "samsung,exynos5250-fimc-lite",
+		.data = &fimc_lite_drvdata_exynos5,
+	},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, flite_of_match);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.h b/drivers/media/platform/exynos4-is/fimc-lite.h
index 71fed51..a35f29e 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.h
+++ b/drivers/media/platform/exynos4-is/fimc-lite.h
@@ -27,7 +27,7 @@
 
 #define FIMC_LITE_DRV_NAME	"exynos-fimc-lite"
 #define FLITE_CLK_NAME		"flite"
-#define FIMC_LITE_MAX_DEVS	2
+#define FIMC_LITE_MAX_DEVS	3
 #define FLITE_REQ_BUFS_MIN	2
 
 /* Bit index definitions for struct fimc_lite::state */
@@ -54,6 +54,8 @@ struct flite_drvdata {
 	unsigned short out_width_align;
 	unsigned short win_hor_offs_align;
 	unsigned short out_hor_offs_align;
+	unsigned short support_multi_dma_buf;
+	unsigned short num_devs;
 };
 
 #define fimc_lite_get_drvdata(_pdev) \
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v2 3/6] media: fimc-lite: Adding support for Exynos5
  2013-04-24  7:41 [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem Shaik Ameer Basha
  2013-04-24  7:41 ` [RFC v2 1/6] media: exynos4-is: modify existing mdev to use common pipeline Shaik Ameer Basha
  2013-04-24  7:41 ` [RFC v2 2/6] fimc-lite: Adding Exynos5 compatibility to fimc-lite driver Shaik Ameer Basha
@ 2013-04-24  7:41 ` Shaik Ameer Basha
  2013-04-29 15:13   ` Sylwester Nawrocki
  2013-04-24  7:41 ` [RFC v2 4/6] media: fimc-lite: Fix for DMA output corruption Shaik Ameer Basha
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Shaik Ameer Basha @ 2013-04-24  7:41 UTC (permalink / raw)
  To: linux-media, devicetree-discuss, linux-samsung-soc
  Cc: s.nawrocki, shaik.samsung, arunkk.samsung

FIMC-LITE supports multiple DMA shadow registers from Exynos5 onwards.
This patch adds the functionality of using shadow registers by
checking the driver data.

Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-lite-reg.c |   13 +++++++
 drivers/media/platform/exynos4-is/fimc-lite-reg.h |   41 ++++++++++++++++++++-
 drivers/media/platform/exynos4-is/fimc-lite.c     |   12 ++++--
 3 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.c b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
index 8cc0d39..a1d566a 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite-reg.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
@@ -215,6 +215,18 @@ void flite_hw_set_camera_bus(struct fimc_lite *dev,
 	flite_hw_set_camera_port(dev, si->mux_id);
 }
 
+static void flite_hw_set_pack12(struct fimc_lite *dev, int on)
+{
+	u32 cfg = readl(dev->regs + FLITE_REG_CIODMAFMT);
+
+	cfg &= ~FLITE_REG_CIODMAFMT_PACK12;
+
+	if (on)
+		cfg |= FLITE_REG_CIODMAFMT_PACK12;
+
+	writel(cfg, dev->regs + FLITE_REG_CIODMAFMT);
+}
+
 static void flite_hw_set_out_order(struct fimc_lite *dev, struct flite_frame *f)
 {
 	static const u32 pixcode[4][2] = {
@@ -267,6 +279,7 @@ void flite_hw_set_output_dma(struct fimc_lite *dev, struct flite_frame *f,
 
 	flite_hw_set_out_order(dev, f);
 	flite_hw_set_dma_window(dev, f);
+	flite_hw_set_pack12(dev, 0);
 }
 
 void flite_hw_dump_regs(struct fimc_lite *dev, const char *label)
diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.h b/drivers/media/platform/exynos4-is/fimc-lite-reg.h
index 3903839..8e57e7a 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite-reg.h
+++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.h
@@ -120,6 +120,10 @@
 /* b0: 1 - camera B, 0 - camera A */
 #define FLITE_REG_CIGENERAL_CAM_B		(1 << 0)
 
+
+#define FLITE_REG_CIFCNTSEQ			0x100
+#define FLITE_REG_CIOSAN(x)			(0x200 + (4 * (x)))
+
 /* ----------------------------------------------------------------------------
  * Function declarations
  */
@@ -143,8 +147,41 @@ void flite_hw_set_dma_window(struct fimc_lite *dev, struct flite_frame *f);
 void flite_hw_set_test_pattern(struct fimc_lite *dev, bool on);
 void flite_hw_dump_regs(struct fimc_lite *dev, const char *label);
 
-static inline void flite_hw_set_output_addr(struct fimc_lite *dev, u32 paddr)
+static inline void flite_hw_set_output_addr(struct fimc_lite *dev,
+	u32 paddr, u32 index)
+{
+	u32 config;
+
+	/* FLITE in EXYNOS4 has only one DMA register */
+	if (!dev->dd->support_multi_dma_buf)
+		index = 0;
+
+	config = readl(dev->regs + FLITE_REG_CIFCNTSEQ);
+	config |= 1 << index;
+	writel(config, dev->regs + FLITE_REG_CIFCNTSEQ);
+
+	if (index == 0)
+		writel(paddr, dev->regs + FLITE_REG_CIOSA);
+	else
+		writel(paddr, dev->regs + FLITE_REG_CIOSAN(index-1));
+}
+
+static inline void flite_hw_clear_output_addr(struct fimc_lite *dev, u32 index)
 {
-	writel(paddr, dev->regs + FLITE_REG_CIOSA);
+	u32 config;
+
+	/* FLITE in EXYNOS4 has only one DMA register */
+	if (!dev->dd->support_multi_dma_buf)
+		index = 0;
+
+	config = readl(dev->regs + FLITE_REG_CIFCNTSEQ);
+	config &= ~(1 << index);
+	writel(config, dev->regs + FLITE_REG_CIFCNTSEQ);
 }
+
+static inline void flite_hw_clear_output_index(struct fimc_lite *dev)
+{
+	writel(0, dev->regs + FLITE_REG_CIFCNTSEQ);
+}
+
 #endif /* FIMC_LITE_REG_H */
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index cb173ec..1b12ea8 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -166,6 +166,8 @@ static int fimc_lite_hw_init(struct fimc_lite *fimc, bool isp_output)
 	if (fimc->inp_frame.fmt == NULL || fimc->out_frame.fmt == NULL)
 		return -EINVAL;
 
+	flite_hw_clear_output_index(fimc);
+
 	/* Get sensor configuration data from the sensor subdev */
 	si = v4l2_get_subdev_hostdata(fimc->sensor);
 	if (!si)
@@ -307,11 +309,12 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
 		tv->tv_sec = ts.tv_sec;
 		tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 		vbuf->vb.v4l2_buf.sequence = fimc->frame_count++;
+		flite_hw_clear_output_addr(fimc, vbuf->vb.v4l2_buf.index);
 		vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
 
 		vbuf = fimc_lite_pending_queue_pop(fimc);
-		flite_hw_set_output_addr(fimc, vbuf->paddr);
-		fimc_lite_active_queue_add(fimc, vbuf);
+		flite_hw_set_output_addr(fimc, vbuf->paddr,
+					vbuf->vb.v4l2_buf.index);
 	}
 
 	if (test_bit(ST_FLITE_CONFIG, &fimc->state))
@@ -439,7 +442,8 @@ static void buffer_queue(struct vb2_buffer *vb)
 	if (!test_bit(ST_FLITE_SUSPENDED, &fimc->state) &&
 	    !test_bit(ST_FLITE_STREAM, &fimc->state) &&
 	    list_empty(&fimc->active_buf_q)) {
-		flite_hw_set_output_addr(fimc, buf->paddr);
+		flite_hw_set_output_addr(fimc, buf->paddr,
+					buf->vb.v4l2_buf.index);
 		fimc_lite_active_queue_add(fimc, buf);
 	} else {
 		fimc_lite_pending_queue_add(fimc, buf);
@@ -643,7 +647,7 @@ static int fimc_vidioc_querycap_capture(struct file *file, void *priv,
 	strlcpy(cap->driver, FIMC_LITE_DRV_NAME, sizeof(cap->driver));
 	cap->bus_info[0] = 0;
 	cap->card[0] = 0;
-	cap->capabilities = V4L2_CAP_STREAMING;
+	cap->capabilities = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE_MPLANE;
 	return 0;
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v2 4/6] media: fimc-lite: Fix for DMA output corruption
  2013-04-24  7:41 [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem Shaik Ameer Basha
                   ` (2 preceding siblings ...)
  2013-04-24  7:41 ` [RFC v2 3/6] media: fimc-lite: Adding support for Exynos5 Shaik Ameer Basha
@ 2013-04-24  7:41 ` Shaik Ameer Basha
  2013-04-24  7:41 ` [RFC v2 5/6] media: s5p-csis: Adding Exynos5250 compatibility Shaik Ameer Basha
  2013-04-24  7:41 ` [RFC v2 6/6] media: exynos5-is: Adding media device driver for exynos5 Shaik Ameer Basha
  5 siblings, 0 replies; 12+ messages in thread
From: Shaik Ameer Basha @ 2013-04-24  7:41 UTC (permalink / raw)
  To: linux-media, devicetree-discuss, linux-samsung-soc
  Cc: s.nawrocki, shaik.samsung, arunkk.samsung

Fixes Buffer corruption on DMA output from fimc-lite

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-lite-reg.c |    3 ++-
 drivers/media/platform/exynos4-is/fimc-lite.c     |   14 +++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.c b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
index a1d566a..46eda5b 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite-reg.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
@@ -68,7 +68,8 @@ void flite_hw_set_interrupt_mask(struct fimc_lite *dev)
 	if (atomic_read(&dev->out_path) == FIMC_IO_DMA) {
 		intsrc = FLITE_REG_CIGCTRL_IRQ_OVFEN |
 			 FLITE_REG_CIGCTRL_IRQ_LASTEN |
-			 FLITE_REG_CIGCTRL_IRQ_STARTEN;
+			 FLITE_REG_CIGCTRL_IRQ_STARTEN |
+			 FLITE_REG_CIGCTRL_IRQ_ENDEN;
 	} else {
 		/* An output to the FIMC-IS */
 		intsrc = FLITE_REG_CIGCTRL_IRQ_OVFEN |
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 1b12ea8..5de2dd4 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -301,8 +301,16 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
 
 	if ((intsrc & FLITE_REG_CISTATUS_IRQ_SRC_FRMSTART) &&
 	    test_bit(ST_FLITE_RUN, &fimc->state) &&
-	    !list_empty(&fimc->active_buf_q) &&
 	    !list_empty(&fimc->pending_buf_q)) {
+		vbuf = fimc_lite_pending_queue_pop(fimc);
+		flite_hw_set_output_addr(fimc, vbuf->paddr,
+					vbuf->vb.v4l2_buf.index);
+		fimc_lite_active_queue_add(fimc, vbuf);
+	}
+
+	if ((intsrc & FLITE_REG_CISTATUS_IRQ_SRC_FRMEND) &&
+	    test_bit(ST_FLITE_RUN, &fimc->state) &&
+	    !list_empty(&fimc->active_buf_q)) {
 		vbuf = fimc_lite_active_queue_pop(fimc);
 		ktime_get_ts(&ts);
 		tv = &vbuf->vb.v4l2_buf.timestamp;
@@ -311,10 +319,6 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
 		vbuf->vb.v4l2_buf.sequence = fimc->frame_count++;
 		flite_hw_clear_output_addr(fimc, vbuf->vb.v4l2_buf.index);
 		vb2_buffer_done(&vbuf->vb, VB2_BUF_STATE_DONE);
-
-		vbuf = fimc_lite_pending_queue_pop(fimc);
-		flite_hw_set_output_addr(fimc, vbuf->paddr,
-					vbuf->vb.v4l2_buf.index);
 	}
 
 	if (test_bit(ST_FLITE_CONFIG, &fimc->state))
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v2 5/6] media: s5p-csis: Adding Exynos5250 compatibility
  2013-04-24  7:41 [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem Shaik Ameer Basha
                   ` (3 preceding siblings ...)
  2013-04-24  7:41 ` [RFC v2 4/6] media: fimc-lite: Fix for DMA output corruption Shaik Ameer Basha
@ 2013-04-24  7:41 ` Shaik Ameer Basha
  2013-04-29 15:14   ` Sylwester Nawrocki
  2013-04-24  7:41 ` [RFC v2 6/6] media: exynos5-is: Adding media device driver for exynos5 Shaik Ameer Basha
  5 siblings, 1 reply; 12+ messages in thread
From: Shaik Ameer Basha @ 2013-04-24  7:41 UTC (permalink / raw)
  To: linux-media, devicetree-discuss, linux-samsung-soc
  Cc: s.nawrocki, shaik.samsung, arunkk.samsung

FIMC-IS firmware needs all the MIPI-CSIS interrupts to be enabled.
This patch enables all those MIPI interrupts and adds the Exynos5
compatible string.

Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 drivers/media/platform/exynos4-is/mipi-csis.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index 8636bcd..51ad9b2 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -66,7 +66,7 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
 
 /* Interrupt mask */
 #define S5PCSIS_INTMSK			0x10
-#define S5PCSIS_INTMSK_EN_ALL		0xf000103f
+#define S5PCSIS_INTMSK_EN_ALL		0xfc00103f
 #define S5PCSIS_INTMSK_EVEN_BEFORE	(1 << 31)
 #define S5PCSIS_INTMSK_EVEN_AFTER	(1 << 30)
 #define S5PCSIS_INTMSK_ODD_BEFORE	(1 << 29)
@@ -1003,6 +1003,7 @@ static const struct dev_pm_ops s5pcsis_pm_ops = {
 static const struct of_device_id s5pcsis_of_match[] = {
 	{ .compatible = "samsung,s5pv210-csis" },
 	{ .compatible = "samsung,exynos4210-csis" },
+	{ .compatible = "samsung,exynos5250-csis" },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, s5pcsis_of_match);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC v2 6/6] media: exynos5-is: Adding media device driver for exynos5
  2013-04-24  7:41 [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem Shaik Ameer Basha
                   ` (4 preceding siblings ...)
  2013-04-24  7:41 ` [RFC v2 5/6] media: s5p-csis: Adding Exynos5250 compatibility Shaik Ameer Basha
@ 2013-04-24  7:41 ` Shaik Ameer Basha
  2013-04-29 16:09   ` Sylwester Nawrocki
  5 siblings, 1 reply; 12+ messages in thread
From: Shaik Ameer Basha @ 2013-04-24  7:41 UTC (permalink / raw)
  To: linux-media, devicetree-discuss, linux-samsung-soc
  Cc: s.nawrocki, shaik.samsung, arunkk.samsung

This patch adds support for media device for EXYNOS5 SoCs.
The current media device supports the following ips to connect
through the media controller framework.

* MIPI-CSIS
  Support interconnection(subdev interface) between devices

* FIMC-LITE
  Support capture interface from device(Sensor, MIPI-CSIS) to memory
  Support interconnection(subdev interface) between devices

G-Scaler will be added later to the current media device.

* Gscaler: general scaler
  Support memory to memory interface
  Support output interface from memory to display device(LCD, TV)
  Support capture interface from device(FIMC-LITE, FIMD) to memory

--> media 0
  Camera Capture path consists of MIPI-CSIS, FIMC-LITE and G-Scaler
  +--------+     +-----------+     +-----------------+
  | Sensor | --> | FIMC-LITE | --> | G-Scaler-capture |
  +--------+     +-----------+     +-----------------+

  +--------+     +-----------+     +-----------+     +-----------------+
  | Sensor | --> | MIPI-CSIS | --> | FIMC-LITE | --> | G-Scaler-capture |
  +--------+     +-----------+     +-----------+     +-----------------+

Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 .../devicetree/bindings/media/exynos5-mdev.txt     |  153 +++
 drivers/media/platform/Kconfig                     |    1 +
 drivers/media/platform/Makefile                    |    1 +
 drivers/media/platform/exynos5-is/Kconfig          |    7 +
 drivers/media/platform/exynos5-is/Makefile         |    4 +
 drivers/media/platform/exynos5-is/exynos5-mdev.c   | 1131 ++++++++++++++++++++
 drivers/media/platform/exynos5-is/exynos5-mdev.h   |  120 +++
 7 files changed, 1417 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/exynos5-mdev.txt
 create mode 100644 drivers/media/platform/exynos5-is/Kconfig
 create mode 100644 drivers/media/platform/exynos5-is/Makefile
 create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.c
 create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.h

diff --git a/Documentation/devicetree/bindings/media/exynos5-mdev.txt b/Documentation/devicetree/bindings/media/exynos5-mdev.txt
new file mode 100644
index 0000000..d7d419b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/exynos5-mdev.txt
@@ -0,0 +1,153 @@
+Samsung EXYNOS5 SoC Camera Subsystem (FIMC)
+----------------------------------------------
+
+The Exynos5 SoC Camera subsystem comprises of multiple sub-devices
+represented by separate device tree nodes. Currently this includes: FIMC-LITE,
+MIPI CSIS and FIMC-IS.
+
+The sub-subdevices are defined as child nodes of the common 'camera' node which
+also includes common properties of the whole subsystem not really specific to
+any single sub-device, like common camera port pins or the CAMCLK clock outputs
+for external image sensors attached to an SoC.
+
+Common 'camera' node
+--------------------
+
+Required properties:
+
+- compatible	: must be "samsung,exynos5-fimc", "simple-bus"
+- clocks	: list of clock specifiers, corresponding to entries in
+		  the clock-names property;
+- clock-names	: must contain "sclk_cam0", "sclk_cam1" entries,
+		  matching entries in the clocks property.
+
+The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used
+to define a required pinctrl state named "default" and optional pinctrl states:
+"idle", "active-a", active-b". These optional states can be used to switch the
+camera port pinmux at runtime. The "idle" state should configure both the camera
+ports A and B into high impedance state, especially the CAMCLK clock output
+should be inactive. For the "active-a" state the camera port A must be activated
+and the port B deactivated and for the state "active-b" it should be the other
+way around.
+
+The 'camera' node must include at least one 'fimc-lite' child node.
+
+'parallel-ports' node
+---------------------
+
+This node should contain child 'port' nodes specifying active parallel video
+input ports. It includes camera A and camera B inputs. 'reg' property in the
+port nodes specifies data input - 0, 1 indicates input A, B respectively.
+
+Optional properties
+
+- samsung,camclk-out : specifies clock output for remote sensor,
+		       0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT;
+
+Image sensor nodes
+------------------
+
+The sensor device nodes should be added to their control bus controller (e.g.
+I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
+using the common video interfaces bindings, defined in video-interfaces.txt.
+The implementation of this bindings requires clock-frequency property to be
+present in the sensor device nodes.
+
+Example:
+
+	aliases {
+		fimc-lite0 = &fimc_lite_0
+	};
+
+	/* Parallel bus IF sensor */
+	i2c_0: i2c@13860000 {
+		s5k6aa: sensor@3c {
+			compatible = "samsung,s5k6aafx";
+			reg = <0x3c>;
+			vddio-supply = <...>;
+
+			clock-frequency = <24000000>;
+			clocks = <...>;
+			clock-names = "mclk";
+
+			port {
+				s5k6aa_ep: endpoint {
+					remote-endpoint = <&fimc0_ep>;
+					bus-width = <8>;
+					hsync-active = <0>;
+					vsync-active = <1>;
+					pclk-sample = <1>;
+				};
+			};
+		};
+	};
+
+	/* MIPI CSI-2 bus IF sensor */
+	s5c73m3: sensor@0x1a {
+		compatible = "samsung,s5c73m3";
+		reg = <0x1a>;
+		vddio-supply = <...>;
+
+		clock-frequency = <24000000>;
+		clocks = <...>;
+		clock-names = "mclk";
+
+		port {
+			s5c73m3_1: endpoint {
+				data-lanes = <1 2 3 4>;
+				remote-endpoint = <&csis0_ep>;
+			};
+		};
+	};
+
+	camera {
+		compatible = "samsung,exynos5-fimc", "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		status = "okay";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&cam_port_a_clk_active>;
+
+		/* parallel camera ports */
+		parallel-ports {
+			/* camera A input */
+			port@0 {
+				reg = <0>;
+				fimc0_ep: endpoint {
+					remote-endpoint = <&s5k6aa_ep>;
+					bus-width = <8>;
+					hsync-active = <0>;
+					vsync-active = <1>;
+					pclk-sample = <1>;
+				};
+			};
+		};
+
+		fimc_lite_0: fimc-lite@13C00000 {
+			compatible = "samsung,exynos5250-fimc-lite";
+			reg = <0x13C00000 0x1000>;
+			interrupts = <0 126 0>;
+			clocks = <&clock 129>;
+			clock-names = "flite";
+			status = "okay";
+		};
+
+		csis_0: csis@11880000 {
+			compatible = "samsung,exynos4210-csis";
+			reg = <0x11880000 0x1000>;
+			interrupts = <0 78 0>;
+			/* camera C input */
+			port@3 {
+				reg = <3>;
+				csis0_ep: endpoint {
+					remote-endpoint = <&s5c73m3_ep>;
+					data-lanes = <1 2 3 4>;
+					samsung,csis-hs-settle = <12>;
+				};
+			};
+		};
+	};
+
+MIPI-CSIS device binding is defined in samsung-mipi-csis.txt and FIMC-LITE
+device binding is defined in exynos-fimc-lite.txt.
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index d25f044..66fbb41 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -123,6 +123,7 @@ config VIDEO_S3C_CAMIF
 
 source "drivers/media/platform/soc_camera/Kconfig"
 source "drivers/media/platform/exynos4-is/Kconfig"
+source "drivers/media/platform/exynos5-is/Kconfig"
 source "drivers/media/platform/s5p-tv/Kconfig"
 
 endif # V4L_PLATFORM_DRIVERS
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index eee28dd..b1225e5 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_TV)	+= s5p-tv/
 
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)	+= s5p-g2d/
 obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)	+= exynos-gsc/
+obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS5_MDEV)	+= exynos5-is/
 
 obj-$(CONFIG_BLACKFIN)                  += blackfin/
 
diff --git a/drivers/media/platform/exynos5-is/Kconfig b/drivers/media/platform/exynos5-is/Kconfig
new file mode 100644
index 0000000..de02d90
--- /dev/null
+++ b/drivers/media/platform/exynos5-is/Kconfig
@@ -0,0 +1,7 @@
+config VIDEO_SAMSUNG_EXYNOS5_MDEV
+	bool "Samsung Exynos5 Media Device driver"
+	depends on VIDEO_DEV && VIDEO_V4L2 && ARCH_EXYNOS5 && OF
+	help
+	  This is a v4l2 based media controller driver for
+	  Exynos5 SoC.
+
diff --git a/drivers/media/platform/exynos5-is/Makefile b/drivers/media/platform/exynos5-is/Makefile
new file mode 100644
index 0000000..002e137
--- /dev/null
+++ b/drivers/media/platform/exynos5-is/Makefile
@@ -0,0 +1,4 @@
+ccflags-y += -Idrivers/media/platform/exynos4-is
+exynos-mdevice-objs := exynos5-mdev.o
+
+obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS5_MDEV) += exynos-mdevice.o
diff --git a/drivers/media/platform/exynos5-is/exynos5-mdev.c b/drivers/media/platform/exynos5-is/exynos5-mdev.c
new file mode 100644
index 0000000..ac3e85e
--- /dev/null
+++ b/drivers/media/platform/exynos5-is/exynos5-mdev.c
@@ -0,0 +1,1131 @@
+/*
+ * EXYNOS5 SoC series camera host interface media device driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Shaik Ameer Basha <shaik.ameer@samsung.com>
+ *
+ * This driver is based on exynos4-is media device driver developed by
+ * 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
+ * by the Free Software Foundation, either version 2 of the License,
+ * or (at your option) any later version.
+ */
+
+#include <linux/bug.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/of_i2c.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-of.h>
+#include <media/media-device.h>
+
+#include "exynos5-mdev.h"
+
+#define dbg(fmt, args...) \
+	pr_debug("%s:%d: " fmt "\n", __func__, __LINE__, ##args)
+
+static int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on);
+static int __fimc_md_set_camclk(struct fimc_md *fmd,
+				struct fimc_source_info *si,
+				bool on);
+/**
+ * fimc_pipeline_prepare - update pipeline information with subdevice pointers
+ * @me: media entity terminating the pipeline
+ *
+ * Caller holds the graph mutex.
+ */
+static void fimc_pipeline_prepare(struct exynos5_pipeline0 *p,
+				  struct media_entity *me)
+{
+	struct v4l2_subdev *sd;
+	int i;
+
+	for (i = 0; i < IDX_MAX; i++)
+		p->subdevs[i] = NULL;
+
+	while (1) {
+		struct media_pad *pad = NULL;
+
+		/* Find remote source pad */
+		for (i = 0; i < me->num_pads; i++) {
+			struct media_pad *spad = &me->pads[i];
+			if (!(spad->flags & MEDIA_PAD_FL_SINK))
+				continue;
+			pad = media_entity_remote_source(spad);
+			if (pad)
+				break;
+		}
+
+		if (pad == NULL ||
+		    media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
+			break;
+		sd = media_entity_to_v4l2_subdev(pad->entity);
+
+		switch (sd->grp_id) {
+		case GRP_ID_FIMC_IS_SENSOR:
+		case GRP_ID_SENSOR:
+			p->subdevs[IDX_SENSOR] = sd;
+			break;
+		case GRP_ID_CSIS:
+			p->subdevs[IDX_CSIS] = sd;
+			break;
+		case GRP_ID_FLITE:
+			p->subdevs[IDX_FLITE] = sd;
+			break;
+		default:
+			pr_warn("%s: Unknown subdev grp_id: %#x\n",
+				__func__, sd->grp_id);
+		}
+		me = &sd->entity;
+		if (me->num_pads == 1)
+			break;
+	}
+}
+
+/**
+ * __subdev_set_power - change power state of a single subdev
+ * @sd: subdevice to change power state for
+ * @on: 1 to enable power or 0 to disable
+ *
+ * Return result of s_power subdev operation or -ENXIO if sd argument
+ * is NULL. Return 0 if the subdevice does not implement s_power.
+ */
+static int __subdev_set_power(struct v4l2_subdev *sd, int on)
+{
+	int *use_count;
+	int ret;
+
+	if (sd == NULL)
+		return -ENXIO;
+
+	use_count = &sd->entity.use_count;
+	if (on && (*use_count)++ > 0)
+		return 0;
+	else if (!on && (*use_count == 0 || --(*use_count) > 0))
+		return 0;
+	ret = v4l2_subdev_call(sd, core, s_power, on);
+
+	return ret != -ENOIOCTLCMD ? ret : 0;
+}
+
+/**
+ * fimc_pipeline_s_power - change power state of all pipeline subdevs
+ * @fimc: fimc device terminating the pipeline
+ * @state: true to power on, false to power off
+ *
+ * Needs to be called with the graph mutex held.
+ */
+static int fimc_pipeline_s_power(struct exynos5_pipeline0 *p, bool state)
+{
+	unsigned int i;
+	int ret;
+
+	if (p->subdevs[IDX_SENSOR] == NULL)
+		return -ENXIO;
+
+	for (i = 0; i < IDX_MAX; i++) {
+		unsigned int idx = state ? (IDX_MAX - 1) - i : i;
+
+		ret = __subdev_set_power(p->subdevs[idx], state);
+		if (ret < 0 && ret != -ENXIO)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * __fimc_pipeline_init - allocate the fimc_pipeline structure and do the
+ *                        basic initialization.
+ * @p: Pointer to fimc_pipeline structure
+ *
+ * Initializes the 'is_init' variable to indicate the pipeline structure
+ * is valid.
+ */
+static int __fimc_pipeline_init(struct fimc_pipeline *p)
+{
+	struct exynos5_pipeline0 *ep;
+
+	ep = kzalloc(sizeof(*ep), GFP_KERNEL);
+	if (!ep)
+		return -ENOMEM;
+
+	ep->is_init = true;
+	p->priv = (void *)ep;
+	return 0;
+}
+
+/**
+ * __fimc_pipeline_free - free the allocated resources for fimc_pipeline
+ * @p: Pointer to fimc_pipeline structure
+ *
+ * sets the 'is_init' variable to false, to indicate the pipeline structure
+ * is not valid.
+ *
+ * RETURNS:
+ * Zero in case of success and -EINVAL in case of failure.
+ */
+static int __fimc_pipeline_free(struct fimc_pipeline *p)
+{
+	struct exynos5_pipeline0 *ep0 = (struct exynos5_pipeline0 *)p->priv;
+
+	if (!ep0 || !ep0->is_init)
+		return -EINVAL;
+
+	ep0->is_init = false;
+	kfree(ep0);
+
+	return 0;
+}
+
+/**
+ * __fimc_pipeline_open - update the pipeline information, enable power
+ *                        of all pipeline subdevs and the sensor clock
+ * @me: media entity to start graph walk with
+ * @prepare: true to walk the current pipeline and acquire all subdevs
+ *
+ * Called with the graph mutex held.
+ */
+static int __fimc_pipeline_open(struct fimc_pipeline *p,
+				struct media_entity *me, bool prepare)
+{
+	struct exynos5_pipeline0 *ep = (struct exynos5_pipeline0 *)p->priv;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	if (WARN_ON(p == NULL || me == NULL))
+		return -EINVAL;
+
+	if (!ep || !ep->is_init)
+		return -EINVAL;
+
+	if (prepare)
+		fimc_pipeline_prepare(ep, me);
+
+	sd = ep->subdevs[IDX_SENSOR];
+	if (sd == NULL)
+		return -EINVAL;
+
+	ret = fimc_md_set_camclk(sd, true);
+	if (ret < 0)
+		return ret;
+
+	ret = fimc_pipeline_s_power(ep, 1);
+	if (!ret)
+		return 0;
+
+	fimc_md_set_camclk(sd, false);
+	return ret;
+}
+
+/**
+ * __fimc_pipeline_close - disable the sensor clock and pipeline power
+ * @fimc: fimc device terminating the pipeline
+ *
+ * Disable power of all subdevs and turn the external sensor clock off.
+ */
+static int __fimc_pipeline_close(struct fimc_pipeline *p)
+{
+	struct exynos5_pipeline0 *ep = (struct exynos5_pipeline0 *)p->priv;
+	struct v4l2_subdev *sd = ep ? ep->subdevs[IDX_SENSOR] : NULL;
+	int ret = 0;
+
+	if (WARN_ON(sd == NULL))
+		return -EINVAL;
+
+	if (ep->subdevs[IDX_SENSOR]) {
+		ret = fimc_pipeline_s_power(ep, 0);
+		fimc_md_set_camclk(sd, false);
+	}
+	return ret == -ENXIO ? 0 : ret;
+}
+
+/**
+ * __fimc_pipeline_s_stream - call s_stream() on pipeline subdevs
+ * @pipeline: video pipeline structure
+ * @on: passed as the s_stream() callback argument
+ */
+static int __fimc_pipeline_s_stream(struct fimc_pipeline *p, bool on)
+{
+	struct exynos5_pipeline0 *ep = (struct exynos5_pipeline0 *)p->priv;
+	int i, ret;
+
+	if (ep->subdevs[IDX_SENSOR] == NULL)
+		return -ENODEV;
+
+	for (i = 0; i < IDX_MAX; i++) {
+		unsigned int idx = on ? (IDX_MAX - 1) - i : i;
+
+		ret = v4l2_subdev_call(ep->subdevs[idx], video, s_stream, on);
+
+		if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
+			return ret;
+	}
+
+	return 0;
+
+}
+
+/**
+ * __fimc_pipeline_get_subdev_sensor - if valid pipeline, returns the sensor
+ *                                     subdev pointer else returns NULL
+ * @p: Pointer to fimc_pipeline structure
+ * @sd_idx: Index of the subdev associated with the current pipeline
+ *
+ * RETURNS:
+ * If success, returns valid subdev pointer or NULL in case of sudbev not found
+ */
+static struct v4l2_subdev *__fimc_pipeline_get_subdev(struct fimc_pipeline *p,
+					enum exynos_subdev_index sd_idx)
+{
+	struct exynos5_pipeline0 *ep = (struct exynos5_pipeline0 *)p->priv;
+	struct v4l2_subdev *sd;
+
+	if (!ep || !ep->is_init)
+		return NULL;
+
+	switch (sd_idx) {
+	case EXYNOS_SD_SENSOR:
+		sd = ep->subdevs[IDX_SENSOR];
+		break;
+	case EXYNOS_SD_CSIS:
+		sd = ep->subdevs[IDX_CSIS];
+		break;
+	case EXYNOS_SD_FLITE:
+		sd = ep->subdevs[IDX_FLITE];
+		break;
+	default:
+		sd = NULL;
+		break;
+	}
+	return sd;
+}
+
+static void __fimc_pipeline_register_notify_callback(
+		struct fimc_pipeline *p,
+		void (*notify_cb)(struct v4l2_subdev *sd,
+				unsigned int notification, void *arg))
+{
+	struct exynos5_pipeline0 *ep = (struct exynos5_pipeline0 *)p->priv;
+
+	if (!notify_cb)
+		return;
+
+	ep->sensor_notify = notify_cb;
+}
+
+/* Media pipeline operations for the FIMC/FIMC-LITE video device driver */
+static const struct fimc_pipeline_ops exynos5_pipeline0_ops = {
+	.init		= __fimc_pipeline_init,
+	.free		= __fimc_pipeline_free,
+	.open		= __fimc_pipeline_open,
+	.close		= __fimc_pipeline_close,
+	.set_stream	= __fimc_pipeline_s_stream,
+	.get_subdev	= __fimc_pipeline_get_subdev,
+	.register_notify_cb	= __fimc_pipeline_register_notify_callback,
+};
+
+/* Register I2C client subdev associated with @node. */
+static int fimc_md_of_add_sensor(struct fimc_md *fmd,
+				 struct device_node *node, int index)
+{
+	struct fimc_sensor_info *si;
+	struct i2c_client *client;
+	struct v4l2_subdev *sd;
+	int ret;
+
+	if (WARN_ON(index >= ARRAY_SIZE(fmd->sensor)))
+		return -EINVAL;
+	si = &fmd->sensor[index];
+
+	client = of_find_i2c_device_by_node(node);
+	if (!client)
+		return -EPROBE_DEFER;
+
+	device_lock(&client->dev);
+
+	if (!client->driver ||
+	    !try_module_get(client->driver->driver.owner)) {
+		ret = -EPROBE_DEFER;
+		v4l2_info(&fmd->v4l2_dev, "No driver found for %s\n",
+						node->full_name);
+		goto dev_put;
+	}
+
+	/* Enable sensor's master clock */
+	ret = __fimc_md_set_camclk(fmd, &si->pdata, true);
+	if (ret < 0)
+		goto mod_put;
+	sd = i2c_get_clientdata(client);
+
+	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
+	__fimc_md_set_camclk(fmd, &si->pdata, false);
+	if (ret < 0)
+		goto mod_put;
+
+	v4l2_set_subdev_hostdata(sd, &si->pdata);
+	sd->grp_id = GRP_ID_SENSOR;
+	si->subdev = sd;
+	v4l2_info(&fmd->v4l2_dev, "Registered sensor subdevice: %s (%d)\n",
+		  sd->name, fmd->num_sensors);
+	fmd->num_sensors++;
+
+mod_put:
+	module_put(client->driver->driver.owner);
+dev_put:
+	device_unlock(&client->dev);
+	put_device(&client->dev);
+	return ret;
+}
+
+/* Parse port node and register as a sub-device any sensor specified there. */
+static int fimc_md_parse_port_node(struct fimc_md *fmd,
+				   struct device_node *port,
+				   unsigned int index)
+{
+	struct device_node *rem, *ep;
+	struct fimc_source_info *pd;
+	struct v4l2_of_endpoint endpoint;
+	int ret;
+	u32 val;
+
+	pd = &fmd->sensor[index].pdata;
+
+	/* Assume here a port node can have only one endpoint node. */
+	ep = of_get_next_child(port, NULL);
+	if (!ep)
+		return 0;
+
+	v4l2_of_parse_endpoint(ep, &endpoint);
+	if (WARN_ON(endpoint.port == 0) || index >= FIMC_MAX_SENSORS)
+		return -EINVAL;
+
+	pd->mux_id = (endpoint.port - 1) & 0x1;
+
+	rem = v4l2_of_get_remote_port_parent(ep);
+	of_node_put(ep);
+	if (rem == NULL) {
+		v4l2_info(&fmd->v4l2_dev, "Remote device at %s not found\n",
+							ep->full_name);
+		return 0;
+	}
+	if (!of_property_read_u32(rem, "samsung,camclk-out", &val))
+		pd->clk_id = val;
+
+	if (!of_property_read_u32(rem, "clock-frequency", &val))
+		pd->clk_frequency = val;
+
+	if (pd->clk_frequency == 0) {
+		v4l2_err(&fmd->v4l2_dev, "Wrong clock frequency at node %s\n",
+			 rem->full_name);
+		of_node_put(rem);
+		return -EINVAL;
+	}
+
+	if (fimc_input_is_parallel(endpoint.port)) {
+		if (endpoint.bus_type == V4L2_MBUS_PARALLEL)
+			pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_601;
+		else
+			pd->sensor_bus_type = FIMC_BUS_TYPE_ITU_656;
+		pd->flags = endpoint.bus.parallel.flags;
+	} else if (fimc_input_is_mipi_csi(endpoint.port)) {
+		/*
+		 * MIPI CSI-2: only input mux selection and
+		 * the sensor's clock frequency is needed.
+		 */
+		pd->sensor_bus_type = FIMC_BUS_TYPE_MIPI_CSI2;
+	} else {
+		v4l2_err(&fmd->v4l2_dev, "Wrong port id (%u) at node %s\n",
+			 endpoint.port, rem->full_name);
+	}
+
+	ret = fimc_md_of_add_sensor(fmd, rem, index);
+	of_node_put(rem);
+
+	return ret;
+}
+
+/* Register all SoC external sub-devices */
+static int fimc_md_of_sensors_register(struct fimc_md *fmd,
+				       struct device_node *np)
+{
+	struct device_node *parent = fmd->pdev->dev.of_node;
+	struct device_node *node, *ports;
+	int index = 0;
+	int ret;
+
+	/* Attach sensors linked to MIPI CSI-2 receivers */
+	for_each_available_child_of_node(parent, node) {
+		struct device_node *port;
+
+		if (of_node_cmp(node->name, "csis"))
+			continue;
+		/* The csis node can have only port subnode. */
+		port = of_get_next_child(node, NULL);
+		if (!port)
+			continue;
+
+		ret = fimc_md_parse_port_node(fmd, port, index);
+		if (ret < 0)
+			return ret;
+		index++;
+	}
+
+	/* Attach sensors listed in the parallel-ports node */
+	ports = of_get_child_by_name(parent, "parallel-ports");
+	if (!ports)
+		return 0;
+
+	for_each_child_of_node(ports, node) {
+		ret = fimc_md_parse_port_node(fmd, node, index);
+		if (ret < 0)
+			break;
+		index++;
+	}
+
+	return 0;
+}
+
+static int __of_get_csis_id(struct device_node *np)
+{
+	u32 reg = 0;
+
+	np = of_get_child_by_name(np, "port");
+	if (!np)
+		return -EINVAL;
+	of_property_read_u32(np, "reg", &reg);
+	return reg - FIMC_INPUT_MIPI_CSI2_0;
+}
+
+/*
+ * MIPI-CSIS, FIMC and FIMC-LITE platform devices registration.
+ */
+
+static int register_fimc_lite_entity(struct fimc_md *fmd,
+				     struct fimc_lite *fimc_lite)
+{
+	struct v4l2_subdev *sd;
+	int ret;
+
+	if (WARN_ON(fimc_lite->index >= FIMC_LITE_MAX_DEVS ||
+		    fmd->fimc_lite[fimc_lite->index]))
+		return -EBUSY;
+
+	sd = &fimc_lite->subdev;
+	sd->grp_id = GRP_ID_FLITE;
+	v4l2_set_subdev_hostdata(sd, (void *)&exynos5_pipeline0_ops);
+
+	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
+	if (!ret)
+		fmd->fimc_lite[fimc_lite->index] = fimc_lite;
+	else
+		v4l2_err(&fmd->v4l2_dev, "Failed to register FIMC.LITE%d\n",
+			 fimc_lite->index);
+	return ret;
+}
+
+static int register_csis_entity(struct fimc_md *fmd,
+				struct platform_device *pdev,
+				struct v4l2_subdev *sd)
+{
+	struct device_node *node = pdev->dev.of_node;
+	int id, ret;
+
+	id = node ? __of_get_csis_id(node) : max(0, pdev->id);
+
+	if (WARN_ON(id < 0 || id >= CSIS_MAX_ENTITIES))
+		return -ENOENT;
+
+	if (WARN_ON(fmd->csis[id].sd))
+		return -EBUSY;
+
+	sd->grp_id = GRP_ID_CSIS;
+	ret = v4l2_device_register_subdev(&fmd->v4l2_dev, sd);
+	if (!ret)
+		fmd->csis[id].sd = sd;
+	else
+		v4l2_err(&fmd->v4l2_dev,
+			 "Failed to register MIPI-CSIS.%d (%d)\n", id, ret);
+	return ret;
+}
+
+static int fimc_md_register_platform_entity(struct fimc_md *fmd,
+					    struct platform_device *pdev,
+					    int plat_entity)
+{
+	struct device *dev = &pdev->dev;
+	int ret = -EPROBE_DEFER;
+	void *drvdata;
+
+	/* Lock to ensure dev->driver won't change. */
+	device_lock(dev);
+
+	if (!dev->driver || !try_module_get(dev->driver->owner))
+		goto dev_unlock;
+
+	drvdata = dev_get_drvdata(dev);
+	/* Some subdev didn't probe succesfully id drvdata is NULL */
+	if (drvdata) {
+		switch (plat_entity) {
+		case IDX_FLITE:
+			ret = register_fimc_lite_entity(fmd, drvdata);
+			break;
+		case IDX_CSIS:
+			ret = register_csis_entity(fmd, pdev, drvdata);
+			break;
+		default:
+			ret = -ENODEV;
+		}
+	}
+
+	module_put(dev->driver->owner);
+dev_unlock:
+	device_unlock(dev);
+	if (ret == -EPROBE_DEFER)
+		dev_info(&fmd->pdev->dev, "deferring %s device registration\n",
+			dev_name(dev));
+	else if (ret < 0)
+		dev_err(&fmd->pdev->dev, "%s device registration failed (%d)\n",
+			dev_name(dev), ret);
+	return ret;
+}
+
+/* Register FIMC, FIMC-LITE and CSIS media entities */
+static int fimc_md_register_of_platform_entities(struct fimc_md *fmd,
+						 struct device_node *parent)
+{
+	struct device_node *node;
+	int ret = 0;
+
+	for_each_available_child_of_node(parent, node) {
+		struct platform_device *pdev;
+		int plat_entity = -1;
+
+		pdev = of_find_device_by_node(node);
+		if (!pdev)
+			continue;
+
+		/* If driver of any entity isn't ready try all again later. */
+		if (!strcmp(node->name, CSIS_OF_NODE_NAME))
+			plat_entity = IDX_CSIS;
+		else if (!strcmp(node->name, FIMC_LITE_OF_NODE_NAME))
+			plat_entity = IDX_FLITE;
+
+		if (plat_entity >= 0)
+			ret = fimc_md_register_platform_entity(fmd, pdev,
+							plat_entity);
+		put_device(&pdev->dev);
+		if (ret < 0)
+			break;
+	}
+
+	return ret;
+}
+
+static void fimc_md_unregister_entities(struct fimc_md *fmd)
+{
+	int i;
+
+	for (i = 0; i < FIMC_LITE_MAX_DEVS; i++) {
+		if (fmd->fimc_lite[i] == NULL)
+			continue;
+		v4l2_device_unregister_subdev(&fmd->fimc_lite[i]->subdev);
+		fmd->fimc_lite[i]->pipeline_ops = NULL;
+		fmd->fimc_lite[i] = NULL;
+	}
+	for (i = 0; i < CSIS_MAX_ENTITIES; i++) {
+		if (fmd->csis[i].sd == NULL)
+			continue;
+		v4l2_device_unregister_subdev(fmd->csis[i].sd);
+		module_put(fmd->csis[i].sd->owner);
+		fmd->csis[i].sd = NULL;
+	}
+	for (i = 0; i < fmd->num_sensors; i++) {
+		if (fmd->sensor[i].subdev == NULL)
+			continue;
+		v4l2_device_unregister_subdev(fmd->sensor[i].subdev);
+		fmd->sensor[i].subdev = NULL;
+	}
+	v4l2_info(&fmd->v4l2_dev, "Unregistered all entities\n");
+}
+
+/**
+ * __fimc_md_create_fimc_links - create links to all FIMC entities
+ * @fmd: fimc media device
+ * @source: the source entity to create links to all fimc entities from
+ * @sensor: sensor subdev linked to FIMC[fimc_id] entity, may be null
+ * @pad: the source entity pad index
+ * @link_mask: bitmask of the fimc devices for which link should be enabled
+ */
+static int __fimc_md_create_fimc_sink_links(struct fimc_md *fmd,
+					    struct media_entity *source,
+					    struct v4l2_subdev *sensor,
+					    int pad, int link_mask)
+{
+	struct media_entity *sink;
+	unsigned int flags = 0;
+	int i, ret = 0;
+
+	for (i = 0; i < FIMC_LITE_MAX_DEVS; i++) {
+		if (!fmd->fimc_lite[i])
+			continue;
+
+		flags = ((1 << i) & link_mask) ? MEDIA_LNK_FL_ENABLED : 0;
+		sink = &fmd->fimc_lite[i]->subdev.entity;
+		ret = media_entity_create_link(source, pad, sink,
+					       FLITE_SD_PAD_SINK, flags);
+		if (ret)
+			return ret;
+
+		/* Notify FIMC-LITE subdev entity */
+		ret = media_entity_call(sink, link_setup, &sink->pads[0],
+					&source->pads[pad], flags);
+		if (ret)
+			break;
+
+		v4l2_info(&fmd->v4l2_dev, "created link [%s] %c> [%s]\n",
+			  source->name, flags ? '=' : '-', sink->name);
+	}
+	return 0;
+}
+
+/**
+ * fimc_md_create_links - create default links between registered entities
+ *
+ * Parallel interface sensor entities are connected directly to FIMC capture
+ * entities. The sensors using MIPI CSIS bus are connected through immutable
+ * link with CSI receiver entity specified by mux_id. Any registered CSIS
+ * entity has a link to each registered FIMC capture entity. Enabled links
+ * are created by default between each subsequent registered sensor and
+ * subsequent FIMC capture entity. The number of default active links is
+ * determined by the number of available sensors or FIMC entities,
+ * whichever is less.
+ */
+static int fimc_md_create_links(struct fimc_md *fmd)
+{
+	struct v4l2_subdev *csi_sensors[CSIS_MAX_ENTITIES] = { NULL };
+	struct v4l2_subdev *sensor, *csis;
+	struct fimc_source_info *pdata;
+	struct media_entity *source, *sink;
+	int i, pad, fimc_id = 0, ret = 0;
+	u32 flags, link_mask = 0;
+
+	for (i = 0; i < fmd->num_sensors; i++) {
+		if (fmd->sensor[i].subdev == NULL)
+			continue;
+
+		sensor = fmd->sensor[i].subdev;
+		pdata = v4l2_get_subdev_hostdata(sensor);
+		if (!pdata)
+			continue;
+
+		source = NULL;
+
+		switch (pdata->sensor_bus_type) {
+		case FIMC_BUS_TYPE_MIPI_CSI2:
+			if (WARN(pdata->mux_id >= CSIS_MAX_ENTITIES,
+				"Wrong CSI channel id: %d\n", pdata->mux_id))
+				return -EINVAL;
+
+			csis = fmd->csis[pdata->mux_id].sd;
+			if (WARN(csis == NULL,
+				 "MIPI-CSI interface specified "
+				 "but s5p-csis module is not loaded!\n"))
+				return -EINVAL;
+
+			pad = sensor->entity.num_pads - 1;
+			ret = media_entity_create_link(&sensor->entity, pad,
+					      &csis->entity, CSIS_PAD_SINK,
+					      MEDIA_LNK_FL_IMMUTABLE |
+					      MEDIA_LNK_FL_ENABLED);
+			if (ret)
+				return ret;
+
+			v4l2_info(&fmd->v4l2_dev, "created link [%s] => [%s]\n",
+				  sensor->entity.name, csis->entity.name);
+
+			source = NULL;
+			csi_sensors[pdata->mux_id] = sensor;
+			break;
+
+		case FIMC_BUS_TYPE_ITU_601...FIMC_BUS_TYPE_ITU_656:
+			source = &sensor->entity;
+			pad = 0;
+			break;
+
+		default:
+			v4l2_err(&fmd->v4l2_dev, "Wrong bus_type: %x\n",
+				 pdata->sensor_bus_type);
+			return -EINVAL;
+		}
+		if (source == NULL)
+			continue;
+
+		link_mask = 1 << fimc_id++;
+		ret = __fimc_md_create_fimc_sink_links(fmd, source, sensor,
+						       pad, link_mask);
+	}
+
+	for (i = 0; i < CSIS_MAX_ENTITIES; i++) {
+		if (fmd->csis[i].sd == NULL)
+			continue;
+
+		source = &fmd->csis[i].sd->entity;
+		pad = CSIS_PAD_SOURCE;
+		sensor = csi_sensors[i];
+
+		link_mask = 1 << fimc_id++;
+		ret = __fimc_md_create_fimc_sink_links(fmd, source, sensor,
+						       pad, link_mask);
+	}
+
+	/* Create immutable links b/w each FIMC-LITE's subdev and video node */
+	flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED;
+	for (i = 0; i < FIMC_LITE_MAX_DEVS; i++) {
+		if (!fmd->fimc_lite[i])
+			continue;
+		source = &fmd->fimc_lite[i]->subdev.entity;
+		sink = &fmd->fimc_lite[i]->vfd.entity;
+		ret = media_entity_create_link(source, FLITE_SD_PAD_SOURCE_DMA,
+					      sink, 0, flags);
+		if (ret)
+			break;
+	}
+
+	return 0;
+}
+
+/*
+ * The peripheral sensor clock management.
+ */
+static void fimc_md_put_clocks(struct fimc_md *fmd)
+{
+	int i = FIMC_MAX_CAMCLKS;
+
+	while (--i >= 0) {
+		if (IS_ERR(fmd->camclk[i].clock))
+			continue;
+		clk_unprepare(fmd->camclk[i].clock);
+		clk_put(fmd->camclk[i].clock);
+		fmd->camclk[i].clock = ERR_PTR(-EINVAL);
+	}
+}
+
+static int fimc_md_get_clocks(struct fimc_md *fmd)
+{
+	struct device *dev = NULL;
+	char clk_name[32];
+	struct clk *clock;
+	int ret, i;
+
+	for (i = 0; i < FIMC_MAX_CAMCLKS; i++)
+		fmd->camclk[i].clock = ERR_PTR(-EINVAL);
+
+	if (fmd->pdev->dev.of_node)
+		dev = &fmd->pdev->dev;
+
+	for (i = 0; i < FIMC_MAX_CAMCLKS; i++) {
+		snprintf(clk_name, sizeof(clk_name), "sclk_cam%u", i);
+		clock = clk_get(dev, clk_name);
+
+		if (IS_ERR(clock)) {
+			dev_err(&fmd->pdev->dev, "Failed to get clock: %s\n",
+								clk_name);
+			ret = PTR_ERR(clock);
+			break;
+		}
+		ret = clk_prepare(clock);
+		if (ret < 0) {
+			clk_put(clock);
+			fmd->camclk[i].clock = ERR_PTR(-EINVAL);
+			break;
+		}
+		fmd->camclk[i].clock = clock;
+	}
+	if (ret)
+		fimc_md_put_clocks(fmd);
+
+	return ret;
+}
+
+static int __fimc_md_set_camclk(struct fimc_md *fmd,
+				struct fimc_source_info *si,
+				bool on)
+{
+	struct fimc_camclk_info *camclk;
+	int ret = 0;
+
+	if (WARN_ON(si->clk_id >= FIMC_MAX_CAMCLKS) || fmd == NULL)
+		return -EINVAL;
+
+	camclk = &fmd->camclk[si->clk_id];
+
+	dbg("camclk %d, f: %lu, use_count: %d, on: %d",
+	    si->clk_id, si->clk_frequency, camclk->use_count, on);
+
+	if (on) {
+		if (camclk->use_count > 0 &&
+		    camclk->frequency != si->clk_frequency)
+			return -EINVAL;
+
+		if (camclk->use_count++ == 0) {
+			clk_set_rate(camclk->clock, si->clk_frequency);
+			camclk->frequency = si->clk_frequency;
+			ret = clk_enable(camclk->clock);
+			dbg("Enabled camclk %d: f: %lu", si->clk_id,
+			    clk_get_rate(camclk->clock));
+		}
+		return ret;
+	}
+
+	if (WARN_ON(camclk->use_count == 0))
+		return 0;
+
+	if (--camclk->use_count == 0) {
+		clk_disable(camclk->clock);
+		dbg("Disabled camclk %d", si->clk_id);
+	}
+	return ret;
+}
+
+/**
+ * fimc_md_set_camclk - peripheral sensor clock setup
+ * @sd: sensor subdev to configure sclk_cam clock for
+ * @on: 1 to enable or 0 to disable the clock
+ *
+ * There are 2 separate clock outputs available in the SoC for external
+ * image processors. These clocks are shared between all registered FIMC
+ * devices to which sensors can be attached, either directly or through
+ * the MIPI CSI receiver. The clock is allowed here to be used by
+ * multiple sensors concurrently if they use same frequency.
+ * This function should only be called when the graph mutex is held.
+ */
+static int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on)
+{
+	struct fimc_source_info *si = v4l2_get_subdev_hostdata(sd);
+	struct fimc_md *fmd = entity_to_fimc_mdev(&sd->entity);
+
+	return __fimc_md_set_camclk(fmd, si, 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_pipeline *pipeline;
+	struct exynos5_pipeline0 *ep;
+	struct v4l2_subdev *sd;
+	struct mutex *lock;
+	int ret = 0, i;
+	int ref_count;
+
+	if (media_entity_type(sink->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
+		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;
+		lock = &fimc_lite->lock;
+		break;
+	default:
+		return 0;
+	}
+
+	mutex_lock(lock);
+	ref_count = fimc_lite->ref_count;
+
+	ep = (struct exynos5_pipeline0 *)pipeline->priv;
+	if (!ep || !ep->is_init)
+		return -EINVAL;
+
+	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
+		if (ref_count > 0)
+			ret = __fimc_pipeline_close(pipeline);
+		for (i = 0; i < IDX_MAX; i++)
+			ep->subdevs[i] = NULL;
+	} else if (ref_count > 0) {
+		/*
+		 * Link activation. Enable power of pipeline elements only if
+		 * the pipeline is already in use, i.e. its video node is open.
+		 */
+		ret = __fimc_pipeline_open(pipeline,
+					   source->entity, true);
+	}
+
+	mutex_unlock(lock);
+	return ret ? -EPIPE : ret;
+}
+
+/**
+ * fimc_md_sensor_notify - v4l2_device notification from a sensor subdev
+ * @sd: pointer to a subdev generating the notification
+ * @notification: the notification type, must be S5P_FIMC_TX_END_NOTIFY
+ * @arg: pointer to an u32 type integer that stores the frame payload value
+ *
+ * Passes the sensor notification to the capture device
+ */
+static void fimc_md_sensor_notify(struct v4l2_subdev *sd,
+				unsigned int notification, void *arg)
+{
+	struct fimc_md *fmd;
+	struct exynos5_pipeline0 *ep;
+	struct fimc_pipeline *p;
+	unsigned long flags;
+
+	fmd = entity_to_fimc_mdev(&sd->entity);
+
+	if (sd == NULL)
+		return;
+
+	p = media_pipe_to_fimc_pipeline(sd->entity.pipe);
+	ep = (struct exynos5_pipeline0 *)p->priv;
+
+	spin_lock_irqsave(&fmd->slock, flags);
+
+	if (ep->sensor_notify)
+		ep->sensor_notify(sd, notification, arg);
+
+	spin_unlock_irqrestore(&fmd->slock, flags);
+}
+
+static int fimc_md_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct v4l2_device *v4l2_dev;
+	struct fimc_md *fmd;
+	int ret;
+
+	fmd = devm_kzalloc(dev, sizeof(*fmd), GFP_KERNEL);
+	if (!fmd)
+		return -ENOMEM;
+
+	spin_lock_init(&fmd->slock);
+	fmd->pdev = pdev;
+
+	strlcpy(fmd->media_dev.model, "SAMSUNG EXYNOS5 IS",
+		sizeof(fmd->media_dev.model));
+	fmd->media_dev.link_notify = fimc_md_link_notify;
+	fmd->media_dev.dev = dev;
+
+	v4l2_dev = &fmd->v4l2_dev;
+	v4l2_dev->mdev = &fmd->media_dev;
+	v4l2_dev->notify = fimc_md_sensor_notify;
+	strlcpy(v4l2_dev->name, "exynos5-fimc-md", sizeof(v4l2_dev->name));
+
+	ret = v4l2_device_register(dev, &fmd->v4l2_dev);
+	if (ret < 0) {
+		v4l2_err(v4l2_dev, "Failed to register v4l2_device: %d\n", ret);
+		return ret;
+	}
+	ret = media_device_register(&fmd->media_dev);
+	if (ret < 0) {
+		v4l2_err(v4l2_dev, "Failed to register media dev: %d\n", ret);
+		goto err_md;
+	}
+	ret = fimc_md_get_clocks(fmd);
+	if (ret)
+		goto err_clk;
+
+	/* Protect the media graph while we're registering entities */
+	mutex_lock(&fmd->media_dev.graph_mutex);
+
+	ret = fimc_md_register_of_platform_entities(fmd, dev->of_node);
+	if (ret)
+		goto err_unlock;
+
+	fmd->num_sensors = 0;
+	ret = fimc_md_of_sensors_register(fmd, dev->of_node);
+	if (ret)
+		goto err_unlock;
+
+	ret = fimc_md_create_links(fmd);
+	if (ret)
+		goto err_unlock;
+	ret = v4l2_device_register_subdev_nodes(&fmd->v4l2_dev);
+	if (ret)
+		goto err_unlock;
+
+	platform_set_drvdata(pdev, fmd);
+	mutex_unlock(&fmd->media_dev.graph_mutex);
+	return 0;
+
+err_unlock:
+	mutex_unlock(&fmd->media_dev.graph_mutex);
+err_clk:
+	media_device_unregister(&fmd->media_dev);
+	fimc_md_put_clocks(fmd);
+	fimc_md_unregister_entities(fmd);
+err_md:
+	v4l2_device_unregister(&fmd->v4l2_dev);
+	return ret;
+}
+
+static int fimc_md_remove(struct platform_device *pdev)
+{
+	struct fimc_md *fmd = platform_get_drvdata(pdev);
+
+	if (!fmd)
+		return 0;
+	fimc_md_unregister_entities(fmd);
+	media_device_unregister(&fmd->media_dev);
+	fimc_md_put_clocks(fmd);
+	return 0;
+}
+
+static struct platform_device_id fimc_driver_ids[] __always_unused = {
+	{ .name = "exynos5-fimc-md" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, fimc_driver_ids);
+
+static const struct of_device_id fimc_md_of_match[] = {
+	{ .compatible = "samsung,exynos5-fimc" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, fimc_md_of_match);
+
+static struct platform_driver fimc_md_driver = {
+	.probe		= fimc_md_probe,
+	.remove		= fimc_md_remove,
+	.driver = {
+		.of_match_table = fimc_md_of_match,
+		.name		= "exynos5-fimc-md",
+		.owner		= THIS_MODULE,
+	}
+};
+
+static int __init fimc_md_init(void)
+{
+	request_module("s5p-csis");
+	return platform_driver_register(&fimc_md_driver);
+}
+
+static void __exit fimc_md_exit(void)
+{
+	platform_driver_unregister(&fimc_md_driver);
+}
+
+module_init(fimc_md_init);
+module_exit(fimc_md_exit);
+
+MODULE_AUTHOR("Shaik Ameer Basha <shaik.ameer@samsung.com>");
+MODULE_DESCRIPTION("EXYNOS5 FIMC media device driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/platform/exynos5-is/exynos5-mdev.h b/drivers/media/platform/exynos5-is/exynos5-mdev.h
new file mode 100644
index 0000000..43621b6
--- /dev/null
+++ b/drivers/media/platform/exynos5-is/exynos5-mdev.h
@@ -0,0 +1,120 @@
+/*
+ * Copyright (C) 2011 - 2012 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef EXYNOS5_MDEVICE_H_
+#define EXYNOS5_MDEVICE_H_
+
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <media/media-device.h>
+#include <media/media-entity.h>
+#include <media/s5p_fimc.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+#include "fimc-lite.h"
+#include "mipi-csis.h"
+
+#define FIMC_OF_NODE_NAME	"fimc"
+#define FIMC_LITE_OF_NODE_NAME	"fimc-lite"
+#define CSIS_OF_NODE_NAME	"csis"
+#define FIMC_IS_OF_NODE_NAME	"fimc-is"
+
+/* Group IDs of sensor, MIPI-CSIS, FIMC-LITE and the writeback subdevs. */
+#define GRP_ID_SENSOR		(1 << 8)
+#define GRP_ID_FIMC_IS_SENSOR	(1 << 9)
+#define GRP_ID_WRITEBACK	(1 << 10)
+#define GRP_ID_CSIS		(1 << 11)
+#define GRP_ID_FLITE		(1 << 13)
+#define GRP_ID_FIMC_IS		(1 << 14)
+
+#define FIMC_MAX_SENSORS	8
+#define FIMC_MAX_CAMCLKS	2
+
+enum fimc_subdev_index {
+	IDX_SENSOR,
+	IDX_CSIS,
+	IDX_FLITE,
+	IDX_FIMC,
+	IDX_MAX,
+};
+
+struct fimc_csis_info {
+	struct v4l2_subdev *sd;
+	int id;
+};
+
+struct fimc_camclk_info {
+	struct clk *clock;
+	int use_count;
+	unsigned long frequency;
+};
+
+/**
+ * struct fimc_sensor_info - image data source subdev information
+ * @pdata: sensor's atrributes passed as media device's platform data
+ * @subdev: image sensor v4l2 subdev
+ * @host: fimc device the sensor is currently linked to
+ *
+ * This data structure applies to image sensor and the writeback subdevs.
+ */
+struct fimc_sensor_info {
+	struct fimc_source_info pdata;
+	struct v4l2_subdev *subdev;
+	struct fimc_dev *host;
+};
+
+/**
+ * struct fimc_md - fimc media device information
+ * @csis: MIPI CSIS subdevs data
+ * @sensor: array of registered sensor subdevs
+ * @num_sensors: actual number of registered sensors
+ * @camclk: external sensor clock information
+ * @fimc: array of registered fimc devices
+ * @media_dev: top level media device
+ * @v4l2_dev: top level v4l2_device holding up the subdevs
+ * @pdev: platform device this media device is hooked up into
+ * @user_subdev_api: true if subdevs are not configured by the host driver
+ * @slock: spinlock protecting @sensor array
+ */
+struct fimc_md {
+	struct fimc_csis_info csis[CSIS_MAX_ENTITIES];
+	struct fimc_sensor_info sensor[FIMC_MAX_SENSORS];
+	int num_sensors;
+	struct fimc_camclk_info camclk[FIMC_MAX_CAMCLKS];
+	struct fimc_lite *fimc_lite[FIMC_LITE_MAX_DEVS];
+	struct media_device media_dev;
+	struct v4l2_device v4l2_dev;
+	struct platform_device *pdev;
+	spinlock_t slock;
+};
+
+struct exynos5_pipeline0 {
+	int is_init;
+	struct fimc_md *fmd;
+	struct v4l2_subdev *subdevs[IDX_MAX];
+	void (*sensor_notify)(struct v4l2_subdev *sd,
+			unsigned int notification, void *arg);
+};
+
+#define is_subdev_pad(pad) (pad == NULL || \
+	media_entity_type(pad->entity) == MEDIA_ENT_T_V4L2_SUBDEV)
+
+#define me_subtype(me) \
+	((me->type) & (MEDIA_ENT_TYPE_MASK | MEDIA_ENT_SUBTYPE_MASK))
+
+#define subdev_has_devnode(__sd) (__sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)
+
+static inline struct fimc_md *entity_to_fimc_mdev(struct media_entity *me)
+{
+	return me->parent == NULL ? NULL :
+		container_of(me->parent, struct fimc_md, media_dev);
+}
+
+#endif /* EXYNOS5_MDEVICE_H_ */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC v2 1/6] media: exynos4-is: modify existing mdev to use common pipeline
  2013-04-24  7:41 ` [RFC v2 1/6] media: exynos4-is: modify existing mdev to use common pipeline Shaik Ameer Basha
@ 2013-04-29 11:24   ` Sylwester Nawrocki
  0 siblings, 0 replies; 12+ messages in thread
From: Sylwester Nawrocki @ 2013-04-29 11:24 UTC (permalink / raw)
  To: Shaik Ameer Basha
  Cc: linux-media, devicetree-discuss, linux-samsung-soc, shaik.samsung,
	arunkk.samsung

Hi Shaik,

Thanks for the updated patch series.

On 04/24/2013 09:41 AM, Shaik Ameer Basha wrote:
> Current fimc_pipeline is tightly coupled with exynos4-is media
> device driver. And this will not allow to use the same pipeline
> across different exynos series media device drivers.
> 
> This patch adds,
> 1] Changing of the existing pipeline as a common pipeline
>    to be used across multiple exynos series media device drivers.
> 2] Modifies the existing exynos4-is media device driver to
>    use the updated common pipeline implementation.
> 
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  drivers/media/platform/exynos4-is/fimc-capture.c |   47 ++++--
>  drivers/media/platform/exynos4-is/fimc-lite.c    |    4 +-
>  drivers/media/platform/exynos4-is/media-dev.c    |  179 +++++++++++++++++++---
>  drivers/media/platform/exynos4-is/media-dev.h    |   16 ++
>  include/media/s5p_fimc.h                         |   46 ++++--
>  5 files changed, 248 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c
> index 72c516a..904d725 100644
> --- a/drivers/media/platform/exynos4-is/fimc-capture.c
> +++ b/drivers/media/platform/exynos4-is/fimc-capture.c
[...]
> @@ -1830,6 +1847,9 @@ static int fimc_capture_subdev_registered(struct v4l2_subdev *sd)
>   		return ret;
>
>   	fimc->pipeline_ops = v4l2_get_subdev_hostdata(sd);
> +	fimc->pipeline_ops->init(&fimc->pipeline);
> +	fimc->pipeline_ops->register_notify_cb(&fimc->pipeline,
> +						fimc_sensor_notify);

Why is this needed ? The 'notify' callback belongs to struct v4l2_device,
which is on same level in the V4L2 API as struct media_device in the MC API.

Once a v4l2 subdev is registered it gets its v4l2_dev field set to its parent
v4l2 device and can send notification, e.g. by using the v4l2_subdev_notify() 
macro.
In exynos4-is the media device driver then routes such notification to the FIMC
capture driver, depending on the media links configuration.

I know, the current code upstream needs to get fixed to work as described above
also after links reconfiguration at run time.

Still it's not clear to, me why you need this new register_notify_cb callback ?

>   	ret = fimc_register_capture_device(fimc, sd->v4l2_dev);
>   	if (ret) {
> @@ -1852,6 +1872,7 @@ static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd)
>   	if (video_is_registered(&fimc->vid_cap.vfd)) {
>   		video_unregister_device(&fimc->vid_cap.vfd);
>   		media_entity_cleanup(&fimc->vid_cap.vfd.entity);
> +		fimc->pipeline_ops->free(&fimc->pipeline);
>   		fimc->pipeline_ops = NULL;
>   	}
>   	kfree(fimc->vid_cap.ctx);

> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 1dbd554..c80633a 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -36,6 +36,8 @@
>   #include "fimc-lite.h"
>   #include "mipi-csis.h"
>
> +static struct exynos4_pipeline exynos4_pl;
> +
[...]
> +static int __fimc_pipeline_init(struct fimc_pipeline *p)
> +{
> +	struct exynos4_pipeline *ep =&exynos4_pl;
> +
> +	if (ep->is_init)
> +		return 0;
> +
> +	ep->is_init = true;
> +	p->priv = (void *)ep;

This will not work since there can be more than one video pipeline operating
simultaneously, and a static variable is used here.

I think we could ditch those fimc_pipeline_init/free ops by designing properly
data structures describing video pipelines. Please see below for more details.

> +	return 0;
> +}
> +
> +/**
> + * __fimc_pipeline_free - free the allocated resources for fimc_pipeline
> + * @p: Pointer to fimc_pipeline structure
> + *
> + * sets the 'is_init' variable to false, to indicate the pipeline structure
> + * is not valid.
> + *
> + * RETURNS:
> + * Zero in case of success and -EINVAL in case of failure.

nit: According to Documentation the correct format is:

+ Return: Zero in case of success and -EINVAL in case of failure.

> + */
> +static int __fimc_pipeline_free(struct fimc_pipeline *p)
> +{
> +	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
> +
> +	if (!ep || !ep->is_init)
> +		return -EINVAL;
> +
> +	ep->is_init = false;

I find it hard to see justification for existence of this function.

> +	return 0;
> +}

> +/**
> + * __fimc_pipeline_get_subdev_sensor - if valid pipeline, returns the sensor
> + *                                     subdev pointer else returns NULL
> + * @p: Pointer to fimc_pipeline structure
> + * @sd_idx: Index of the subdev associated with the current pipeline
> + *
> + * RETURNS:
> + * If success, returns valid subdev pointer or NULL in case of sudbev not found
> + */
> +static struct v4l2_subdev *__fimc_pipeline_get_subdev(struct fimc_pipeline *p,
> +					enum exynos_subdev_index sd_idx)
> +{
> +	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
> +	struct v4l2_subdev *sd;
> +
> +	if (!ep || !ep->is_init)
> +		return NULL;
> +
> +	switch (sd_idx) {
> +	case EXYNOS_SD_SENSOR:
> +		sd = ep->subdevs[IDX_SENSOR];
> +		break;
> +	case EXYNOS_SD_CSIS:
> +		sd = ep->subdevs[IDX_CSIS];
> +		break;
> +	case EXYNOS_SD_FLITE:
> +		sd = ep->subdevs[IDX_FLITE];
> +		break;
> +	case EXYNOS_SD_IS_ISP:
> +		sd = ep->subdevs[IDX_IS_ISP];
> +		break;
> +	case EXYNOS_SD_FIMC:
> +		sd = ep->subdevs[IDX_FIMC];
> +		break;

There is no need for new EXYNOS_SD_* enumeration, subdev group ids could be 
used instead. And for exynos4-is I don't want such get_subdev callback. Only
one entity would be using it (FIMC video capture device). I prefer to make 
enum fimc_subdev_index local to the exynos4-is driver and use IDX_* there 
directly. Until such usage is replaced by more generic code - not a driver 
specific data pipeline callback.

Also, with so many elements in the array using switch looks pretty ugly 
to me. Iterating over the array and comparing against subdev group id might
be a better idea, e.g.

	unsigned int i;
	for (i = 0; i < IDX_MAX; i++) {
		if (ep->subdevs[i] && ep->subdevs[i]->grp_id == requested_grp_id)
			return ep->subdevs[i];
	}
	return NULL;	

> +	default:
> +		sd = NULL;
> +		break;
> +	}
> +	return sd;
> +}
> +
> +static void __fimc_pipeline_register_notify_callback(
> +		struct fimc_pipeline *p,
> +		void (*notify_cb)(struct v4l2_subdev *sd,
> +					unsigned int notification, void *arg))
> +{
> +	struct exynos4_pipeline *ep = (struct exynos4_pipeline *)p->priv;
> +
> +	if (!notify_cb)
> +		return;
> +
> +	ep->sensor_notify = notify_cb;
> +}

This really seems redundant code to me. Could you explain a bit what is the
purpose of this ?

> +
>   /* Media pipeline operations for the FIMC/FIMC-LITE video device driver */
>   static const struct fimc_pipeline_ops fimc_pipeline_ops = {
> +	.init		= __fimc_pipeline_init,
> +	.free		= __fimc_pipeline_free,
>   	.open		= __fimc_pipeline_open,
>   	.close		= __fimc_pipeline_close,
>   	.set_stream	= __fimc_pipeline_s_stream,
> +	.get_subdev	= __fimc_pipeline_get_subdev,
> +	.register_notify_cb     = __fimc_pipeline_register_notify_callback,
>   };

> +/**
> + * fimc_md_sensor_notify - v4l2_device notification from a sensor subdev
> + * @sd: pointer to a subdev generating the notification
> + * @notification: the notification type, must be S5P_FIMC_TX_END_NOTIFY
> + * @arg: pointer to an u32 type integer that stores the frame payload value
> + *
> + * Passes the sensor notification to the capture device
> + */
> +void fimc_md_sensor_notify(struct v4l2_subdev *sd,
> +				unsigned int notification, void *arg)
> +{
> +	struct fimc_md *fmd;
> +	struct exynos4_pipeline *ep;
> +	struct fimc_pipeline *p;
> +	unsigned long flags;
> +
> +	fmd = entity_to_fimc_mdev(&sd->entity);
> +
> +	if (sd == NULL)
> +		return;
> +
> +	p = media_pipe_to_fimc_pipeline(sd->entity.pipe);

When video streaming is off (before VIDIOC_STREAMON and after VIDIOC_STREAMOFF
ioctl) sd->entity.pipe will be NULL. So this code is not safe.

> +	ep = (struct exynos4_pipeline *)p->priv;
> +
> +	spin_lock_irqsave(&fmd->slock, flags);
> +
> +	if (ep->sensor_notify)
> +		ep->sensor_notify(sd, notification, arg);
> +
> +	spin_unlock_irqrestore(&fmd->slock, flags);
> +}

Creating the notification handler function in this file might be a good idea,
I remember having something like this done for some experiments. But I'm not
happy with introducing yet another callback for it in the pipeline struct.
The pipeline callbacks are supposed to be called by the video node drivers
and implemented by the media driver, not the other way around.

Actually what is the problem you tried to solve by introducing this code ?

> @@ -1375,7 +1518,7 @@ static int fimc_md_probe(struct platform_device *pdev)
>
>   	v4l2_dev =&fmd->v4l2_dev;
>   	v4l2_dev->mdev =&fmd->media_dev;
> -	v4l2_dev->notify = fimc_sensor_notify;
> +	v4l2_dev->notify = fimc_md_sensor_notify;
>   	strlcpy(v4l2_dev->name, "s5p-fimc-md", sizeof(v4l2_dev->name));
>
>   	fmd->use_isp = fimc_md_is_isp_available(dev->of_node);
> diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
> index 44d86b6..7ee8dd8 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.h
> +++ b/drivers/media/platform/exynos4-is/media-dev.h
> @@ -40,6 +40,15 @@ enum {
>   	FIMC_MAX_WBCLKS
>   };
>
> +enum exynos4_subdev_index {
> +	IDX_SENSOR,
> +	IDX_CSIS,
> +	IDX_FLITE,
> +	IDX_IS_ISP,
> +	IDX_FIMC,
> +	IDX_MAX,
> +};
>   struct fimc_csis_info {
>   	struct v4l2_subdev *sd;
>   	int id;
> @@ -107,6 +116,13 @@ struct fimc_md {
>   	spinlock_t slock;
>   };
>
> +struct exynos4_pipeline {
> +	int is_init;
> +	struct v4l2_subdev *subdevs[IDX_MAX];
> +	void (*sensor_notify)(struct v4l2_subdev *sd,
> +			unsigned int notification, void *arg);
> +};

> diff --git a/include/media/s5p_fimc.h b/include/media/s5p_fimc.h
> index f509690..2908a02 100644
> --- a/include/media/s5p_fimc.h
> +++ b/include/media/s5p_fimc.h
> @@ -105,6 +105,24 @@ struct s5p_platform_fimc {
>    */
>   #define S5P_FIMC_TX_END_NOTIFY _IO('e', 0)
>
> +/* Group IDs of sensor, MIPI-CSIS, FIMC-LITE and the writeback subdevs. */
> +#define GRP_ID_SENSOR		(1<<  8)
> +#define GRP_ID_FIMC_IS_SENSOR	(1<<  9)
> +#define GRP_ID_WRITEBACK	(1<<  10)
> +#define GRP_ID_CSIS		(1<<  11)
> +#define GRP_ID_FIMC		(1<<  12)
> +#define GRP_ID_FLITE		(1<<  13)
> +#define GRP_ID_FIMC_IS		(1<<  14)

These definitions are already in this file, I made recently a patch moving 
those from media-dev.h. So this chunk must not be repeated here.

> +enum exynos_subdev_index {
> +	EXYNOS_SD_SENSOR,
> +	EXYNOS_SD_CSIS,
> +	EXYNOS_SD_FLITE,
> +	EXYNOS_SD_IS_ISP,
> +	EXYNOS_SD_FIMC,
> +	EXYNOS_SD_MAX,

So we now have 3 different types of the subdev identifiers: GRP_ID_*, EXYNOS_SD_*, 
IDX_* ? As I mentioned above subdev group ids could be used instead of this 
new enumeration.

I think we could just rename GRP_ID_* constants that are already in this file 
to EXYNOS_SD_GROUP_ID_* and use it. Probably there is even no need for renaming.

>   /**
> @@ -140,21 +158,12 @@ 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,
> -};

We probably need to create new header file, include/media/exynos_is.h, which
would hold all common definitions for the exynos4-is and exynos5-is driver.

I intend to rename s5p_fimc.h to exynos4_is.h, once most of board files using
it are dropped, presumably for 3.11.

>   struct fimc_pipeline {
> -	struct v4l2_subdev *subdevs[IDX_MAX];
> -	struct media_pipeline *m_pipeline;
> +	struct media_pipeline m_pipeline;
> +	void *priv;

Since there is already going to be plenty of different structures describing
video pipeline, i.e.:

 - struct media_pipeline
 - struct fimc_pipeline
 - struct exynos4_pipeline
 - struct exynos5_pipeline

I think we could have specific pipeline structures "inherited" from struct
media_pipeline. I though initially about using only struct media_pipeline
and a exynos4/exynos5 pipeline structure, e.g.

struct exynos4_media_pipeline {
	struct media_pipeline mp;
	...
};

struct exynos5_media_pipeline {
	struct media_pipeline mp;
	...
};

And have entities passing around and using pointers to struct media_pipeline.
So the ops would have become something like:

struct exynos_pipeline_ops {
	int (*open)(struct media_pipeline *p, struct media_entity *me,
					bool resume);
	int (*close)(struct media_pipeline *p);
	int (*set_stream)(struct media_pipeline *p, bool state);
	...
};

But if we added struct exynos_pipeline_ops * to struct exynos_pipeline, which
is needed to handle link reconfiguration events when multiple subdevs/video
nodes are involved in a single video pipeline, some shared subdevs (FIMC-LITE)
would need to dereference more specific pipeline struct than struct 
media_pipeline to access the pipeline ops. So those shared subdevs would need
a common pipeline data structure type, since we wanted them to perform some
operations on a pipeline struct, but still allow a specific media device
driver to define its pipeline object type, inherited from base type.

So would have something like below, which becomes a bit uglier, but is likely
no different from performance POV. Since struct media_pipeline is an empty
data structure, and 'ep' is first field of struct exynos*_media_pipeline.

struct exynos_media_pipeline {
	struct media_pipeline mp;
	struct exynos_pipeline_ops *ops;
};


struct exynos4_media_pipeline {
	struct exynos_media_pipeline ep;
	...
};

struct exynos5_media_pipeline {
	struct exynos_media_pipeline ep;
	...
};

The shared subdevs would (and should) in general not be aware of details 
of the the pipeline structure specific to a given media device driver.

Would then still init/free ops be necessary ?

BTW, I would really like to limit usage of struct fimc_pipeline to minimum,
i.e. use generic graph/media entity operations where possible.


>   };
>
>   /*
> @@ -163,14 +172,27 @@ struct fimc_pipeline {
>    * by corresponding media device driver.
>    */
>   struct fimc_pipeline_ops {
> +	int (*init) (struct fimc_pipeline *p);
> +	int (*free) (struct fimc_pipeline *p);
>   	int (*open)(struct fimc_pipeline *p, struct media_entity *me,
> -			  bool resume);
> +					bool resume);
>   	int (*close)(struct fimc_pipeline *p);
>   	int (*set_stream)(struct fimc_pipeline *p, bool state);
> +	struct v4l2_subdev *(*get_subdev)(struct fimc_pipeline *p,
> +					enum exynos_subdev_index sd_idx);
> +	void (*register_notify_cb)(struct fimc_pipeline *p,
> +					void (*cb)(struct v4l2_subdev *sd,
> +					unsigned int notification, void *arg));
>   };
>
>   #define fimc_pipeline_call(f, op, p, args...)				\
>   	(!(f) ? -ENODEV : (((f)->pipeline_ops&&  (f)->pipeline_ops->op) ? \
>   			    (f)->pipeline_ops->op((p), ##args) : -ENOIOCTLCMD))
>
> +#define fimc_pipeline_get_subdev(ops, p, idx_subdev)			\
> +	((ops&&  ops->get_subdev) ? ops->get_subdev(p, idx_subdev) : NULL)

I'm not sure it belongs here. The shared subdevs should use generic APIs
to deal with media entities. And the not shared ones could make assumptions 
about their media device specific pipeline data structure.

Regards,
Sylwester

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v2 2/6] fimc-lite: Adding Exynos5 compatibility to fimc-lite driver
  2013-04-24  7:41 ` [RFC v2 2/6] fimc-lite: Adding Exynos5 compatibility to fimc-lite driver Shaik Ameer Basha
@ 2013-04-29 12:08   ` Sylwester Nawrocki
  0 siblings, 0 replies; 12+ messages in thread
From: Sylwester Nawrocki @ 2013-04-29 12:08 UTC (permalink / raw)
  To: Shaik Ameer Basha
  Cc: linux-media, devicetree-discuss, linux-samsung-soc, shaik.samsung,
	arunkk.samsung

On 04/24/2013 09:41 AM, Shaik Ameer Basha wrote:
> This patch adds,
> 1] Exynos5 soc compatibility to the fimc-lite driver
> 2] Multiple dma output buffer support as from Exynos5 onwards,
>    fimc-lite h/w ip supports multiple dma buffers.
> 
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  drivers/media/platform/exynos4-is/fimc-lite.c |   19 ++++++++++++++++++-
>  drivers/media/platform/exynos4-is/fimc-lite.h |    4 +++-
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
> index 4878089..cb173ec 100644
> --- a/drivers/media/platform/exynos4-is/fimc-lite.c
> +++ b/drivers/media/platform/exynos4-is/fimc-lite.c
> @@ -1467,7 +1467,7 @@ static int fimc_lite_probe(struct platform_device *pdev)
>  		fimc->index = pdev->id;
>  	}
>  
> -	if (!drv_data || fimc->index < 0 || fimc->index >= FIMC_LITE_MAX_DEVS)
> +	if (!drv_data || fimc->index < 0 || fimc->index >= drv_data->num_devs)
>  		return -EINVAL;
>  
>  	fimc->dd = drv_data;
> @@ -1625,6 +1625,19 @@ static struct flite_drvdata fimc_lite_drvdata_exynos4 = {
>  	.out_width_align	= 8,
>  	.win_hor_offs_align	= 2,
>  	.out_hor_offs_align	= 8,
> +	.support_multi_dma_buf	= false,
> +	.num_devs = 2,
> +};

Could you include this in your patch:

/**
 * struct flite_drvdata - FIMC-LITE IP variant data structure
 * @max_width: maximum camera interface input width in pixels
 * @max_height: maximum camera interface input height in pixels
 * @out_width_align: minimum output width alignment in pixels
 * @win_hor_offs_align: minimum camera interface crop window horizontal
 * 			offset alignment in pixels
 * @out_hor_offs_align: minimum output DMA compose rectangle horizontal
 * 			offset alignment in pixels
 * @num_out_dma_bufs: number of output DMA buffer start address registers
 */
?
> +/* EXYNOS5250 */
> +static struct flite_drvdata fimc_lite_drvdata_exynos5 = {
> +	.max_width		= 8192,
> +	.max_height		= 8192,
> +	.out_width_align	= 8,
> +	.win_hor_offs_align	= 2,
> +	.out_hor_offs_align	= 8,
> +	.support_multi_dma_buf	= true,

How about changing it to 'num_out_dma_bufs' ? And instead of

if (dd->support_multi_dma_buf)

having

if (dd->num_out_dma_bufs > 1)

?

Otherwise the patch looks good to me. I will make a separate patch
to update the binding documentation.

I've found there is a mistake in the compatible property's documentation.
I would like to fix it in the 3.10-rc cycle, I'm not 100% sure if it
qualifies as the -rc material, but having kernel released with wrongly
documented compatible property would have been bad I think.


Thanks,
Sylwester

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v2 3/6] media: fimc-lite: Adding support for Exynos5
  2013-04-24  7:41 ` [RFC v2 3/6] media: fimc-lite: Adding support for Exynos5 Shaik Ameer Basha
@ 2013-04-29 15:13   ` Sylwester Nawrocki
  0 siblings, 0 replies; 12+ messages in thread
From: Sylwester Nawrocki @ 2013-04-29 15:13 UTC (permalink / raw)
  To: Shaik Ameer Basha
  Cc: linux-media, devicetree-discuss, linux-samsung-soc, shaik.samsung,
	arunkk.samsung

On 04/24/2013 09:41 AM, Shaik Ameer Basha wrote:
> FIMC-LITE supports multiple DMA shadow registers from Exynos5 onwards.
> This patch adds the functionality of using shadow registers by
> checking the driver data.
> 
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  drivers/media/platform/exynos4-is/fimc-lite-reg.c |   13 +++++++
>  drivers/media/platform/exynos4-is/fimc-lite-reg.h |   41 ++++++++++++++++++++-
>  drivers/media/platform/exynos4-is/fimc-lite.c     |   12 ++++--
>  3 files changed, 60 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.c b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
> index 8cc0d39..a1d566a 100644
> --- a/drivers/media/platform/exynos4-is/fimc-lite-reg.c
> +++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
> @@ -215,6 +215,18 @@ void flite_hw_set_camera_bus(struct fimc_lite *dev,
>  	flite_hw_set_camera_port(dev, si->mux_id);
>  }
>  
> +static void flite_hw_set_pack12(struct fimc_lite *dev, int on)
> +{
> +	u32 cfg = readl(dev->regs + FLITE_REG_CIODMAFMT);
> +
> +	cfg &= ~FLITE_REG_CIODMAFMT_PACK12;
> +
> +	if (on)
> +		cfg |= FLITE_REG_CIODMAFMT_PACK12;
> +
> +	writel(cfg, dev->regs + FLITE_REG_CIODMAFMT);
> +}
> +
>  static void flite_hw_set_out_order(struct fimc_lite *dev, struct flite_frame *f)
>  {
>  	static const u32 pixcode[4][2] = {
> @@ -267,6 +279,7 @@ void flite_hw_set_output_dma(struct fimc_lite *dev, struct flite_frame *f,
>  
>  	flite_hw_set_out_order(dev, f);
>  	flite_hw_set_dma_window(dev, f);
> +	flite_hw_set_pack12(dev, 0);

Are you planning to use this function with the second argument's different
value than 0 ? FLITE_REG_CIODMAFMT_PACK12 default value after reset is 0. 
I'm not opposed to keeping this code, just was wondering if it is not needed 
only when support for some packed media bus formats is added and we actually
set the PACK12 bit to 1 ?

>  }
>  
>  void flite_hw_dump_regs(struct fimc_lite *dev, const char *label)
> diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.h b/drivers/media/platform/exynos4-is/fimc-lite-reg.h
> index 3903839..8e57e7a 100644
> --- a/drivers/media/platform/exynos4-is/fimc-lite-reg.h
> +++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.h
> @@ -120,6 +120,10 @@
>  /* b0: 1 - camera B, 0 - camera A */
>  #define FLITE_REG_CIGENERAL_CAM_B		(1 << 0)
>  
> +
> +#define FLITE_REG_CIFCNTSEQ			0x100
> +#define FLITE_REG_CIOSAN(x)			(0x200 + (4 * (x)))
> +
>  /* ----------------------------------------------------------------------------
>   * Function declarations
>   */
> @@ -143,8 +147,41 @@ void flite_hw_set_dma_window(struct fimc_lite *dev, struct flite_frame *f);
>  void flite_hw_set_test_pattern(struct fimc_lite *dev, bool on);
>  void flite_hw_dump_regs(struct fimc_lite *dev, const char *label);
>  
> -static inline void flite_hw_set_output_addr(struct fimc_lite *dev, u32 paddr)
> +static inline void flite_hw_set_output_addr(struct fimc_lite *dev,
> +	u32 paddr, u32 index)
> +{
> +	u32 config;
> +
> +	/* FLITE in EXYNOS4 has only one DMA register */
> +	if (!dev->dd->support_multi_dma_buf)
> +		index = 0;
> +
> +	config = readl(dev->regs + FLITE_REG_CIFCNTSEQ);
> +	config |= 1 << index;

nit: Could be also written more explicitly:

config |= BIT(index);

(needs #include <linux/bitops.h>)

> +static inline void flite_hw_set_output_addr(struct fimc_lite *dev,
> +	u32 paddr, u32 index)

> +	writel(config, dev->regs + FLITE_REG_CIFCNTSEQ);
> +
> +	if (index == 0)
> +		writel(paddr, dev->regs + FLITE_REG_CIOSA);
> +	else
> +		writel(paddr, dev->regs + FLITE_REG_CIOSAN(index-1));
> +}
> +
> +static inline void flite_hw_clear_output_addr(struct fimc_lite *dev, u32 index)
>  {
> -	writel(paddr, dev->regs + FLITE_REG_CIOSA);
> +	u32 config;
> +
> +	/* FLITE in EXYNOS4 has only one DMA register */
> +	if (!dev->dd->support_multi_dma_buf)
> +		index = 0;
> +
> +	config = readl(dev->regs + FLITE_REG_CIFCNTSEQ);
> +	config &= ~(1 << index);
> +	writel(config, dev->regs + FLITE_REG_CIFCNTSEQ);
>  }
> +
> +static inline void flite_hw_clear_output_index(struct fimc_lite *dev)
> +{
> +	writel(0, dev->regs + FLITE_REG_CIFCNTSEQ);
> +}
> +
>  #endif /* FIMC_LITE_REG_H */
> diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
> index cb173ec..1b12ea8 100644
> --- a/drivers/media/platform/exynos4-is/fimc-lite.c
> +++ b/drivers/media/platform/exynos4-is/fimc-lite.c
> @@ -166,6 +166,8 @@ static int fimc_lite_hw_init(struct fimc_lite *fimc, bool isp_output)
>  	if (fimc->inp_frame.fmt == NULL || fimc->out_frame.fmt == NULL)
>  		return -EINVAL;
>  
> +	flite_hw_clear_output_index(fimc);
> +
>  	/* Get sensor configuration data from the sensor subdev */
>  	si = v4l2_get_subdev_hostdata(fimc->sensor);
>  	if (!si)
> @@ -307,11 +309,12 @@ static irqreturn_t flite_irq_handler(int irq, void *priv)
>  		tv->tv_sec = ts.tv_sec;
>  		tv->tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  		vbuf->vb.v4l2_buf.sequence = fimc->frame_count++;
> +		flite_hw_clear_output_addr(fimc, vbuf->vb.v4l2_buf.index);

This patch looks OK except that it might not be a good idea to use v4l2_buf.index
in the kernel to control how frame buffers are queued in the hardware.
I think it can break with USERPTR buffers, when user space is free to re-assign
memory buffer address to a v4l2_buffer with given index.
So IMO the kernel should maintain it's own id of a buffer in hardware, 
associated with a buffer in e.g. buffer_queue op.

> @@ -643,7 +647,7 @@ static int fimc_vidioc_querycap_capture(struct file *file, void *priv,
>  	strlcpy(cap->driver, FIMC_LITE_DRV_NAME, sizeof(cap->driver));
>  	cap->bus_info[0] = 0;
>  	cap->card[0] = 0;
> -	cap->capabilities = V4L2_CAP_STREAMING;
> +	cap->capabilities = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE_MPLANE;

The FIMC-LITE video nodes require some subdevices to be configured and video
capture will not work if user space skips these steps. Hence it cannot be
assumed at user space that this video node is a standard V4L2 video capture
device and V4L2_CAP_VIDEO_CAPTURE_MPLANE must not be set for it.

I have tested patches 2...4/6 on Exynos4412 SoC and a side effect is that 
frame rate seems to be twice lower. In addition, there is twice as many
FIMC-LITE interrupts, since you have turned on FRAME_END interrupt 
permanently. With two additional interrupts from the MIPI-CSI receiver per 
each frame it's a bit worrying. I'll comment on that at patch 5/6.


Regards,
Sylwester

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v2 5/6] media: s5p-csis: Adding Exynos5250 compatibility
  2013-04-24  7:41 ` [RFC v2 5/6] media: s5p-csis: Adding Exynos5250 compatibility Shaik Ameer Basha
@ 2013-04-29 15:14   ` Sylwester Nawrocki
  0 siblings, 0 replies; 12+ messages in thread
From: Sylwester Nawrocki @ 2013-04-29 15:14 UTC (permalink / raw)
  To: Shaik Ameer Basha
  Cc: linux-media, devicetree-discuss, linux-samsung-soc, shaik.samsung,
	arunkk.samsung

On 04/24/2013 09:41 AM, Shaik Ameer Basha wrote:
> FIMC-IS firmware needs all the MIPI-CSIS interrupts to be enabled.
> This patch enables all those MIPI interrupts and adds the Exynos5
> compatible string.
> 
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  drivers/media/platform/exynos4-is/mipi-csis.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
> index 8636bcd..51ad9b2 100644
> --- a/drivers/media/platform/exynos4-is/mipi-csis.c
> +++ b/drivers/media/platform/exynos4-is/mipi-csis.c
> @@ -66,7 +66,7 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
>  
>  /* Interrupt mask */
>  #define S5PCSIS_INTMSK			0x10
> -#define S5PCSIS_INTMSK_EN_ALL		0xf000103f
> +#define S5PCSIS_INTMSK_EN_ALL		0xfc00103f

I'm a bit reluctant to apply this patch as is. These interrupts should not
be enabled if are not required. I'll try to make some patch to allow a media
device driver to enable/disable the frame start/end interrupts when needed.
But it would presumably be on top of this patch.

>  #define S5PCSIS_INTMSK_EVEN_BEFORE	(1 << 31)
>  #define S5PCSIS_INTMSK_EVEN_AFTER	(1 << 30)
>  #define S5PCSIS_INTMSK_ODD_BEFORE	(1 << 29)
> @@ -1003,6 +1003,7 @@ static const struct dev_pm_ops s5pcsis_pm_ops = {
>  static const struct of_device_id s5pcsis_of_match[] = {
>  	{ .compatible = "samsung,s5pv210-csis" },
>  	{ .compatible = "samsung,exynos4210-csis" },
> +	{ .compatible = "samsung,exynos5250-csis" },
>  	{ /* sentinel */ },
>  };

Regards,
Sylwester

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC v2 6/6] media: exynos5-is: Adding media device driver for exynos5
  2013-04-24  7:41 ` [RFC v2 6/6] media: exynos5-is: Adding media device driver for exynos5 Shaik Ameer Basha
@ 2013-04-29 16:09   ` Sylwester Nawrocki
  0 siblings, 0 replies; 12+ messages in thread
From: Sylwester Nawrocki @ 2013-04-29 16:09 UTC (permalink / raw)
  To: Shaik Ameer Basha
  Cc: linux-media, devicetree-discuss, linux-samsung-soc, shaik.samsung,
	arunkk.samsung

On 04/24/2013 09:41 AM, Shaik Ameer Basha wrote:
> This patch adds support for media device for EXYNOS5 SoCs.
> The current media device supports the following ips to connect
> through the media controller framework.

> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  .../devicetree/bindings/media/exynos5-mdev.txt     |  153 +++
>  drivers/media/platform/Kconfig                     |    1 +
>  drivers/media/platform/Makefile                    |    1 +
>  drivers/media/platform/exynos5-is/Kconfig          |    7 +
>  drivers/media/platform/exynos5-is/Makefile         |    4 +
>  drivers/media/platform/exynos5-is/exynos5-mdev.c   | 1131 ++++++++++++++++++++
>  drivers/media/platform/exynos5-is/exynos5-mdev.h   |  120 +++
>  7 files changed, 1417 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/exynos5-mdev.txt
>  create mode 100644 drivers/media/platform/exynos5-is/Kconfig
>  create mode 100644 drivers/media/platform/exynos5-is/Makefile
>  create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.c
>  create mode 100644 drivers/media/platform/exynos5-is/exynos5-mdev.h
> 
> diff --git a/Documentation/devicetree/bindings/media/exynos5-mdev.txt b/Documentation/devicetree/bindings/media/exynos5-mdev.txt
> new file mode 100644
> index 0000000..d7d419b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/exynos5-mdev.txt
> @@ -0,0 +1,153 @@
> +Samsung EXYNOS5 SoC Camera Subsystem (FIMC)
> +----------------------------------------------
> +
> +The Exynos5 SoC Camera subsystem comprises of multiple sub-devices
> +represented by separate device tree nodes. Currently this includes: FIMC-LITE,
> +MIPI CSIS and FIMC-IS.
> +
> +The sub-subdevices are defined as child nodes of the common 'camera' node which
> +also includes common properties of the whole subsystem not really specific to
> +any single sub-device, like common camera port pins or the CAMCLK clock outputs
> +for external image sensors attached to an SoC.
> +
> +Common 'camera' node
> +--------------------
> +
> +Required properties:
> +
> +- compatible	: must be "samsung,exynos5-fimc", "simple-bus"
> +- clocks	: list of clock specifiers, corresponding to entries in
> +		  the clock-names property;
> +- clock-names	: must contain "sclk_cam0", "sclk_cam1" entries,
> +		  matching entries in the clocks property.
> +
> +The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used
> +to define a required pinctrl state named "default" and optional pinctrl states:
> +"idle", "active-a", active-b". These optional states can be used to switch the
> +camera port pinmux at runtime. The "idle" state should configure both the camera
> +ports A and B into high impedance state, especially the CAMCLK clock output
> +should be inactive. For the "active-a" state the camera port A must be activated
> +and the port B deactivated and for the state "active-b" it should be the other
> +way around.
> +
> +The 'camera' node must include at least one 'fimc-lite' child node.

Why such a restriction ? Still you could have a valid video capture pipeline
using Gscaler, couldn't you ? 

> +'parallel-ports' node
> +---------------------
> +
> +This node should contain child 'port' nodes specifying active parallel video
> +input ports. It includes camera A and camera B inputs. 'reg' property in the
> +port nodes specifies data input - 0, 1 indicates input A, B respectively.
> +
> +Optional properties
> +
> +- samsung,camclk-out : specifies clock output for remote sensor,
> +		       0 - CAM_A_CLKOUT, 1 - CAM_B_CLKOUT;

I think with this driver we should try to start using the asynchronous subdev
registration API and the clock bindings right from the beginning, so the
above property would become unnecessary.

> +Image sensor nodes
> +------------------
> +
> +The sensor device nodes should be added to their control bus controller (e.g.
> +I2C0) nodes and linked to a port node in the csis or the parallel-ports node,
> +using the common video interfaces bindings, defined in video-interfaces.txt.
> +The implementation of this bindings requires clock-frequency property to be
> +present in the sensor device nodes.
> +
> +Example:
> +
> +	aliases {
> +		fimc-lite0 = &fimc_lite_0
> +	};
> +
> +	/* Parallel bus IF sensor */
> +	i2c_0: i2c@13860000 {
> +		s5k6aa: sensor@3c {
> +			compatible = "samsung,s5k6aafx";
> +			reg = <0x3c>;
> +			vddio-supply = <...>;
> +
> +			clock-frequency = <24000000>;
> +			clocks = <...>;
> +			clock-names = "mclk";
> +
> +			port {
> +				s5k6aa_ep: endpoint {
> +					remote-endpoint = <&fimc0_ep>;
> +					bus-width = <8>;
> +					hsync-active = <0>;
> +					vsync-active = <1>;
> +					pclk-sample = <1>;
> +				};
> +			};
> +		};
> +	};
> +
> +	/* MIPI CSI-2 bus IF sensor */
> +	s5c73m3: sensor@0x1a {

This node should be off some I2C bus controller node, e.g. be child node of
the i2c@13860000 node. 

I realise a similar issue is in Documentation/devicetree/bindings/media/
samsung-fimc.txt. And nobody pointed it out... :P

> +		compatible = "samsung,s5c73m3";
> +		reg = <0x1a>;
> +		vddio-supply = <...>;
> +
> +		clock-frequency = <24000000>;
> +		clocks = <...>;
> +		clock-names = "mclk";
> +
> +		port {
> +			s5c73m3_1: endpoint {
> +				data-lanes = <1 2 3 4>;
> +				remote-endpoint = <&csis0_ep>;
> +			};
> +		};
> +	};

> diff --git a/drivers/media/platform/exynos5-is/exynos5-mdev.c b/drivers/media/platform/exynos5-is/exynos5-mdev.c
> new file mode 100644
> index 0000000..ac3e85e
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/exynos5-mdev.c
> @@ -0,0 +1,1131 @@
> +/*
> + * EXYNOS5 SoC series camera host interface media device driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Shaik Ameer Basha <shaik.ameer@samsung.com>
> + *
> + * This driver is based on exynos4-is media device driver developed by

s/developed/written ?

> + * 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
> + * by the Free Software Foundation, either version 2 of the License,
> + * or (at your option) any later version.
> + */

> +static void fimc_md_unregister_entities(struct fimc_md *fmd)
> +{
> +	int i;
> +
> +	for (i = 0; i < FIMC_LITE_MAX_DEVS; i++) {
> +		if (fmd->fimc_lite[i] == NULL)
> +			continue;
> +		v4l2_device_unregister_subdev(&fmd->fimc_lite[i]->subdev);
> +		fmd->fimc_lite[i]->pipeline_ops = NULL;
> +		fmd->fimc_lite[i] = NULL;
> +	}
> +	for (i = 0; i < CSIS_MAX_ENTITIES; i++) {
> +		if (fmd->csis[i].sd == NULL)
> +			continue;
> +		v4l2_device_unregister_subdev(fmd->csis[i].sd);
> +		module_put(fmd->csis[i].sd->owner);

This module_put() call needs to be removed.

> +		fmd->csis[i].sd = NULL;
> +	}
> +	for (i = 0; i < fmd->num_sensors; i++) {
> +		if (fmd->sensor[i].subdev == NULL)
> +			continue;
> +		v4l2_device_unregister_subdev(fmd->sensor[i].subdev);
> +		fmd->sensor[i].subdev = NULL;
> +	}
> +	v4l2_info(&fmd->v4l2_dev, "Unregistered all entities\n");
> +}

> +static struct platform_device_id fimc_driver_ids[] __always_unused = {
> +	{ .name = "exynos5-fimc-md" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(platform, fimc_driver_ids);

This table is not used, you can remove it.

> +static const struct of_device_id fimc_md_of_match[] = {
> +	{ .compatible = "samsung,exynos5-fimc" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, fimc_md_of_match);
> +
> +static struct platform_driver fimc_md_driver = {
> +	.probe		= fimc_md_probe,
> +	.remove		= fimc_md_remove,
> +	.driver = {
> +		.of_match_table = fimc_md_of_match,
> +		.name		= "exynos5-fimc-md",
> +		.owner		= THIS_MODULE,
> +	}
> +};

> +MODULE_AUTHOR("Shaik Ameer Basha <shaik.ameer@samsung.com>");
> +MODULE_DESCRIPTION("EXYNOS5 FIMC media device driver");

Perhaps "EXYNOS5 SoC camera subsystem media device driver" ?

> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/exynos5-is/exynos5-mdev.h b/drivers/media/platform/exynos5-is/exynos5-mdev.h
> new file mode 100644
> index 0000000..43621b6
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/exynos5-mdev.h
> @@ -0,0 +1,120 @@

> +#include <media/s5p_fimc.h>

> +/* Group IDs of sensor, MIPI-CSIS, FIMC-LITE and the writeback subdevs. */
> +#define GRP_ID_SENSOR		(1 << 8)
> +#define GRP_ID_FIMC_IS_SENSOR	(1 << 9)
> +#define GRP_ID_WRITEBACK	(1 << 10)
> +#define GRP_ID_CSIS		(1 << 11)
> +#define GRP_ID_FLITE		(1 << 13)
> +#define GRP_ID_FIMC_IS		(1 << 14)

You are re-defining these constants third time now ;-)

> +struct exynos5_pipeline0 {

How is going struct exynos5_pipeline1 to look like ? Why do you need 
multiple video pipeline data structures ? Couldn't one be designed 
for exynos5 to cover all cases ?

> +	int is_init;
> +	struct fimc_md *fmd;
> +	struct v4l2_subdev *subdevs[IDX_MAX];
> +	void (*sensor_notify)(struct v4l2_subdev *sd,
> +			unsigned int notification, void *arg);
> +};

Regards,
Sylwester

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-04-29 16:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-24  7:41 [RFC v2 0/6] Adding media device driver for Exynos5 imaging subsystem Shaik Ameer Basha
2013-04-24  7:41 ` [RFC v2 1/6] media: exynos4-is: modify existing mdev to use common pipeline Shaik Ameer Basha
2013-04-29 11:24   ` Sylwester Nawrocki
2013-04-24  7:41 ` [RFC v2 2/6] fimc-lite: Adding Exynos5 compatibility to fimc-lite driver Shaik Ameer Basha
2013-04-29 12:08   ` Sylwester Nawrocki
2013-04-24  7:41 ` [RFC v2 3/6] media: fimc-lite: Adding support for Exynos5 Shaik Ameer Basha
2013-04-29 15:13   ` Sylwester Nawrocki
2013-04-24  7:41 ` [RFC v2 4/6] media: fimc-lite: Fix for DMA output corruption Shaik Ameer Basha
2013-04-24  7:41 ` [RFC v2 5/6] media: s5p-csis: Adding Exynos5250 compatibility Shaik Ameer Basha
2013-04-29 15:14   ` Sylwester Nawrocki
2013-04-24  7:41 ` [RFC v2 6/6] media: exynos5-is: Adding media device driver for exynos5 Shaik Ameer Basha
2013-04-29 16:09   ` Sylwester Nawrocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).