linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates
@ 2013-05-09 15:36 Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 01/13] exynos4-is: Remove platform_device_id table at fimc-lite driver Sylwester Nawrocki
                   ` (12 more replies)
  0 siblings, 13 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk, Sylwester Nawrocki

Hi All,

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

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

Any comments/suggestions on this patch are welcome.

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

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

Thanks,
Sylwester

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

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

--
1.7.9.5


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

* [PATCH 01/13] exynos4-is: Remove platform_device_id table at fimc-lite driver
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 02/13] exynos4-is: Correct querycap ioctl handling " Sylwester Nawrocki
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

The driver id_table is unused since all SoCs containing the FIMC-LITE
IP block have been dt-only. Just remove it.

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

diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 14bb7bc..8af8add 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1,8 +1,8 @@
 /*
  * Samsung EXYNOS FIMC-LITE (camera host interface) driver
 *
- * Copyright (C) 2012 Samsung Electronics Co., Ltd.
- * Sylwester Nawrocki <s.nawrocki@samsung.com>
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki@samsung.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -1626,15 +1626,6 @@ static struct flite_drvdata fimc_lite_drvdata_exynos4 = {
 	.out_hor_offs_align	= 8,
 };
 
-static struct platform_device_id fimc_lite_driver_ids[] = {
-	{
-		.name		= "exynos-fimc-lite",
-		.driver_data	= (unsigned long)&fimc_lite_drvdata_exynos4,
-	},
-	{ /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(platform, fimc_lite_driver_ids);
-
 static const struct of_device_id flite_of_match[] = {
 	{
 		.compatible = "samsung,exynos4212-fimc-lite",
@@ -1647,7 +1638,6 @@ MODULE_DEVICE_TABLE(of, flite_of_match);
 static struct platform_driver fimc_lite_driver = {
 	.probe		= fimc_lite_probe,
 	.remove		= fimc_lite_remove,
-	.id_table	= fimc_lite_driver_ids,
 	.driver = {
 		.of_match_table = flite_of_match,
 		.name		= FIMC_LITE_DRV_NAME,
-- 
1.7.9.5


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

* [PATCH 02/13] exynos4-is: Correct querycap ioctl handling at fimc-lite driver
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 01/13] exynos4-is: Remove platform_device_id table at fimc-lite driver Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 03/13] exynos4-is: Move common functions to a separate module Sylwester Nawrocki
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

Fill in properly bus_info and card fields and set device_caps.
The querycap ioctl handler is renamed for consistency with the
other ioctls.

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

diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 8af8add..c57fa65 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -637,13 +637,18 @@ static void fimc_lite_try_compose(struct fimc_lite *fimc, struct v4l2_rect *r)
 /*
  * Video node ioctl operations
  */
-static int fimc_vidioc_querycap_capture(struct file *file, void *priv,
+static int fimc_lite_querycap(struct file *file, void *priv,
 					struct v4l2_capability *cap)
 {
+	struct fimc_lite *fimc = video_drvdata(file);
+
 	strlcpy(cap->driver, FIMC_LITE_DRV_NAME, sizeof(cap->driver));
-	cap->bus_info[0] = 0;
-	cap->card[0] = 0;
-	cap->capabilities = V4L2_CAP_STREAMING;
+	strlcpy(cap->card, FIMC_LITE_DRV_NAME, sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
+					dev_name(&fimc->pdev->dev));
+
+	cap->device_caps = V4L2_CAP_STREAMING;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 	return 0;
 }
 
@@ -938,7 +943,7 @@ static int fimc_lite_s_selection(struct file *file, void *fh,
 }
 
 static const struct v4l2_ioctl_ops fimc_lite_ioctl_ops = {
-	.vidioc_querycap		= fimc_vidioc_querycap_capture,
+	.vidioc_querycap		= fimc_lite_querycap,
 	.vidioc_enum_fmt_vid_cap_mplane	= fimc_lite_enum_fmt_mplane,
 	.vidioc_try_fmt_vid_cap_mplane	= fimc_lite_try_fmt_mplane,
 	.vidioc_s_fmt_vid_cap_mplane	= fimc_lite_s_fmt_mplane,
-- 
1.7.9.5


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

* [PATCH 03/13] exynos4-is: Move common functions to a separate module
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 01/13] exynos4-is: Remove platform_device_id table at fimc-lite driver Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 02/13] exynos4-is: Correct querycap ioctl handling " Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 04/13] exynos4-is: Add struct exynos_video_entity Sylwester Nawrocki
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/Kconfig     |    5 +++
 drivers/media/platform/exynos4-is/Makefile    |    2 ++
 drivers/media/platform/exynos4-is/common.c    |   41 +++++++++++++++++++++++++
 drivers/media/platform/exynos4-is/common.h    |   12 ++++++++
 drivers/media/platform/exynos4-is/fimc-lite.c |   29 ++---------------
 5 files changed, 63 insertions(+), 26 deletions(-)
 create mode 100644 drivers/media/platform/exynos4-is/common.c
 create mode 100644 drivers/media/platform/exynos4-is/common.h

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


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

* [PATCH 04/13] exynos4-is: Add struct exynos_video_entity
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (2 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 03/13] exynos4-is: Move common functions to a separate module Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 05/13] exynos4-is: Preserve state of controls between /dev/video open/close Sylwester Nawrocki
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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


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

* [PATCH 05/13] exynos4-is: Preserve state of controls between /dev/video open/close
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (3 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 04/13] exynos4-is: Add struct exynos_video_entity Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 06/13] exynos4-is: Media graph/video device locking rework Sylwester Nawrocki
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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

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

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


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

* [PATCH 06/13] exynos4-is: Media graph/video device locking rework
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (4 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 05/13] exynos4-is: Preserve state of controls between /dev/video open/close Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 07/13] exynos4-is: Do not use asynchronous runtime PM in release fop Sylwester Nawrocki
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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

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


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

* [PATCH 07/13] exynos4-is: Do not use asynchronous runtime PM in release fop
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (5 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 06/13] exynos4-is: Media graph/video device locking rework Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 08/13] exynos4-is: Use common exynos_media_pipeline data structure Sylwester Nawrocki
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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


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

* [PATCH 08/13] exynos4-is: Use common exynos_media_pipeline data structure
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (6 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 07/13] exynos4-is: Do not use asynchronous runtime PM in release fop Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 09/13] media: Change media device link_notify behaviour Sylwester Nawrocki
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-capture.c |   67 ++++++++-----
 drivers/media/platform/exynos4-is/fimc-core.h    |    2 -
 drivers/media/platform/exynos4-is/fimc-lite.c    |   29 +++---
 drivers/media/platform/exynos4-is/fimc-lite.h    |    2 -
 drivers/media/platform/exynos4-is/media-dev.c    |  110 ++++++++++++++--------
 drivers/media/platform/exynos4-is/media-dev.h    |   43 +++++++++
 include/media/s5p_fimc.h                         |   53 +++++------
 7 files changed, 192 insertions(+), 114 deletions(-)

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


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

* [PATCH 09/13] media: Change media device link_notify behaviour
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (7 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 08/13] exynos4-is: Use common exynos_media_pipeline data structure Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-28 17:39   ` Sylwester Nawrocki
  2013-05-30  2:02   ` Laurent Pinchart
  2013-05-09 15:36 ` [PATCH 10/13] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines Sylwester Nawrocki
                   ` (3 subsequent siblings)
  12 siblings, 2 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/media-entity.c                  |   18 +++--------
 drivers/media/platform/exynos4-is/media-dev.c |   16 +++++-----
 drivers/media/platform/omap3isp/isp.c         |   41 +++++++++++++++----------
 include/media/media-device.h                  |    9 ++++--
 4 files changed, 46 insertions(+), 38 deletions(-)

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


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

* [PATCH 10/13] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (8 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 09/13] media: Change media device link_notify behaviour Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 11/13] exynos4-is: Fix sensor subdev -> FIMC notification setup Sylwester Nawrocki
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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

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

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


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

* [PATCH 11/13] exynos4-is: Fix sensor subdev -> FIMC notification setup
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (9 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 10/13] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 12/13] exynos4-is: Add locking at fimc(-lite) subdev unregistered handler Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 13/13] exynos4-is: Remove WARN_ON() from __fimc_pipeline_close() Sylwester Nawrocki
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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


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

* [PATCH 12/13] exynos4-is: Add locking at fimc(-lite) subdev unregistered handler
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (10 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 11/13] exynos4-is: Fix sensor subdev -> FIMC notification setup Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  2013-05-09 15:36 ` [PATCH 13/13] exynos4-is: Remove WARN_ON() from __fimc_pipeline_close() Sylwester Nawrocki
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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


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

