linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera
@ 2014-11-21 16:14 Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure Jacek Anaszewski
                   ` (10 more replies)
  0 siblings, 11 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

This is a fourth version of the patch series adding a plugin for the 
Exynos4 camera.

Temporarily the plugin doesn't link against libmediactl, but
has its sources compiled in. Currently libmediactl is built
after the plugins and it can't be easily changed because
it depends on the core libs.

================
Changes from v3:
================

- added struct v4l2_subdev and put entity fd and 
  information about supported controls to it
- improved functions for negotiating and setting
  pipeline format by using available libv4lsubdev API
- applied minor improvements and cleanups

================
Changes from v2:
================

- switched to using mediatext library for parsing
  the media device configuration
- extended libmediactl
- switched to using libmediactl

================
Changes from v1:
================

- removed redundant mbus code negotiation
- split the parser, media device helpers and ioctl wrappers
  to the separate modules
- added mechanism for querying extended controls
- applied various fixes and modifications

The plugin was tested on latest media-tree.git master with patches for
exynos4-is that fix failing open ioctl when a sensor sub-device is not
linked [1] [2] [3].

The plugin expects a configuration file:
/var/lib/libv4l/exynos4_capture_conf

Exemplary configuration file:

==========================================

link-conf "s5p-mipi-csis.0":1 -> "FIMC.0":0 [1]
ctrl-to-subdev-conf 0x0098091f -> "fimc.0.capture"
ctrl-to-subdev-conf 0x00980902 -> "S5C73M3"
ctrl-to-subdev-conf 0x00980922 -> "fimc.0.capture"
ctrl-to-subdev-conf 0x009a0914 -> "S5C73M3"

==========================================

With this settings the plugin can be tested on the exynos4412-trats2 board
using following gstreamer pipeline:

gst-launch-1.0 v4l2src device=/dev/video1 ! video/x-raw,width=960,height=720 ! fbdevsink

In order to avoid fbdevsink element failure the fix [4]
for exynos-drm driver is required.

Thanks,
Jacek Anaszewski

[1] https://patchwork.linuxtv.org/patch/26366/
[2] https://patchwork.linuxtv.org/patch/26367/
[3] https://patchwork.linuxtv.org/patch/26368/
[4] http://www.spinics.net/lists/dri-devel/msg66494.html

Jacek Anaszewski (11):
  mediactl: Introduce v4l2_subdev structure
  mediactl: Add support for v4l2 controls
  mediactl: Separate entity and pad parsing
  mediatext: Add library
  mediactl: Add media device graph helpers
  mediactl: Add media_device creation helpers
  media-ctl: libv4l2subdev: add VYUY8_2X8 mbus code
  mediactl: Add support for media device pipelines
  mediactl: Close only pipeline sub-devices
  mediactl: Add media device ioctl API
  Add a libv4l plugin for Exynos4 camera

 configure.ac                                      |    1 +
 lib/Makefile.am                                   |    7 +-
 lib/libv4l-exynos4-camera/Makefile.am             |    7 +
 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c |  595 +++++++++++++++++++++
 utils/media-ctl/Makefile.am                       |   12 +-
 utils/media-ctl/libmediactl.c                     |  486 ++++++++++++++++-
 utils/media-ctl/libmediatext.pc.in                |   10 +
 utils/media-ctl/libv4l2media_ioctl.c              |  325 +++++++++++
 utils/media-ctl/libv4l2media_ioctl.h              |   40 ++
 utils/media-ctl/libv4l2subdev.c                   |  204 ++++++-
 utils/media-ctl/mediactl-priv.h                   |   15 +
 utils/media-ctl/mediactl.h                        |  256 +++++++++
 utils/media-ctl/mediatext-test.c                  |   66 +++
 utils/media-ctl/mediatext.c                       |  286 ++++++++++
 utils/media-ctl/mediatext.h                       |   52 ++
 utils/media-ctl/v4l2subdev.h                      |   70 +++
 16 files changed, 2401 insertions(+), 31 deletions(-)
 create mode 100644 lib/libv4l-exynos4-camera/Makefile.am
 create mode 100644 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
 create mode 100644 utils/media-ctl/libmediatext.pc.in
 create mode 100644 utils/media-ctl/libv4l2media_ioctl.c
 create mode 100644 utils/media-ctl/libv4l2media_ioctl.h
 create mode 100644 utils/media-ctl/mediatext-test.c
 create mode 100644 utils/media-ctl/mediatext.c
 create mode 100644 utils/media-ctl/mediatext.h

-- 
1.7.9.5


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

* [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-25 11:36   ` Sakari Ailus
  2014-11-21 16:14 ` [PATCH/RFC v4 02/11] mediactl: Add support for v4l2 controls Jacek Anaszewski
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

Add struct v4l2_subdev as a representation of the v4l2 sub-device
related to a media entity. Add sd property, the pointer to
the newly introduced structure, to the struct media_entity
and move fd property to it.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/libmediactl.c   |   30 +++++++++++++++++++++++++-----
 utils/media-ctl/libv4l2subdev.c |   34 +++++++++++++++++-----------------
 utils/media-ctl/mediactl-priv.h |    5 +++++
 utils/media-ctl/mediactl.h      |   22 ++++++++++++++++++++++
 4 files changed, 69 insertions(+), 22 deletions(-)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index ec360bd..53921f5 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -511,7 +511,6 @@ static int media_enum_entities(struct media_device *media)
 
 		entity = &media->entities[media->entities_count];
 		memset(entity, 0, sizeof(*entity));
-		entity->fd = -1;
 		entity->info.id = id | MEDIA_ENT_ID_FLAG_NEXT;
 		entity->media = media;
 
@@ -529,11 +528,13 @@ static int media_enum_entities(struct media_device *media)
 
 		entity->pads = malloc(entity->info.pads * sizeof(*entity->pads));
 		entity->links = malloc(entity->max_links * sizeof(*entity->links));
-		if (entity->pads == NULL || entity->links == NULL) {
+		entity->sd = calloc(1, sizeof(*entity->sd));
+		if (entity->pads == NULL || entity->links == NULL || entity->sd == NULL) {
 			ret = -ENOMEM;
 			break;
 		}
 
+		entity->sd->fd = -1;
 		media->entities_count++;
 
 		if (entity->info.flags & MEDIA_ENT_FL_DEFAULT) {
@@ -704,8 +705,9 @@ void media_device_unref(struct media_device *media)
 
 		free(entity->pads);
 		free(entity->links);
-		if (entity->fd != -1)
-			close(entity->fd);
+		if (entity->sd->fd != -1)
+			close(entity->sd->fd);
+		free(entity->sd);
 	}
 
 	free(media->entities);
@@ -726,13 +728,17 @@ int media_device_add_entity(struct media_device *media,
 	if (entity == NULL)
 		return -ENOMEM;
 
+	entity->sd = calloc(1, sizeof(*entity->sd));
+	if (entity->sd == NULL)
+		return -ENOMEM;
+
 	media->entities = entity;
 	media->entities_count++;
 
 	entity = &media->entities[media->entities_count - 1];
 	memset(entity, 0, sizeof *entity);
 
-	entity->fd = -1;
+	entity->sd->fd = -1;
 	entity->media = media;
 	strncpy(entity->devname, devnode, sizeof entity->devname);
 	entity->devname[sizeof entity->devname - 1] = '\0';
@@ -955,3 +961,17 @@ int media_parse_setup_links(struct media_device *media, const char *p)
 
 	return *end ? -EINVAL : 0;
 }
+
+/* -----------------------------------------------------------------------------
+ * Media entity access
+ */
+
+int media_entity_get_fd(struct media_entity *entity)
+{
+	return entity->sd->fd;
+}
+
+void media_entity_set_fd(struct media_entity *entity, int fd)
+{
+	entity->sd->fd = fd;
+}
diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index 8015330..09e0081 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -41,11 +41,11 @@
 
 int v4l2_subdev_open(struct media_entity *entity)
 {
-	if (entity->fd != -1)
+	if (entity->sd->fd != -1)
 		return 0;
 
-	entity->fd = open(entity->devname, O_RDWR);
-	if (entity->fd == -1) {
+	entity->sd->fd = open(entity->devname, O_RDWR);
+	if (entity->sd->fd == -1) {
 		int ret = -errno;
 		media_dbg(entity->media,
 			  "%s: Failed to open subdev device node %s\n", __func__,
@@ -58,8 +58,8 @@ int v4l2_subdev_open(struct media_entity *entity)
 
 void v4l2_subdev_close(struct media_entity *entity)
 {
-	close(entity->fd);
-	entity->fd = -1;
+	close(entity->sd->fd);
+	entity->sd->fd = -1;
 }
 
 int v4l2_subdev_get_format(struct media_entity *entity,
@@ -77,7 +77,7 @@ int v4l2_subdev_get_format(struct media_entity *entity,
 	fmt.pad = pad;
 	fmt.which = which;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
 	if (ret < 0)
 		return -errno;
 
@@ -101,7 +101,7 @@ int v4l2_subdev_set_format(struct media_entity *entity,
 	fmt.which = which;
 	fmt.format = *format;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FMT, &fmt);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_FMT, &fmt);
 	if (ret < 0)
 		return -errno;
 
@@ -128,7 +128,7 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
 	u.sel.target = target;
 	u.sel.which = which;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_SELECTION, &u.sel);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_SELECTION, &u.sel);
 	if (ret >= 0) {
 		*rect = u.sel.r;
 		return 0;
@@ -140,7 +140,7 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
 	u.crop.pad = pad;
 	u.crop.which = which;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_CROP, &u.crop);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_CROP, &u.crop);
 	if (ret < 0)
 		return -errno;
 
@@ -168,7 +168,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
 	u.sel.which = which;
 	u.sel.r = *rect;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_SELECTION, &u.sel);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_SELECTION, &u.sel);
 	if (ret >= 0) {
 		*rect = u.sel.r;
 		return 0;
@@ -181,7 +181,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
 	u.crop.which = which;
 	u.crop.rect = *rect;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_CROP, &u.crop);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_CROP, &u.crop);
 	if (ret < 0)
 		return -errno;
 
@@ -202,7 +202,7 @@ int v4l2_subdev_get_dv_timings_caps(struct media_entity *entity,
 	memset(caps, 0, sizeof(*caps));
 	caps->pad = pad;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_DV_TIMINGS_CAP, caps);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_DV_TIMINGS_CAP, caps);
 	if (ret < 0)
 		return -errno;
 
@@ -220,7 +220,7 @@ int v4l2_subdev_query_dv_timings(struct media_entity *entity,
 
 	memset(timings, 0, sizeof(*timings));
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_QUERY_DV_TIMINGS, timings);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_QUERY_DV_TIMINGS, timings);
 	if (ret < 0)
 		return -errno;
 
@@ -238,7 +238,7 @@ int v4l2_subdev_get_dv_timings(struct media_entity *entity,
 
 	memset(timings, 0, sizeof(*timings));
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_DV_TIMINGS, timings);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_DV_TIMINGS, timings);
 	if (ret < 0)
 		return -errno;
 
@@ -254,7 +254,7 @@ int v4l2_subdev_set_dv_timings(struct media_entity *entity,
 	if (ret < 0)
 		return ret;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_DV_TIMINGS, timings);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_DV_TIMINGS, timings);
 	if (ret < 0)
 		return -errno;
 
@@ -273,7 +273,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity *entity,
 
 	memset(&ival, 0, sizeof(ival));
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
 	if (ret < 0)
 		return -errno;
 
@@ -294,7 +294,7 @@ int v4l2_subdev_set_frame_interval(struct media_entity *entity,
 	memset(&ival, 0, sizeof(ival));
 	ival.interval = *interval;
 
-	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
+	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
 	if (ret < 0)
 		return -errno;
 
diff --git a/utils/media-ctl/mediactl-priv.h b/utils/media-ctl/mediactl-priv.h
index a0d3a55..4bcb1e0 100644
--- a/utils/media-ctl/mediactl-priv.h
+++ b/utils/media-ctl/mediactl-priv.h
@@ -34,7 +34,12 @@ struct media_entity {
 	unsigned int max_links;
 	unsigned int num_links;
 
+	struct v4l2_subdev *sd;
+
 	char devname[32];
+};
+
+struct v4l2_subdev {
 	int fd;
 };
 
diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
index 77ac182..b8cefe8 100644
--- a/utils/media-ctl/mediactl.h
+++ b/utils/media-ctl/mediactl.h
@@ -420,4 +420,26 @@ int media_parse_setup_link(struct media_device *media,
  */
 int media_parse_setup_links(struct media_device *media, const char *p);
 
+/**
+ * @brief Get file descriptor of the entity sub-device
+ * @param entity - media entity
+ *
+ * This function gets the file descriptor of the opened
+ * sub-device node related to the entity.
+ *
+ * @return file descriptor of the opened sub-device,
+	   or -1 if the sub-device is closed
+ */
+int media_entity_get_fd(struct media_entity *entity);
+
+/**
+ * @brief Set file descriptor of the entity sub-device
+ * @param entity - media entity
+ * @param fd - entity sub-device file descriptor
+ *
+ * This function sets the file descriptor of the opened
+ * sub-device node related to the entity.
+ */
+void media_entity_set_fd(struct media_entity *entity, int fd);
+
 #endif
-- 
1.7.9.5


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

* [PATCH/RFC v4 02/11] mediactl: Add support for v4l2 controls
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 03/11] mediactl: Separate entity and pad parsing Jacek Anaszewski
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

Make struct v4l2_subdev capable of aggregating information
on its validated v4l2 controls. A control needs to be validated
for a v4l2_subdev in case it is mentioned in the media
configuraion file. Added are also functions for validating
controls and finding validated controls.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/libmediactl.c   |   11 +++++++
 utils/media-ctl/libv4l2subdev.c |   60 ++++++++++++++++++++++++++++++++++++++-
 utils/media-ctl/mediactl-priv.h |    3 ++
 utils/media-ctl/v4l2subdev.h    |   28 ++++++++++++++++++
 4 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index 53921f5..4c4ddbe 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -537,6 +537,12 @@ static int media_enum_entities(struct media_device *media)
 		entity->sd->fd = -1;
 		media->entities_count++;
 
+		entity->sd->v4l2_controls = malloc(sizeof(__u32));
+		if (entity->sd->v4l2_controls == NULL) {
+			ret = -ENOMEM;
+			break;
+		}
+
 		if (entity->info.flags & MEDIA_ENT_FL_DEFAULT) {
 			switch (entity->info.type) {
 			case MEDIA_ENT_T_DEVNODE_V4L:
@@ -707,6 +713,7 @@ void media_device_unref(struct media_device *media)
 		free(entity->links);
 		if (entity->sd->fd != -1)
 			close(entity->sd->fd);
+		free(entity->sd->v4l2_controls);
 		free(entity->sd);
 	}
 
@@ -732,6 +739,10 @@ int media_device_add_entity(struct media_device *media,
 	if (entity->sd == NULL)
 		return -ENOMEM;
 
+	entity->sd->v4l2_controls = malloc(sizeof(__u32));
+	if (entity->sd->v4l2_controls == NULL)
+		return -ENOMEM;
+
 	media->entities = entity;
 	media->entities_count++;
 
diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index 09e0081..4c5fb12 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -26,7 +26,6 @@
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -755,3 +754,62 @@ enum v4l2_mbus_pixelcode v4l2_subdev_string_to_pixelcode(const char *string,
 
 	return mbus_formats[i].code;
 }
+
+int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media,
+				   struct media_entity *entity,
+				   __u32 ctrl_id)
+{
+	struct v4l2_query_ext_ctrl queryctrl;
+	int ret;
+
+	if (v4l2_subdev_has_v4l2_control(media, entity, ctrl_id))
+		goto done;
+
+	ret = v4l2_subdev_open(entity);
+	if (ret < 0)
+		return ret;
+
+	/* Iterate through control ids */
+
+	queryctrl.id = ctrl_id;
+
+	ret = ioctl(entity->sd->fd, VIDIOC_QUERY_EXT_CTRL, &queryctrl);
+	if (ret < 0) {
+		media_dbg(media, "Control (0x%8.8x) not supported on entity %s\n",
+			  queryctrl.name,
+			  ctrl_id,
+			  entity->info.name);
+		return ret;
+	}
+
+	entity->sd->v4l2_controls = realloc(entity->sd->v4l2_controls,
+					    sizeof(*entity->sd->v4l2_controls) *
+					    (entity->sd->v4l2_controls_count + 1));
+	if (!entity->sd->v4l2_controls)
+		return -ENOMEM;
+
+	entity->sd->v4l2_controls[entity->sd->v4l2_controls_count] = ctrl_id;
+	++entity->sd->v4l2_controls_count;
+
+done:
+	media_dbg(media, "Validated control \"%s\" (0x%8.8x) on entity %s\n",
+		  queryctrl.name,
+		  ctrl_id,
+		  entity->info.name);
+
+	return 0;
+}
+
+bool v4l2_subdev_has_v4l2_control(struct media_device *media,
+				  struct media_entity *entity,
+				  int ctrl_id)
+{
+	struct v4l2_subdev *sd = entity->sd;
+	int i;
+
+	for (i = 0; i < sd->v4l2_controls_count; ++i)
+		if (sd->v4l2_controls[i] == ctrl_id)
+			return true;
+
+	return false;
+}
diff --git a/utils/media-ctl/mediactl-priv.h b/utils/media-ctl/mediactl-priv.h
index 4bcb1e0..fbf1989 100644
--- a/utils/media-ctl/mediactl-priv.h
+++ b/utils/media-ctl/mediactl-priv.h
@@ -41,6 +41,9 @@ struct media_entity {
 
 struct v4l2_subdev {
 	int fd;
+
+	__u32 *v4l2_controls;
+	unsigned int v4l2_controls_count;
 };
 
 struct media_device {
diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
index 1cb53ff..ac98b61 100644
--- a/utils/media-ctl/v4l2subdev.h
+++ b/utils/media-ctl/v4l2subdev.h
@@ -22,6 +22,7 @@
 #ifndef __SUBDEV_H__
 #define __SUBDEV_H__
 
+#include <stdbool.h>
 #include <linux/v4l2-subdev.h>
 
 struct media_entity;
@@ -255,4 +256,31 @@ const char *v4l2_subdev_pixelcode_to_string(enum v4l2_mbus_pixelcode code);
  */
 enum v4l2_mbus_pixelcode v4l2_subdev_string_to_pixelcode(const char *string,
 							 unsigned int length);
+
+/**
+ * @brief Validate v4l2 control for a sub-device
+ * @param media - media device.
+ * @param entity - subdev-device media entity.
+ * @param ctrl_id - v4l2 control identifier
+ *
+ * Verify if the entity supports v4l2-control with ctrl_id.
+ *
+ * @return 1 if the control is supported, 0 otherwise.
+ */
+int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media,
+	struct media_entity *entity, __u32 ctrl_id);
+
+/**
+ * @brief Check if the sub-device has a validated control
+ * @param media - media device.
+ * @param entity - subdev-device media entity.
+ * @param ctrl_id - v4l2 control identifier
+ *
+ * Check if the sub-device has validated v4l2-control.
+ *
+ * @return true on success, false otherwise
+ */
+bool v4l2_subdev_has_v4l2_control(struct media_device *media,
+	struct media_entity *entity, int ctrl_id);
+
 #endif
-- 
1.7.9.5


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

* [PATCH/RFC v4 03/11] mediactl: Separate entity and pad parsing
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 02/11] mediactl: Add support for v4l2 controls Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 04/11] mediatext: Add library Jacek Anaszewski
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

Sometimes it's useful to be able to parse the entity independent of the pad.
Separate entity parsing into media_parse_entity().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 utils/media-ctl/libmediactl.c |   28 ++++++++++++++++++++++++----
 utils/media-ctl/mediactl.h    |   14 ++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index 4c4ddbe..af7dd43 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -787,10 +787,10 @@ int media_device_add_entity(struct media_device *media,
 	return 0;
 }
 
-struct media_pad *media_parse_pad(struct media_device *media,
-				  const char *p, char **endp)
+struct media_entity *media_parse_entity(struct media_device *media,
+					const char *p, char **endp)
 {
-	unsigned int entity_id, pad;
+	unsigned int entity_id;
 	struct media_entity *entity;
 	char *end;
 
@@ -827,7 +827,27 @@ struct media_pad *media_parse_pad(struct media_device *media,
 			return NULL;
 		}
 	}
-	for (; isspace(*end); ++end);
+	for (p = end; isspace(*p); ++p);
+
+	*endp = (char *)p;
+
+	return entity;
+}
+
+struct media_pad *media_parse_pad(struct media_device *media,
+				  const char *p, char **endp)
+{
+	unsigned int pad;
+	struct media_entity *entity;
+	char *end;
+
+	if (endp == NULL)
+		endp = &end;
+
+	entity = media_parse_entity(media, p, &end);
+	if (!entity)
+		return NULL;
+	*endp = end;
 
 	if (*end != ':') {
 		media_dbg(media, "Expected ':'\n", *end);
diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
index b8cefe8..7309b16 100644
--- a/utils/media-ctl/mediactl.h
+++ b/utils/media-ctl/mediactl.h
@@ -368,6 +368,20 @@ int media_setup_link(struct media_device *media,
 int media_reset_links(struct media_device *media);
 
 /**
+ * @brief Parse string to an entity on the media device.
+ * @param media - media device.
+ * @param p - input string
+ * @param endp - pointer to string where parsing ended
+ *
+ * Parse NULL terminated string describing an entity and return its
+ * struct media_entity instance.
+ *
+ * @return Pointer to struct media_entity on success, NULL on failure.
+ */
+struct media_entity *media_parse_entity(struct media_device *media,
+					const char *p, char **endp);
+
+/**
  * @brief Parse string to a pad on the media device.
  * @param media - media device.
  * @param p - input string
-- 
1.7.9.5


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

* [PATCH/RFC v4 04/11] mediatext: Add library
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (2 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 03/11] mediactl: Separate entity and pad parsing Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers Jacek Anaszewski
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski,
	Teemu Tuominen

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 13927 bytes --]

libmediatext is a helper library for converting configurations (Media
controller links, V4L2 controls and V4L2 sub-device media bus formats and
selections) from text-based form into IOCTLs.

libmediatext depends on libv4l2subdev and libmediactl.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Teemu Tuominen <teemu.tuominen@intel.com>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 utils/media-ctl/Makefile.am        |   10 +-
 utils/media-ctl/libmediatext.pc.in |   10 ++
 utils/media-ctl/mediatext-test.c   |   66 +++++++++
 utils/media-ctl/mediatext.c        |  286 ++++++++++++++++++++++++++++++++++++
 utils/media-ctl/mediatext.h        |   52 +++++++
 5 files changed, 422 insertions(+), 2 deletions(-)
 create mode 100644 utils/media-ctl/libmediatext.pc.in
 create mode 100644 utils/media-ctl/mediatext-test.c
 create mode 100644 utils/media-ctl/mediatext.c
 create mode 100644 utils/media-ctl/mediatext.h

diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
index a3931fb..3e883e0 100644
--- a/utils/media-ctl/Makefile.am
+++ b/utils/media-ctl/Makefile.am
@@ -1,4 +1,4 @@
-noinst_LTLIBRARIES = libmediactl.la libv4l2subdev.la
+noinst_LTLIBRARIES = libmediactl.la libv4l2subdev.la libmediatext.la
 
 libmediactl_la_SOURCES = libmediactl.c mediactl-priv.h
 libmediactl_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
@@ -9,9 +9,15 @@ libv4l2subdev_la_LIBADD = libmediactl.la
 libv4l2subdev_la_CFLAGS = -static
 libv4l2subdev_la_LDFLAGS = -static
 
+libmediatext_la_SOURCES = mediatext.c
+libmediatext_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
+libmediatext_la_LDFLAGS = -static $(LIBUDEV_LIBS)
+
 mediactl_includedir=$(includedir)/mediactl
 noinst_HEADERS = mediactl.h v4l2subdev.h
 
-bin_PROGRAMS = media-ctl
+bin_PROGRAMS = media-ctl mediatext-test
 media_ctl_SOURCES = media-ctl.c options.c options.h tools.h
 media_ctl_LDADD = libmediactl.la libv4l2subdev.la
+mediatext_test_SOURCES = mediatext-test.c
+mediatext_test_LDADD = libmediatext.la libmediactl.la libv4l2subdev.la
diff --git a/utils/media-ctl/libmediatext.pc.in b/utils/media-ctl/libmediatext.pc.in
new file mode 100644
index 0000000..6aa6353
--- /dev/null
+++ b/utils/media-ctl/libmediatext.pc.in
@@ -0,0 +1,10 @@
+prefix=@prefix@
+exec_prefix=@exec_prefix@
+libdir=@libdir@
+includedir=@includedir@
+
+Name: libmediatext
+Description: Media controller and V4L2 text-based configuration library
+Version: @PACKAGE_VERSION@
+Cflags: -I${includedir}
+Libs: -L${libdir} -lmediatext
diff --git a/utils/media-ctl/mediatext-test.c b/utils/media-ctl/mediatext-test.c
new file mode 100644
index 0000000..29ed38b
--- /dev/null
+++ b/utils/media-ctl/mediatext-test.c
@@ -0,0 +1,66 @@
+/*
+ * libmediatext test program
+ *
+ * Copyright (C) 2013 Intel Corporation
+ *
+ * Contact: Sakari Ailus <sakari.ailus@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "mediactl.h"
+#include "mediatext.h"
+
+int main(int argc, char *argv[])
+{
+	struct media_device *device;
+	int rval;
+
+	if (argc != 3) {
+		fprintf(stderr, "usage: %s <media device> <string>\n\n", argv[0]);
+		fprintf(stderr, "\tstring := [ v4l2-ctrl | v4l2-mbus | link-reset | link-conf]\n\n");
+		fprintf(stderr, "\tv4l2-ctrl := \"entity\" ctrl_type ctrl_id ctrl_value\n");
+		fprintf(stderr, "\tctrl_type := [ int | int64 | bitmask ]\n");
+		fprintf(stderr, "\tctrl_value := [ %%d | %%PRId64 | bitmask_value ]\n");
+		fprintf(stderr, "\tbitmask_value := b<binary_number>\n\n");
+		fprintf(stderr, "\tv4l2-mbus := \n");
+		fprintf(stderr, "\tlink-conf := \"entity\":pad -> \"entity\":pad[link-flags]\n");
+		fprintf(stderr, "\tctrl-to-subdev-conf := ctrl_id -> \"entity\"\n");
+		return EXIT_FAILURE;
+	}
+
+	device = media_device_new(argv[1]);
+	if (!device)
+		return EXIT_FAILURE;
+
+	media_debug_set_handler(device, (void (*)(void *, ...))fprintf, stdout);
+
+	rval = media_device_enumerate(device);
+	if (rval)
+		return EXIT_FAILURE;
+
+	rval = mediatext_parse(device, argv[2]);
+	if (rval) {
+		fprintf(stderr, "bad string %s (%s)\n", argv[2], strerror(-rval));
+		return EXIT_FAILURE;
+	}
+
+	media_device_unref(device);
+
+	return EXIT_SUCCESS;
+}
diff --git a/utils/media-ctl/mediatext.c b/utils/media-ctl/mediatext.c
new file mode 100644
index 0000000..7e816f9
--- /dev/null
+++ b/utils/media-ctl/mediatext.c
@@ -0,0 +1,286 @@
+/*
+ * Media controller text-based configuration library
+ *
+ * Copyright (C) 2013 Intel Corporation
+ *
+ * Contact: Sakari Ailus <sakari.ailus@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <sys/ioctl.h>
+
+#include <ctype.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+
+#include <linux/types.h>
+
+#include "mediactl.h"
+#include "mediactl-priv.h"
+#include "tools.h"
+#include "v4l2subdev.h"
+
+struct parser {
+	char *prefix;
+	int (*parse)(struct media_device *media, const struct parser *p,
+		     char *string);
+	struct parser *next;
+	bool no_args;
+};
+
+static int parse(struct media_device *media, const struct parser *p, char *string)
+{
+	for (; p->prefix; p++) {
+		size_t len = strlen(p->prefix);
+
+		if (strncmp(p->prefix, string, len))
+			continue;
+
+		string += len;
+
+		for (; isspace(*string); string++);
+
+		if (p->no_args)
+			return p->parse(media, p->next, NULL);
+
+		if (strlen(string) == 0)
+			return -ENOEXEC;
+
+		return p->parse(media, p->next, string);
+	}
+
+	media_dbg(media, "Unknown parser prefix\n");
+
+	return -ENOENT;
+}
+
+struct ctrl_type {
+	uint32_t type;
+	char *str;
+} ctrltypes[] = {
+	{ V4L2_CTRL_TYPE_INTEGER, "int" },
+	{ V4L2_CTRL_TYPE_MENU, "menu" },
+	{ V4L2_CTRL_TYPE_INTEGER_MENU, "intmenu" },
+	{ V4L2_CTRL_TYPE_BITMASK, "bitmask" },
+	{ V4L2_CTRL_TYPE_INTEGER64, "int64" },
+};
+
+static int parse_v4l2_ctrl_id(struct media_device *media, const struct parser *p,
+			      char *string, char **endp, __u32 *ctrl_id)
+{
+	int rval;
+
+	for (; isspace(*string); string++);
+	rval = sscanf(string, "0x%" PRIx32, ctrl_id);
+	if (rval <= 0)
+		return -EINVAL;
+
+	for (; !isspace(*string) && *string; string++);
+	for (; isspace(*string); string++);
+
+	*endp = string;
+
+	return 0;
+}
+
+/* adapted from yavta.c */
+static int parse_v4l2_ctrl(struct media_device *media, const struct parser *p,
+			   char *string)
+{
+	struct v4l2_ext_control ctrl = { 0 };
+	struct v4l2_ext_controls ctrls = { .count = 1,
+					   .controls = &ctrl };
+	int64_t val;
+	int rval;
+	struct media_entity *entity;
+	struct ctrl_type *ctype;
+	unsigned int i;
+
+	entity = media_parse_entity(media, string, &string);
+	if (!entity)
+		return -ENOENT;
+
+	for (i = 0; i < ARRAY_SIZE(ctrltypes); i++)
+		if (!strncmp(string, ctrltypes[i].str,
+			     strlen(ctrltypes[i].str)))
+			break;
+
+	if (i == ARRAY_SIZE(ctrltypes))
+		return -ENOENT;
+
+	ctype = &ctrltypes[i];
+
+	string += strlen(ctrltypes[i].str);
+
+	rval = parse_v4l2_ctrl_id(media, p, string, &string, &ctrl.id);
+	if (rval < 0)
+		return -EINVAL;
+
+	ctrls.ctrl_class = V4L2_CTRL_ID2CLASS(ctrl.id);
+
+	switch (ctype->type) {
+	case V4L2_CTRL_TYPE_BITMASK:
+		if (*string++ != 'b')
+			return -EINVAL;
+		while (*string == '1' || *string == '0') {
+			val <<= 1;
+			if (*string == '1')
+				val++;
+			string++;
+		}
+		break;
+	default:
+		rval = sscanf(string, "%" PRId64, &val);
+		break;
+	}
+	if (rval <= 0)
+		return -EINVAL;
+
+	media_dbg(media, "Setting control 0x%8.8x (type %s), value %" PRId64 "\n",
+		  ctrl.id, ctype->str, val);
+
+	if (ctype->type == V4L2_CTRL_TYPE_INTEGER64)
+		ctrl.value64 = val;
+	else
+		ctrl.value = val;
+
+	rval = v4l2_subdev_open(entity);
+	if (rval < 0)
+		return rval;
+
+	rval = ioctl(entity->sd->fd, VIDIOC_S_EXT_CTRLS, &ctrls);
+	if (ctype->type != V4L2_CTRL_TYPE_INTEGER64) {
+		if (rval != -1) {
+			ctrl.value64 = ctrl.value;
+		} else if (ctype->type != V4L2_CTRL_TYPE_STRING &&
+			   (errno == EINVAL || errno == ENOTTY)) {
+			struct v4l2_control old = { .id = ctrl.id,
+						    .value = val };
+
+			rval = ioctl(entity->sd->fd, VIDIOC_S_CTRL, &old);
+			if (rval != -1)
+				ctrl.value64 = old.value;
+		}
+	}
+	if (rval == -1) {
+		media_dbg(media,
+			  "Failed setting control 0x%8.8x: %s (%d) to value %"
+			  PRId64 "\n", ctrl.id, strerror(errno), errno, val);
+		return -errno;
+	}
+
+	if (val != ctrl.value64)
+		media_dbg(media, "Asking for %" PRId64 ", got %" PRId64 "\n",
+			  val, ctrl.value64);
+
+	return 0;
+}
+
+int parse_ctrl_to_subdev_conf(struct media_device *media, const struct parser *p,
+			   char *string)
+{
+	struct media_entity *entity;
+	__u32 ctrl_id;
+	int rval;
+
+	media_dbg(media, "Configuring v4l2-control target: %s\n", string);
+
+	rval = parse_v4l2_ctrl_id(media, p, string, &string, &ctrl_id);
+	if (rval < 0)
+		return -EINVAL;
+
+	if (string[0] != '-' || string[1] != '>') {
+		media_dbg(media, "Expected '->'\n");
+		return -EINVAL;
+	}
+
+	string += 2;
+
+	entity = media_parse_entity(media, string, &string);
+	if (!entity)
+		return -ENOENT;
+
+	return v4l2_subdev_validate_v4l2_ctrl(media, entity, ctrl_id);
+}
+
+static int parse_v4l2_mbus(struct media_device *media, const struct parser *p,
+			   char *string)
+{
+	media_dbg(media, "Media bus format setup: %s\n", string);
+	return v4l2_subdev_parse_setup_formats(media, string);
+}
+
+static int parse_link_reset(struct media_device *media, const struct parser *p,
+			    char *string)
+{
+	media_dbg(media, "Resetting links\n");
+	return media_reset_links(media);
+}
+
+static int parse_link_conf(struct media_device *media, const struct parser *p,
+			   char *string)
+{
+	media_dbg(media, "Configuring links: %s\n", string);
+	return media_parse_setup_links(media, string);
+}
+
+static const struct parser parsers[] = {
+	{ "v4l2-ctrl", parse_v4l2_ctrl },
+	{ "ctrl-to-subdev-conf", parse_ctrl_to_subdev_conf },
+	{ "v4l2-mbus", parse_v4l2_mbus },
+	{ "link-reset", parse_link_reset, NULL, true },
+	{ "link-conf", parse_link_conf },
+	{ 0 }
+};
+
+int mediatext_parse(struct media_device *media, char *string)
+{
+	return parse(media, parsers, string);
+}
+
+int mediatext_parse_setup_config(struct media_device *device, const char *conf_path)
+{
+	char *line;
+	size_t n = 0;
+	FILE *f;
+	int ret;
+
+	if (conf_path == NULL)
+		return -EINVAL;
+
+	f = fopen(conf_path, "r");
+	if (!f)
+		return -EINVAL;
+
+	while (getline(&line, &n, f) != -1) {
+		ret = mediatext_parse(device, line);
+		if (ret < 0)
+			goto err_parse;
+		free(line);
+		line = NULL;
+		n = 0;
+	}
+
+err_parse:
+	fclose(f);
+	return ret;
+}
diff --git a/utils/media-ctl/mediatext.h b/utils/media-ctl/mediatext.h
new file mode 100644
index 0000000..7dfbaf6
--- /dev/null
+++ b/utils/media-ctl/mediatext.h
@@ -0,0 +1,52 @@
+/*
+ * Media controller text-based configuration library
+ *
+ * Copyright (C) 2013 Intel Corporation
+ *
+ * Contact: Sakari Ailus <sakari.ailus@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __MEDIATEXT_H__
+#define __MEDIATEXT_H__
+
+struct media_device;
+
+/**
+ * @brief Parse and apply media device command
+ * @param device - media device
+ * @param string - string to parse
+ *
+ * Parse media device command and apply it to the media device
+ * passed in the device argument.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int mediatext_parse(struct media_device *device, char *string);
+
+/**
+ * @brief Parse and apply media device configuration
+ * @param media - media device
+ * @param conf_path - path to the configuration file
+ *
+ * Parse the media device commands listed in the file under
+ * conf_path and apply them to the media device passed in the
+ * device argument.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int mediatext_parse_setup_config(struct media_device *device, const char *conf_path);
+
+#endif /* __MEDIATEXT_H__ */
-- 
1.7.9.5


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

* [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (3 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 04/11] mediatext: Add library Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-28 17:06   ` Sakari Ailus
  2014-11-21 16:14 ` [PATCH/RFC v4 06/11] mediactl: Add media_device creation helpers Jacek Anaszewski
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

Add new graph helpers useful for video pipeline discovering.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/libmediactl.c |  174 +++++++++++++++++++++++++++++++++++++++++
 utils/media-ctl/mediactl.h    |  121 ++++++++++++++++++++++++++++
 2 files changed, 295 insertions(+)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index af7dd43..a476601 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -35,6 +35,7 @@
 #include <unistd.h>
 
 #include <linux/media.h>
+#include <linux/kdev_t.h>
 #include <linux/videodev2.h>
 
 #include "mediactl.h"
@@ -87,6 +88,28 @@ struct media_entity *media_get_entity_by_name(struct media_device *media,
 	return NULL;
 }
 
+struct media_entity *media_get_entity_by_devname(struct media_device *media,
+					      const char *devname, size_t length)
+{
+	unsigned int i;
+
+	/* A match is impossible if the entity devname is longer than the maximum
+	 * size we can get from the kernel.
+	 */
+	if (length >= FIELD_SIZEOF(struct media_entity, devname))
+		return NULL;
+
+	for (i = 0; i < media->entities_count; ++i) {
+		struct media_entity *entity = &media->entities[i];
+
+		if (strncmp(entity->devname, devname, length) == 0 &&
+		    entity->devname[length] == '\0')
+			return entity;
+	}
+
+	return NULL;
+}
+
 struct media_entity *media_get_entity_by_id(struct media_device *media,
 					    __u32 id)
 {
@@ -145,6 +168,11 @@ const char *media_entity_get_devname(struct media_entity *entity)
 	return entity->devname[0] ? entity->devname : NULL;
 }
 
+const char *media_entity_get_name(struct media_entity *entity)
+{
+	return entity->info.name ? entity->info.name : NULL;
+}
+
 struct media_entity *media_get_default_entity(struct media_device *media,
 					      unsigned int type)
 {
@@ -177,6 +205,152 @@ const struct media_entity_desc *media_entity_get_info(struct media_entity *entit
 	return &entity->info;
 }
 
+int media_get_link_by_sink_pad(struct media_device *media,
+				struct media_pad *pad,
+				struct media_link **link)
+{
+	struct media_link *cur_link = NULL;
+	int i, j;
+
+	if (pad == NULL || link == NULL)
+		return -EINVAL;
+
+	for (i = 0; i < media->entities_count; ++i) {
+		for (j = 0; j < media->entities[i].num_links; ++j) {
+			cur_link = &media->entities[i].links[j];
+			if ((cur_link->flags & MEDIA_LNK_FL_ENABLED) &&
+			    /* check if cur_link's sink entity matches the pad parent entity */
+			    (cur_link->sink->entity->info.id == pad->entity->info.id) &&
+			    /* check if cur_link's sink pad id matches */
+			    (cur_link->sink->index == pad->index)) {
+				*link = cur_link;
+				return 0;
+			}
+		}
+	}
+
+	return -EINVAL;
+}
+
+int media_get_link_by_source_pad(struct media_entity *entity,
+				struct media_pad *pad,
+				struct media_link **link)
+{
+	int i;
+
+	if (entity == NULL || pad == NULL || link == NULL)
+		return -EINVAL;
+
+	for (i = 0; i < entity->num_links; ++i) {
+		if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) &&
+		    (entity->links[i].source->index == pad->index)) {
+			*link = &entity->links[i];
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+int media_get_pads_by_entity(struct media_entity *entity, unsigned int type,
+				struct media_pad **pads, int *num_pads)
+{
+	struct media_pad *entity_pads;
+	int cnt_pads, i;
+
+	if (entity == NULL || pads == NULL || num_pads == NULL)
+		return -EINVAL;
+
+	entity_pads = malloc(sizeof(*entity_pads));
+	cnt_pads = 0;
+
+	for (i = 0; i < entity->info.pads; ++i) {
+		if (entity->pads[i].flags & type) {
+			entity_pads = realloc(entity_pads, (i + 1) *
+					      sizeof(*entity_pads));
+			entity_pads[cnt_pads++] = entity->pads[i];
+		}
+	}
+
+	if (cnt_pads == 0)
+		free(entity_pads);
+
+	*pads = entity_pads;
+	*num_pads = cnt_pads;
+
+	return 0;
+}
+
+int media_get_busy_pads_by_entity(struct media_device *media,
+				struct media_entity *entity,
+				unsigned int type,
+				struct media_pad **busy_pads,
+				int *num_busy_pads)
+{
+	struct media_pad *bpads, *pads;
+	struct media_link *link;
+	int cnt_bpads = 0, num_pads, i, ret;
+
+	if (entity == NULL || busy_pads == NULL || num_busy_pads == NULL ||
+	    (type == MEDIA_PAD_FL_SINK && media == NULL))
+		return -EINVAL;
+
+	ret = media_get_pads_by_entity(entity, type, &pads, &num_pads);
+	if (ret < 0)
+		return -EINVAL;
+
+	if (num_pads == 0)
+		goto done;
+
+	bpads = malloc(sizeof(*pads));
+	if (bpads == NULL)
+		goto error_ret;
+
+	for (i = 0; i < num_pads; ++i) {
+		if (type == MEDIA_PAD_FL_SINK)
+			ret = media_get_link_by_sink_pad(media, &pads[i], &link);
+		else
+			ret = media_get_link_by_source_pad(entity, &pads[i], &link);
+		if (ret == 0) {
+			bpads = realloc(bpads, (i + 1) *
+						sizeof(*bpads));
+			bpads[cnt_bpads++] = pads[i];
+		}
+	}
+	if (num_pads > 0)
+		free(pads);
+
+	if (cnt_bpads == 0)
+		free(bpads);
+
+done:
+	*busy_pads = bpads;
+	*num_busy_pads = cnt_bpads;
+
+	return 0;
+
+error_ret:
+	return -EINVAL;
+}
+
+int media_get_pad_by_index(struct media_pad *pads, int num_pads,
+				int index, struct media_pad *out_pad)
+{
+	int i;
+
+	if (pads == NULL || out_pad == NULL)
+		return -EINVAL;
+
+	for (i = 0; i < num_pads; ++i) {
+		if (pads[i].index == index) {
+			*out_pad = pads[i];
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
 /* -----------------------------------------------------------------------------
  * Open/close
  */
diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
index 7309b16..18a1e0e 100644
--- a/utils/media-ctl/mediactl.h
+++ b/utils/media-ctl/mediactl.h
@@ -23,6 +23,7 @@
 #define __MEDIA_H__
 
 #include <linux/media.h>
+#include <sys/types.h>
 
 struct media_link {
 	struct media_pad *source;
@@ -231,6 +232,16 @@ const struct media_link *media_entity_get_link(struct media_entity *entity,
 const char *media_entity_get_devname(struct media_entity *entity);
 
 /**
+ * @brief Get the name for an entity
+ * @param entity - media entity.
+ *
+ * This function returns the name of the entity.
+ *
+ * @return A pointer to the string with entity name
+ */
+const char *media_entity_get_name(struct media_entity *entity);
+
+/**
  * @brief Get the type of an entity.
  * @param entity - the entity.
  *
@@ -255,6 +266,19 @@ struct media_entity *media_get_entity_by_name(struct media_device *media,
 	const char *name, size_t length);
 
 /**
+ * @brief Find an entity by the corresponding device node name.
+ * @param media - media device.
+ * @param devname - device node name.
+ * @param length - size of @a devname.
+ *
+ * Search for an entity with a device node name equal to @a devname.
+ *
+ * @return A pointer to the entity if found, or NULL otherwise.
+ */
+struct media_entity *media_get_entity_by_devname(struct media_device *media,
+	const char *devname, size_t length);
+
+/**
  * @brief Find an entity by its ID.
  * @param media - media device.
  * @param id - entity ID.
@@ -435,6 +459,103 @@ int media_parse_setup_link(struct media_device *media,
 int media_parse_setup_links(struct media_device *media, const char *p);
 
 /**
+ * @brief Get entity's pads of a given type
+ * @param entity - media entity
+ * @param type - pad type (MEDIA_PAD_FL_SINK or MEDIA_PAD_FL_SOURCE)
+ * @param pads - array of matching pads
+ * @param num_pads - number of matching pads
+ *
+ * Get only sink or source pads for an entity. The returned pads
+ * array has to be freed by the caller.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_get_pads_by_entity(struct media_entity *entity,
+				unsigned int type,
+				struct media_pad **pads,
+				int *num_pads);
+/**
+ * @brief Get occupied entity's pads of a given type
+ * @param media - media device
+ * @param entity - media entity
+ * @param type - pad type (MEDIA_PAD_FL_SINK or MEDIA_PAD_FL_SOURCE)
+ * @param busy_pads - array of matching pads
+ * @param num_busy_pads - number of matching pads
+ *
+ * Get only sink or source pads for an entity, with active links.
+ * The returned pads array has to be freed by the caller.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_get_busy_pads_by_entity(struct media_device *media,
+				struct media_entity *entity,
+				unsigned int type,
+				struct media_pad **busy_pads,
+				int *num_busy_pads);
+
+/**
+ * @brief Get link for  sink pad
+ * @param media - media device
+ * @param pad - pad to search the link for
+ * @param link - matching link
+ *
+ * Get the link connected to the entity's sink pad.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_get_link_by_sink_pad(struct media_device *media,
+				struct media_pad *pad,
+				struct media_link **link);
+
+/**
+ * @brief Get link for source pad
+ * @param entity - media entity
+ * @param pad - pad to search the link for
+ * @param link - matching link
+ *
+ * Get the link connected to the entity's source pad.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_get_link_by_source_pad(struct media_entity *entity,
+				struct media_pad *pad,
+				struct media_link **link);
+
+/**
+ * @brief Get pad with given index
+ * @param pads - array of pads
+ * @param num_pads - number of pads in the array
+ * @param index - index of a pad to search for
+ * @param out_pad - matching pad
+ *
+ * Get pad with given index from the given pads array.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_get_pad_by_index(struct media_pad *pads, int num_pads,
+				int index, struct media_pad *out_pad);
+
+/**
+ * @brief Get source pad of the pipeline entity
+ * @param entity - media entity
+ *
+ * This function returns the source pad of the entity.
+ *
+ * @return entity source pad, or NULL if the entity is not linked.
+ */
+int media_entity_get_src_pad_index(struct media_entity *entity);
+
+/**
+ * @brief Get sink pad of the pipeline entity
+ * @param entity - media entity
+ *
+ * This function returns the sink pad of the entity.
+ *
+ * @return entity sink pad, or NULL if the entity is not linked
+ */
+int media_entity_get_sink_pad_index(struct media_entity *entity);
+
+/**
  * @brief Get file descriptor of the entity sub-device
  * @param entity - media entity
  *
-- 
1.7.9.5


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

* [PATCH/RFC v4 06/11] mediactl: Add media_device creation helpers
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (4 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 07/11] media-ctl: libv4l2subdev: add VYUY8_2X8 mbus code Jacek Anaszewski
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

Add helper functions that allow for easy instantiation
of media_device object basing on whether the media device
contains video device with given node name.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/libmediactl.c |   75 +++++++++++++++++++++++++++++++++++++++++
 utils/media-ctl/mediactl.h    |   29 ++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index a476601..9c81711 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -853,6 +853,43 @@ struct media_device *media_device_new(const char *devnode)
 	return media;
 }
 
+struct media_device *media_device_new_by_entity_devname(char *entity_devname)
+{
+	struct media_device *media;
+	char media_devname[32];
+	int i, ret;
+
+	if (entity_devname == NULL)
+		return NULL;
+
+	/* query all available media devices */
+	for (i = 0;; ++i) {
+		sprintf(media_devname, "/dev/media%d", i);
+
+		media = media_device_new(media_devname);
+		if (media == NULL)
+			return NULL;
+
+		ret = media_device_enumerate(media);
+		if (ret < 0) {
+			media_dbg(media, "Failed to enumerate %s (%d)\n", media_devname, ret);
+			goto err_dev_enum;
+		}
+
+		/* Check if the media device contains entity with entity_devname */
+		if (media_get_entity_by_devname(media, entity_devname, strlen(entity_devname)))
+			return media;
+
+		if (media)
+			media_device_unref(media);
+	}
+
+err_dev_enum:
+	if (media)
+		media_device_unref(media);
+	return NULL;
+}
+
 struct media_device *media_device_new_emulated(struct media_device_info *info)
 {
 	struct media_device *media;
@@ -896,6 +933,44 @@ void media_device_unref(struct media_device *media)
 	free(media);
 }
 
+int media_get_devname_by_fd(int fd, char *node_name)
+{
+	struct udev *udev;
+	struct media_entity tmp_entity;
+	struct stat stat;
+	int ret;
+
+	if (node_name == NULL)
+		return -EINVAL;
+
+	ret = fstat(fd, &stat);
+	if (ret < 0)
+		return -EINVAL;
+
+	tmp_entity.info.v4l.major = MAJOR(stat.st_rdev);
+	tmp_entity.info.v4l.minor = MINOR(stat.st_rdev);
+
+	ret = media_udev_open(&udev);
+	if (ret < 0)
+		printf("Can't get udev context\n");
+
+	/* Try to get the device name via udev */
+	ret = media_get_devname_udev(udev, &tmp_entity);
+	if (!ret)
+		goto out;
+
+	ret = media_get_devname_sysfs(&tmp_entity);
+	if (ret < 0)
+		goto err_get_devname;
+
+out:
+	strcpy(node_name, tmp_entity.devname);
+err_get_devname:
+	media_udev_close(udev);
+	return ret;
+}
+
+
 int media_device_add_entity(struct media_device *media,
 			    const struct media_entity_desc *desc,
 			    const char *devnode)
diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
index 18a1e0e..7f16097 100644
--- a/utils/media-ctl/mediactl.h
+++ b/utils/media-ctl/mediactl.h
@@ -77,6 +77,23 @@ struct media_device *media_device_new(const char *devnode);
 struct media_device *media_device_new_emulated(struct media_device_info *info);
 
 /**
+ * @brief Create a new media device if it comprises entity with entity_devname
+ * @param entity_devname - device node name of the entity to be matched
+ *
+ * Query all media devices available in the system to find the one comprising
+ * the entity with device node name equal to entity_devname. If the media
+ * device is matched then its instance is created and initialized with
+ * enumerated entities and links. The returned device can be accessed.
+ *
+ * Media devices are reference-counted, see media_device_ref() and
+ * media_device_unref() for more information.
+ *
+ * @return A pointer to the new media device or NULL if video_devname cannot
+ * be matched or memory cannot be allocated.
+ */
+struct media_device *media_device_new_by_entity_devname(char *video_devname);
+
+/**
  * @brief Take a reference to the device.
  * @param media - device instance.
  *
@@ -242,6 +259,18 @@ const char *media_entity_get_devname(struct media_entity *entity);
 const char *media_entity_get_name(struct media_entity *entity);
 
 /**
+ * @brief Get the device node name by its file descriptor
+ * @param fd - file descriptor of a device
+ * @param node_name - output device node name string
+ *
+ * This function returns the full path and name to the device node corresponding
+ * to the given file descriptor.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_get_devname_by_fd(int fd, char *node_name);
+
+/**
  * @brief Get the type of an entity.
  * @param entity - the entity.
  *
-- 
1.7.9.5


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

* [PATCH/RFC v4 07/11] media-ctl: libv4l2subdev: add VYUY8_2X8 mbus code
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (5 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 06/11] mediactl: Add media_device creation helpers Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-28 17:10   ` Sakari Ailus
  2014-11-21 16:14 ` [PATCH/RFC v4 08/11] mediactl: Add support for media device pipelines Jacek Anaszewski
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

The VYUY8_2X8 media bus format is the only one supported
by the S5C73M3 camera sensor, that is a part of the media
device on the Exynos4412-trats2 board.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/libv4l2subdev.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index 4c5fb12..a96ed7a 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -704,6 +704,7 @@ static struct {
 	{ "YUYV", V4L2_MBUS_FMT_YUYV8_1X16 },
 	{ "YUYV1_5X8", V4L2_MBUS_FMT_YUYV8_1_5X8 },
 	{ "YUYV2X8", V4L2_MBUS_FMT_YUYV8_2X8 },
+	{ "VYUY8_2X8", V4L2_MBUS_FMT_VYUY8_2X8 },
 	{ "UYVY", V4L2_MBUS_FMT_UYVY8_1X16 },
 	{ "UYVY1_5X8", V4L2_MBUS_FMT_UYVY8_1_5X8 },
 	{ "UYVY2X8", V4L2_MBUS_FMT_UYVY8_2X8 },
-- 
1.7.9.5


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

* [PATCH/RFC v4 08/11] mediactl: Add support for media device pipelines
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (6 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 07/11] media-ctl: libv4l2subdev: add VYUY8_2X8 mbus code Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 09/11] mediactl: Close only pipeline sub-devices Jacek Anaszewski
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

Add infrastructure for linking media entities,
discovering pipelines of media entities and
opening/closing all sub-devices in the pipeline
at one go.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/libmediactl.c   |  168 +++++++++++++++++++++++++++++++++++++++
 utils/media-ctl/libv4l2subdev.c |  109 +++++++++++++++++++++++++
 utils/media-ctl/mediactl-priv.h |    7 ++
 utils/media-ctl/mediactl.h      |   70 ++++++++++++++++
 utils/media-ctl/v4l2subdev.h    |   42 ++++++++++
 5 files changed, 396 insertions(+)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index 9c81711..003902b 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -1243,9 +1243,172 @@ int media_parse_setup_links(struct media_device *media, const char *p)
 }
 
 /* -----------------------------------------------------------------------------
+ * Pipeline operations
+ */
+
+int media_discover_pipeline_by_entity(struct media_device *media,
+				      struct media_entity *entity)
+{
+	struct media_entity *pipe_head = NULL;
+	struct media_pad *sink_pads, sink_pad, *src_pad;
+	struct media_link *link = NULL;
+	int num_sink_pads, ret;
+
+	if (entity == NULL)
+		return -EINVAL;
+
+	for (;;) {
+		/* Make recently discovered entity the pipeline head */
+		if (pipe_head == NULL) {
+			pipe_head = entity;
+		} else {
+			entity->next = pipe_head;
+			pipe_head = entity;
+		}
+
+		/* Cache a source pad used for linking the entity */
+		if (link)
+			entity->pipe_src_pad = link->source;
+
+		ret = media_get_busy_pads_by_entity(media, entity,
+						    MEDIA_PAD_FL_SINK,
+						    &sink_pads,
+						    &num_sink_pads);
+		if (ret < 0)
+			return ret;
+
+		/* check if pipeline source entity has been reached */
+		if (num_sink_pads > 2) {
+			media_dbg(media, "Unexpected number of busy sink pads (%d)\n", num_sink_pads);
+			goto err_check_sink_pads;
+		} else if (num_sink_pads == 2) {
+			/* Allow two active pads only in case of S5C73M3-OIF entity */
+			if (strcmp(entity->info.name, "S5C73M3-OIF")) {
+				media_dbg(media, "Ambiguous media device topology: two busy sink pads");
+				goto err_check_sink_pads;
+			}
+			/*
+			 * Two active links are allowed betwen S5C73M3-OIF and S5C73M3 entities.
+			 * In such a case a route through pad 0 has to be selected.
+			 */
+			ret = media_get_pad_by_index(sink_pads, num_sink_pads,
+							0, &sink_pad);
+			if (ret < 0)
+				goto err_check_sink_pads;
+		} else if (num_sink_pads == 1) {
+			sink_pad = sink_pads[0];
+		} else {
+			break;
+		}
+
+		free(sink_pads);
+
+		ret = media_get_link_by_sink_pad(media, &sink_pad, &link);
+		if (ret < 0)
+			return -EINVAL;
+
+		/* Cache a sink pad used for linking the entity */
+		entity->pipe_sink_pad = link->sink;
+
+		media_dbg(media, "Discovered sink pad %d for the %s entity\n",
+			  entity->pipe_sink_pad->index, media_entity_get_name(entity));
+
+		src_pad = media_entity_remote_source(link->sink);
+		if (!src_pad)
+			return -EINVAL;
+
+		entity = src_pad->entity;
+	}
+
+	media->pipeline = pipe_head;
+
+	return 0;
+
+err_check_sink_pads:
+	free(sink_pads);
+	return -EINVAL;
+}
+
+int media_has_pipeline_entity(struct media_entity *pipeline, char *entity_name)
+{
+	if (pipeline == NULL || entity_name == NULL)
+		return -EINVAL;
+
+	while (pipeline) {
+		if (!strcmp(pipeline->info.name, entity_name))
+			return 1;
+		pipeline = pipeline->next;
+	}
+
+	return 0;
+}
+
+int media_open_pipeline_subdevs(struct media_device *media)
+{
+	struct media_entity *entity = media->pipeline;
+
+	if (entity == NULL)
+		return 0;
+
+	while (entity->next) {
+		media_dbg(media, "Opening sub-device: %s\n", entity->devname);
+		entity->sd->fd = open(entity->devname, O_RDWR);
+
+		if (entity->sd->fd < 0) {
+			media_dbg(media, "Could not open device %s", entity->devname);
+			goto err_open_subdev;
+		}
+
+		entity = entity->next;
+	}
+
+	return 0;
+
+err_open_subdev:
+	media_close_pipeline_subdevs(media);
+
+	return -EINVAL;
+}
+
+void media_close_pipeline_subdevs(struct media_device *media)
+{
+	struct media_entity *entity = media->pipeline;
+
+	if (entity == NULL)
+		return;
+
+	while (entity->next) {
+		if (entity->sd->fd >= 0) {
+			media_dbg(media, "Closing sub-device: %s\n", entity->devname);
+			close(entity->sd->fd);
+		} else {
+			break;
+		}
+
+		entity->sd->fd = -1;
+		entity = entity->next;
+	}
+}
+
+/* -----------------------------------------------------------------------------
  * Media entity access
  */
 
+struct media_entity *media_get_pipeline(struct media_device *media)
+{
+	return media->pipeline;
+}
+
+int media_entity_get_src_pad_index(struct media_entity *entity)
+{
+	return entity->pipe_src_pad->index;
+}
+
+int media_entity_get_sink_pad_index(struct media_entity *entity)
+{
+	return entity->pipe_sink_pad->index;
+}
+
 int media_entity_get_fd(struct media_entity *entity)
 {
 	return entity->sd->fd;
@@ -1255,3 +1418,8 @@ void media_entity_set_fd(struct media_entity *entity, int fd)
 {
 	entity->sd->fd = fd;
 }
+
+struct media_entity *media_entity_get_next(struct media_entity *entity)
+{
+	return entity->next;
+}
diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
index a96ed7a..458923b 100644
--- a/utils/media-ctl/libv4l2subdev.c
+++ b/utils/media-ctl/libv4l2subdev.c
@@ -108,6 +108,67 @@ int v4l2_subdev_set_format(struct media_entity *entity,
 	return 0;
 }
 
+int v4l2_subdev_apply_pipeline_fmt(struct media_device *media,
+				   struct v4l2_format *fmt)
+{
+	struct v4l2_mbus_framefmt mbus_fmt = { 0 };
+	struct media_entity *entity = media->pipeline;
+	struct media_pad *pad;
+	int ret;
+
+	while (entity) {
+		/*
+		 * Source entity is linked only through a source pad
+		 * and this pad should be used for setting the format.
+		 * For other entities set the format on a sink pad.
+		 */
+		pad = entity->pipe_sink_pad ? entity->pipe_sink_pad :
+					      entity->pipe_src_pad;
+		if (pad == NULL)
+			return -EINVAL;
+
+		ret = v4l2_subdev_get_format(entity, &mbus_fmt, pad->index,
+					     V4L2_SUBDEV_FORMAT_TRY);
+
+		if (ret < 0)
+			return ret;
+
+		media_dbg(media, "Format read from entity pad %s:%d: mbus_code: %s, width: %d, height: %d\n",
+			  media_entity_get_name(entity), pad->index,
+			  v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
+			  mbus_fmt.width, mbus_fmt.height);
+
+		ret = v4l2_subdev_set_format(entity, &mbus_fmt, pad->index,
+					     V4L2_SUBDEV_FORMAT_ACTIVE);
+		if (ret < 0)
+			return ret;
+
+		media_dbg(media, "Format set on the entity pad %s:%d: mbus_code: %s, width: %d, height: %d\n",
+			  media_entity_get_name(entity), pad->index,
+			  v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
+			  mbus_fmt.width, mbus_fmt.height);
+
+		entity = entity->next;
+
+		/* Last entity in the pipeline is not a sub-device */
+		if (entity->next == NULL)
+			break;
+	}
+
+	/*
+	 * Sink entity represents a video device node and is not
+	 * a sub-device. Nonetheless because it has associated
+	 * file descriptor and can expose v4l2-controls the
+	 * v4l2-subdev structure is used for caching the
+	 * related data.
+	 */
+	ret = ioctl(entity->sd->fd, VIDIOC_S_FMT, fmt);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 int v4l2_subdev_get_selection(struct media_entity *entity,
 	struct v4l2_rect *rect, unsigned int pad, unsigned int target,
 	enum v4l2_subdev_format_whence which)
@@ -801,6 +862,40 @@ done:
 	return 0;
 }
 
+int v4l2_subdev_format_compare(struct v4l2_mbus_framefmt *fmt1,
+				struct v4l2_mbus_framefmt *fmt2)
+{
+	if (fmt1 == NULL || fmt2 == NULL)
+		return 0;
+
+	if (fmt1->width != fmt2->width) {
+		printf("width mismatch\n");
+		return 0;
+	}
+
+	if (fmt1->height != fmt2->height) {
+		printf("height mismatch\n");
+		return 0;
+	}
+
+	if (fmt1->code != fmt2->code) {
+		printf("mbus code mismatch\n");
+		return 0;
+	}
+
+	if (fmt1->field != fmt2->field) {
+		printf("field mismatch\n");
+		return 0;
+	}
+
+	if (fmt1->colorspace != fmt2->colorspace) {
+		printf("colorspace mismatch\n");
+		return 0;
+	}
+
+	return 1;
+}
+
 bool v4l2_subdev_has_v4l2_control(struct media_device *media,
 				  struct media_entity *entity,
 				  int ctrl_id)
@@ -814,3 +909,17 @@ bool v4l2_subdev_has_v4l2_control(struct media_device *media,
 
 	return false;
 }
+
+struct media_entity *v4l2_subdev_get_pipeline_entity_by_cid(struct media_device *media,
+						int cid)
+{
+	struct media_entity *entity = media->pipeline;
+
+	while (entity) {
+		if (v4l2_subdev_has_v4l2_control(media, entity, cid))
+			return entity;
+		entity = entity->next;
+	}
+
+	return NULL;
+}
diff --git a/utils/media-ctl/mediactl-priv.h b/utils/media-ctl/mediactl-priv.h
index fbf1989..0878776 100644
--- a/utils/media-ctl/mediactl-priv.h
+++ b/utils/media-ctl/mediactl-priv.h
@@ -34,9 +34,14 @@ struct media_entity {
 	unsigned int max_links;
 	unsigned int num_links;
 
+	struct media_pad *pipe_src_pad;
+	struct media_pad *pipe_sink_pad;
+
 	struct v4l2_subdev *sd;
 
 	char devname[32];
+
+	struct media_entity *next;
 };
 
 struct v4l2_subdev {
@@ -54,6 +59,8 @@ struct media_device {
 	struct media_device_info info;
 	struct media_entity *entities;
 	unsigned int entities_count;
+	struct media_entity *pipeline;
+
 
 	void (*debug_handler)(void *, ...);
 	void *debug_priv;
diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
index 7f16097..b0451b2 100644
--- a/utils/media-ctl/mediactl.h
+++ b/utils/media-ctl/mediactl.h
@@ -565,6 +565,32 @@ int media_get_pad_by_index(struct media_pad *pads, int num_pads,
 				int index, struct media_pad *out_pad);
 
 /**
+ * @brief Check presence of the entity in the pipeline
+ * @param pipeline - video pipeline within a media device
+ * @param entity_name - name of the entity to search for
+ *
+ * Check if the entity with entity_name belongs to
+ * the pipeline.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_has_pipeline_entity(struct media_entity *pipeline, char *entity_name);
+
+/**
+ * @brief Discover the video pipeline
+ * @param media - media device
+ * @param entity - media entity
+ *
+ * Discover the pipeline of sub-devices, by walking
+ * upstream starting from the passed sink entity until
+ * the camera sensor entity is encountered.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_discover_pipeline_by_entity(struct media_device *media,
+				struct media_entity *entity);
+
+/**
  * @brief Get source pad of the pipeline entity
  * @param entity - media entity
  *
@@ -606,4 +632,48 @@ int media_entity_get_fd(struct media_entity *entity);
  */
 void media_entity_set_fd(struct media_entity *entity, int fd);
 
+/**
+ * @brief Get next entity in the pipeline
+ * @param entity - media entity
+ *
+ * This function gets the entity connected to a
+ * source pad of this entity.
+ *
+ * @return next enetity in the pipeline,
+ *	   or NULL if the entity is not linked
+ */
+struct media_entity *media_entity_get_next(struct media_entity *entity);
+
+/**
+ * @brief Get the video pipeline
+ * @param media - media device
+ *
+ * This function gets the pipeline of media entities. The pipeline
+ * source entity is a camera sensor and the sink one is the entity
+ * representing opened video device node. The pipeline has to be
+ * discovered with use of the function media_discover_pipeline_by_entity.
+ *
+ * @return first media_entity in the pipeline,
+ *	   or NULL if the pipeline hasn't been discovered
+ */
+struct media_entity *media_get_pipeline(struct media_device *media);
+
+/**
+ * @brief Open pipeline sub-devices
+ * @param media - media device
+ *
+ * Open all sub-devices in the pipeline.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_open_pipeline_subdevs(struct media_device *media);
+
+/**
+ * @brief Close pipeline sub-devices
+ * @param media - media device
+ *
+ * Close all sub-devices in the pipeline.
+ */
+void media_close_pipeline_subdevs(struct media_device *media);
+
 #endif
diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
index ac98b61..bae53f0 100644
--- a/utils/media-ctl/v4l2subdev.h
+++ b/utils/media-ctl/v4l2subdev.h
@@ -89,6 +89,21 @@ int v4l2_subdev_set_format(struct media_entity *entity,
 	enum v4l2_subdev_format_whence which);
 
 /**
+ * @brief Set media device pipeline format
+ * @param entity - media entity
+ * @param fmt - negotiated format
+ *
+ * Set the active format on all the pipeline entities.
+ * The format has to be at first negotiated with VIDIOC_SUBDEV_S_FMT
+ * by struct v4l2_subdev_format's 'whence' property set to
+ * V4L2_SUBDEV_FORMAT_TRY.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int v4l2_subdev_apply_pipeline_fmt(struct media_device *media,
+				   struct v4l2_format *fmt);
+
+/**
  * @brief Retrieve a selection rectangle on a pad.
  * @param entity - subdev-device media entity.
  * @param r - rectangle to be filled.
@@ -271,6 +286,18 @@ int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media,
 	struct media_entity *entity, __u32 ctrl_id);
 
 /**
+ * @brief Compare mbus formats
+ * @param fmt1 - 1st mbus format to compare
+ * @param fmt2 - 2nd mbus format to compare
+ *
+ * Check whether two mbus formats are compatible.
+ *
+ * @return 1 if formats are compatible, 0 otherwise
+ */
+int v4l2_subdev_format_compare(struct v4l2_mbus_framefmt *fmt1,
+	struct v4l2_mbus_framefmt *fmt2);
+
+/**
  * @brief Check if the sub-device has a validated control
  * @param media - media device.
  * @param entity - subdev-device media entity.
@@ -283,4 +310,19 @@ int v4l2_subdev_validate_v4l2_ctrl(struct media_device *media,
 bool v4l2_subdev_has_v4l2_control(struct media_device *media,
 	struct media_entity *entity, int ctrl_id);
 
+/**
+ * @brief Get the first pipeline entity supporting the control
+ * @param media - media device
+ * @param cid - v4l2 control identifier
+ *
+ * Get the first entity in the media device pipeline,
+ * on which the ctrl with cid has been validated.
+ *
+ * @return associated entity if defined, or NULL when the
+ *	   control hasn't been validated on any entity
+ *	   in the pipeline
+ */
+struct media_entity *v4l2_subdev_get_pipeline_entity_by_cid(
+	struct media_device *media, int cid);
+
 #endif
-- 
1.7.9.5


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

* [PATCH/RFC v4 09/11] mediactl: Close only pipeline sub-devices
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (7 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 08/11] mediactl: Add support for media device pipelines Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 10/11] mediactl: Add media device ioctl API Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera Jacek Anaszewski
  10 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

The function media_device_new_by_entity_devname queries
media devices available in the system for containment
if given media entity. If a verification is negative
the media_device is released with media_device_unref.
In the previous approach media_device_unref was closing
all media entities it contained, which was undesirable
behavior as there might exist other initialized plugins
which had opened the same media_device and initialized
a pipeline. With this patch only the sub-devices that
belong to the pipeline of current media_device instance
will be closed.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/libmediactl.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index 003902b..9419fb4 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -917,13 +917,13 @@ void media_device_unref(struct media_device *media)
 	if (media->refcount > 0)
 		return;
 
+	media_close_pipeline_subdevs(media);
+
 	for (i = 0; i < media->entities_count; ++i) {
 		struct media_entity *entity = &media->entities[i];
 
 		free(entity->pads);
 		free(entity->links);
-		if (entity->sd->fd != -1)
-			close(entity->sd->fd);
 		free(entity->sd->v4l2_controls);
 		free(entity->sd);
 	}
-- 
1.7.9.5


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

* [PATCH/RFC v4 10/11] mediactl: Add media device ioctl API
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (8 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 09/11] mediactl: Close only pipeline sub-devices Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-21 16:14 ` [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera Jacek Anaszewski
  10 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

Ioctls executed on complex media devices need special
handling. E.g. S_FMT requires negotiation for the whole
pipeline of sub-devices. On the other hand some ioctls
need to be targeted for specific sub-devices. The API
being introduced address such requirements.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 utils/media-ctl/Makefile.am          |    2 +-
 utils/media-ctl/libv4l2media_ioctl.c |  325 ++++++++++++++++++++++++++++++++++
 utils/media-ctl/libv4l2media_ioctl.h |   40 +++++
 3 files changed, 366 insertions(+), 1 deletion(-)
 create mode 100644 utils/media-ctl/libv4l2media_ioctl.c
 create mode 100644 utils/media-ctl/libv4l2media_ioctl.h

diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
index 3e883e0..7f18624 100644
--- a/utils/media-ctl/Makefile.am
+++ b/utils/media-ctl/Makefile.am
@@ -1,6 +1,6 @@
 noinst_LTLIBRARIES = libmediactl.la libv4l2subdev.la libmediatext.la
 
-libmediactl_la_SOURCES = libmediactl.c mediactl-priv.h
+libmediactl_la_SOURCES = libmediactl.c mediactl-priv.h libv4l2media_ioctl.c libv4l2media_ioctl.h
 libmediactl_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
 libmediactl_la_LDFLAGS = -static $(LIBUDEV_LIBS)
 
diff --git a/utils/media-ctl/libv4l2media_ioctl.c b/utils/media-ctl/libv4l2media_ioctl.c
new file mode 100644
index 0000000..f2498e5
--- /dev/null
+++ b/utils/media-ctl/libv4l2media_ioctl.c
@@ -0,0 +1,325 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *              http://www.samsung.com
+ *
+ * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ */
+
+#include <errno.h>
+#include <unistd.h>
+#include <linux/videodev2.h>
+#include <sys/syscall.h>
+
+#include "libv4l2media_ioctl.h"
+#include "../../utils/media-ctl/v4l2subdev.h"
+#include "mediactl-priv.h"
+#include "mediactl.h"
+
+#define SYS_IOCTL(fd, cmd, arg) \
+	syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg))
+
+int media_ioctl_ctrl(struct media_device *media, int request,
+			struct v4l2_control *arg)
+{
+	struct media_entity *entity = media->pipeline;
+	struct v4l2_control ctrl = *arg;
+	struct v4l2_queryctrl queryctrl;
+	int ret = -EINVAL;
+
+	/*
+	 * The control has to be reset to the default value
+	 * on all of the pipeline entities, prior setting a new
+	 * value. This is required in cases when the control config
+	 * is changed between subsequent calls to VIDIOC_S_CTRL,
+	 * to avoid the situation when a control is set on more
+	 * than one sub-device.
+	 */
+	if (request == VIDIOC_S_CTRL) {
+		while (entity) {
+			queryctrl.id = ctrl.id;
+
+			ret = SYS_IOCTL(entity->sd->fd, VIDIOC_QUERYCTRL,
+								&queryctrl);
+			if (ret < 0) {
+				entity = entity->next;
+				continue;
+			}
+
+			ctrl.value = queryctrl.default_value;
+			ret = SYS_IOCTL(entity->sd->fd, VIDIOC_S_CTRL, &ctrl);
+			if (ret < 0)
+				return -EINVAL;
+
+			entity = entity->next;
+		}
+
+		ctrl.value = arg->value;
+	}
+
+	entity = v4l2_subdev_get_pipeline_entity_by_cid(media, ctrl.id);
+	if (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, request, &ctrl);
+		media_dbg(media, "Ioctl result for user control 0x%8.8x on %s: %d\n",
+			  ctrl.id, entity->info.name, ret);
+		goto exit;
+	}
+
+	media_dbg(media, "No config for control id 0x%8.8x\n", ctrl.id);
+
+	/* Walk the pipeline until the request succeeds */
+	entity = media->pipeline;
+
+	while (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, request, &ctrl);
+		if (!ret) {
+			media_dbg(media, "Ioctl succeeded for user control 0x%8.8x on %s\n",
+				  ctrl.id, entity->info.name);
+			goto exit;
+		}
+
+		entity = entity->next;
+	}
+
+	media_dbg(media, "Setting control 0x%8.8x failed\n", ctrl.id);
+
+exit:
+	*arg = ctrl;
+	return ret;
+}
+
+static int media_ioctl_single_ext_ctrl(struct media_device *media,
+				int request, struct v4l2_ext_controls *arg)
+{
+	struct media_entity *entity = media->pipeline;
+	struct v4l2_ext_controls ctrls = *arg;
+	struct v4l2_ext_control *ctrl;
+	struct v4l2_query_ext_ctrl queryctrl;
+	int ret = -EINVAL;
+
+	ctrl = &ctrls.controls[0];
+
+	/*
+	 * The control has to be reset to the default value
+	 * on all of the pipeline entities, prior setting a new
+	 * value. This is required in cases when the control config
+	 * is changed between subsequent calls to VIDIOC_S_EXT_CTRLS,
+	 * to avoid the situation when a control is set on more
+	 * than one sub-device.
+	 */
+	if (request == VIDIOC_S_EXT_CTRLS) {
+		while (entity) {
+			queryctrl.id = ctrl->id;
+
+			ret = SYS_IOCTL(entity->sd->fd, VIDIOC_QUERY_EXT_CTRL,
+								&queryctrl);
+			if (ret < 0) {
+				entity = entity->next;
+				continue;
+			}
+
+			ctrl->value64 = queryctrl.default_value;
+
+			ret = SYS_IOCTL(entity->sd->fd, VIDIOC_S_EXT_CTRLS, &ctrls);
+			if (ret < 0)
+				return -EINVAL;
+
+			entity = entity->next;
+		}
+
+		ctrl->value64 = arg->controls[0].value64;
+	}
+
+	entity = v4l2_subdev_get_pipeline_entity_by_cid(media, ctrl->id);
+	if (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, request, &ctrls);
+		media_dbg(media, "Ioctl result for extended control 0x%8.8x on %s: %d\n",
+			  ctrl->id, entity->info.name, ret);
+		goto exit;
+	}
+
+	media_dbg(media, "No config for control id 0x%8.8x\n", ctrl->id);
+
+	/* Walk the pipeline until the request succeeds */
+	entity = media->pipeline;
+
+	while (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, request, &ctrls);
+		if (!ret) {
+			media_dbg(media, "Ioctl succeeded for extended control 0x%8.8x on %s\n",
+				  ctrl->id, entity->info.name);
+			goto exit;
+		}
+
+
+		entity = entity->next;
+	}
+
+exit:
+	*arg = ctrls;
+	return ret;
+}
+
+int media_ioctl_ext_ctrl(struct media_device *media, int request,
+			struct v4l2_ext_controls *arg)
+{
+	struct v4l2_ext_controls out_ctrls = *arg, ctrls = *arg;
+	int ret = -EINVAL, i;
+
+	ctrls.count = 1;
+
+	/*
+	 * Split cluster to individual ioctl calls for each control
+	 * from the array, to make possible redirection of every
+	 * single control to different sub-device, according to the
+	 * configuration settings.
+	 */
+	for (i = 0; i < arg->count; ++i) {
+		ctrls.controls = &arg->controls[i];
+
+		ret = media_ioctl_single_ext_ctrl(media, request, &ctrls);
+		out_ctrls.controls[i] = ctrls.controls[i];
+		if (ret < 0) {
+			if (ctrls.error_idx == 1)
+				out_ctrls.error_idx = ctrls.count;
+			else
+				out_ctrls.error_idx = i;
+			goto exit;
+		}
+	}
+
+exit:
+	*arg = out_ctrls;
+	return ret;
+}
+
+int media_ioctl_queryctrl(struct media_device *media,
+				struct v4l2_queryctrl *arg)
+{
+	struct media_entity *entity = media->pipeline, *target_ent;
+	struct v4l2_queryctrl queryctrl = *arg;
+	int ret = -EINVAL;
+
+	while (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, VIDIOC_QUERYCTRL, &queryctrl);
+		if (!ret) {
+			/*
+			 * Control id might be or'ed with V4L2_CTRL_FLAG_NEXT_CTRL,
+			 * therefore the check for the config settings must be
+			 * done no sooner than after first successful ioctl execution
+			 * on a pipeline sub-device.
+			 */
+			target_ent = v4l2_subdev_get_pipeline_entity_by_cid(media, queryctrl.id);
+
+			/*
+			 * If the control is in the config, then
+			 * query the associated sub-device.
+			 */
+			if (target_ent) {
+				media_dbg(media, "Queryctrl: \"%s\" control found in the config\n",
+					  queryctrl.name);
+				/* Check if the control hasn't been already queried */
+				if (target_ent != entity)
+					ret = SYS_IOCTL(target_ent->sd->fd, VIDIOC_QUERYCTRL, &queryctrl);
+			}
+
+			media_dbg(media, "Queryctrl: \"%s\" control on %s (%d)\n",
+				  queryctrl.name, target_ent ? target_ent->info.name :
+							entity->info.name,
+							ret);
+			break;
+		}
+
+		entity = entity->next;
+	}
+
+	*arg = queryctrl;
+	return ret;
+}
+
+int media_ioctl_query_ext_ctrl(struct media_device *media,
+				struct v4l2_query_ext_ctrl *arg)
+{
+	struct media_entity *entity = media->pipeline, *target_ent;
+	struct v4l2_query_ext_ctrl query_ext_ctrl = *arg;
+	int ret = -EINVAL;
+
+	while (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, VIDIOC_QUERY_EXT_CTRL, &query_ext_ctrl);
+		if (!ret) {
+			/*
+			 * Control id might be or'ed with V4L2_CTRL_FLAG_NEXT_CTRL,
+			 * therefore the check for the config settings must be
+			 * done no sooner than after first successful ioctl execution
+			 * on a pipeline sub-device.
+			 */
+			target_ent = v4l2_subdev_get_pipeline_entity_by_cid(media, query_ext_ctrl.id);
+			/*
+			 * If the control is in the config, then
+			 * query the associated sub-device.
+			 */
+			if (target_ent) {
+				media_dbg(media, "Query ext control: \"%s\" found in config\n",
+					  query_ext_ctrl.name);
+				/* Check if the control hasn't been already queried */
+				if (target_ent != entity)
+					ret = SYS_IOCTL(target_ent->sd->fd, VIDIOC_QUERY_EXT_CTRL,
+							&query_ext_ctrl);
+			}
+
+			media_dbg(media, "Query ext control: \"%s\" on %s (%d)\n",
+				  query_ext_ctrl.name, target_ent ?
+							target_ent->info.name :
+							entity->info.name,
+							ret);
+			break;
+		}
+
+		entity = entity->next;
+	}
+
+	*arg = query_ext_ctrl;
+	return ret;
+}
+
+int media_ioctl_querymenu(struct media_device *media,
+				struct v4l2_querymenu *arg)
+{
+	struct media_entity *entity = media->pipeline;
+	struct v4l2_querymenu querymenu = *arg;
+	int ret = -EINVAL;
+
+	entity = v4l2_subdev_get_pipeline_entity_by_cid(media, querymenu.id);
+	if (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, VIDIOC_QUERYMENU, &querymenu);
+		media_dbg(media, "Querymenu result for the control 0x%8.8x on %s: %d\n",
+			  querymenu.id, entity->info.name, ret);
+		goto exit;
+	}
+
+	entity = media->pipeline;
+
+	while (entity) {
+		ret = SYS_IOCTL(entity->sd->fd, VIDIOC_QUERYMENU, &querymenu);
+		if (!ret) {
+			media_dbg(media, "Querymenu succeeded for the control 0x%8.8x on %s\n",
+				  querymenu.id, entity->info.name);
+			goto exit;
+		}
+
+		entity = entity->next;
+	}
+
+exit:
+	*arg = querymenu;
+	return ret;
+}
diff --git a/utils/media-ctl/libv4l2media_ioctl.h b/utils/media-ctl/libv4l2media_ioctl.h
new file mode 100644
index 0000000..08547d4
--- /dev/null
+++ b/utils/media-ctl/libv4l2media_ioctl.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *              http://www.samsung.com
+ *
+ * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ */
+
+#ifndef __LIBV4L2MEDIA_IOCTL_H
+#define __LIBV4L2MEDIA_IOCTL_H
+
+#include <linux/videodev2.h>
+
+struct media_device;
+
+int media_ioctl_ctrl(struct media_device *media, int request,
+			struct v4l2_control *arg);
+
+int media_ioctl_ext_ctrl(struct media_device *media, int request,
+			struct v4l2_ext_controls *arg);
+
+int media_ioctl_queryctrl(struct media_device *media,
+			struct v4l2_queryctrl *arg);
+
+int media_ioctl_query_ext_ctrl(struct media_device *media,
+			struct v4l2_query_ext_ctrl *arg);
+
+int media_ioctl_querymenu(struct media_device *media,
+			struct v4l2_querymenu *arg);
+
+#endif /* __LIBV4L2MEDIA_IOCTL_H */
-- 
1.7.9.5


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