* [PATCH 13/13] exynos4-is: Remove WARN_ON() from __fimc_pipeline_close()
  2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
                   ` (11 preceding siblings ...)
  2013-05-09 15:36 ` [PATCH 12/13] exynos4-is: Add locking at fimc(-lite) subdev unregistered handler Sylwester Nawrocki
@ 2013-05-09 15:36 ` Sylwester Nawrocki
  12 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-09 15:36 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Sylwester Nawrocki, Kyungmin Park

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

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

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


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

* Re: [PATCH 09/13] media: Change media device link_notify behaviour
  2013-05-09 15:36 ` [PATCH 09/13] media: Change media device link_notify behaviour Sylwester Nawrocki
@ 2013-05-28 17:39   ` Sylwester Nawrocki
  2013-05-30  2:02   ` Laurent Pinchart
  1 sibling, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-28 17:39 UTC (permalink / raw)
  To: linux-media
  Cc: hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Kyungmin Park, Seung-Woo Kim, Laurent Pinchart, Sakari Ailus

Hi All,

(replying to myself, probably a bad sign... ;))

On 05/09/2013 05:36 PM, Sylwester Nawrocki wrote:
> Currently the media device link_notify callback is invoked before the
> actual change of state of a link when the link is being enabled, and
> after the actual change of state when the link is being disabled.
> 
> This doesn't allow a media device driver to perform any operations
> on a full graph before a link is disabled, as well as performing
> any tasks on a modified graph right after a link's state is changed.
> 
> This patch modifies signature of the link_notify callback. This
> callback is now called always before and after a link's state change.
> To distinguish the notifications a 'notification' argument is added
> to the link_notify callback: MEDIA_DEV_NOTIFY_PRE_LINK_CH indicates
> notification before link's state change and
> MEDIA_DEV_NOTIFY_POST_LINK_CH corresponds to a notification after
> link flags change.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/media-entity.c                  |   18 +++--------
>  drivers/media/platform/exynos4-is/media-dev.c |   16 +++++-----
>  drivers/media/platform/omap3isp/isp.c         |   41 +++++++++++++++----------
>  include/media/media-device.h                  |    9 ++++--
>  4 files changed, 46 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 7c2b51c..0fcf4c0 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -547,25 +547,17 @@ int __media_entity_setup_link(struct media_link *link, u32 flags)
>  
>  	mdev = source->parent;
>  
> -	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify) {
> -		ret = mdev->link_notify(link->source, link->sink,
> -					MEDIA_LNK_FL_ENABLED);
> +	if (mdev->link_notify) {
> +		ret = mdev->link_notify(link, MEDIA_LNK_FL_ENABLED,
> +						MEDIA_DEV_NOTIFY_PRE_LINK_CH);

After some testing I found it really should have been

		ret = mdev->link_notify(link, flags,
					MEDIA_DEV_NOTIFY_PRE_LINK_CH);

Otherwise the pipeline never gets powered off upon link disconnection.

I will repost the whole series in the end of week, after some more testing.
Still, any further comments on this are appreciated.

Thanks,
Sylwester
>  		if (ret < 0)
>  			return ret;
>  	}
>  
>  	ret = __media_entity_setup_link_notify(link, flags);
> -	if (ret < 0)
> -		goto err;
>  
> -	if (!(flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
> -		mdev->link_notify(link->source, link->sink, 0);
> -
> -	return 0;
> -
> -err:
> -	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
> -		mdev->link_notify(link->source, link->sink, 0);
> +	if (mdev->link_notify)
> +		mdev->link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);
>  
>  	return ret;
>  }

-- 
Sylwester Nawrocki
Samsung R&D Institute Poland
Samsung Electronics

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

* Re: [PATCH 09/13] media: Change media device link_notify behaviour
  2013-05-09 15:36 ` [PATCH 09/13] media: Change media device link_notify behaviour Sylwester Nawrocki
  2013-05-28 17:39   ` Sylwester Nawrocki
@ 2013-05-30  2:02   ` Laurent Pinchart
  2013-05-31 10:02     ` Sylwester Nawrocki
  1 sibling, 1 reply; 17+ messages in thread
From: Laurent Pinchart @ 2013-05-30  2:02 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-media, hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Kyungmin Park

Hi Sylwester,

Thank you for the patch, and sorry for the late reply.

On Thursday 09 May 2013 17:36:41 Sylwester Nawrocki wrote:
> Currently the media device link_notify callback is invoked before the
> actual change of state of a link when the link is being enabled, and
> after the actual change of state when the link is being disabled.
> 
> This doesn't allow a media device driver to perform any operations
> on a full graph before a link is disabled, as well as performing
> any tasks on a modified graph right after a link's state is changed.
> 
> This patch modifies signature of the link_notify callback. This
> callback is now called always before and after a link's state change.
> To distinguish the notifications a 'notification' argument is added
> to the link_notify callback: MEDIA_DEV_NOTIFY_PRE_LINK_CH indicates
> notification before link's state change and
> MEDIA_DEV_NOTIFY_POST_LINK_CH corresponds to a notification after
> link flags change.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/media-entity.c                  |   18 +++--------
>  drivers/media/platform/exynos4-is/media-dev.c |   16 +++++-----
>  drivers/media/platform/omap3isp/isp.c         |   41 +++++++++++++---------
>  include/media/media-device.h                  |    9 ++++--
>  4 files changed, 46 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 7c2b51c..0fcf4c0 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -547,25 +547,17 @@ int __media_entity_setup_link(struct media_link *link,
> u32 flags)
> 
>  	mdev = source->parent;
> 
> -	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify) {
> -		ret = mdev->link_notify(link->source, link->sink,
> -					MEDIA_LNK_FL_ENABLED);
> +	if (mdev->link_notify) {
> +		ret = mdev->link_notify(link, MEDIA_LNK_FL_ENABLED,
> +						MEDIA_DEV_NOTIFY_PRE_LINK_CH);

As you correctly pointed out in a self-reply to your patch, you should pass 
the flags here instead of MEDIA_LNK_FL_ENABLED.

>  		if (ret < 0)
>  			return ret;
>  	}
> 
>  	ret = __media_entity_setup_link_notify(link, flags);
> -	if (ret < 0)
> -		goto err;
> 
> -	if (!(flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
> -		mdev->link_notify(link->source, link->sink, 0);
> -
> -	return 0;
> -
> -err:
> -	if ((flags & MEDIA_LNK_FL_ENABLED) && mdev->link_notify)
> -		mdev->link_notify(link->source, link->sink, 0);
> +	if (mdev->link_notify)
> +		mdev->link_notify(link, flags, MEDIA_DEV_NOTIFY_POST_LINK_CH);
> 
>  	return ret;
>  }
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c
> b/drivers/media/platform/exynos4-is/media-dev.c index e95a6d5..ca58dfc
> 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -1274,34 +1274,36 @@ int fimc_md_set_camclk(struct v4l2_subdev *sd, bool
> on) return __fimc_md_set_camclk(fmd, si, on);
>  }
> 
> -static int fimc_md_link_notify(struct media_pad *source,
> -			       struct media_pad *sink, u32 flags)
> +static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
> +						unsigned int notification)
>  {
> +	struct media_entity *sink = link->sink->entity;
>  	struct exynos_video_entity *ve;
>  	struct video_device *vdev;
>  	struct fimc_pipeline *pipeline;
>  	int i, ret = 0;
> 
> -	if (media_entity_type(sink->entity) != MEDIA_ENT_T_DEVNODE_V4L)
> +	if (media_entity_type(sink) != MEDIA_ENT_T_DEVNODE_V4L ||
> +	    notification == MEDIA_DEV_NOTIFY_LINK_PRE_CH)

Don't you need to call __fimc_pipeline_open() on post-notify instead of pre-
notified below ?

>  		return 0;
> 
> -	vdev = media_entity_to_video_device(sink->entity);
> +	vdev = media_entity_to_video_device(sink);
>  	ve = vdev_to_exynos_video_entity(vdev);
>  	pipeline = to_fimc_pipeline(ve->pipe);
> 
>  	if (!(flags & MEDIA_LNK_FL_ENABLED) && pipeline->subdevs[IDX_SENSOR]) {
> -		if (sink->entity->use_count > 0)
> +		if (sink->use_count > 0)
>  			ret = __fimc_pipeline_close(ve->pipe);
> 
>  		for (i = 0; i < IDX_MAX; i++)
>  			pipeline->subdevs[i] = NULL;
> -	} else if (sink->entity->use_count > 0) {
> +	} else if (sink->use_count > 0) {
>  		/*
>  		 * Link activation. Enable power of pipeline elements only if
>  		 * the pipeline is already in use, i.e. its video node is open.
>  		 * Recreate the controls destroyed during the link deactivation.
>  		 */
> -		ret = __fimc_pipeline_open(ve->pipe, sink->entity, true);
> +		ret = __fimc_pipeline_open(ve->pipe, sink, true);
>  	}
> 
>  	return ret ? -EPIPE : ret;
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 6e5ad8e..a762aeb 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -670,9 +670,9 @@ int omap3isp_pipeline_pm_use(struct media_entity
> *entity, int use)
> 
>  /*
>   * isp_pipeline_link_notify - Link management notification callback
> - * @source: Pad at the start of the link
> - * @sink: Pad at the end of the link
> + * @link: The link
>   * @flags: New link flags that will be applied
> + * @notification: The link's state change notification type
> (MEDIA_DEV_NOTIFY_*) *
>   * React to link management on powered pipelines by updating the use count
>   * of all entities in the source and sink sides of the link. Entities are
>   * powered
> @@ -682,29 +682,38 @@ int omap3isp_pipeline_pm_use(struct
> media_entity *entity, int use)
>   * off is assumed to never fail. This function will not fail for
>   * disconnection events.
>   */
> -static int isp_pipeline_link_notify(struct media_pad *source,
> -				    struct media_pad *sink, u32 flags)
> +static int isp_pipeline_link_notify(struct media_link *link, unsigned int
> +						flags, unsigned int notification)
>  {
> -	int source_use = isp_pipeline_pm_use_count(source->entity);
> -	int sink_use = isp_pipeline_pm_use_count(sink->entity);
> +	struct media_entity *source = link->source->entity;
> +	struct media_entity *sink = link->sink->entity;
> +	int source_use = isp_pipeline_pm_use_count(source);
> +	int sink_use = isp_pipeline_pm_use_count(sink);
>  	int ret;
> 
> -	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
> +	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
> +	    !(flags & MEDIA_LNK_FL_ENABLED)) {

Shouldn't the condition be

	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
	    (flags & MEDIA_LNK_FL_ENABLED))

here ? The code currently pre-notifies on link enable and post-notifies on 
link disable.

>  		/* Powering off entities is assumed to never fail. */
> -		isp_pipeline_pm_power(source->entity, -sink_use);
> -		isp_pipeline_pm_power(sink->entity, -source_use);
> +		isp_pipeline_pm_power(source, -sink_use);
> +		isp_pipeline_pm_power(sink, -source_use);
>  		return 0;
>  	}
> 
> -	ret = isp_pipeline_pm_power(source->entity, sink_use);
> -	if (ret < 0)
> -		return ret;
> +	if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
> +	        (flags & MEDIA_LNK_FL_ENABLED)) {

Similarly, shouldn't this be

	if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
	    !(flags & MEDIA_LNK_FL_ENABLED))

> -	ret = isp_pipeline_pm_power(sink->entity, source_use);
> -	if (ret < 0)
> -		isp_pipeline_pm_power(source->entity, -sink_use);
> +		ret = isp_pipeline_pm_power(source, sink_use);
> +		if (ret < 0)
> +			return ret;
> 
> -	return ret;
> +		ret = isp_pipeline_pm_power(sink, source_use);
> +		if (ret < 0)
> +			isp_pipeline_pm_power(source, -sink_use);
> +
> +		return ret;
> +	}
> +
> +	return 0;
>  }
> 
>  /*
> ---------------------------------------------------------------------------
> -- diff --git a/include/media/media-device.h b/include/media/media-device.h
> index eaade98..353c4ee 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -45,6 +45,7 @@ struct device;
>   * @entities:	List of registered entities
>   * @lock:	Entities list lock
>   * @graph_mutex: Entities graph operation lock
> + * @link_notify: Link state change notification callback
>   *
>   * This structure represents an abstract high-level media device. It allows
> easy * access to entities and provides basic media device-level support.
> The @@ -75,10 +76,14 @@ struct media_device {
>  	/* Serializes graph operations. */
>  	struct mutex graph_mutex;
> 
> -	int (*link_notify)(struct media_pad *source,
> -			   struct media_pad *sink, u32 flags);
> +	int (*link_notify)(struct media_link *link, unsigned int flags,
> +			   unsigned int notification);
>  };
> 
> +/* Supported link_notify @notification values. */
> +#define MEDIA_DEV_NOTIFY_PRE_LINK_CH	0
> +#define MEDIA_DEV_NOTIFY_POST_LINK_CH	1
> +
>  /* media_devnode to media_device */
>  #define to_media_device(node) container_of(node, struct media_device,
> devnode)
-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 09/13] media: Change media device link_notify behaviour
  2013-05-30  2:02   ` Laurent Pinchart
@ 2013-05-31 10:02     ` Sylwester Nawrocki
  0 siblings, 0 replies; 17+ messages in thread