* [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
  2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
                   ` (9 preceding siblings ...)
  2014-11-21 16:14 ` [PATCH/RFC v4 10/11] mediactl: Add media device ioctl API Jacek Anaszewski
@ 2014-11-21 16:14 ` Jacek Anaszewski
  2014-11-27  8:41   ` Sakari Ailus
                     ` (2 more replies)
  10 siblings, 3 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-21 16:14 UTC (permalink / raw)
  To: linux-media
  Cc: m.chehab, gjasny, hdegoede, hans.verkuil, b.zolnierkie,
	kyungmin.park, sakari.ailus, laurent.pinchart, Jacek Anaszewski

The plugin provides support for the media device on Exynos4 SoC.
It performs single plane <-> multi plane API conversion,
video pipeline linking and takes care of automatic data format
negotiation for the whole pipeline, after intercepting
VIDIOC_S_FMT or VIDIOC_TRY_FMT ioctls.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 configure.ac                                      |    1 +
 lib/Makefile.am                                   |    7 +-
 lib/libv4l-exynos4-camera/Makefile.am             |    7 +
 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c |  595 +++++++++++++++++++++
 4 files changed, 609 insertions(+), 1 deletion(-)
 create mode 100644 lib/libv4l-exynos4-camera/Makefile.am
 create mode 100644 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c

diff --git a/configure.ac b/configure.ac
index c9b0524..ae653b9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,6 +17,7 @@ AC_CONFIG_FILES([Makefile
 	lib/libdvbv5/Makefile
 	lib/libv4l2rds/Makefile
 	lib/libv4l-mplane/Makefile
+	lib/libv4l-exynos4-camera/Makefile
 
 	utils/Makefile
 	utils/libv4l2util/Makefile
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 3a0e19c..56b3a9f 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -5,7 +5,12 @@ SUBDIRS = \
 	libv4l2rds \
 	libv4l-mplane
 
+if WITH_V4LUTILS
+SUBDIRS += \
+	libv4l-exynos4-camera
+endif
+
 if LINUX_OS
 SUBDIRS += \
 	libdvbv5
-endif
\ No newline at end of file
+endif
diff --git a/lib/libv4l-exynos4-camera/Makefile.am b/lib/libv4l-exynos4-camera/Makefile.am
new file mode 100644
index 0000000..23c60c6
--- /dev/null
+++ b/lib/libv4l-exynos4-camera/Makefile.am
@@ -0,0 +1,7 @@
+if WITH_V4L_PLUGINS
+libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la
+endif
+
+libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c ../../utils/media-ctl/libv4l2media_ioctl.c ../../utils/media-ctl/mediatext.c
+libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99
+libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared -export-dynamic -lpthread
diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
new file mode 100644
index 0000000..119c75c
--- /dev/null
+++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
@@ -0,0 +1,595 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *              http://www.samsung.com
+ *
+ * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ */
+
+#include <config.h>
+#include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/syscall.h>
+#include <linux/types.h>
+
+#include "../../utils/media-ctl/libv4l2media_ioctl.h"
+#include "../../utils/media-ctl/mediactl.h"
+#include "../../utils/media-ctl/mediatext.h"
+#include "../../utils/media-ctl/v4l2subdev.h"
+#include "libv4l-plugin.h"
+
+struct media_device;
+struct media_entity;
+
+/*
+ * struct exynos4_camera_plugin - libv4l exynos4 camera plugin
+ * @media:		media device comprising the vid_fd related video device
+ */
+struct exynos4_camera_plugin {
+	struct media_device *media;
+};
+
+#ifdef DEBUG
+#define V4L2_EXYNOS4_DBG(format, ARG...)\
+	printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, ##ARG)
+#else
+#define V4L2_EXYNOS4_DBG(format, ARG...)
+#endif
+
+#define V4L2_EXYNOS4_ERR(format, ARG...)\
+	fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
+
+#define V4L2_EXYNOS4_LOG(format, ARG...)\
+	fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
+
+#if HAVE_VISIBILITY
+#define PLUGIN_PUBLIC __attribute__ ((visibility("default")))
+#else
+#define PLUGIN_PUBLIC
+#endif
+
+#define SYS_IOCTL(fd, cmd, arg) \
+	syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg))
+
+#define SIMPLE_CONVERT_IOCTL(fd, cmd, arg, __struc) ({  \
+	int __ret;                                      \
+	struct __struc *req = arg;                      \
+	uint32_t type = req->type;                      \
+	req->type = convert_type(type);                 \
+	__ret = SYS_IOCTL(fd, cmd, arg);                \
+	req->type = type;                               \
+	__ret;                                          \
+	})
+
+#define EXYNOS4_FIMC_DRV	"exynos4-fimc"
+#define EXYNOS4_FIMC_LITE_DRV	"exynos-fimc-lit"
+#define EXYNOS4_FIMC_IS_ISP_DRV	"exynos4-fimc-is"
+#define ENTITY_CAPTURE_SEGMENT	"capture"
+#define EXYNOS4_CAPTURE_CONF	"/var/lib/libv4l/exynos4_capture_conf"
+#define EXYNOS4_FIMC_IS_ISP	"FIMC-IS-ISP"
+#define EXYNOS4_FIMC_PREFIX	"FIMC."
+#define MAX_FMT_NEGO_NUM	50
+
+
+static int __capture_entity(const char *name)
+{
+	int cap_segment_pos;
+
+	if (name == NULL)
+		return 0;
+
+	cap_segment_pos = strlen(name) - strlen(ENTITY_CAPTURE_SEGMENT);
+
+	if (strcmp(name + cap_segment_pos, ENTITY_CAPTURE_SEGMENT) == 0)
+		return 1;
+
+	return 0;
+}
+
+static int __adjust_format_to_fimc_is_isp(struct v4l2_mbus_framefmt *mbus_fmt)
+{
+	if (mbus_fmt == NULL)
+		return -EINVAL;
+
+	mbus_fmt->width += 16;
+	mbus_fmt->height += 12;
+
+	return 0;
+}
+
+static int negotiate_pipeline_fmt(struct media_entity *pipeline,
+				  struct v4l2_format *dev_fmt)
+{
+	struct media_entity *entity = pipeline;
+	struct v4l2_subdev_format subdev_fmt = { 0 };
+	struct v4l2_mbus_framefmt mbus_fmt = { 0 }, common_fmt;
+	int repeat_negotiation, cnt_negotiation = 0, ret, pad_id;
+
+	if (pipeline == NULL || dev_fmt == NULL)
+		return -EINVAL;
+
+	mbus_fmt.width = dev_fmt->fmt.pix_mp.width;
+	mbus_fmt.height = dev_fmt->fmt.pix_mp.height;
+	mbus_fmt.field = dev_fmt->fmt.pix_mp.field;
+	mbus_fmt.colorspace = dev_fmt->fmt.pix_mp.colorspace;
+
+	if (media_has_pipeline_entity(entity, EXYNOS4_FIMC_IS_ISP)) {
+		ret = __adjust_format_to_fimc_is_isp(&mbus_fmt);
+		if (ret < 0)
+			return ret;
+	}
+
+	V4L2_EXYNOS4_DBG("Begin pipeline format negotiation...");
+
+	for (;;) {
+		repeat_negotiation = 0;
+		entity = pipeline;
+
+		pad_id = media_entity_get_src_pad_index(entity);
+
+		V4L2_EXYNOS4_DBG("Setting format on entity %s, pad: %d",
+				 media_entity_get_name(entity), pad_id);
+
+		ret = v4l2_subdev_set_format(entity, &mbus_fmt,
+					     pad_id, V4L2_SUBDEV_FORMAT_TRY);
+		if (ret < 0)
+			return ret;
+
+		common_fmt = mbus_fmt;
+
+		entity = media_entity_get_next(entity);
+
+		while (entity) {
+			pad_id = media_entity_get_sink_pad_index(entity);
+
+			/* Set format on the entity src pad */
+			V4L2_EXYNOS4_DBG("Setting format on the entity pad %s:%d: mbus_code: %s, width: %d, height: %d",
+					 media_entity_get_name(entity), pad_id,
+					 v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
+					 mbus_fmt.width, mbus_fmt.height);
+
+			ret = v4l2_subdev_set_format(entity, &mbus_fmt, pad_id,
+							V4L2_SUBDEV_FORMAT_TRY);
+			if (ret < 0)
+				return ret;
+
+			if (!v4l2_subdev_format_compare(&mbus_fmt, &common_fmt)) {
+				repeat_negotiation = 1;
+				break;
+			}
+
+			/*
+			 * Do not check format on FIMC.[n] source pad
+			 * and stop negotiation.
+			 */
+			if (!strncmp(media_entity_get_name(entity),
+				     EXYNOS4_FIMC_PREFIX,
+				     strlen(EXYNOS4_FIMC_PREFIX)))
+				break;
+
+			pad_id = media_entity_get_src_pad_index(entity);
+
+			/* Get format on the entity sink pad */
+			ret = v4l2_subdev_get_format(entity, &mbus_fmt, pad_id,
+							V4L2_SUBDEV_FORMAT_TRY);
+			if (ret < 0)
+				return -EINVAL;
+
+			V4L2_EXYNOS4_DBG("Format propagated to the entity pad %s:%d: mbus_code: %s, width: %d, height: %d",
+					 media_entity_get_name(entity), pad_id,
+					 v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
+					 mbus_fmt.width, mbus_fmt.height);
+
+			if (!strcmp(media_entity_get_name(entity),
+				    EXYNOS4_FIMC_IS_ISP)) {
+				common_fmt.code = subdev_fmt.format.code;
+				common_fmt.colorspace =
+						subdev_fmt.format.colorspace;
+				common_fmt.width -= 16;
+				common_fmt.height -= 12;
+			}
+
+			if (!v4l2_subdev_format_compare(&mbus_fmt, &common_fmt)) {
+				repeat_negotiation = 1;
+				break;
+			}
+
+			entity = media_entity_get_next(entity);
+
+			/*
+			 * Stop if this is last element in the
+			 * pipeline as it is not a sub-device.
+			 */
+			if (media_entity_get_next(entity) == NULL)
+				break;
+		}
+
+		if (!repeat_negotiation) {
+			break;
+		} else if (++cnt_negotiation > MAX_FMT_NEGO_NUM) {
+			V4L2_EXYNOS4_DBG("Pipeline format negotiation failed!");
+			return -EINVAL;
+		}
+	}
+
+	dev_fmt->fmt.pix_mp.width = mbus_fmt.width;
+	dev_fmt->fmt.pix_mp.height = mbus_fmt.height;
+	dev_fmt->fmt.pix_mp.field = mbus_fmt.field;
+	dev_fmt->fmt.pix_mp.colorspace = mbus_fmt.colorspace;
+
+	V4L2_EXYNOS4_DBG("Pipeline format successfuly negotiated");
+
+	return 0;
+}
+
+static int convert_type(int type)
+{
+	switch (type) {
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+		return V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+	default:
+		return type;
+	}
+}
+
+static int set_fmt_ioctl(struct media_device *media,
+			 unsigned long int cmd,
+			 struct v4l2_format *arg,
+			 enum v4l2_subdev_format_whence set_mode)
+{
+	struct v4l2_format fmt = { 0 };
+	struct v4l2_format *org = arg;
+	int ret;
+
+	fmt.type = convert_type(arg->type);
+	if (fmt.type != arg->type) {
+		fmt.fmt.pix_mp.width = org->fmt.pix.width;
+		fmt.fmt.pix_mp.height = org->fmt.pix.height;
+		fmt.fmt.pix_mp.pixelformat = org->fmt.pix.pixelformat;
+		fmt.fmt.pix_mp.field = org->fmt.pix.field;
+		fmt.fmt.pix_mp.colorspace = org->fmt.pix.colorspace;
+		fmt.fmt.pix_mp.num_planes = 1;
+		fmt.fmt.pix_mp.flags = org->fmt.pix.flags;
+		fmt.fmt.pix_mp.plane_fmt[0].bytesperline = org->fmt.pix.bytesperline;
+		fmt.fmt.pix_mp.plane_fmt[0].sizeimage = org->fmt.pix.sizeimage;
+	} else {
+		fmt = *org;
+	}
+
+	ret = negotiate_pipeline_fmt(media_get_pipeline(media), &fmt);
+	if (ret < 0)
+		return ret;
+
+	if (set_mode == V4L2_SUBDEV_FORMAT_ACTIVE) {
+		ret = v4l2_subdev_apply_pipeline_fmt(media, &fmt);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (fmt.type != arg->type) {
+		org->fmt.pix.width = fmt.fmt.pix_mp.width;
+		org->fmt.pix.height = fmt.fmt.pix_mp.height;
+		org->fmt.pix.pixelformat = fmt.fmt.pix_mp.pixelformat;
+		org->fmt.pix.field = fmt.fmt.pix_mp.field;
+		org->fmt.pix.colorspace = fmt.fmt.pix_mp.colorspace;
+		org->fmt.pix.bytesperline = fmt.fmt.pix_mp.plane_fmt[0].bytesperline;
+		org->fmt.pix.sizeimage = fmt.fmt.pix_mp.plane_fmt[0].sizeimage;
+		org->fmt.pix.flags = fmt.fmt.pix_mp.flags;
+	} else {
+		*org = fmt;
+	}
+
+	return 0;
+}
+
+static int get_fmt_ioctl(int fd,
+			 unsigned long int cmd,
+			 struct v4l2_format *arg)
+{
+	struct v4l2_format fmt = { 0 };
+	struct v4l2_format *org = arg;
+	int ret;
+
+	fmt.type = convert_type(arg->type);
+
+	if (fmt.type == arg->type)
+		return SYS_IOCTL(fd, cmd, arg);
+
+	ret = SYS_IOCTL(fd, cmd, &fmt);
+	if (ret < 0)
+		return ret;
+
+	memset(&org->fmt.pix, 0, sizeof(org->fmt.pix));
+	org->fmt.pix.width = fmt.fmt.pix_mp.width;
+	org->fmt.pix.height = fmt.fmt.pix_mp.height;
+	org->fmt.pix.pixelformat = fmt.fmt.pix_mp.pixelformat;
+	org->fmt.pix.field = fmt.fmt.pix_mp.field;
+	org->fmt.pix.colorspace = fmt.fmt.pix_mp.colorspace;
+	org->fmt.pix.bytesperline = fmt.fmt.pix_mp.plane_fmt[0].bytesperline;
+	org->fmt.pix.sizeimage = fmt.fmt.pix_mp.plane_fmt[0].sizeimage;
+	org->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
+	org->fmt.pix.flags = fmt.fmt.pix_mp.flags;
+
+	/*
+	 * If the device doesn't support just one plane, there's
+	 * nothing we can do, except return an error condition.
+	 */
+	if (fmt.fmt.pix_mp.num_planes > 1) {
+		errno = EINVAL;
+		return -1;
+	}
+
+
+	return ret;
+}
+
+static int buf_ioctl(int fd,
+		     unsigned long int cmd,
+		     struct v4l2_buffer *arg)
+{
+	struct v4l2_buffer buf = *arg;
+	struct v4l2_plane plane = { 0 };
+	int ret;
+
+	buf.type = convert_type(arg->type);
+
+	if (buf.type == arg->type)
+		return SYS_IOCTL(fd, cmd, arg);
+
+	memcpy(&plane.m, &arg->m, sizeof(plane.m));
+	plane.length = arg->length;
+	plane.bytesused = arg->bytesused;
+
+	buf.m.planes = &plane;
+	buf.length = 1;
+
+	ret = SYS_IOCTL(fd, cmd, &buf);
+
+	arg->index = buf.index;
+	arg->memory = buf.memory;
+	arg->flags = buf.flags;
+	arg->field = buf.field;
+	arg->timestamp = buf.timestamp;
+	arg->timecode = buf.timecode;
+	arg->sequence = buf.sequence;
+
+	arg->length = plane.length;
+	arg->bytesused = plane.bytesused;
+	memcpy(&arg->m, &plane.m, sizeof(arg->m));
+
+	return ret;
+}
+
+static int querycap_ioctl(int fd, struct v4l2_capability *arg)
+{
+	int ret;
+
+	ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, arg);
+	if (ret < 0)
+		return ret;
+
+	arg->device_caps |= V4L2_CAP_VIDEO_CAPTURE;
+	arg->capabilities |= V4L2_CAP_VIDEO_CAPTURE;
+
+	return ret;
+}
+
+static void *plugin_init(int fd)
+{
+	struct v4l2_capability cap;
+	struct exynos4_camera_plugin *plugin = NULL;
+	const char *sink_entity_name;
+	struct media_device *media;
+	struct media_entity *sink_entity;
+	char video_devname[32];
+	int ret;
+
+	V4L2_EXYNOS4_ERR("fd: %d\n", fd);
+
+	memset(&plugin, 0, sizeof(plugin));
+
+	memset(&cap, 0, sizeof(cap));
+	ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, &cap);
+	if (ret < 0) {
+		V4L2_EXYNOS4_ERR("Failed to query video capabilities.");
+		return NULL;
+	}
+
+	/* Check if this is Exynos4 media device */
+	if (strcmp((char *) cap.driver, EXYNOS4_FIMC_DRV) &&
+	    strcmp((char *) cap.driver, EXYNOS4_FIMC_LITE_DRV) &&
+	    strcmp((char *) cap.driver, EXYNOS4_FIMC_IS_ISP_DRV)) {
+		V4L2_EXYNOS4_ERR("Not an Exynos4 media device.");
+		return NULL;
+	}
+
+	/* Obtain the node name of the opened device */
+	ret = media_get_devname_by_fd(fd, video_devname);
+	if (ret < 0) {
+		V4L2_EXYNOS4_ERR("Failed to get video device node name.");
+		return NULL;
+	}
+
+	/*
+	 * Create the representation of a media device
+	 * containing the opened video device.
+	 */
+	media = media_device_new_by_entity_devname(video_devname);
+	if (media == NULL) {
+		V4L2_EXYNOS4_ERR("Failed to create media device.");
+		return NULL;
+	}
+
+#ifdef DEBUG
+	media_debug_set_handler(media, (void (*)(void *, ...))fprintf, stdout);
+#endif
+
+	/* Get the entity representing the opened video device node */
+	sink_entity = media_get_entity_by_devname(media, video_devname, strlen(video_devname));
+	if (sink_entity == NULL) {
+		V4L2_EXYNOS4_ERR("Failed to get sinkd entity name.");
+		goto err_get_sink_entity;
+	}
+
+	/* The last entity in the pipeline represents video device node */
+	media_entity_set_fd(sink_entity, fd);
+
+	sink_entity_name = media_entity_get_name(sink_entity);
+
+	/* Check if video entity is of capture type, not m2m */
+	if (!__capture_entity(sink_entity_name)) {
+		V4L2_EXYNOS4_ERR("Device not of capture type.");
+		goto err_get_sink_entity;
+	}
+
+	/* Parse media configuration file and apply its settings */
+	ret = mediatext_parse_setup_config(media, EXYNOS4_CAPTURE_CONF);
+	if (ret < 0) {
+		V4L2_EXYNOS4_ERR("Media config parser error.");
+		goto err_get_sink_entity;
+	}
+
+	/*
+	 * Discover the pipeline of sub-devices from a camera sensor
+	 * to the opened video device.
+	 */
+	ret = media_discover_pipeline_by_entity(media, sink_entity);
+	if (ret < 0) {
+		V4L2_EXYNOS4_ERR("Error discovering video pipeline.");
+		goto err_get_sink_entity;
+	}
+
+	/* Open all sub-devices in the discovered pipeline */
+	ret = media_open_pipeline_subdevs(media);
+	if (ret < 0) {
+		V4L2_EXYNOS4_ERR("Error opening video pipeline.");
+		goto err_get_sink_entity;
+	}
+
+	/* Allocate private data */
+	plugin = calloc(1, sizeof(*plugin));
+	if (!plugin)
+		goto err_validate_controls;
+
+	plugin->media = media;
+
+	V4L2_EXYNOS4_LOG("Initialized exynos4-camera plugin.");
+
+	return plugin;
+
+err_validate_controls:
+	media_close_pipeline_subdevs(media);
+err_get_sink_entity:
+	if (media)
+		media_device_unref(media);
+	return NULL;
+}
+
+static void plugin_close(void *dev_ops_priv)
+{
+	struct exynos4_camera_plugin *plugin;
+	struct media_device *media;
+
+	if (dev_ops_priv == NULL)
+		return;
+
+	plugin = (struct exynos4_camera_plugin *) dev_ops_priv;
+	media = plugin->media;
+
+	media_close_pipeline_subdevs(media);
+	media_device_unref(media);
+
+	free(plugin);
+}
+
+static int plugin_ioctl(void *dev_ops_priv, int fd, unsigned long int cmd,
+							void *arg)
+{
+	struct exynos4_camera_plugin *plugin = dev_ops_priv;
+	struct media_device *media;
+
+	if (plugin == NULL || arg == NULL)
+		return -EINVAL;
+
+	media = plugin->media;
+
+	if (media == NULL)
+		return -EINVAL;
+
+	switch (cmd) {
+	case VIDIOC_S_CTRL:
+	case VIDIOC_G_CTRL:
+		return media_ioctl_ctrl(media, cmd, arg);
+	case VIDIOC_S_EXT_CTRLS:
+	case VIDIOC_G_EXT_CTRLS:
+	case VIDIOC_TRY_EXT_CTRLS:
+		return media_ioctl_ext_ctrl(media, cmd, arg);
+	case VIDIOC_QUERYCTRL:
+		return media_ioctl_queryctrl(media, arg);
+	case VIDIOC_QUERY_EXT_CTRL:
+		return media_ioctl_query_ext_ctrl(media, arg);
+	case VIDIOC_QUERYMENU:
+		return media_ioctl_querymenu(media, arg);
+	case VIDIOC_TRY_FMT:
+		return set_fmt_ioctl(media, cmd, arg,
+				     V4L2_SUBDEV_FORMAT_TRY);
+	case VIDIOC_S_FMT:
+		return set_fmt_ioctl(media, cmd, arg,
+				     V4L2_SUBDEV_FORMAT_ACTIVE);
+	case VIDIOC_G_FMT:
+		return get_fmt_ioctl(fd, cmd, arg);
+	case VIDIOC_QUERYCAP:
+		return querycap_ioctl(fd, arg);
+	case VIDIOC_QBUF:
+	case VIDIOC_DQBUF:
+	case VIDIOC_QUERYBUF:
+	case VIDIOC_PREPARE_BUF:
+		return buf_ioctl(fd, cmd, arg);
+	case VIDIOC_REQBUFS:
+		return SIMPLE_CONVERT_IOCTL(fd, cmd, arg,
+					    v4l2_requestbuffers);
+	case VIDIOC_ENUM_FMT:
+		return SIMPLE_CONVERT_IOCTL(fd, cmd, arg, v4l2_fmtdesc);
+	case VIDIOC_CROPCAP:
+		return SIMPLE_CONVERT_IOCTL(fd, cmd, arg, v4l2_cropcap);
+	case VIDIOC_STREAMON:
+	case VIDIOC_STREAMOFF:
+		{
+			int *arg_type = (int *) arg;
+			int type;
+
+			type = convert_type(*arg_type);
+
+			if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+			    type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+				V4L2_EXYNOS4_ERR("Invalid buffer type.");
+				return -EINVAL;
+			}
+
+			return SYS_IOCTL(fd, cmd, &type);
+		}
+
+	default:
+		return SYS_IOCTL(fd, cmd, arg);
+	}
+}
+
+PLUGIN_PUBLIC const struct libv4l_dev_ops libv4l2_plugin = {
+	.init = &plugin_init,
+	.close = &plugin_close,
+	.ioctl = &plugin_ioctl,
+};
-- 
1.7.9.5


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

* Re: [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure
  2014-11-21 16:14 ` [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure Jacek Anaszewski
@ 2014-11-25 11:36   ` Sakari Ailus
  2014-11-25 12:22     ` Jacek Anaszewski
  0 siblings, 1 reply; 26+ messages in thread
From: Sakari Ailus @ 2014-11-25 11:36 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Jacek,

Thank you for the updated patchset.

On Fri, Nov 21, 2014 at 05:14:30PM +0100, Jacek Anaszewski wrote:
> Add struct v4l2_subdev as a representation of the v4l2 sub-device
> related to a media entity. Add sd property, the pointer to
> the newly introduced structure, to the struct media_entity
> and move fd property to it.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  utils/media-ctl/libmediactl.c   |   30 +++++++++++++++++++++++++-----
>  utils/media-ctl/libv4l2subdev.c |   34 +++++++++++++++++-----------------
>  utils/media-ctl/mediactl-priv.h |    5 +++++
>  utils/media-ctl/mediactl.h      |   22 ++++++++++++++++++++++
>  4 files changed, 69 insertions(+), 22 deletions(-)
> 
> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> index ec360bd..53921f5 100644
> --- a/utils/media-ctl/libmediactl.c
> +++ b/utils/media-ctl/libmediactl.c
> @@ -511,7 +511,6 @@ static int media_enum_entities(struct media_device *media)
>  
>  		entity = &media->entities[media->entities_count];
>  		memset(entity, 0, sizeof(*entity));
> -		entity->fd = -1;

I think I'd definitely leave the fd to the media_entity itself. Not all the
entities are sub-devices, even right now.

>  		entity->info.id = id | MEDIA_ENT_ID_FLAG_NEXT;
>  		entity->media = media;
>  
> @@ -529,11 +528,13 @@ static int media_enum_entities(struct media_device *media)
>  
>  		entity->pads = malloc(entity->info.pads * sizeof(*entity->pads));
>  		entity->links = malloc(entity->max_links * sizeof(*entity->links));
> -		if (entity->pads == NULL || entity->links == NULL) {
> +		entity->sd = calloc(1, sizeof(*entity->sd));
> +		if (entity->pads == NULL || entity->links == NULL || entity->sd == NULL) {
>  			ret = -ENOMEM;
>  			break;
>  		}
>  
> +		entity->sd->fd = -1;
>  		media->entities_count++;
>  
>  		if (entity->info.flags & MEDIA_ENT_FL_DEFAULT) {
> @@ -704,8 +705,9 @@ void media_device_unref(struct media_device *media)
>  
>  		free(entity->pads);
>  		free(entity->links);
> -		if (entity->fd != -1)
> -			close(entity->fd);
> +		if (entity->sd->fd != -1)
> +			close(entity->sd->fd);
> +		free(entity->sd);
>  	}
>  
>  	free(media->entities);
> @@ -726,13 +728,17 @@ int media_device_add_entity(struct media_device *media,
>  	if (entity == NULL)
>  		return -ENOMEM;
>  
> +	entity->sd = calloc(1, sizeof(*entity->sd));
> +	if (entity->sd == NULL)
> +		return -ENOMEM;
> +
>  	media->entities = entity;
>  	media->entities_count++;
>  
>  	entity = &media->entities[media->entities_count - 1];
>  	memset(entity, 0, sizeof *entity);
>  
> -	entity->fd = -1;
> +	entity->sd->fd = -1;
>  	entity->media = media;
>  	strncpy(entity->devname, devnode, sizeof entity->devname);
>  	entity->devname[sizeof entity->devname - 1] = '\0';
> @@ -955,3 +961,17 @@ int media_parse_setup_links(struct media_device *media, const char *p)
>  
>  	return *end ? -EINVAL : 0;
>  }
> +
> +/* -----------------------------------------------------------------------------
> + * Media entity access
> + */
> +
> +int media_entity_get_fd(struct media_entity *entity)
> +{
> +	return entity->sd->fd;
> +}
> +
> +void media_entity_set_fd(struct media_entity *entity, int fd)
> +{
> +	entity->sd->fd = fd;
> +}

You access the fd directly now inside the library. I don't think there
should be a need to set it.

> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index 8015330..09e0081 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -41,11 +41,11 @@
>  
>  int v4l2_subdev_open(struct media_entity *entity)
>  {
> -	if (entity->fd != -1)
> +	if (entity->sd->fd != -1)
>  		return 0;
>  
> -	entity->fd = open(entity->devname, O_RDWR);
> -	if (entity->fd == -1) {
> +	entity->sd->fd = open(entity->devname, O_RDWR);
> +	if (entity->sd->fd == -1) {
>  		int ret = -errno;
>  		media_dbg(entity->media,
>  			  "%s: Failed to open subdev device node %s\n", __func__,
> @@ -58,8 +58,8 @@ int v4l2_subdev_open(struct media_entity *entity)
>  
>  void v4l2_subdev_close(struct media_entity *entity)
>  {
> -	close(entity->fd);
> -	entity->fd = -1;
> +	close(entity->sd->fd);
> +	entity->sd->fd = -1;
>  }
>  
>  int v4l2_subdev_get_format(struct media_entity *entity,
> @@ -77,7 +77,7 @@ int v4l2_subdev_get_format(struct media_entity *entity,
>  	fmt.pad = pad;
>  	fmt.which = which;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -101,7 +101,7 @@ int v4l2_subdev_set_format(struct media_entity *entity,
>  	fmt.which = which;
>  	fmt.format = *format;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FMT, &fmt);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_FMT, &fmt);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -128,7 +128,7 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
>  	u.sel.target = target;
>  	u.sel.which = which;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_SELECTION, &u.sel);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_SELECTION, &u.sel);
>  	if (ret >= 0) {
>  		*rect = u.sel.r;
>  		return 0;
> @@ -140,7 +140,7 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
>  	u.crop.pad = pad;
>  	u.crop.which = which;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_CROP, &u.crop);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_CROP, &u.crop);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -168,7 +168,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
>  	u.sel.which = which;
>  	u.sel.r = *rect;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_SELECTION, &u.sel);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_SELECTION, &u.sel);
>  	if (ret >= 0) {
>  		*rect = u.sel.r;
>  		return 0;
> @@ -181,7 +181,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
>  	u.crop.which = which;
>  	u.crop.rect = *rect;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_CROP, &u.crop);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_CROP, &u.crop);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -202,7 +202,7 @@ int v4l2_subdev_get_dv_timings_caps(struct media_entity *entity,
>  	memset(caps, 0, sizeof(*caps));
>  	caps->pad = pad;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_DV_TIMINGS_CAP, caps);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_DV_TIMINGS_CAP, caps);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -220,7 +220,7 @@ int v4l2_subdev_query_dv_timings(struct media_entity *entity,
>  
>  	memset(timings, 0, sizeof(*timings));
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_QUERY_DV_TIMINGS, timings);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_QUERY_DV_TIMINGS, timings);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -238,7 +238,7 @@ int v4l2_subdev_get_dv_timings(struct media_entity *entity,
>  
>  	memset(timings, 0, sizeof(*timings));
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_DV_TIMINGS, timings);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_DV_TIMINGS, timings);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -254,7 +254,7 @@ int v4l2_subdev_set_dv_timings(struct media_entity *entity,
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_DV_TIMINGS, timings);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_DV_TIMINGS, timings);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -273,7 +273,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity *entity,
>  
>  	memset(&ival, 0, sizeof(ival));
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
>  	if (ret < 0)
>  		return -errno;
>  
> @@ -294,7 +294,7 @@ int v4l2_subdev_set_frame_interval(struct media_entity *entity,
>  	memset(&ival, 0, sizeof(ival));
>  	ival.interval = *interval;
>  
> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
>  	if (ret < 0)
>  		return -errno;
>  
> diff --git a/utils/media-ctl/mediactl-priv.h b/utils/media-ctl/mediactl-priv.h
> index a0d3a55..4bcb1e0 100644
> --- a/utils/media-ctl/mediactl-priv.h
> +++ b/utils/media-ctl/mediactl-priv.h
> @@ -34,7 +34,12 @@ struct media_entity {
>  	unsigned int max_links;
>  	unsigned int num_links;
>  
> +	struct v4l2_subdev *sd;
> +
>  	char devname[32];
> +};
> +
> +struct v4l2_subdev {
>  	int fd;

struct v4l2_subdev should be defined in v4l2subdev.h.

>  };
>  
> diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
> index 77ac182..b8cefe8 100644
> --- a/utils/media-ctl/mediactl.h
> +++ b/utils/media-ctl/mediactl.h
> @@ -420,4 +420,26 @@ int media_parse_setup_link(struct media_device *media,
>   */
>  int media_parse_setup_links(struct media_device *media, const char *p);
>  
> +/**
> + * @brief Get file descriptor of the entity sub-device
> + * @param entity - media entity
> + *
> + * This function gets the file descriptor of the opened
> + * sub-device node related to the entity.
> + *
> + * @return file descriptor of the opened sub-device,
> +	   or -1 if the sub-device is closed
> + */
> +int media_entity_get_fd(struct media_entity *entity);
> +
> +/**
> + * @brief Set file descriptor of the entity sub-device
> + * @param entity - media entity
> + * @param fd - entity sub-device file descriptor
> + *
> + * This function sets the file descriptor of the opened
> + * sub-device node related to the entity.
> + */
> +void media_entity_set_fd(struct media_entity *entity, int fd);
> +
>  #endif

-- 
Kind regards,

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

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

* Re: [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure
  2014-11-25 11:36   ` Sakari Ailus
@ 2014-11-25 12:22     ` Jacek Anaszewski
  2014-11-26 10:20       ` Sakari Ailus
  0 siblings, 1 reply; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-25 12:22 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Sakari,

On 11/25/2014 12:36 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> Thank you for the updated patchset.
>
> On Fri, Nov 21, 2014 at 05:14:30PM +0100, Jacek Anaszewski wrote:
>> Add struct v4l2_subdev as a representation of the v4l2 sub-device
>> related to a media entity. Add sd property, the pointer to
>> the newly introduced structure, to the struct media_entity
>> and move fd property to it.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   utils/media-ctl/libmediactl.c   |   30 +++++++++++++++++++++++++-----
>>   utils/media-ctl/libv4l2subdev.c |   34 +++++++++++++++++-----------------
>>   utils/media-ctl/mediactl-priv.h |    5 +++++
>>   utils/media-ctl/mediactl.h      |   22 ++++++++++++++++++++++
>>   4 files changed, 69 insertions(+), 22 deletions(-)
>>
>> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
>> index ec360bd..53921f5 100644
>> --- a/utils/media-ctl/libmediactl.c
>> +++ b/utils/media-ctl/libmediactl.c
>> @@ -511,7 +511,6 @@ static int media_enum_entities(struct media_device *media)
>>
>>   		entity = &media->entities[media->entities_count];
>>   		memset(entity, 0, sizeof(*entity));
>> -		entity->fd = -1;
>
> I think I'd definitely leave the fd to the media_entity itself. Not all the
> entities are sub-devices, even right now.

I am aware of it, I even came across this issue while implementing the
function v4l2_subdev_apply_pipeline_fmt. I added suitable comment
explaining why the entity not being a sub-device has its representation.

I moved the fd out of media_entity by following Laurent's message [1],
where he mentioned this, however I think that it would be indeed
best if it remained intact.


>>   		entity->info.id = id | MEDIA_ENT_ID_FLAG_NEXT;
>>   		entity->media = media;
>>
>> @@ -529,11 +528,13 @@ static int media_enum_entities(struct media_device *media)
>>
>>   		entity->pads = malloc(entity->info.pads * sizeof(*entity->pads));
>>   		entity->links = malloc(entity->max_links * sizeof(*entity->links));
>> -		if (entity->pads == NULL || entity->links == NULL) {
>> +		entity->sd = calloc(1, sizeof(*entity->sd));
>> +		if (entity->pads == NULL || entity->links == NULL || entity->sd == NULL) {
>>   			ret = -ENOMEM;
>>   			break;
>>   		}
>>
>> +		entity->sd->fd = -1;
>>   		media->entities_count++;
>>
>>   		if (entity->info.flags & MEDIA_ENT_FL_DEFAULT) {
>> @@ -704,8 +705,9 @@ void media_device_unref(struct media_device *media)
>>
>>   		free(entity->pads);
>>   		free(entity->links);
>> -		if (entity->fd != -1)
>> -			close(entity->fd);
>> +		if (entity->sd->fd != -1)
>> +			close(entity->sd->fd);
>> +		free(entity->sd);
>>   	}
>>
>>   	free(media->entities);
>> @@ -726,13 +728,17 @@ int media_device_add_entity(struct media_device *media,
>>   	if (entity == NULL)
>>   		return -ENOMEM;
>>
>> +	entity->sd = calloc(1, sizeof(*entity->sd));
>> +	if (entity->sd == NULL)
>> +		return -ENOMEM;
>> +
>>   	media->entities = entity;
>>   	media->entities_count++;
>>
>>   	entity = &media->entities[media->entities_count - 1];
>>   	memset(entity, 0, sizeof *entity);
>>
>> -	entity->fd = -1;
>> +	entity->sd->fd = -1;
>>   	entity->media = media;
>>   	strncpy(entity->devname, devnode, sizeof entity->devname);
>>   	entity->devname[sizeof entity->devname - 1] = '\0';
>> @@ -955,3 +961,17 @@ int media_parse_setup_links(struct media_device *media, const char *p)
>>
>>   	return *end ? -EINVAL : 0;
>>   }
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Media entity access
>> + */
>> +
>> +int media_entity_get_fd(struct media_entity *entity)
>> +{
>> +	return entity->sd->fd;
>> +}
>> +
>> +void media_entity_set_fd(struct media_entity *entity, int fd)
>> +{
>> +	entity->sd->fd = fd;
>> +}
>
> You access the fd directly now inside the library. I don't think there
> should be a need to set it.

struct media_entity is defined in mediactl-priv.h, whose name implies
that it shouldn't be made public. Thats way I implemented the setter.
I use it in the libv4l-exynos4-camera.c.

>> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
>> index 8015330..09e0081 100644
>> --- a/utils/media-ctl/libv4l2subdev.c
>> +++ b/utils/media-ctl/libv4l2subdev.c
>> @@ -41,11 +41,11 @@
>>
>>   int v4l2_subdev_open(struct media_entity *entity)
>>   {
>> -	if (entity->fd != -1)
>> +	if (entity->sd->fd != -1)
>>   		return 0;
>>
>> -	entity->fd = open(entity->devname, O_RDWR);
>> -	if (entity->fd == -1) {
>> +	entity->sd->fd = open(entity->devname, O_RDWR);
>> +	if (entity->sd->fd == -1) {
>>   		int ret = -errno;
>>   		media_dbg(entity->media,
>>   			  "%s: Failed to open subdev device node %s\n", __func__,
>> @@ -58,8 +58,8 @@ int v4l2_subdev_open(struct media_entity *entity)
>>
>>   void v4l2_subdev_close(struct media_entity *entity)
>>   {
>> -	close(entity->fd);
>> -	entity->fd = -1;
>> +	close(entity->sd->fd);
>> +	entity->sd->fd = -1;
>>   }
>>
>>   int v4l2_subdev_get_format(struct media_entity *entity,
>> @@ -77,7 +77,7 @@ int v4l2_subdev_get_format(struct media_entity *entity,
>>   	fmt.pad = pad;
>>   	fmt.which = which;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -101,7 +101,7 @@ int v4l2_subdev_set_format(struct media_entity *entity,
>>   	fmt.which = which;
>>   	fmt.format = *format;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FMT, &fmt);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_FMT, &fmt);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -128,7 +128,7 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
>>   	u.sel.target = target;
>>   	u.sel.which = which;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_SELECTION, &u.sel);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_SELECTION, &u.sel);
>>   	if (ret >= 0) {
>>   		*rect = u.sel.r;
>>   		return 0;
>> @@ -140,7 +140,7 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
>>   	u.crop.pad = pad;
>>   	u.crop.which = which;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_CROP, &u.crop);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_CROP, &u.crop);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -168,7 +168,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
>>   	u.sel.which = which;
>>   	u.sel.r = *rect;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_SELECTION, &u.sel);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_SELECTION, &u.sel);
>>   	if (ret >= 0) {
>>   		*rect = u.sel.r;
>>   		return 0;
>> @@ -181,7 +181,7 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
>>   	u.crop.which = which;
>>   	u.crop.rect = *rect;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_CROP, &u.crop);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_CROP, &u.crop);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -202,7 +202,7 @@ int v4l2_subdev_get_dv_timings_caps(struct media_entity *entity,
>>   	memset(caps, 0, sizeof(*caps));
>>   	caps->pad = pad;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_DV_TIMINGS_CAP, caps);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_DV_TIMINGS_CAP, caps);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -220,7 +220,7 @@ int v4l2_subdev_query_dv_timings(struct media_entity *entity,
>>
>>   	memset(timings, 0, sizeof(*timings));
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_QUERY_DV_TIMINGS, timings);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_QUERY_DV_TIMINGS, timings);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -238,7 +238,7 @@ int v4l2_subdev_get_dv_timings(struct media_entity *entity,
>>
>>   	memset(timings, 0, sizeof(*timings));
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_DV_TIMINGS, timings);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_DV_TIMINGS, timings);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -254,7 +254,7 @@ int v4l2_subdev_set_dv_timings(struct media_entity *entity,
>>   	if (ret < 0)
>>   		return ret;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_DV_TIMINGS, timings);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_DV_TIMINGS, timings);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -273,7 +273,7 @@ int v4l2_subdev_get_frame_interval(struct media_entity *entity,
>>
>>   	memset(&ival, 0, sizeof(ival));
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_G_FRAME_INTERVAL, &ival);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> @@ -294,7 +294,7 @@ int v4l2_subdev_set_frame_interval(struct media_entity *entity,
>>   	memset(&ival, 0, sizeof(ival));
>>   	ival.interval = *interval;
>>
>> -	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
>> +	ret = ioctl(entity->sd->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
>>   	if (ret < 0)
>>   		return -errno;
>>
>> diff --git a/utils/media-ctl/mediactl-priv.h b/utils/media-ctl/mediactl-priv.h
>> index a0d3a55..4bcb1e0 100644
>> --- a/utils/media-ctl/mediactl-priv.h
>> +++ b/utils/media-ctl/mediactl-priv.h
>> @@ -34,7 +34,12 @@ struct media_entity {
>>   	unsigned int max_links;
>>   	unsigned int num_links;
>>
>> +	struct v4l2_subdev *sd;
>> +
>>   	char devname[32];
>> +};
>> +
>> +struct v4l2_subdev {
>>   	int fd;
>
> struct v4l2_subdev should be defined in v4l2subdev.h.
>
>>   };

Right.

>> diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
>> index 77ac182..b8cefe8 100644
>> --- a/utils/media-ctl/mediactl.h
>> +++ b/utils/media-ctl/mediactl.h
>> @@ -420,4 +420,26 @@ int media_parse_setup_link(struct media_device *media,
>>    */
>>   int media_parse_setup_links(struct media_device *media, const char *p);
>>
>> +/**
>> + * @brief Get file descriptor of the entity sub-device
>> + * @param entity - media entity
>> + *
>> + * This function gets the file descriptor of the opened
>> + * sub-device node related to the entity.
>> + *
>> + * @return file descriptor of the opened sub-device,
>> +	   or -1 if the sub-device is closed
>> + */
>> +int media_entity_get_fd(struct media_entity *entity);
>> +
>> +/**
>> + * @brief Set file descriptor of the entity sub-device
>> + * @param entity - media entity
>> + * @param fd - entity sub-device file descriptor
>> + *
>> + * This function sets the file descriptor of the opened
>> + * sub-device node related to the entity.
>> + */
>> +void media_entity_set_fd(struct media_entity *entity, int fd);
>> +
>>   #endif
>

Best Regards,
Jacek Anaszewski

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

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

* Re: [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure
  2014-11-25 12:22     ` Jacek Anaszewski
@ 2014-11-26 10:20       ` Sakari Ailus
  0 siblings, 0 replies; 26+ messages in thread
From: Sakari Ailus @ 2014-11-26 10:20 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Jacek,

On Tue, Nov 25, 2014 at 01:22:50PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 11/25/2014 12:36 PM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >Thank you for the updated patchset.
> >
> >On Fri, Nov 21, 2014 at 05:14:30PM +0100, Jacek Anaszewski wrote:
> >>Add struct v4l2_subdev as a representation of the v4l2 sub-device
> >>related to a media entity. Add sd property, the pointer to
> >>the newly introduced structure, to the struct media_entity
> >>and move fd property to it.
> >>
> >>Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> >>Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>---
> >>  utils/media-ctl/libmediactl.c   |   30 +++++++++++++++++++++++++-----
> >>  utils/media-ctl/libv4l2subdev.c |   34 +++++++++++++++++-----------------
> >>  utils/media-ctl/mediactl-priv.h |    5 +++++
> >>  utils/media-ctl/mediactl.h      |   22 ++++++++++++++++++++++
> >>  4 files changed, 69 insertions(+), 22 deletions(-)
> >>
> >>diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> >>index ec360bd..53921f5 100644
> >>--- a/utils/media-ctl/libmediactl.c
> >>+++ b/utils/media-ctl/libmediactl.c
> >>@@ -511,7 +511,6 @@ static int media_enum_entities(struct media_device *media)
> >>
> >>  		entity = &media->entities[media->entities_count];
> >>  		memset(entity, 0, sizeof(*entity));
> >>-		entity->fd = -1;
> >
> >I think I'd definitely leave the fd to the media_entity itself. Not all the
> >entities are sub-devices, even right now.
> 
> I am aware of it, I even came across this issue while implementing the
> function v4l2_subdev_apply_pipeline_fmt. I added suitable comment
> explaining why the entity not being a sub-device has its representation.
> 
> I moved the fd out of media_entity by following Laurent's message [1],
> where he mentioned this, however I think that it would be indeed
> best if it remained intact.

I read Laurent's reply again, and I can see why he suggested that. I
wouldn't mind, but then we should avoid touching it from libmediactl, and
only access it from libv4l2subdev.

> >>  		entity->info.id = id | MEDIA_ENT_ID_FLAG_NEXT;
> >>  		entity->media = media;
> >>
> >>@@ -529,11 +528,13 @@ static int media_enum_entities(struct media_device *media)
> >>
> >>  		entity->pads = malloc(entity->info.pads * sizeof(*entity->pads));
> >>  		entity->links = malloc(entity->max_links * sizeof(*entity->links));
> >>-		if (entity->pads == NULL || entity->links == NULL) {
> >>+		entity->sd = calloc(1, sizeof(*entity->sd));
> >>+		if (entity->pads == NULL || entity->links == NULL || entity->sd == NULL) {
> >>  			ret = -ENOMEM;
> >>  			break;
> >>  		}
> >>
> >>+		entity->sd->fd = -1;
> >>  		media->entities_count++;
> >>
> >>  		if (entity->info.flags & MEDIA_ENT_FL_DEFAULT) {
> >>@@ -704,8 +705,9 @@ void media_device_unref(struct media_device *media)
> >>
> >>  		free(entity->pads);
> >>  		free(entity->links);
> >>-		if (entity->fd != -1)
> >>-			close(entity->fd);
> >>+		if (entity->sd->fd != -1)
> >>+			close(entity->sd->fd);
> >>+		free(entity->sd);
> >>  	}
> >>
> >>  	free(media->entities);
> >>@@ -726,13 +728,17 @@ int media_device_add_entity(struct media_device *media,
> >>  	if (entity == NULL)
> >>  		return -ENOMEM;
> >>
> >>+	entity->sd = calloc(1, sizeof(*entity->sd));
> >>+	if (entity->sd == NULL)
> >>+		return -ENOMEM;
> >>+
> >>  	media->entities = entity;
> >>  	media->entities_count++;
> >>
> >>  	entity = &media->entities[media->entities_count - 1];
> >>  	memset(entity, 0, sizeof *entity);
> >>
> >>-	entity->fd = -1;
> >>+	entity->sd->fd = -1;
> >>  	entity->media = media;
> >>  	strncpy(entity->devname, devnode, sizeof entity->devname);
> >>  	entity->devname[sizeof entity->devname - 1] = '\0';
> >>@@ -955,3 +961,17 @@ int media_parse_setup_links(struct media_device *media, const char *p)
> >>
> >>  	return *end ? -EINVAL : 0;
> >>  }
> >>+
> >>+/* -----------------------------------------------------------------------------
> >>+ * Media entity access
> >>+ */
> >>+
> >>+int media_entity_get_fd(struct media_entity *entity)
> >>+{
> >>+	return entity->sd->fd;
> >>+}
> >>+
> >>+void media_entity_set_fd(struct media_entity *entity, int fd)
> >>+{
> >>+	entity->sd->fd = fd;
> >>+}
> >
> >You access the fd directly now inside the library. I don't think there
> >should be a need to set it.
> 
> struct media_entity is defined in mediactl-priv.h, whose name implies
> that it shouldn't be made public. Thats way I implemented the setter.
> I use it in the libv4l-exynos4-camera.c.

Ah, I now understand why you wnat to do this. You should also close the file
handle --- this is used internally by the library, and simply setting the
value will lead the loss of the existing handle.

-- 
Kind regards,

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

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

* Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
  2014-11-21 16:14 ` [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera Jacek Anaszewski
@ 2014-11-27  8:41   ` Sakari Ailus
  2014-11-28 13:29     ` Jacek Anaszewski
  2015-02-27  9:32     ` Jacek Anaszewski
  2015-03-15 19:07   ` Gregor Jasny
  2015-03-15 19:12   ` Gregor Jasny
  2 siblings, 2 replies; 26+ messages in thread
From: Sakari Ailus @ 2014-11-27  8:41 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Jacek,

On Fri, Nov 21, 2014 at 05:14:40PM +0100, Jacek Anaszewski wrote:
> The plugin provides support for the media device on Exynos4 SoC.
> It performs single plane <-> multi plane API conversion,
> video pipeline linking and takes care of automatic data format
> negotiation for the whole pipeline, after intercepting
> VIDIOC_S_FMT or VIDIOC_TRY_FMT ioctls.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  configure.ac                                      |    1 +
>  lib/Makefile.am                                   |    7 +-
>  lib/libv4l-exynos4-camera/Makefile.am             |    7 +
>  lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c |  595 +++++++++++++++++++++
>  4 files changed, 609 insertions(+), 1 deletion(-)
>  create mode 100644 lib/libv4l-exynos4-camera/Makefile.am
>  create mode 100644 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
> 
> diff --git a/configure.ac b/configure.ac
> index c9b0524..ae653b9 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -17,6 +17,7 @@ AC_CONFIG_FILES([Makefile
>  	lib/libdvbv5/Makefile
>  	lib/libv4l2rds/Makefile
>  	lib/libv4l-mplane/Makefile
> +	lib/libv4l-exynos4-camera/Makefile
>  
>  	utils/Makefile
>  	utils/libv4l2util/Makefile
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 3a0e19c..56b3a9f 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -5,7 +5,12 @@ SUBDIRS = \
>  	libv4l2rds \
>  	libv4l-mplane
>  
> +if WITH_V4LUTILS
> +SUBDIRS += \
> +	libv4l-exynos4-camera
> +endif
> +
>  if LINUX_OS
>  SUBDIRS += \
>  	libdvbv5
> -endif
> \ No newline at end of file
> +endif
> diff --git a/lib/libv4l-exynos4-camera/Makefile.am b/lib/libv4l-exynos4-camera/Makefile.am
> new file mode 100644
> index 0000000..23c60c6
> --- /dev/null
> +++ b/lib/libv4l-exynos4-camera/Makefile.am
> @@ -0,0 +1,7 @@
> +if WITH_V4L_PLUGINS
> +libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la
> +endif
> +
> +libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c ../../utils/media-ctl/libv4l2media_ioctl.c ../../utils/media-ctl/mediatext.c
> +libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99
> +libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared -export-dynamic -lpthread
> diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
> new file mode 100644
> index 0000000..119c75c
> --- /dev/null
> +++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
> @@ -0,0 +1,595 @@
> +/*
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + *              http://www.samsung.com
> + *
> + * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + */
> +
> +#include <config.h>
> +#include <errno.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <sys/syscall.h>
> +#include <linux/types.h>
> +
> +#include "../../utils/media-ctl/libv4l2media_ioctl.h"
> +#include "../../utils/media-ctl/mediactl.h"
> +#include "../../utils/media-ctl/mediatext.h"
> +#include "../../utils/media-ctl/v4l2subdev.h"
> +#include "libv4l-plugin.h"
> +
> +struct media_device;
> +struct media_entity;
> +
> +/*
> + * struct exynos4_camera_plugin - libv4l exynos4 camera plugin
> + * @media:		media device comprising the vid_fd related video device
> + */
> +struct exynos4_camera_plugin {
> +	struct media_device *media;
> +};
> +
> +#ifdef DEBUG
> +#define V4L2_EXYNOS4_DBG(format, ARG...)\
> +	printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, ##ARG)
> +#else
> +#define V4L2_EXYNOS4_DBG(format, ARG...)
> +#endif
> +
> +#define V4L2_EXYNOS4_ERR(format, ARG...)\
> +	fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
> +
> +#define V4L2_EXYNOS4_LOG(format, ARG...)\
> +	fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
> +
> +#if HAVE_VISIBILITY
> +#define PLUGIN_PUBLIC __attribute__ ((visibility("default")))
> +#else
> +#define PLUGIN_PUBLIC
> +#endif
> +
> +#define SYS_IOCTL(fd, cmd, arg) \
> +	syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg))
> +#define SIMPLE_CONVERT_IOCTL(fd, cmd, arg, __struc) ({  \
> +	int __ret;                                      \
> +	struct __struc *req = arg;                      \
> +	uint32_t type = req->type;                      \
> +	req->type = convert_type(type);                 \
> +	__ret = SYS_IOCTL(fd, cmd, arg);                \
> +	req->type = type;                               \
> +	__ret;                                          \
> +	})
> +
> +#define EXYNOS4_FIMC_DRV	"exynos4-fimc"
> +#define EXYNOS4_FIMC_LITE_DRV	"exynos-fimc-lit"
> +#define EXYNOS4_FIMC_IS_ISP_DRV	"exynos4-fimc-is"
> +#define ENTITY_CAPTURE_SEGMENT	"capture"
> +#define EXYNOS4_CAPTURE_CONF	"/var/lib/libv4l/exynos4_capture_conf"

If you have a different sensor, such as one using the smiapp driver, would
you still have the same configuration file? Just wondering whether this
should be under /etc or not. But this is a minor detail in any case.

> +#define EXYNOS4_FIMC_IS_ISP	"FIMC-IS-ISP"
> +#define EXYNOS4_FIMC_PREFIX	"FIMC."
> +#define MAX_FMT_NEGO_NUM	50
> +
> +
> +static int __capture_entity(const char *name)
> +{
> +	int cap_segment_pos;
> +
> +	if (name == NULL)
> +		return 0;
> +
> +	cap_segment_pos = strlen(name) - strlen(ENTITY_CAPTURE_SEGMENT);
> +
> +	if (strcmp(name + cap_segment_pos, ENTITY_CAPTURE_SEGMENT) == 0)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static int __adjust_format_to_fimc_is_isp(struct v4l2_mbus_framefmt *mbus_fmt)
> +{
> +	if (mbus_fmt == NULL)
> +		return -EINVAL;
> +
> +	mbus_fmt->width += 16;
> +	mbus_fmt->height += 12;
> +
> +	return 0;
> +}
> +
> +static int negotiate_pipeline_fmt(struct media_entity *pipeline,
> +				  struct v4l2_format *dev_fmt)
> +{
> +	struct media_entity *entity = pipeline;
> +	struct v4l2_subdev_format subdev_fmt = { 0 };
> +	struct v4l2_mbus_framefmt mbus_fmt = { 0 }, common_fmt;
> +	int repeat_negotiation, cnt_negotiation = 0, ret, pad_id;
> +
> +	if (pipeline == NULL || dev_fmt == NULL)
> +		return -EINVAL;
> +
> +	mbus_fmt.width = dev_fmt->fmt.pix_mp.width;
> +	mbus_fmt.height = dev_fmt->fmt.pix_mp.height;
> +	mbus_fmt.field = dev_fmt->fmt.pix_mp.field;
> +	mbus_fmt.colorspace = dev_fmt->fmt.pix_mp.colorspace;
> +
> +	if (media_has_pipeline_entity(entity, EXYNOS4_FIMC_IS_ISP)) {
> +		ret = __adjust_format_to_fimc_is_isp(&mbus_fmt);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	V4L2_EXYNOS4_DBG("Begin pipeline format negotiation...");
> +
> +	for (;;) {
> +		repeat_negotiation = 0;
> +		entity = pipeline;
> +
> +		pad_id = media_entity_get_src_pad_index(entity);
> +
> +		V4L2_EXYNOS4_DBG("Setting format on entity %s, pad: %d",
> +				 media_entity_get_name(entity), pad_id);
> +
> +		ret = v4l2_subdev_set_format(entity, &mbus_fmt,
> +					     pad_id, V4L2_SUBDEV_FORMAT_TRY);
> +		if (ret < 0)
> +			return ret;
> +
> +		common_fmt = mbus_fmt;
> +
> +		entity = media_entity_get_next(entity);
> +
> +		while (entity) {
> +			pad_id = media_entity_get_sink_pad_index(entity);
> +
> +			/* Set format on the entity src pad */
> +			V4L2_EXYNOS4_DBG("Setting format on the entity pad %s:%d: mbus_code: %s, width: %d, height: %d",
> +					 media_entity_get_name(entity), pad_id,
> +					 v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
> +					 mbus_fmt.width, mbus_fmt.height);
> +
> +			ret = v4l2_subdev_set_format(entity, &mbus_fmt, pad_id,
> +							V4L2_SUBDEV_FORMAT_TRY);
> +			if (ret < 0)
> +				return ret;
> +
> +			if (!v4l2_subdev_format_compare(&mbus_fmt, &common_fmt)) {
> +				repeat_negotiation = 1;
> +				break;
> +			}
> +
> +			/*
> +			 * Do not check format on FIMC.[n] source pad
> +			 * and stop negotiation.
> +			 */
> +			if (!strncmp(media_entity_get_name(entity),
> +				     EXYNOS4_FIMC_PREFIX,
> +				     strlen(EXYNOS4_FIMC_PREFIX)))
> +				break;
> +
> +			pad_id = media_entity_get_src_pad_index(entity);
> +
> +			/* Get format on the entity sink pad */
> +			ret = v4l2_subdev_get_format(entity, &mbus_fmt, pad_id,
> +							V4L2_SUBDEV_FORMAT_TRY);
> +			if (ret < 0)
> +				return -EINVAL;
> +
> +			V4L2_EXYNOS4_DBG("Format propagated to the entity pad %s:%d: mbus_code: %s, width: %d, height: %d",
> +					 media_entity_get_name(entity), pad_id,
> +					 v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
> +					 mbus_fmt.width, mbus_fmt.height);
> +
> +			if (!strcmp(media_entity_get_name(entity),
> +				    EXYNOS4_FIMC_IS_ISP)) {
> +				common_fmt.code = subdev_fmt.format.code;
> +				common_fmt.colorspace =
> +						subdev_fmt.format.colorspace;
> +				common_fmt.width -= 16;
> +				common_fmt.height -= 12;
> +			}
> +
> +			if (!v4l2_subdev_format_compare(&mbus_fmt, &common_fmt)) {
> +				repeat_negotiation = 1;
> +				break;
> +			}
> +
> +			entity = media_entity_get_next(entity);
> +
> +			/*
> +			 * Stop if this is last element in the
> +			 * pipeline as it is not a sub-device.
> +			 */
> +			if (media_entity_get_next(entity) == NULL)
> +				break;
> +		}
> +
> +		if (!repeat_negotiation) {
> +			break;
> +		} else if (++cnt_negotiation > MAX_FMT_NEGO_NUM) {
> +			V4L2_EXYNOS4_DBG("Pipeline format negotiation failed!");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	dev_fmt->fmt.pix_mp.width = mbus_fmt.width;
> +	dev_fmt->fmt.pix_mp.height = mbus_fmt.height;
> +	dev_fmt->fmt.pix_mp.field = mbus_fmt.field;
> +	dev_fmt->fmt.pix_mp.colorspace = mbus_fmt.colorspace;
> +
> +	V4L2_EXYNOS4_DBG("Pipeline format successfuly negotiated");
> +
> +	return 0;
> +}
> +
> +static int convert_type(int type)

How about __u32 instead?

> +{
> +	switch (type) {
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> +		return V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	default:
> +		return type;
> +	}
> +}
> +
> +static int set_fmt_ioctl(struct media_device *media,
> +			 unsigned long int cmd,
> +			 struct v4l2_format *arg,
> +			 enum v4l2_subdev_format_whence set_mode)
> +{
> +	struct v4l2_format fmt = { 0 };
> +	struct v4l2_format *org = arg;

You never change org, why a new local variable?

> +	int ret;
> +
> +	fmt.type = convert_type(arg->type);
> +	if (fmt.type != arg->type) {
> +		fmt.fmt.pix_mp.width = org->fmt.pix.width;
> +		fmt.fmt.pix_mp.height = org->fmt.pix.height;
> +		fmt.fmt.pix_mp.pixelformat = org->fmt.pix.pixelformat;
> +		fmt.fmt.pix_mp.field = org->fmt.pix.field;
> +		fmt.fmt.pix_mp.colorspace = org->fmt.pix.colorspace;
> +		fmt.fmt.pix_mp.num_planes = 1;
> +		fmt.fmt.pix_mp.flags = org->fmt.pix.flags;
> +		fmt.fmt.pix_mp.plane_fmt[0].bytesperline = org->fmt.pix.bytesperline;
> +		fmt.fmt.pix_mp.plane_fmt[0].sizeimage = org->fmt.pix.sizeimage;
> +	} else {
> +		fmt = *org;
> +	}
> +
> +	ret = negotiate_pipeline_fmt(media_get_pipeline(media), &fmt);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (set_mode == V4L2_SUBDEV_FORMAT_ACTIVE) {
> +		ret = v4l2_subdev_apply_pipeline_fmt(media, &fmt);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	if (fmt.type != arg->type) {
> +		org->fmt.pix.width = fmt.fmt.pix_mp.width;
> +		org->fmt.pix.height = fmt.fmt.pix_mp.height;
> +		org->fmt.pix.pixelformat = fmt.fmt.pix_mp.pixelformat;
> +		org->fmt.pix.field = fmt.fmt.pix_mp.field;
> +		org->fmt.pix.colorspace = fmt.fmt.pix_mp.colorspace;
> +		org->fmt.pix.bytesperline = fmt.fmt.pix_mp.plane_fmt[0].bytesperline;
> +		org->fmt.pix.sizeimage = fmt.fmt.pix_mp.plane_fmt[0].sizeimage;
> +		org->fmt.pix.flags = fmt.fmt.pix_mp.flags;
> +	} else {
> +		*org = fmt;
> +	}
> +
> +	return 0;
> +}
> +
> +static int get_fmt_ioctl(int fd,
> +			 unsigned long int cmd,
> +			 struct v4l2_format *arg)
> +{
> +	struct v4l2_format fmt = { 0 };
> +	struct v4l2_format *org = arg;
> +	int ret;
> +
> +	fmt.type = convert_type(arg->type);
> +
> +	if (fmt.type == arg->type)
> +		return SYS_IOCTL(fd, cmd, arg);
> +
> +	ret = SYS_IOCTL(fd, cmd, &fmt);
> +	if (ret < 0)
> +		return ret;
> +
> +	memset(&org->fmt.pix, 0, sizeof(org->fmt.pix));
> +	org->fmt.pix.width = fmt.fmt.pix_mp.width;
> +	org->fmt.pix.height = fmt.fmt.pix_mp.height;
> +	org->fmt.pix.pixelformat = fmt.fmt.pix_mp.pixelformat;
> +	org->fmt.pix.field = fmt.fmt.pix_mp.field;
> +	org->fmt.pix.colorspace = fmt.fmt.pix_mp.colorspace;
> +	org->fmt.pix.bytesperline = fmt.fmt.pix_mp.plane_fmt[0].bytesperline;
> +	org->fmt.pix.sizeimage = fmt.fmt.pix_mp.plane_fmt[0].sizeimage;
> +	org->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;

What's this is for?

> +	org->fmt.pix.flags = fmt.fmt.pix_mp.flags;
> +
> +	/*
> +	 * If the device doesn't support just one plane, there's
> +	 * nothing we can do, except return an error condition.
> +	 */
> +	if (fmt.fmt.pix_mp.num_planes > 1) {

Wouldn't you notice this right after the IOCTL?

What's the reason btw. to support only single-plane formats?

> +		errno = EINVAL;
> +		return -1;
> +	}
> +
> +
> +	return ret;
> +}
> +
> +static int buf_ioctl(int fd,
> +		     unsigned long int cmd,
> +		     struct v4l2_buffer *arg)
> +{
> +	struct v4l2_buffer buf = *arg;
> +	struct v4l2_plane plane = { 0 };
> +	int ret;
> +
> +	buf.type = convert_type(arg->type);
> +
> +	if (buf.type == arg->type)
> +		return SYS_IOCTL(fd, cmd, arg);
> +
> +	memcpy(&plane.m, &arg->m, sizeof(plane.m));
> +	plane.length = arg->length;
> +	plane.bytesused = arg->bytesused;
> +
> +	buf.m.planes = &plane;
> +	buf.length = 1;
> +
> +	ret = SYS_IOCTL(fd, cmd, &buf);
> +
> +	arg->index = buf.index;
> +	arg->memory = buf.memory;
> +	arg->flags = buf.flags;
> +	arg->field = buf.field;
> +	arg->timestamp = buf.timestamp;
> +	arg->timecode = buf.timecode;
> +	arg->sequence = buf.sequence;
> +
> +	arg->length = plane.length;
> +	arg->bytesused = plane.bytesused;
> +	memcpy(&arg->m, &plane.m, sizeof(arg->m));
> +
> +	return ret;
> +}
> +
> +static int querycap_ioctl(int fd, struct v4l2_capability *arg)
> +{
> +	int ret;
> +
> +	ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, arg);
> +	if (ret < 0)
> +		return ret;
> +
> +	arg->device_caps |= V4L2_CAP_VIDEO_CAPTURE;
> +	arg->capabilities |= V4L2_CAP_VIDEO_CAPTURE;
> +
> +	return ret;
> +}
> +
> +static void *plugin_init(int fd)
> +{
> +	struct v4l2_capability cap;
> +	struct exynos4_camera_plugin *plugin = NULL;
> +	const char *sink_entity_name;
> +	struct media_device *media;
> +	struct media_entity *sink_entity;
> +	char video_devname[32];
> +	int ret;
> +
> +	V4L2_EXYNOS4_ERR("fd: %d\n", fd);
> +
> +	memset(&plugin, 0, sizeof(plugin));

This is an interesting way to set a pointer's value to NULL. But I think
it's redundant.

> +	memset(&cap, 0, sizeof(cap));

You could use cap = { 0 } in declaration.

> +	ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, &cap);
> +	if (ret < 0) {
> +		V4L2_EXYNOS4_ERR("Failed to query video capabilities.");
> +		return NULL;
> +	}
> +
> +	/* Check if this is Exynos4 media device */
> +	if (strcmp((char *) cap.driver, EXYNOS4_FIMC_DRV) &&
> +	    strcmp((char *) cap.driver, EXYNOS4_FIMC_LITE_DRV) &&
> +	    strcmp((char *) cap.driver, EXYNOS4_FIMC_IS_ISP_DRV)) {
> +		V4L2_EXYNOS4_ERR("Not an Exynos4 media device.");
> +		return NULL;
> +	}
> +
> +	/* Obtain the node name of the opened device */
> +	ret = media_get_devname_by_fd(fd, video_devname);
> +	if (ret < 0) {
> +		V4L2_EXYNOS4_ERR("Failed to get video device node name.");
> +		return NULL;
> +	}
> +
> +	/*
> +	 * Create the representation of a media device
> +	 * containing the opened video device.
> +	 */
> +	media = media_device_new_by_entity_devname(video_devname);
> +	if (media == NULL) {
> +		V4L2_EXYNOS4_ERR("Failed to create media device.");
> +		return NULL;
> +	}
> +
> +#ifdef DEBUG
> +	media_debug_set_handler(media, (void (*)(void *, ...))fprintf, stdout);
> +#endif
> +
> +	/* Get the entity representing the opened video device node */
> +	sink_entity = media_get_entity_by_devname(media, video_devname, strlen(video_devname));

Could you use the fd directly instead of translating that to the device
node? fstat(2) gives you directly inode / device major + minor which you can
then use to find the MC device.

> +	if (sink_entity == NULL) {
> +		V4L2_EXYNOS4_ERR("Failed to get sinkd entity name.");
> +		goto err_get_sink_entity;
> +	}
> +
> +	/* The last entity in the pipeline represents video device node */
> +	media_entity_set_fd(sink_entity, fd);
> +
> +	sink_entity_name = media_entity_get_name(sink_entity);
> +
> +	/* Check if video entity is of capture type, not m2m */
> +	if (!__capture_entity(sink_entity_name)) {
> +		V4L2_EXYNOS4_ERR("Device not of capture type.");
> +		goto err_get_sink_entity;
> +	}
> +
> +	/* Parse media configuration file and apply its settings */
> +	ret = mediatext_parse_setup_config(media, EXYNOS4_CAPTURE_CONF);
> +	if (ret < 0) {
> +		V4L2_EXYNOS4_ERR("Media config parser error.");
> +		goto err_get_sink_entity;
> +	}
> +
> +	/*
> +	 * Discover the pipeline of sub-devices from a camera sensor
> +	 * to the opened video device.
> +	 */
> +	ret = media_discover_pipeline_by_entity(media, sink_entity);
> +	if (ret < 0) {
> +		V4L2_EXYNOS4_ERR("Error discovering video pipeline.");
> +		goto err_get_sink_entity;
> +	}
> +
> +	/* Open all sub-devices in the discovered pipeline */
> +	ret = media_open_pipeline_subdevs(media);
> +	if (ret < 0) {
> +		V4L2_EXYNOS4_ERR("Error opening video pipeline.");
> +		goto err_get_sink_entity;
> +	}
> +
> +	/* Allocate private data */
> +	plugin = calloc(1, sizeof(*plugin));
> +	if (!plugin)
> +		goto err_validate_controls;
> +
> +	plugin->media = media;
> +
> +	V4L2_EXYNOS4_LOG("Initialized exynos4-camera plugin.");
> +
> +	return plugin;
> +
> +err_validate_controls:
> +	media_close_pipeline_subdevs(media);
> +err_get_sink_entity:
> +	if (media)
> +		media_device_unref(media);
> +	return NULL;
> +}
> +
> +static void plugin_close(void *dev_ops_priv)
> +{
> +	struct exynos4_camera_plugin *plugin;
> +	struct media_device *media;
> +
> +	if (dev_ops_priv == NULL)
> +		return;
> +
> +	plugin = (struct exynos4_camera_plugin *) dev_ops_priv;

You don't need to cast a void pointer to another pointer. You could also
make the assignment in variable declaration.

> +	media = plugin->media;
> +
> +	media_close_pipeline_subdevs(media);
> +	media_device_unref(media);
> +
> +	free(plugin);
> +}
> +
> +static int plugin_ioctl(void *dev_ops_priv, int fd, unsigned long int cmd,
> +							void *arg)
> +{
> +	struct exynos4_camera_plugin *plugin = dev_ops_priv;
> +	struct media_device *media;
> +
> +	if (plugin == NULL || arg == NULL)
> +		return -EINVAL;
> +
> +	media = plugin->media;
> +
> +	if (media == NULL)
> +		return -EINVAL;
> +
> +	switch (cmd) {
> +	case VIDIOC_S_CTRL:
> +	case VIDIOC_G_CTRL:
> +		return media_ioctl_ctrl(media, cmd, arg);
> +	case VIDIOC_S_EXT_CTRLS:
> +	case VIDIOC_G_EXT_CTRLS:
> +	case VIDIOC_TRY_EXT_CTRLS:
> +		return media_ioctl_ext_ctrl(media, cmd, arg);
> +	case VIDIOC_QUERYCTRL:
> +		return media_ioctl_queryctrl(media, arg);
> +	case VIDIOC_QUERY_EXT_CTRL:
> +		return media_ioctl_query_ext_ctrl(media, arg);
> +	case VIDIOC_QUERYMENU:
> +		return media_ioctl_querymenu(media, arg);
> +	case VIDIOC_TRY_FMT:
> +		return set_fmt_ioctl(media, cmd, arg,
> +				     V4L2_SUBDEV_FORMAT_TRY);
> +	case VIDIOC_S_FMT:
> +		return set_fmt_ioctl(media, cmd, arg,
> +				     V4L2_SUBDEV_FORMAT_ACTIVE);
> +	case VIDIOC_G_FMT:
> +		return get_fmt_ioctl(fd, cmd, arg);
> +	case VIDIOC_QUERYCAP:
> +		return querycap_ioctl(fd, arg);
> +	case VIDIOC_QBUF:
> +	case VIDIOC_DQBUF:
> +	case VIDIOC_QUERYBUF:
> +	case VIDIOC_PREPARE_BUF:
> +		return buf_ioctl(fd, cmd, arg);
> +	case VIDIOC_REQBUFS:
> +		return SIMPLE_CONVERT_IOCTL(fd, cmd, arg,
> +					    v4l2_requestbuffers);
> +	case VIDIOC_ENUM_FMT:
> +		return SIMPLE_CONVERT_IOCTL(fd, cmd, arg, v4l2_fmtdesc);
> +	case VIDIOC_CROPCAP:
> +		return SIMPLE_CONVERT_IOCTL(fd, cmd, arg, v4l2_cropcap);
> +	case VIDIOC_STREAMON:
> +	case VIDIOC_STREAMOFF:
> +		{
> +			int *arg_type = (int *) arg;
> +			int type;
> +
> +			type = convert_type(*arg_type);
> +
> +			if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> +			    type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> +				V4L2_EXYNOS4_ERR("Invalid buffer type.");
> +				return -EINVAL;
> +			}
> +
> +			return SYS_IOCTL(fd, cmd, &type);
> +		}
> +
> +	default:
> +		return SYS_IOCTL(fd, cmd, arg);
> +	}
> +}
> +
> +PLUGIN_PUBLIC const struct libv4l_dev_ops libv4l2_plugin = {
> +	.init = &plugin_init,
> +	.close = &plugin_close,
> +	.ioctl = &plugin_ioctl,
> +};

-- 
Kind regards,

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

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

* Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
  2014-11-27  8:41   ` Sakari Ailus
@ 2014-11-28 13:29     ` Jacek Anaszewski
  2015-02-27  9:32     ` Jacek Anaszewski
  1 sibling, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-11-28 13:29 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Sakari,

Thanks for the review.

On 11/27/2014 09:41 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Fri, Nov 21, 2014 at 05:14:40PM +0100, Jacek Anaszewski wrote:
>> The plugin provides support for the media device on Exynos4 SoC.
>> It performs single plane <-> multi plane API conversion,
>> video pipeline linking and takes care of automatic data format
>> negotiation for the whole pipeline, after intercepting
>> VIDIOC_S_FMT or VIDIOC_TRY_FMT ioctls.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   configure.ac                                      |    1 +
>>   lib/Makefile.am                                   |    7 +-
>>   lib/libv4l-exynos4-camera/Makefile.am             |    7 +
>>   lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c |  595 +++++++++++++++++++++
>>   4 files changed, 609 insertions(+), 1 deletion(-)
>>   create mode 100644 lib/libv4l-exynos4-camera/Makefile.am
>>   create mode 100644 lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index c9b0524..ae653b9 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -17,6 +17,7 @@ AC_CONFIG_FILES([Makefile
>>   	lib/libdvbv5/Makefile
>>   	lib/libv4l2rds/Makefile
>>   	lib/libv4l-mplane/Makefile
>> +	lib/libv4l-exynos4-camera/Makefile
>>
>>   	utils/Makefile
>>   	utils/libv4l2util/Makefile
>> diff --git a/lib/Makefile.am b/lib/Makefile.am
>> index 3a0e19c..56b3a9f 100644
>> --- a/lib/Makefile.am
>> +++ b/lib/Makefile.am
>> @@ -5,7 +5,12 @@ SUBDIRS = \
>>   	libv4l2rds \
>>   	libv4l-mplane
>>
>> +if WITH_V4LUTILS
>> +SUBDIRS += \
>> +	libv4l-exynos4-camera
>> +endif
>> +
>>   if LINUX_OS
>>   SUBDIRS += \
>>   	libdvbv5
>> -endif
>> \ No newline at end of file
>> +endif
>> diff --git a/lib/libv4l-exynos4-camera/Makefile.am b/lib/libv4l-exynos4-camera/Makefile.am
>> new file mode 100644
>> index 0000000..23c60c6
>> --- /dev/null
>> +++ b/lib/libv4l-exynos4-camera/Makefile.am
>> @@ -0,0 +1,7 @@
>> +if WITH_V4L_PLUGINS
>> +libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la
>> +endif
>> +
>> +libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c ../../utils/media-ctl/libv4l2media_ioctl.c ../../utils/media-ctl/mediatext.c
>> +libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99
>> +libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared -export-dynamic -lpthread
>> diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
>> new file mode 100644
>> index 0000000..119c75c
>> --- /dev/null
>> +++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
>> @@ -0,0 +1,595 @@
>> +/*
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + *              http://www.samsung.com
>> + *
>> + * Author: Jacek Anaszewski <j.anaszewski@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU Lesser General Public License as published by
>> + * the Free Software Foundation; either version 2.1 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + */
>> +
>> +#include <config.h>
>> +#include <errno.h>
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +
>> +#include <sys/syscall.h>
>> +#include <linux/types.h>
>> +
>> +#include "../../utils/media-ctl/libv4l2media_ioctl.h"
>> +#include "../../utils/media-ctl/mediactl.h"
>> +#include "../../utils/media-ctl/mediatext.h"
>> +#include "../../utils/media-ctl/v4l2subdev.h"
>> +#include "libv4l-plugin.h"
>> +
>> +struct media_device;
>> +struct media_entity;
>> +
>> +/*
>> + * struct exynos4_camera_plugin - libv4l exynos4 camera plugin
>> + * @media:		media device comprising the vid_fd related video device
>> + */
>> +struct exynos4_camera_plugin {
>> +	struct media_device *media;
>> +};
>> +
>> +#ifdef DEBUG
>> +#define V4L2_EXYNOS4_DBG(format, ARG...)\
>> +	printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, ##ARG)
>> +#else
>> +#define V4L2_EXYNOS4_DBG(format, ARG...)
>> +#endif
>> +
>> +#define V4L2_EXYNOS4_ERR(format, ARG...)\
>> +	fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
>> +
>> +#define V4L2_EXYNOS4_LOG(format, ARG...)\
>> +	fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
>> +
>> +#if HAVE_VISIBILITY
>> +#define PLUGIN_PUBLIC __attribute__ ((visibility("default")))
>> +#else
>> +#define PLUGIN_PUBLIC
>> +#endif
>> +
>> +#define SYS_IOCTL(fd, cmd, arg) \
>> +	syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg))
>> +#define SIMPLE_CONVERT_IOCTL(fd, cmd, arg, __struc) ({  \
>> +	int __ret;                                      \
>> +	struct __struc *req = arg;                      \
>> +	uint32_t type = req->type;                      \
>> +	req->type = convert_type(type);                 \
>> +	__ret = SYS_IOCTL(fd, cmd, arg);                \
>> +	req->type = type;                               \
>> +	__ret;                                          \
>> +	})
>> +
>> +#define EXYNOS4_FIMC_DRV	"exynos4-fimc"
>> +#define EXYNOS4_FIMC_LITE_DRV	"exynos-fimc-lit"
>> +#define EXYNOS4_FIMC_IS_ISP_DRV	"exynos4-fimc-is"
>> +#define ENTITY_CAPTURE_SEGMENT	"capture"
>> +#define EXYNOS4_CAPTURE_CONF	"/var/lib/libv4l/exynos4_capture_conf"
>
> If you have a different sensor, such as one using the smiapp driver, would
> you still have the same configuration file? Just wondering whether this
> should be under /etc or not. But this is a minor detail in any case.

Sensor entity name is used for ctrl-to-subdev-conf in case
a v4l2 control related ioctls are to be redirected to it.
In such a case the entity name would have to be changed.

>> +#define EXYNOS4_FIMC_IS_ISP	"FIMC-IS-ISP"
>> +#define EXYNOS4_FIMC_PREFIX	"FIMC."
>> +#define MAX_FMT_NEGO_NUM	50
>> +
>> +
>> +static int __capture_entity(const char *name)
>> +{
>> +	int cap_segment_pos;
>> +
>> +	if (name == NULL)
>> +		return 0;
>> +
>> +	cap_segment_pos = strlen(name) - strlen(ENTITY_CAPTURE_SEGMENT);
>> +
>> +	if (strcmp(name + cap_segment_pos, ENTITY_CAPTURE_SEGMENT) == 0)
>> +		return 1;
>> +
>> +	return 0;
>> +}
>> +
>> +static int __adjust_format_to_fimc_is_isp(struct v4l2_mbus_framefmt *mbus_fmt)
>> +{
>> +	if (mbus_fmt == NULL)
>> +		return -EINVAL;
>> +
>> +	mbus_fmt->width += 16;
>> +	mbus_fmt->height += 12;
>> +
>> +	return 0;
>> +}
>> +
>> +static int negotiate_pipeline_fmt(struct media_entity *pipeline,
>> +				  struct v4l2_format *dev_fmt)
>> +{
>> +	struct media_entity *entity = pipeline;
>> +	struct v4l2_subdev_format subdev_fmt = { 0 };
>> +	struct v4l2_mbus_framefmt mbus_fmt = { 0 }, common_fmt;
>> +	int repeat_negotiation, cnt_negotiation = 0, ret, pad_id;
>> +
>> +	if (pipeline == NULL || dev_fmt == NULL)
>> +		return -EINVAL;
>> +
>> +	mbus_fmt.width = dev_fmt->fmt.pix_mp.width;
>> +	mbus_fmt.height = dev_fmt->fmt.pix_mp.height;
>> +	mbus_fmt.field = dev_fmt->fmt.pix_mp.field;
>> +	mbus_fmt.colorspace = dev_fmt->fmt.pix_mp.colorspace;
>> +
>> +	if (media_has_pipeline_entity(entity, EXYNOS4_FIMC_IS_ISP)) {
>> +		ret = __adjust_format_to_fimc_is_isp(&mbus_fmt);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	V4L2_EXYNOS4_DBG("Begin pipeline format negotiation...");
>> +
>> +	for (;;) {
>> +		repeat_negotiation = 0;
>> +		entity = pipeline;
>> +
>> +		pad_id = media_entity_get_src_pad_index(entity);
>> +
>> +		V4L2_EXYNOS4_DBG("Setting format on entity %s, pad: %d",
>> +				 media_entity_get_name(entity), pad_id);
>> +
>> +		ret = v4l2_subdev_set_format(entity, &mbus_fmt,
>> +					     pad_id, V4L2_SUBDEV_FORMAT_TRY);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		common_fmt = mbus_fmt;
>> +
>> +		entity = media_entity_get_next(entity);
>> +
>> +		while (entity) {
>> +			pad_id = media_entity_get_sink_pad_index(entity);
>> +
>> +			/* Set format on the entity src pad */
>> +			V4L2_EXYNOS4_DBG("Setting format on the entity pad %s:%d: mbus_code: %s, width: %d, height: %d",
>> +					 media_entity_get_name(entity), pad_id,
>> +					 v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
>> +					 mbus_fmt.width, mbus_fmt.height);
>> +
>> +			ret = v4l2_subdev_set_format(entity, &mbus_fmt, pad_id,
>> +							V4L2_SUBDEV_FORMAT_TRY);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			if (!v4l2_subdev_format_compare(&mbus_fmt, &common_fmt)) {
>> +				repeat_negotiation = 1;
>> +				break;
>> +			}
>> +
>> +			/*
>> +			 * Do not check format on FIMC.[n] source pad
>> +			 * and stop negotiation.
>> +			 */
>> +			if (!strncmp(media_entity_get_name(entity),
>> +				     EXYNOS4_FIMC_PREFIX,
>> +				     strlen(EXYNOS4_FIMC_PREFIX)))
>> +				break;
>> +
>> +			pad_id = media_entity_get_src_pad_index(entity);
>> +
>> +			/* Get format on the entity sink pad */
>> +			ret = v4l2_subdev_get_format(entity, &mbus_fmt, pad_id,
>> +							V4L2_SUBDEV_FORMAT_TRY);
>> +			if (ret < 0)
>> +				return -EINVAL;
>> +
>> +			V4L2_EXYNOS4_DBG("Format propagated to the entity pad %s:%d: mbus_code: %s, width: %d, height: %d",
>> +					 media_entity_get_name(entity), pad_id,
>> +					 v4l2_subdev_pixelcode_to_string(mbus_fmt.code),
>> +					 mbus_fmt.width, mbus_fmt.height);
>> +
>> +			if (!strcmp(media_entity_get_name(entity),
>> +				    EXYNOS4_FIMC_IS_ISP)) {
>> +				common_fmt.code = subdev_fmt.format.code;
>> +				common_fmt.colorspace =
>> +						subdev_fmt.format.colorspace;
>> +				common_fmt.width -= 16;
>> +				common_fmt.height -= 12;
>> +			}
>> +
>> +			if (!v4l2_subdev_format_compare(&mbus_fmt, &common_fmt)) {
>> +				repeat_negotiation = 1;
>> +				break;
>> +			}
>> +
>> +			entity = media_entity_get_next(entity);
>> +
>> +			/*
>> +			 * Stop if this is last element in the
>> +			 * pipeline as it is not a sub-device.
>> +			 */
>> +			if (media_entity_get_next(entity) == NULL)
>> +				break;
>> +		}
>> +
>> +		if (!repeat_negotiation) {
>> +			break;
>> +		} else if (++cnt_negotiation > MAX_FMT_NEGO_NUM) {
>> +			V4L2_EXYNOS4_DBG("Pipeline format negotiation failed!");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	dev_fmt->fmt.pix_mp.width = mbus_fmt.width;
>> +	dev_fmt->fmt.pix_mp.height = mbus_fmt.height;
>> +	dev_fmt->fmt.pix_mp.field = mbus_fmt.field;
>> +	dev_fmt->fmt.pix_mp.colorspace = mbus_fmt.colorspace;
>> +
>> +	V4L2_EXYNOS4_DBG("Pipeline format successfuly negotiated");
>> +
>> +	return 0;
>> +}
>> +
>> +static int convert_type(int type)
>
> How about __u32 instead?

OK.

>> +{
>> +	switch (type) {
>> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>> +		return V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> +	default:
>> +		return type;
>> +	}
>> +}
>> +
>> +static int set_fmt_ioctl(struct media_device *media,
>> +			 unsigned long int cmd,
>> +			 struct v4l2_format *arg,
>> +			 enum v4l2_subdev_format_whence set_mode)
>> +{
>> +	struct v4l2_format fmt = { 0 };
>> +	struct v4l2_format *org = arg;
>
> You never change org, why a new local variable?

I do change it in the last condition in the function.
I followed a convention from the libv4l-mplane.c

>> +	int ret;
>> +
>> +	fmt.type = convert_type(arg->type);
>> +	if (fmt.type != arg->type) {
>> +		fmt.fmt.pix_mp.width = org->fmt.pix.width;
>> +		fmt.fmt.pix_mp.height = org->fmt.pix.height;
>> +		fmt.fmt.pix_mp.pixelformat = org->fmt.pix.pixelformat;
>> +		fmt.fmt.pix_mp.field = org->fmt.pix.field;
>> +		fmt.fmt.pix_mp.colorspace = org->fmt.pix.colorspace;
>> +		fmt.fmt.pix_mp.num_planes = 1;
>> +		fmt.fmt.pix_mp.flags = org->fmt.pix.flags;
>> +		fmt.fmt.pix_mp.plane_fmt[0].bytesperline = org->fmt.pix.bytesperline;
>> +		fmt.fmt.pix_mp.plane_fmt[0].sizeimage = org->fmt.pix.sizeimage;
>> +	} else {
>> +		fmt = *org;
>> +	}
>> +
>> +	ret = negotiate_pipeline_fmt(media_get_pipeline(media), &fmt);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (set_mode == V4L2_SUBDEV_FORMAT_ACTIVE) {
>> +		ret = v4l2_subdev_apply_pipeline_fmt(media, &fmt);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	if (fmt.type != arg->type) {
>> +		org->fmt.pix.width = fmt.fmt.pix_mp.width;
>> +		org->fmt.pix.height = fmt.fmt.pix_mp.height;
>> +		org->fmt.pix.pixelformat = fmt.fmt.pix_mp.pixelformat;
>> +		org->fmt.pix.field = fmt.fmt.pix_mp.field;
>> +		org->fmt.pix.colorspace = fmt.fmt.pix_mp.colorspace;
>> +		org->fmt.pix.bytesperline = fmt.fmt.pix_mp.plane_fmt[0].bytesperline;
>> +		org->fmt.pix.sizeimage = fmt.fmt.pix_mp.plane_fmt[0].sizeimage;
>> +		org->fmt.pix.flags = fmt.fmt.pix_mp.flags;
>> +	} else {
>> +		*org = fmt;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int get_fmt_ioctl(int fd,
>> +			 unsigned long int cmd,
>> +			 struct v4l2_format *arg)
>> +{
>> +	struct v4l2_format fmt = { 0 };
>> +	struct v4l2_format *org = arg;
>> +	int ret;
>> +
>> +	fmt.type = convert_type(arg->type);
>> +
>> +	if (fmt.type == arg->type)
>> +		return SYS_IOCTL(fd, cmd, arg);
>> +
>> +	ret = SYS_IOCTL(fd, cmd, &fmt);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	memset(&org->fmt.pix, 0, sizeof(org->fmt.pix));
>> +	org->fmt.pix.width = fmt.fmt.pix_mp.width;
>> +	org->fmt.pix.height = fmt.fmt.pix_mp.height;
>> +	org->fmt.pix.pixelformat = fmt.fmt.pix_mp.pixelformat;
>> +	org->fmt.pix.field = fmt.fmt.pix_mp.field;
>> +	org->fmt.pix.colorspace = fmt.fmt.pix_mp.colorspace;
>> +	org->fmt.pix.bytesperline = fmt.fmt.pix_mp.plane_fmt[0].bytesperline;
>> +	org->fmt.pix.sizeimage = fmt.fmt.pix_mp.plane_fmt[0].sizeimage;
>> +	org->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
>
> What's this is for?

Hmm, I copied this part of code from libv4l-mplane.c and forgot :-/
The last line is to be removed.

>> +	org->fmt.pix.flags = fmt.fmt.pix_mp.flags;
>> +
>> +	/*
>> +	 * If the device doesn't support just one plane, there's
>> +	 * nothing we can do, except return an error condition.
>> +	 */
>> +	if (fmt.fmt.pix_mp.num_planes > 1) {
>
> Wouldn't you notice this right after the IOCTL?
>
> What's the reason btw. to support only single-plane formats?

I copied it too, but it doesn't fit for this plugin.
Thanks for spotting this.

>> +		errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>> +
>> +	return ret;
>> +}
>> +
>> +static int buf_ioctl(int fd,
>> +		     unsigned long int cmd,
>> +		     struct v4l2_buffer *arg)
>> +{
>> +	struct v4l2_buffer buf = *arg;
>> +	struct v4l2_plane plane = { 0 };
>> +	int ret;
>> +
>> +	buf.type = convert_type(arg->type);
>> +
>> +	if (buf.type == arg->type)
>> +		return SYS_IOCTL(fd, cmd, arg);
>> +
>> +	memcpy(&plane.m, &arg->m, sizeof(plane.m));
>> +	plane.length = arg->length;
>> +	plane.bytesused = arg->bytesused;
>> +
>> +	buf.m.planes = &plane;
>> +	buf.length = 1;
>> +
>> +	ret = SYS_IOCTL(fd, cmd, &buf);
>> +
>> +	arg->index = buf.index;
>> +	arg->memory = buf.memory;
>> +	arg->flags = buf.flags;
>> +	arg->field = buf.field;
>> +	arg->timestamp = buf.timestamp;
>> +	arg->timecode = buf.timecode;
>> +	arg->sequence = buf.sequence;
>> +
>> +	arg->length = plane.length;
>> +	arg->bytesused = plane.bytesused;
>> +	memcpy(&arg->m, &plane.m, sizeof(arg->m));
>> +
>> +	return ret;
>> +}
>> +
>> +static int querycap_ioctl(int fd, struct v4l2_capability *arg)
>> +{
>> +	int ret;
>> +
>> +	ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, arg);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	arg->device_caps |= V4L2_CAP_VIDEO_CAPTURE;
>> +	arg->capabilities |= V4L2_CAP_VIDEO_CAPTURE;
>> +
>> +	return ret;
>> +}
>> +
>> +static void *plugin_init(int fd)
>> +{
>> +	struct v4l2_capability cap;
>> +	struct exynos4_camera_plugin *plugin = NULL;
>> +	const char *sink_entity_name;
>> +	struct media_device *media;
>> +	struct media_entity *sink_entity;
>> +	char video_devname[32];
>> +	int ret;
>> +
>> +	V4L2_EXYNOS4_ERR("fd: %d\n", fd);
>> +
>> +	memset(&plugin, 0, sizeof(plugin));
>
> This is an interesting way to set a pointer's value to NULL. But I think
> it's redundant.

You're right. This is a stray code - it made sense in the older versions
when the plugin variable was static.

>> +	memset(&cap, 0, sizeof(cap));
>
> You could use cap = { 0 } in declaration.

OK.

>> +	ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, &cap);
>> +	if (ret < 0) {
>> +		V4L2_EXYNOS4_ERR("Failed to query video capabilities.");
>> +		return NULL;
>> +	}
>> +
>> +	/* Check if this is Exynos4 media device */
>> +	if (strcmp((char *) cap.driver, EXYNOS4_FIMC_DRV) &&
>> +	    strcmp((char *) cap.driver, EXYNOS4_FIMC_LITE_DRV) &&
>> +	    strcmp((char *) cap.driver, EXYNOS4_FIMC_IS_ISP_DRV)) {
>> +		V4L2_EXYNOS4_ERR("Not an Exynos4 media device.");
>> +		return NULL;
>> +	}
>> +
>> +	/* Obtain the node name of the opened device */
>> +	ret = media_get_devname_by_fd(fd, video_devname);
>> +	if (ret < 0) {
>> +		V4L2_EXYNOS4_ERR("Failed to get video device node name.");
>> +		return NULL;
>> +	}
>> +
>> +	/*
>> +	 * Create the representation of a media device
>> +	 * containing the opened video device.
>> +	 */
>> +	media = media_device_new_by_entity_devname(video_devname);
>> +	if (media == NULL) {
>> +		V4L2_EXYNOS4_ERR("Failed to create media device.");
>> +		return NULL;
>> +	}
>> +
>> +#ifdef DEBUG
>> +	media_debug_set_handler(media, (void (*)(void *, ...))fprintf, stdout);
>> +#endif
>> +
>> +	/* Get the entity representing the opened video device node */
>> +	sink_entity = media_get_entity_by_devname(media, video_devname, strlen(video_devname));
>
> Could you use the fd directly instead of translating that to the device
> node? fstat(2) gives you directly inode / device major + minor which you can
> then use to find the MC device.

OK.

>> +	if (sink_entity == NULL) {
>> +		V4L2_EXYNOS4_ERR("Failed to get sinkd entity name.");
>> +		goto err_get_sink_entity;
>> +	}
>> +
>> +	/* The last entity in the pipeline represents video device node */
>> +	media_entity_set_fd(sink_entity, fd);
>> +
>> +	sink_entity_name = media_entity_get_name(sink_entity);
>> +
>> +	/* Check if video entity is of capture type, not m2m */
>> +	if (!__capture_entity(sink_entity_name)) {
>> +		V4L2_EXYNOS4_ERR("Device not of capture type.");
>> +		goto err_get_sink_entity;
>> +	}
>> +
>> +	/* Parse media configuration file and apply its settings */
>> +	ret = mediatext_parse_setup_config(media, EXYNOS4_CAPTURE_CONF);
>> +	if (ret < 0) {
>> +		V4L2_EXYNOS4_ERR("Media config parser error.");
>> +		goto err_get_sink_entity;
>> +	}
>> +
>> +	/*
>> +	 * Discover the pipeline of sub-devices from a camera sensor
>> +	 * to the opened video device.
>> +	 */
>> +	ret = media_discover_pipeline_by_entity(media, sink_entity);
>> +	if (ret < 0) {
>> +		V4L2_EXYNOS4_ERR("Error discovering video pipeline.");
>> +		goto err_get_sink_entity;
>> +	}
>> +
>> +	/* Open all sub-devices in the discovered pipeline */
>> +	ret = media_open_pipeline_subdevs(media);
>> +	if (ret < 0) {
>> +		V4L2_EXYNOS4_ERR("Error opening video pipeline.");
>> +		goto err_get_sink_entity;
>> +	}
>> +
>> +	/* Allocate private data */
>> +	plugin = calloc(1, sizeof(*plugin));
>> +	if (!plugin)
>> +		goto err_validate_controls;
>> +
>> +	plugin->media = media;
>> +
>> +	V4L2_EXYNOS4_LOG("Initialized exynos4-camera plugin.");
>> +
>> +	return plugin;
>> +
>> +err_validate_controls:
>> +	media_close_pipeline_subdevs(media);
>> +err_get_sink_entity:
>> +	if (media)
>> +		media_device_unref(media);
>> +	return NULL;
>> +}
>> +
>> +static void plugin_close(void *dev_ops_priv)
>> +{
>> +	struct exynos4_camera_plugin *plugin;
>> +	struct media_device *media;
>> +
>> +	if (dev_ops_priv == NULL)
>> +		return;
>> +
>> +	plugin = (struct exynos4_camera_plugin *) dev_ops_priv;
>
> You don't need to cast a void pointer to another pointer. You could also
> make the assignment in variable declaration.

Good point, thanks.

Best Regards,
Jacek Anaszewski


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

* Re: [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers
  2014-11-21 16:14 ` [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers Jacek Anaszewski
@ 2014-11-28 17:06   ` Sakari Ailus
  2014-12-01 11:23     ` Jacek Anaszewski
  0 siblings, 1 reply; 26+ messages in thread
From: Sakari Ailus @ 2014-11-28 17:06 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Jacek,

On Fri, Nov 21, 2014 at 05:14:34PM +0100, Jacek Anaszewski wrote:
> Add new graph helpers useful for video pipeline discovering.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  utils/media-ctl/libmediactl.c |  174 +++++++++++++++++++++++++++++++++++++++++
>  utils/media-ctl/mediactl.h    |  121 ++++++++++++++++++++++++++++
>  2 files changed, 295 insertions(+)
> 
> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> index af7dd43..a476601 100644
> --- a/utils/media-ctl/libmediactl.c
> +++ b/utils/media-ctl/libmediactl.c
> @@ -35,6 +35,7 @@
>  #include <unistd.h>
>  
>  #include <linux/media.h>
> +#include <linux/kdev_t.h>
>  #include <linux/videodev2.h>
>  
>  #include "mediactl.h"
> @@ -87,6 +88,28 @@ struct media_entity *media_get_entity_by_name(struct media_device *media,
>  	return NULL;
>  }
>  
> +struct media_entity *media_get_entity_by_devname(struct media_device *media,
> +					      const char *devname, size_t length)
> +{
> +	unsigned int i;
> +
> +	/* A match is impossible if the entity devname is longer than the maximum

Over 80 characters per line.

> +	 * size we can get from the kernel.
> +	 */
> +	if (length >= FIELD_SIZEOF(struct media_entity, devname))
> +		return NULL;
> +
> +	for (i = 0; i < media->entities_count; ++i) {
> +		struct media_entity *entity = &media->entities[i];
> +
> +		if (strncmp(entity->devname, devname, length) == 0 &&
> +		    entity->devname[length] == '\0')
> +			return entity;
> +	}
> +
> +	return NULL;
> +}
> +
>  struct media_entity *media_get_entity_by_id(struct media_device *media,
>  					    __u32 id)
>  {
> @@ -145,6 +168,11 @@ const char *media_entity_get_devname(struct media_entity *entity)
>  	return entity->devname[0] ? entity->devname : NULL;
>  }
>  
> +const char *media_entity_get_name(struct media_entity *entity)
> +{
> +	return entity->info.name ? entity->info.name : NULL;

You could simply return entity->info.name.

> +}
> +
>  struct media_entity *media_get_default_entity(struct media_device *media,
>  					      unsigned int type)
>  {
> @@ -177,6 +205,152 @@ const struct media_entity_desc *media_entity_get_info(struct media_entity *entit
>  	return &entity->info;
>  }
>  
> +int media_get_link_by_sink_pad(struct media_device *media,
> +				struct media_pad *pad,
> +				struct media_link **link)
> +{
> +	struct media_link *cur_link = NULL;
> +	int i, j;
> +
> +	if (pad == NULL || link == NULL)
> +		return -EINVAL;
> +
> +	for (i = 0; i < media->entities_count; ++i) {
> +		for (j = 0; j < media->entities[i].num_links; ++j) {
> +			cur_link = &media->entities[i].links[j];
> +			if ((cur_link->flags & MEDIA_LNK_FL_ENABLED) &&
> +			    /* check if cur_link's sink entity matches the pad parent entity */
> +			    (cur_link->sink->entity->info.id == pad->entity->info.id) &&

Hmm. This looks harder than it should be. Would it be possible loop over the
array of links in struct media_entity instead, and look for a sink?

> +			    /* check if cur_link's sink pad id matches */
> +			    (cur_link->sink->index == pad->index)) {
> +				*link = cur_link;
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +int media_get_link_by_source_pad(struct media_entity *entity,
> +				struct media_pad *pad,
> +				struct media_link **link)
> +{
> +	int i;
> +
> +	if (entity == NULL || pad == NULL || link == NULL)
> +		return -EINVAL;
> +
> +	for (i = 0; i < entity->num_links; ++i) {

...just like you do it here. :-)

> +		if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) &&
> +		    (entity->links[i].source->index == pad->index)) {
> +			*link = &entity->links[i];
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +int media_get_pads_by_entity(struct media_entity *entity, unsigned int type,

type should be flags, and flags uses __u32 type.

> +				struct media_pad **pads, int *num_pads)
> +{
> +	struct media_pad *entity_pads;
> +	int cnt_pads, i;
> +
> +	if (entity == NULL || pads == NULL || num_pads == NULL)
> +		return -EINVAL;
> +
> +	entity_pads = malloc(sizeof(*entity_pads));

How about allocating room for entity->info.pads pads, and then making it
smaller as needed?

> +	cnt_pads = 0;

You could use *num_pads instead of cnt_pads.

> +	for (i = 0; i < entity->info.pads; ++i) {
> +		if (entity->pads[i].flags & type) {
> +			entity_pads = realloc(entity_pads, (i + 1) *
> +					      sizeof(*entity_pads));
> +			entity_pads[cnt_pads++] = entity->pads[i];
> +		}
> +	}
> +
> +	if (cnt_pads == 0)
> +		free(entity_pads);
> +
> +	*pads = entity_pads;
> +	*num_pads = cnt_pads;
> +
> +	return 0;
> +}
> +
> +int media_get_busy_pads_by_entity(struct media_device *media,
> +				struct media_entity *entity,
> +				unsigned int type,
> +				struct media_pad **busy_pads,
> +				int *num_busy_pads)

Are you looking for enabled links that someone else would have configured
here?

I think we should have a more generic solution to that. This one still does
not guard against concurrent user space processes that attempt to configure
the media device.

One possibility would be to add IOCTLs to grant and release exclusive write
(i.e. change configuration) access to the device. Once streaming is started,
exclusive access could be released by the user. I wonder what Laurent would
think about that. I think this would be very robust --- one could start with
resetting all the links one can, and then configure those that are needed;
if this fails, then the pipeline is already used by someone else and
streaming cannot taken place on it. No cleanup of the configuration is
needed.

But this is definitely out of scope of this patchset (also because this is
for the user space).

> +{
> +	struct media_pad *bpads, *pads;
> +	struct media_link *link;
> +	int cnt_bpads = 0, num_pads, i, ret;
> +
> +	if (entity == NULL || busy_pads == NULL || num_busy_pads == NULL ||
> +	    (type == MEDIA_PAD_FL_SINK && media == NULL))
> +		return -EINVAL;
> +
> +	ret = media_get_pads_by_entity(entity, type, &pads, &num_pads);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	if (num_pads == 0)
> +		goto done;
> +
> +	bpads = malloc(sizeof(*pads));
> +	if (bpads == NULL)
> +		goto error_ret;
> +
> +	for (i = 0; i < num_pads; ++i) {
> +		if (type == MEDIA_PAD_FL_SINK)
> +			ret = media_get_link_by_sink_pad(media, &pads[i], &link);
> +		else
> +			ret = media_get_link_by_source_pad(entity, &pads[i], &link);
> +		if (ret == 0) {
> +			bpads = realloc(bpads, (i + 1) *
> +						sizeof(*bpads));
> +			bpads[cnt_bpads++] = pads[i];
> +		}
> +	}
> +	if (num_pads > 0)
> +		free(pads);
> +
> +	if (cnt_bpads == 0)
> +		free(bpads);
> +
> +done:
> +	*busy_pads = bpads;
> +	*num_busy_pads = cnt_bpads;
> +
> +	return 0;
> +
> +error_ret:
> +	return -EINVAL;
> +}
> +
> +int media_get_pad_by_index(struct media_pad *pads, int num_pads,
> +				int index, struct media_pad *out_pad)
> +{
> +	int i;
> +
> +	if (pads == NULL || out_pad == NULL)
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_pads; ++i) {
> +		if (pads[i].index == index) {
> +			*out_pad = pads[i];
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Open/close
>   */
> diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
> index 7309b16..18a1e0e 100644
> --- a/utils/media-ctl/mediactl.h
> +++ b/utils/media-ctl/mediactl.h
> @@ -23,6 +23,7 @@
>  #define __MEDIA_H__
>  
>  #include <linux/media.h>
> +#include <sys/types.h>
>  
>  struct media_link {
>  	struct media_pad *source;
> @@ -231,6 +232,16 @@ const struct media_link *media_entity_get_link(struct media_entity *entity,
>  const char *media_entity_get_devname(struct media_entity *entity);
>  
>  /**
> + * @brief Get the name for an entity
> + * @param entity - media entity.
> + *
> + * This function returns the name of the entity.
> + *
> + * @return A pointer to the string with entity name
> + */
> +const char *media_entity_get_name(struct media_entity *entity);
> +
> +/**
>   * @brief Get the type of an entity.
>   * @param entity - the entity.
>   *
> @@ -255,6 +266,19 @@ struct media_entity *media_get_entity_by_name(struct media_device *media,
>  	const char *name, size_t length);
>  
>  /**
> + * @brief Find an entity by the corresponding device node name.
> + * @param media - media device.
> + * @param devname - device node name.
> + * @param length - size of @a devname.
> + *
> + * Search for an entity with a device node name equal to @a devname.
> + *
> + * @return A pointer to the entity if found, or NULL otherwise.
> + */
> +struct media_entity *media_get_entity_by_devname(struct media_device *media,
> +	const char *devname, size_t length);
> +
> +/**
>   * @brief Find an entity by its ID.
>   * @param media - media device.
>   * @param id - entity ID.
> @@ -435,6 +459,103 @@ int media_parse_setup_link(struct media_device *media,
>  int media_parse_setup_links(struct media_device *media, const char *p);
>  
>  /**
> + * @brief Get entity's pads of a given type
> + * @param entity - media entity
> + * @param type - pad type (MEDIA_PAD_FL_SINK or MEDIA_PAD_FL_SOURCE)
> + * @param pads - array of matching pads
> + * @param num_pads - number of matching pads
> + *
> + * Get only sink or source pads for an entity. The returned pads
> + * array has to be freed by the caller.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int media_get_pads_by_entity(struct media_entity *entity,
> +				unsigned int type,
> +				struct media_pad **pads,
> +				int *num_pads);
> +/**
> + * @brief Get occupied entity's pads of a given type
> + * @param media - media device
> + * @param entity - media entity
> + * @param type - pad type (MEDIA_PAD_FL_SINK or MEDIA_PAD_FL_SOURCE)
> + * @param busy_pads - array of matching pads
> + * @param num_busy_pads - number of matching pads
> + *
> + * Get only sink or source pads for an entity, with active links.
> + * The returned pads array has to be freed by the caller.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int media_get_busy_pads_by_entity(struct media_device *media,
> +				struct media_entity *entity,
> +				unsigned int type,
> +				struct media_pad **busy_pads,
> +				int *num_busy_pads);
> +
> +/**
> + * @brief Get link for  sink pad
> + * @param media - media device
> + * @param pad - pad to search the link for
> + * @param link - matching link
> + *
> + * Get the link connected to the entity's sink pad.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int media_get_link_by_sink_pad(struct media_device *media,
> +				struct media_pad *pad,
> +				struct media_link **link);
> +
> +/**
> + * @brief Get link for source pad
> + * @param entity - media entity
> + * @param pad - pad to search the link for
> + * @param link - matching link
> + *
> + * Get the link connected to the entity's source pad.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int media_get_link_by_source_pad(struct media_entity *entity,
> +				struct media_pad *pad,
> +				struct media_link **link);
> +
> +/**
> + * @brief Get pad with given index
> + * @param pads - array of pads
> + * @param num_pads - number of pads in the array
> + * @param index - index of a pad to search for
> + * @param out_pad - matching pad
> + *
> + * Get pad with given index from the given pads array.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int media_get_pad_by_index(struct media_pad *pads, int num_pads,
> +				int index, struct media_pad *out_pad);
> +
> +/**
> + * @brief Get source pad of the pipeline entity
> + * @param entity - media entity
> + *
> + * This function returns the source pad of the entity.
> + *
> + * @return entity source pad, or NULL if the entity is not linked.
> + */
> +int media_entity_get_src_pad_index(struct media_entity *entity);
> +
> +/**
> + * @brief Get sink pad of the pipeline entity
> + * @param entity - media entity
> + *
> + * This function returns the sink pad of the entity.
> + *
> + * @return entity sink pad, or NULL if the entity is not linked
> + */
> +int media_entity_get_sink_pad_index(struct media_entity *entity);
> +
> +/**
>   * @brief Get file descriptor of the entity sub-device
>   * @param entity - media entity
>   *

-- 
Kind regards,

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

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

* Re: [PATCH/RFC v4 07/11] media-ctl: libv4l2subdev: add VYUY8_2X8 mbus code
  2014-11-21 16:14 ` [PATCH/RFC v4 07/11] media-ctl: libv4l2subdev: add VYUY8_2X8 mbus code Jacek Anaszewski
@ 2014-11-28 17:10   ` Sakari Ailus
  0 siblings, 0 replies; 26+ messages in thread
From: Sakari Ailus @ 2014-11-28 17:10 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

On Fri, Nov 21, 2014 at 05:14:36PM +0100, Jacek Anaszewski wrote:
> The VYUY8_2X8 media bus format is the only one supported
> by the S5C73M3 camera sensor, that is a part of the media
> device on the Exynos4412-trats2 board.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  utils/media-ctl/libv4l2subdev.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index 4c5fb12..a96ed7a 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -704,6 +704,7 @@ static struct {
>  	{ "YUYV", V4L2_MBUS_FMT_YUYV8_1X16 },
>  	{ "YUYV1_5X8", V4L2_MBUS_FMT_YUYV8_1_5X8 },
>  	{ "YUYV2X8", V4L2_MBUS_FMT_YUYV8_2X8 },
> +	{ "VYUY8_2X8", V4L2_MBUS_FMT_VYUY8_2X8 },
>  	{ "UYVY", V4L2_MBUS_FMT_UYVY8_1X16 },
>  	{ "UYVY1_5X8", V4L2_MBUS_FMT_UYVY8_1_5X8 },
>  	{ "UYVY2X8", V4L2_MBUS_FMT_UYVY8_2X8 },

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

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

* Re: [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers
  2014-11-28 17:06   ` Sakari Ailus
@ 2014-12-01 11:23     ` Jacek Anaszewski
  2014-12-01 12:30       ` Sakari Ailus
  0 siblings, 1 reply; 26+ messages in thread
From: Jacek Anaszewski @ 2014-12-01 11:23 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Sakari,

Thanks for a review.

On 11/28/2014 06:06 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Fri, Nov 21, 2014 at 05:14:34PM +0100, Jacek Anaszewski wrote:
>> Add new graph helpers useful for video pipeline discovering.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   utils/media-ctl/libmediactl.c |  174 +++++++++++++++++++++++++++++++++++++++++
>>   utils/media-ctl/mediactl.h    |  121 ++++++++++++++++++++++++++++
>>   2 files changed, 295 insertions(+)
>>
>> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
>> index af7dd43..a476601 100644
>> --- a/utils/media-ctl/libmediactl.c
>> +++ b/utils/media-ctl/libmediactl.c
>> @@ -35,6 +35,7 @@
>>   #include <unistd.h>
>>
>>   #include <linux/media.h>
>> +#include <linux/kdev_t.h>
>>   #include <linux/videodev2.h>
>>
>>   #include "mediactl.h"
>> @@ -87,6 +88,28 @@ struct media_entity *media_get_entity_by_name(struct media_device *media,
>>   	return NULL;
>>   }
>>
>> +struct media_entity *media_get_entity_by_devname(struct media_device *media,
>> +					      const char *devname, size_t length)
>> +{
>> +	unsigned int i;
>> +
>> +	/* A match is impossible if the entity devname is longer than the maximum
>
> Over 80 characters per line.
>
>> +	 * size we can get from the kernel.
>> +	 */
>> +	if (length >= FIELD_SIZEOF(struct media_entity, devname))
>> +		return NULL;
>> +
>> +	for (i = 0; i < media->entities_count; ++i) {
>> +		struct media_entity *entity = &media->entities[i];
>> +
>> +		if (strncmp(entity->devname, devname, length) == 0 &&
>> +		    entity->devname[length] == '\0')
>> +			return entity;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>>   struct media_entity *media_get_entity_by_id(struct media_device *media,
>>   					    __u32 id)
>>   {
>> @@ -145,6 +168,11 @@ const char *media_entity_get_devname(struct media_entity *entity)
>>   	return entity->devname[0] ? entity->devname : NULL;
>>   }
>>
>> +const char *media_entity_get_name(struct media_entity *entity)
>> +{
>> +	return entity->info.name ? entity->info.name : NULL;
>
> You could simply return entity->info.name.

Right.

>> +}
>> +
>>   struct media_entity *media_get_default_entity(struct media_device *media,
>>   					      unsigned int type)
>>   {
>> @@ -177,6 +205,152 @@ const struct media_entity_desc *media_entity_get_info(struct media_entity *entit
>>   	return &entity->info;
>>   }
>>
>> +int media_get_link_by_sink_pad(struct media_device *media,
>> +				struct media_pad *pad,
>> +				struct media_link **link)
>> +{
>> +	struct media_link *cur_link = NULL;
>> +	int i, j;
>> +
>> +	if (pad == NULL || link == NULL)
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < media->entities_count; ++i) {
>> +		for (j = 0; j < media->entities[i].num_links; ++j) {
>> +			cur_link = &media->entities[i].links[j];
>> +			if ((cur_link->flags & MEDIA_LNK_FL_ENABLED) &&
>> +			    /* check if cur_link's sink entity matches the pad parent entity */
>> +			    (cur_link->sink->entity->info.id == pad->entity->info.id) &&
>
> Hmm. This looks harder than it should be. Would it be possible loop over the
> array of links in struct media_entity instead, and look for a sink?

I thought that the entity has only outbound links as it is in case of
MEDIA_IOC_ENUM_LINKS ioctl, but it seems that it has also the
inbound ones. Nice.

>> +			    /* check if cur_link's sink pad id matches */
>> +			    (cur_link->sink->index == pad->index)) {
>> +				*link = cur_link;
>> +				return 0;
>> +			}
>> +		}
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +int media_get_link_by_source_pad(struct media_entity *entity,
>> +				struct media_pad *pad,
>> +				struct media_link **link)
>> +{
>> +	int i;
>> +
>> +	if (entity == NULL || pad == NULL || link == NULL)
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < entity->num_links; ++i) {
>
> ...just like you do it here. :-)
>
>> +		if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) &&
>> +		    (entity->links[i].source->index == pad->index)) {
>> +			*link = &entity->links[i];
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +int media_get_pads_by_entity(struct media_entity *entity, unsigned int type,
>
> type should be flags, and flags uses __u32 type.
>
>> +				struct media_pad **pads, int *num_pads)
>> +{
>> +	struct media_pad *entity_pads;
>> +	int cnt_pads, i;
>> +
>> +	if (entity == NULL || pads == NULL || num_pads == NULL)
>> +		return -EINVAL;
>> +
>> +	entity_pads = malloc(sizeof(*entity_pads));
>
> How about allocating room for entity->info.pads pads, and then making it
> smaller as needed?

Sounds reasonable.

>> +	cnt_pads = 0;
>
> You could use *num_pads instead of cnt_pads.
>
>> +	for (i = 0; i < entity->info.pads; ++i) {
>> +		if (entity->pads[i].flags & type) {
>> +			entity_pads = realloc(entity_pads, (i + 1) *
>> +					      sizeof(*entity_pads));
>> +			entity_pads[cnt_pads++] = entity->pads[i];
>> +		}
>> +	}
>> +
>> +	if (cnt_pads == 0)
>> +		free(entity_pads);
>> +
>> +	*pads = entity_pads;
>> +	*num_pads = cnt_pads;
>> +
>> +	return 0;
>> +}
>> +
>> +int media_get_busy_pads_by_entity(struct media_device *media,
>> +				struct media_entity *entity,
>> +				unsigned int type,
>> +				struct media_pad **busy_pads,
>> +				int *num_busy_pads)
>
> Are you looking for enabled links that someone else would have configured
> here?

The assumption is made here that there will be no concurrent users of
a media device and an entity will have no more than one link connected
to its sink pad. If this assumption is not valid than all the links
in the pipeline would have to be defined in the media config and
the pipeline would have to be only validated not discovered.
By pipeline validation I mean checking whether all config links are
enabled

> I think we should have a more generic solution to that. This one still does
> not guard against concurrent user space processes that attempt to configure
> the media device.
> One possibility would be to add IOCTLs to grant and release exclusive write
> (i.e. change configuration) access to the device. Once streaming is started,
> exclusive access could be released by the user. I wonder what Laurent would
> think about that. I think this would be very robust --- one could start with
> resetting all the links one can, and then configure those that are needed;
> if this fails, then the pipeline is already used by someone else and
> streaming cannot taken place on it. No cleanup of the configuration is
> needed.

This approach would preclude having more than one pipeline configured
in a media device.

> But this is definitely out of scope of this patchset (also because this is
> for the user space).

Taking into account that there are cases when it would be useful
to allow for having more than one active pipelines in a media device
I think that we would require changes in the media controller API.

I would hide from the user a possibility of reconfiguring the links
one by one, but instead provide an ioctl which would accept
a definition of a whole pipeline to be linked. Something
similar to extended controls.
A user space process calling such an ioctl would take the ownership
of the all involved sub-devices, and their linkage couldn't be
reconfigured until released.

Best Regards,
Jacek Anaszewski

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

* Re: [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers
  2014-12-01 11:23     ` Jacek Anaszewski
@ 2014-12-01 12:30       ` Sakari Ailus
  2014-12-01 14:21         ` Jacek Anaszewski
  0 siblings, 1 reply; 26+ messages in thread
From: Sakari Ailus @ 2014-12-01 12:30 UTC (permalink / raw)
  To: Jacek Anaszewski, Sakari Ailus
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, laurent.pinchart

Hi Jacek,

Jacek Anaszewski wrote:
...
>>> +int media_get_busy_pads_by_entity(struct media_device *media,
>>> +                struct media_entity *entity,
>>> +                unsigned int type,
>>> +                struct media_pad **busy_pads,
>>> +                int *num_busy_pads)
>>
>> Are you looking for enabled links that someone else would have configured
>> here?
>
> The assumption is made here that there will be no concurrent users of
> a media device and an entity will have no more than one link connected
> to its sink pad. If this assumption is not valid than all the links
> in the pipeline would have to be defined in the media config and
> the pipeline would have to be only validated not discovered.
> By pipeline validation I mean checking whether all config links are
> enabled

You do get an error from MEDIA_IOC_LINK_SETUP if enabling a link fails.

>> I think we should have a more generic solution to that. This one still
>> does
>> not guard against concurrent user space processes that attempt to
>> configure
>> the media device.
>> One possibility would be to add IOCTLs to grant and release exclusive
>> write
>> (i.e. change configuration) access to the device. Once streaming is
>> started,
>> exclusive access could be released by the user. I wonder what Laurent
>> would
>> think about that. I think this would be very robust --- one could
>> start with
>> resetting all the links one can, and then configure those that are
>> needed;
>> if this fails, then the pipeline is already used by someone else and
>> streaming cannot taken place on it. No cleanup of the configuration is
>> needed.
>
> This approach would preclude having more than one pipeline configured
> in a media device.

That's not true. You can *configure* a single pipeline at once, but once 
that one is streaming (or write access is allowed from other file 
handles again), you can configure another one that does no conflict with 
the first one.

>> But this is definitely out of scope of this patchset (also because
>> this is
>> for the user space).
>
> Taking into account that there are cases when it would be useful
> to allow for having more than one active pipelines in a media device
> I think that we would require changes in the media controller API.
>
> I would hide from the user a possibility of reconfiguring the links
> one by one, but instead provide an ioctl which would accept
> a definition of a whole pipeline to be linked. Something
> similar to extended controls.
> A user space process calling such an ioctl would take the ownership
> of the all involved sub-devices, and their linkage couldn't be
> reconfigured until released.

That does not mean someone else could reconfigure the links before you 
attempt to start streaming.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

* Re: [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers
  2014-12-01 12:30       ` Sakari Ailus
@ 2014-12-01 14:21         ` Jacek Anaszewski
  0 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2014-12-01 14:21 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Sakari Ailus, linux-media, m.chehab, gjasny, hdegoede,
	hans.verkuil, b.zolnierkie, kyungmin.park, laurent.pinchart

Hi Sakari,

On 12/01/2014 01:30 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> Jacek Anaszewski wrote:
> ...
>>>> +int media_get_busy_pads_by_entity(struct media_device *media,
>>>> +                struct media_entity *entity,
>>>> +                unsigned int type,
>>>> +                struct media_pad **busy_pads,
>>>> +                int *num_busy_pads)
>>>
>>> Are you looking for enabled links that someone else would have
>>> configured
>>> here?
>>
>> The assumption is made here that there will be no concurrent users of
>> a media device and an entity will have no more than one link connected
>> to its sink pad. If this assumption is not valid than all the links
>> in the pipeline would have to be defined in the media config and
>> the pipeline would have to be only validated not discovered.
>> By pipeline validation I mean checking whether all config links are
>> enabled
>
> You do get an error from MEDIA_IOC_LINK_SETUP if enabling a link fails.

My intention here was to discuss the situation when there is more
than one active link connected to an entity sink pad(s).

There are two options:
a) more than one active link connected to the same sink pad
    (I don't know if this has been ever considered a valid arrangement)
b) more than one active link connected to separate sink pads
    if the same entity.

In both cases discovering a pipeline will require different approach
than in the recent patch set, where there are only configurable
links expected in the media config and those fixed aren't taken
into account. After the config links are set up, the pipeline
is discovered by walking from the sink entity upstream, until
the entity with no sink pads is encountered.

If more than one link is connected to the sink pad(s) of
an entity the situation becomes ambiguous.

Therefore I propose to define all the links for the pipeline
in the media config. The fixed links could be marked with
relevant flags. By validating the pipeline I meant checking
whether all links from the media configuration file are
active.

>>> I think we should have a more generic solution to that. This one still
>>> does
>>> not guard against concurrent user space processes that attempt to
>>> configure
>>> the media device.
>>> One possibility would be to add IOCTLs to grant and release exclusive
>>> write
>>> (i.e. change configuration) access to the device. Once streaming is
>>> started,
>>> exclusive access could be released by the user. I wonder what Laurent
>>> would
>>> think about that. I think this would be very robust --- one could
>>> start with
>>> resetting all the links one can, and then configure those that are
>>> needed;
>>> if this fails, then the pipeline is already used by someone else and
>>> streaming cannot taken place on it. No cleanup of the configuration is
>>> needed.
>>
>> This approach would preclude having more than one pipeline configured
>> in a media device.
>
> That's not true. You can *configure* a single pipeline at once, but once
> that one is streaming (or write access is allowed from other file
> handles again), you can configure another one that does no conflict with
> the first one.

OK, I missed that.

>>> But this is definitely out of scope of this patchset (also because
>>> this is
>>> for the user space).
>>
>> Taking into account that there are cases when it would be useful
>> to allow for having more than one active pipelines in a media device
>> I think that we would require changes in the media controller API.
>>
>> I would hide from the user a possibility of reconfiguring the links
>> one by one, but instead provide an ioctl which would accept
>> a definition of a whole pipeline to be linked. Something
>> similar to extended controls.
>> A user space process calling such an ioctl would take the ownership
>> of the all involved sub-devices, and their linkage couldn't be
>> reconfigured until released.
>
> That does not mean someone else could reconfigure the links before you
> attempt to start streaming.

Therefore this ioctl should lock media device until VIDIOC_STREAMON
on the configured pipeline. I am not sure about other implications and
feasibility of such a design though.

Best Regards,
Jacek Anaszewski

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

* Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
  2014-11-27  8:41   ` Sakari Ailus
  2014-11-28 13:29     ` Jacek Anaszewski
@ 2015-02-27  9:32     ` Jacek Anaszewski
  1 sibling, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2015-02-27  9:32 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, m.chehab, gjasny, hdegoede, hans.verkuil,
	b.zolnierkie, kyungmin.park, sakari.ailus, laurent.pinchart

Hi Sakari,

On 11/27/2014 09:41 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Fri, Nov 21, 2014 at 05:14:40PM +0100, Jacek Anaszewski wrote:
>> The plugin provides support for the media device on Exynos4 SoC.
>> It performs single plane <-> multi plane API conversion,
>> video pipeline linking and takes care of automatic data format
>> negotiation for the whole pipeline, after intercepting
>> VIDIOC_S_FMT or VIDIOC_TRY_FMT ioctls.
>>
[...]
>> +
>> +static void *plugin_init(int fd)
>> +{
>> +	struct v4l2_capability cap;
>> +	struct exynos4_camera_plugin *plugin = NULL;
>> +	const char *sink_entity_name;
>> +	struct media_device *media;
>> +	struct media_entity *sink_entity;
>> +	char video_devname[32];
>> +	int ret;
[...]
>> +	ret = SYS_IOCTL(fd, VIDIOC_QUERYCAP, &cap);
>> +	if (ret < 0) {
>> +		V4L2_EXYNOS4_ERR("Failed to query video capabilities.");
>> +		return NULL;
>> +	}
>> +
>> +	/* Check if this is Exynos4 media device */
>> +	if (strcmp((char *) cap.driver, EXYNOS4_FIMC_DRV) &&
>> +	    strcmp((char *) cap.driver, EXYNOS4_FIMC_LITE_DRV) &&
>> +	    strcmp((char *) cap.driver, EXYNOS4_FIMC_IS_ISP_DRV)) {
>> +		V4L2_EXYNOS4_ERR("Not an Exynos4 media device.");
>> +		return NULL;
>> +	}
>> +
>> +	/* Obtain the node name of the opened device */
>> +	ret = media_get_devname_by_fd(fd, video_devname);
>> +	if (ret < 0) {
>> +		V4L2_EXYNOS4_ERR("Failed to get video device node name.");
>> +		return NULL;
>> +	}
>> +
>> +	/*
>> +	 * Create the representation of a media device
>> +	 * containing the opened video device.
>> +	 */
>> +	media = media_device_new_by_entity_devname(video_devname);
>> +	if (media == NULL) {
>> +		V4L2_EXYNOS4_ERR("Failed to create media device.");
>> +		return NULL;
>> +	}
>> +
>> +#ifdef DEBUG
>> +	media_debug_set_handler(media, (void (*)(void *, ...))fprintf, stdout);
>> +#endif
>> +
>> +	/* Get the entity representing the opened video device node */
>> +	sink_entity = media_get_entity_by_devname(media, video_devname, strlen(video_devname));
>
> Could you use the fd directly instead of translating that to the device
> node? fstat(2) gives you directly inode / device major + minor which you can
> then use to find the MC device.

After trying to switch it as you requested I decided to stay by current
implementation to avoid the need for translating fd to device node
twice.

If we changed:

media_device_new_by_entity_devname -> media_device_new_by_entity_fd

then media_device_new_by_entity_fd would have to call fstat to find
out the entity node name. Nonetheless we would have to call fstat
one more time to obtain sink_entity to be passed below to
media_entity_get_name.

Therefore, it is better to obtain devname once and use it for
both creating media_device and obtaining sink_entity node name.


>> +	if (sink_entity == NULL) {
>> +		V4L2_EXYNOS4_ERR("Failed to get sinkd entity name.");
>> +		goto err_get_sink_entity;
>> +	}
>> +
>> +	/* The last entity in the pipeline represents video device node */
>> +	media_entity_set_fd(sink_entity, fd);
>> +
>> +	sink_entity_name = media_entity_get_name(sink_entity);
>> +
>> +	/* Check if video entity is of capture type, not m2m */
>> +	if (!__capture_entity(sink_entity_name)) {
>> +		V4L2_EXYNOS4_ERR("Device not of capture type.");
>> +		goto err_get_sink_entity;
>> +	}

-- 
Best Regards,
Jacek Anaszewski

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

* Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
  2014-11-21 16:14 ` [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera Jacek Anaszewski
  2014-11-27  8:41   ` Sakari Ailus
@ 2015-03-15 19:07   ` Gregor Jasny
  2015-03-16  8:54     ` Jacek Anaszewski
  2015-03-15 19:12   ` Gregor Jasny
  2 siblings, 1 reply; 26+ messages in thread
From: Gregor Jasny @ 2015-03-15 19:07 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-media
  Cc: hdegoede, hans.verkuil, b.zolnierkie, kyungmin.park,
	sakari.ailus, laurent.pinchart

On 21/11/14 17:14, Jacek Anaszewski wrote:

> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 3a0e19c..56b3a9f 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -5,7 +5,12 @@ SUBDIRS = \
>  	libv4l2rds \
>  	libv4l-mplane
>  
> +if WITH_V4LUTILS
> +SUBDIRS += \
> +	libv4l-exynos4-camera
> +endif

Why do you depend on WITH_V4LUTILS for a libv4l plugin? This looks
wrong. WITH_V4LUTILS is intended to only switch off the utilities in
utils (see root Makefile.am).

Thanks,
Gregor

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

* Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
  2014-11-21 16:14 ` [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera Jacek Anaszewski
  2014-11-27  8:41   ` Sakari Ailus
  2015-03-15 19:07   ` Gregor Jasny
@ 2015-03-15 19:12   ` Gregor Jasny
  2 siblings, 0 replies; 26+ messages in thread
From: Gregor Jasny @ 2015-03-15 19:12 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-media
  Cc: hdegoede, hans.verkuil, b.zolnierkie, kyungmin.park,
	sakari.ailus, laurent.pinchart

> diff --git a/lib/libv4l-exynos4-camera/Makefile.am b/lib/libv4l-exynos4-camera/Makefile.am
> new file mode 100644
> index 0000000..23c60c6
> --- /dev/null
> +++ b/lib/libv4l-exynos4-camera/Makefile.am
> @@ -0,0 +1,7 @@
> +if WITH_V4L_PLUGINS
> +libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la
> +endif
> +
> +libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c ../../utils/media-ctl/libv4l2media_ioctl.c ../../utils/media-ctl/mediatext.c
> +libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99

Please use $(CFLAG_VISIBILITY) instead of -fvisibility=hidden. Also c99
is default. If you don't need GNU extensions, please drop the -std=gnu99.

Thanks,
Gregor

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

* Re: [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera
  2015-03-15 19:07   ` Gregor Jasny
@ 2015-03-16  8:54     ` Jacek Anaszewski
  0 siblings, 0 replies; 26+ messages in thread
From: Jacek Anaszewski @ 2015-03-16  8:54 UTC (permalink / raw)
  To: Gregor Jasny
  Cc: linux-media, hdegoede, hans.verkuil, b.zolnierkie, kyungmin.park,
	sakari.ailus, laurent.pinchart

On 03/15/2015 08:07 PM, Gregor Jasny wrote:
> On 21/11/14 17:14, Jacek Anaszewski wrote:
>
>> diff --git a/lib/Makefile.am b/lib/Makefile.am
>> index 3a0e19c..56b3a9f 100644
>> --- a/lib/Makefile.am
>> +++ b/lib/Makefile.am
>> @@ -5,7 +5,12 @@ SUBDIRS = \
>>   	libv4l2rds \
>>   	libv4l-mplane
>>
>> +if WITH_V4LUTILS
>> +SUBDIRS += \
>> +	libv4l-exynos4-camera
>> +endif
>
> Why do you depend on WITH_V4LUTILS for a libv4l plugin? This looks
> wrong. WITH_V4LUTILS is intended to only switch off the utilities in
> utils (see root Makefile.am).

This is an issue to be resolved. I need to depend on WITH_V4LUTILS,
because the plugin depends on utils' libraries (e.g. libmediactl.so and
lib4lsubdev.so). On the other hand some utils depend on core libs.

Temporarily the libv4-exynos4-camera plugin doesn't link against utils
libraries but has their sources compiled-in to avoid cyclic
dependencies. I submit this as a subject for discussion on how to adjust
the build system to handle such a configuration.


-- 
Best Regards,
Jacek Anaszewski

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

end of thread, other threads:[~2015-03-16  8:54 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-21 16:14 [PATCH/RFC v4 00/11] Add a plugin for Exynos4 camera Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 01/11] mediactl: Introduce v4l2_subdev structure Jacek Anaszewski
2014-11-25 11:36   ` Sakari Ailus
2014-11-25 12:22     ` Jacek Anaszewski
2014-11-26 10:20       ` Sakari Ailus
2014-11-21 16:14 ` [PATCH/RFC v4 02/11] mediactl: Add support for v4l2 controls Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 03/11] mediactl: Separate entity and pad parsing Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 04/11] mediatext: Add library Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 05/11] mediactl: Add media device graph helpers Jacek Anaszewski
2014-11-28 17:06   ` Sakari Ailus
2014-12-01 11:23     ` Jacek Anaszewski
2014-12-01 12:30       ` Sakari Ailus
2014-12-01 14:21         ` Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 06/11] mediactl: Add media_device creation helpers Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 07/11] media-ctl: libv4l2subdev: add VYUY8_2X8 mbus code Jacek Anaszewski
2014-11-28 17:10   ` Sakari Ailus
2014-11-21 16:14 ` [PATCH/RFC v4 08/11] mediactl: Add support for media device pipelines Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 09/11] mediactl: Close only pipeline sub-devices Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 10/11] mediactl: Add media device ioctl API Jacek Anaszewski
2014-11-21 16:14 ` [PATCH/RFC v4 11/11] Add a libv4l plugin for Exynos4 camera Jacek Anaszewski
2014-11-27  8:41   ` Sakari Ailus
2014-11-28 13:29     ` Jacek Anaszewski
2015-02-27  9:32     ` Jacek Anaszewski
2015-03-15 19:07   ` Gregor Jasny
2015-03-16  8:54     ` Jacek Anaszewski
2015-03-15 19:12   ` Gregor Jasny

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).