From: Sylwester Nawrocki @ 2013-05-31 10:02 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, hj210.choi, dh09.lee, a.hajda, shaik.ameer, arun.kk,
	Kyungmin Park

Hi Laurent,

On 05/30/2013 04:02 AM, Laurent Pinchart wrote:
> Hi Sylwester,
> 
> Thank you for the patch, and sorry for the late reply.

Thank you for review, the timing is just fine. :-)

> On Thursday 09 May 2013 17:36:41 Sylwester Nawrocki wrote:
>> Currently the media device link_notify callback is invoked before the
>> actual change of state of a link when the link is being enabled, and
>> after the actual change of state when the link is being disabled.
>>
>> This doesn't allow a media device driver to perform any operations
>> on a full graph before a link is disabled, as well as performing
>> any tasks on a modified graph right after a link's state is changed.
>>
>> This patch modifies signature of the link_notify callback. This
>> callback is now called always before and after a link's state change.
>> To distinguish the notifications a 'notification' argument is added
>> to the link_notify callback: MEDIA_DEV_NOTIFY_PRE_LINK_CH indicates
>> notification before link's state change and
>> MEDIA_DEV_NOTIFY_POST_LINK_CH corresponds to a notification after
>> link flags change.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/media/media-entity.c                  |   18 +++--------
>>  drivers/media/platform/exynos4-is/media-dev.c |   16 +++++-----
>>  drivers/media/platform/omap3isp/isp.c         |   41 +++++++++++++---------
>>  include/media/media-device.h                  |    9 ++++--
>>  4 files changed, 46 insertions(+), 38 deletions(-)
[...]
>> --- a/drivers/media/platform/exynos4-is/media-dev.c
>> +++ b/drivers/media/platform/exynos4-is/media-dev.c
>> @@ -1274,34 +1274,36 @@ int fimc_md_set_camclk(struct v4l2_subdev *sd, bool
>> on) return __fimc_md_set_camclk(fmd, si, on);
>>  }
>>
>> -static int fimc_md_link_notify(struct media_pad *source,
>> -			       struct media_pad *sink, u32 flags)
>> +static int fimc_md_link_notify(struct media_link *link, unsigned int flags,
>> +						unsigned int notification)
>>  {
>> +	struct media_entity *sink = link->sink->entity;
>>  	struct exynos_video_entity *ve;
>>  	struct video_device *vdev;
>>  	struct fimc_pipeline *pipeline;
>>  	int i, ret = 0;
>>
>> -	if (media_entity_type(sink->entity) != MEDIA_ENT_T_DEVNODE_V4L)
>> +	if (media_entity_type(sink) != MEDIA_ENT_T_DEVNODE_V4L ||
>> +	    notification == MEDIA_DEV_NOTIFY_LINK_PRE_CH)
> 
> Don't you need to call __fimc_pipeline_open() on post-notify instead of pre-
> notified below ?

Yes, that was the intention. I wanted to minimize changes to the drivers in
this patch. Thus this notifier function is further modified in patch 10/13.

>>  		return 0;
>>
>> -	vdev = media_entity_to_video_device(sink->entity);
>> +	vdev = media_entity_to_video_device(sink);
>>  	ve = vdev_to_exynos_video_entity(vdev);
>>  	pipeline = to_fimc_pipeline(ve->pipe);
>>
>>  	if (!(flags & MEDIA_LNK_FL_ENABLED) && pipeline->subdevs[IDX_SENSOR]) {
>> -		if (sink->entity->use_count > 0)
>> +		if (sink->use_count > 0)
>>  			ret = __fimc_pipeline_close(ve->pipe);
>>
>>  		for (i = 0; i < IDX_MAX; i++)
>>  			pipeline->subdevs[i] = NULL;
>> -	} else if (sink->entity->use_count > 0) {
>> +	} else if (sink->use_count > 0) {
>>  		/*
>>  		 * Link activation. Enable power of pipeline elements only if
>>  		 * the pipeline is already in use, i.e. its video node is open.
>>  		 * Recreate the controls destroyed during the link deactivation.
>>  		 */
>> -		ret = __fimc_pipeline_open(ve->pipe, sink->entity, true);
>> +		ret = __fimc_pipeline_open(ve->pipe, sink, true);
>>  	}
>>
>>  	return ret ? -EPIPE : ret;
>> diff --git a/drivers/media/platform/omap3isp/isp.c
>> b/drivers/media/platform/omap3isp/isp.c index 6e5ad8e..a762aeb 100644
>> --- a/drivers/media/platform/omap3isp/isp.c
>> +++ b/drivers/media/platform/omap3isp/isp.c
>> @@ -670,9 +670,9 @@ int omap3isp_pipeline_pm_use(struct media_entity
>> *entity, int use)
>>
>>  /*
>>   * isp_pipeline_link_notify - Link management notification callback
>> - * @source: Pad at the start of the link
>> - * @sink: Pad at the end of the link
>> + * @link: The link
>>   * @flags: New link flags that will be applied
>> + * @notification: The link's state change notification type
>> (MEDIA_DEV_NOTIFY_*) *
>>   * React to link management on powered pipelines by updating the use count
>>   * of all entities in the source and sink sides of the link. Entities are
>>   * powered
>> @@ -682,29 +682,38 @@ int omap3isp_pipeline_pm_use(struct
>> media_entity *entity, int use)
>>   * off is assumed to never fail. This function will not fail for
>>   * disconnection events.
>>   */
>> -static int isp_pipeline_link_notify(struct media_pad *source,
>> -				    struct media_pad *sink, u32 flags)
>> +static int isp_pipeline_link_notify(struct media_link *link, unsigned int
>> +						flags, unsigned int notification)
>>  {
>> -	int source_use = isp_pipeline_pm_use_count(source->entity);
>> -	int sink_use = isp_pipeline_pm_use_count(sink->entity);
>> +	struct media_entity *source = link->source->entity;
>> +	struct media_entity *sink = link->sink->entity;
>> +	int source_use = isp_pipeline_pm_use_count(source);
>> +	int sink_use = isp_pipeline_pm_use_count(sink);
>>  	int ret;
>>
>> -	if (!(flags & MEDIA_LNK_FL_ENABLED)) {
>> +	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
>> +	    !(flags & MEDIA_LNK_FL_ENABLED)) {
> 
> Shouldn't the condition be
> 
> 	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
> 	    (flags & MEDIA_LNK_FL_ENABLED))
> 
> here ? The code currently pre-notifies on link enable and post-notifies on 
> link disable.

Hmm, it seems I failed to retain the original behaviour :-/
But shouldn't the condition really be:

	if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
	    !(flags & MEDIA_LNK_FL_ENABLED)) {
?
>>  		/* Powering off entities is assumed to never fail. */
>> -		isp_pipeline_pm_power(source->entity, -sink_use);
>> -		isp_pipeline_pm_power(sink->entity, -source_use);
>> +		isp_pipeline_pm_power(source, -sink_use);
>> +		isp_pipeline_pm_power(sink, -source_use);
>>  		return 0;
>>  	}
>>
>> -	ret = isp_pipeline_pm_power(source->entity, sink_use);
>> -	if (ret < 0)
>> -		return ret;
>> +	if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
>> +	        (flags & MEDIA_LNK_FL_ENABLED)) {
> 
> Similarly, shouldn't this be
> 
> 	if (notification == MEDIA_DEV_NOTIFY_POST_LINK_CH &&
> 	    !(flags & MEDIA_LNK_FL_ENABLED))

And this one

	if (notification == MEDIA_DEV_NOTIFY_PRE_LINK_CH &&
	        (flags & MEDIA_LNK_FL_ENABLED)) {
?
Since originally the code notified about link activation before
activating the link ?

Regards,
Sylwester

-- 
Sylwester Nawrocki
Samsung R&D Institute Poland
Samsung Electronics

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

end of thread, other threads:[~2013-05-31 10:02 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-09 15:36 [PATCH RFC 00/13] Media link_notify behaviour change an exynos4-is updates Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 01/13] exynos4-is: Remove platform_device_id table at fimc-lite driver Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 02/13] exynos4-is: Correct querycap ioctl handling " Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 03/13] exynos4-is: Move common functions to a separate module Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 04/13] exynos4-is: Add struct exynos_video_entity Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 05/13] exynos4-is: Preserve state of controls between /dev/video open/close Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 06/13] exynos4-is: Media graph/video device locking rework Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 07/13] exynos4-is: Do not use asynchronous runtime PM in release fop Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 08/13] exynos4-is: Use common exynos_media_pipeline data structure Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 09/13] media: Change media device link_notify behaviour Sylwester Nawrocki
2013-05-28 17:39   ` Sylwester Nawrocki
2013-05-30  2:02   ` Laurent Pinchart
2013-05-31 10:02     ` Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 10/13] exynos4-is: Extend link_notify handler to support fimc-is/lite pipelines Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 11/13] exynos4-is: Fix sensor subdev -> FIMC notification setup Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 12/13] exynos4-is: Add locking at fimc(-lite) subdev unregistered handler Sylwester Nawrocki
2013-05-09 15:36 ` [PATCH 13/13] exynos4-is: Remove WARN_ON() from __fimc_pipeline_close() 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).