[media-ctl PATCH 0/3] Reference counting to subdev file handles and media bus code enumeration

[media-ctl PATCH 0/3] Reference counting to subdev file handles and media bus code enumeration

[Sakari Ailus]

Hi,

This patchset adds reference counting to subdev file handles, which
leaves it up to the user whether to keep the file handles open and thus
the devices powered.

Media bus code enumeration is added by the two latter patches of which
the second one makes the result sorted by codes.

Sakari Ailus (3):
      Count users for entities
      Implement v4l2_subdev_enum_mbus_code()
      Sort enumerated media bus codes

 src/mediactl.c   |    1 -
 src/mediactl.h   |    1 +
 src/v4l2subdev.c |  154
+++++++++++++++++++++++++++++++++++++++++++++---------
 src/v4l2subdev.h |   32 ++++++++++--
 4 files changed, 158 insertions(+), 30 deletions(-)

-- 
Kind regards,

Sakari Ailus
sakari.ailus@iki.fi

[media-ctl PATCH 1/3] Count users for entities

[Sakari Ailus]

The subdev nodes used to be closed immediately on v4l2_subdev_close(), now
the subdev is only closed if there are no users left anymore. This changes the
API from immediate effect (close) to a reference counting one.

Also make functions opening subdevs to close them before returning. This
resolves issues on some machines where not all subdevs can be opened at the
same time. Power management wise this is a sound choice since it forces to
make the decision when to keep a device powered rather than keeping
everything open all the time.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 src/mediactl.c   |    1 -
 src/mediactl.h   |    1 +
 src/v4l2subdev.c |   94 ++++++++++++++++++++++++++++++++++++++++--------------
 src/v4l2subdev.h |   12 ++++---
 4 files changed, 78 insertions(+), 30 deletions(-)

diff --git a/src/mediactl.c b/src/mediactl.c
index 46562de..7600714 100644
--- a/src/mediactl.c
+++ b/src/mediactl.c
@@ -382,7 +382,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;
 
diff --git a/src/mediactl.h b/src/mediactl.h
index 2296fe2..79934a7 100644
--- a/src/mediactl.h
+++ b/src/mediactl.h
@@ -49,6 +49,7 @@ struct media_entity {
 
 	char devname[32];
 	int fd;
+	unsigned int open_count;
 	__u32 padding[6];
 };
 
diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
index d0c37f3..56964ce 100644
--- a/src/v4l2subdev.c
+++ b/src/v4l2subdev.c
@@ -38,7 +38,8 @@
 
 int v4l2_subdev_open(struct media_entity *entity)
 {
-	if (entity->fd != -1)
+	entity->open_count++;
+	if (entity->open_count > 1)
 		return 0;
 
 	entity->fd = open(entity->devname, O_RDWR);
@@ -46,6 +47,7 @@ int v4l2_subdev_open(struct media_entity *entity)
 		media_dbg(entity->media,
 			  "%s: Failed to open subdev device node %s\n", __func__,
 			  entity->devname);
+		entity->open_count--;
 		return -errno;
 	}
 
@@ -54,8 +56,12 @@ int v4l2_subdev_open(struct media_entity *entity)
 
 void v4l2_subdev_close(struct media_entity *entity)
 {
-	close(entity->fd);
-	entity->fd = -1;
+	entity->open_count--;
+
+	if (!entity->open_count) {
+		close(entity->fd);
+		entity->fd = -1;
+	}
 }
 
 int v4l2_subdev_get_format(struct media_entity *entity,
@@ -74,10 +80,16 @@ int v4l2_subdev_get_format(struct media_entity *entity,
 	fmt.which = which;
 
 	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_FMT, &fmt);
-	if (ret < 0)
-		return -errno;
+	if (ret < 0) {
+		ret = -errno;
+		goto err;
+	}
 
 	*format = fmt.format;
+
+err:
+	v4l2_subdev_close(entity);
+
 	return 0;
 }
 
@@ -98,10 +110,16 @@ int v4l2_subdev_set_format(struct media_entity *entity,
 	fmt.format = *format;
 
 	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FMT, &fmt);
-	if (ret < 0)
-		return -errno;
+	if (ret < 0) {
+		ret = -errno;
+		goto err;
+	}
 
 	*format = fmt.format;
+
+err:
+	v4l2_subdev_close(entity);
+
 	return 0;
 }
 
@@ -127,21 +145,29 @@ int v4l2_subdev_get_selection(struct media_entity *entity,
 	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_SELECTION, &u.sel);
 	if (ret >= 0) {
 		*rect = u.sel.r;
-		return 0;
+		goto done;
+	}
+	if (errno != ENOTTY || target != V4L2_SEL_TGT_CROP) {
+		ret = -errno;
+		goto done;
 	}
-	if (errno != ENOTTY || target != V4L2_SEL_TGT_CROP)
-		return -errno;
 
 	memset(&u.crop, 0, sizeof(u.crop));
 	u.crop.pad = pad;
 	u.crop.which = which;
 
 	ret = ioctl(entity->fd, VIDIOC_SUBDEV_G_CROP, &u.crop);
-	if (ret < 0)
-		return -errno;
+	if (ret < 0) {
+		ret = -errno;
+		goto done;
+	}
 
 	*rect = u.crop.rect;
-	return 0;
+
+done:
+	v4l2_subdev_close(entity);
+
+	return ret;
 }
 
 int v4l2_subdev_set_selection(struct media_entity *entity,
@@ -167,10 +193,12 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
 	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_SELECTION, &u.sel);
 	if (ret >= 0) {
 		*rect = u.sel.r;
-		return 0;
+		goto done;
+	}
+	if (errno != ENOTTY || target != V4L2_SEL_TGT_CROP) {
+		ret = -errno;
+		goto done;
 	}
-	if (errno != ENOTTY || target != V4L2_SEL_TGT_CROP)
-		return -errno;
 
 	memset(&u.crop, 0, sizeof(u.crop));
 	u.crop.pad = pad;
@@ -178,11 +206,17 @@ int v4l2_subdev_set_selection(struct media_entity *entity,
 	u.crop.rect = *rect;
 
 	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_CROP, &u.crop);
-	if (ret < 0)
-		return -errno;
+	if (ret < 0) {
+		ret = -errno;
+		goto done;
+	}
 
 	*rect = u.crop.rect;
-	return 0;
+
+done:
+	v4l2_subdev_close(entity);
+
+	return ret;
 }
 
 int v4l2_subdev_get_frame_interval(struct media_entity *entity,
@@ -198,11 +232,17 @@ 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);
-	if (ret < 0)
-		return -errno;
+	if (ret < 0) {
+		ret = -errno;
+		goto err;
+	}
 
 	*interval = ival.interval;
-	return 0;
+
+err:
+	v4l2_subdev_close(entity);
+
+	return ret;
 }
 
 int v4l2_subdev_set_frame_interval(struct media_entity *entity,
@@ -219,10 +259,16 @@ int v4l2_subdev_set_frame_interval(struct media_entity *entity,
 	ival.interval = *interval;
 
 	ret = ioctl(entity->fd, VIDIOC_SUBDEV_S_FRAME_INTERVAL, &ival);
-	if (ret < 0)
-		return -errno;
+	if (ret < 0) {
+		ret = -errno;
+		goto err;
+	}
 
 	*interval = ival.interval;
+
+err:
+	v4l2_subdev_close(entity);
+
 	return 0;
 }
 
diff --git a/src/v4l2subdev.h b/src/v4l2subdev.h
index 5d55482..43d5c1f 100644
--- a/src/v4l2subdev.h
+++ b/src/v4l2subdev.h
@@ -30,19 +30,21 @@ struct media_entity;
  * @brief Open a sub-device.
  * @param entity - sub-device media entity.
  *
- * Open the V4L2 subdev device node associated with @a entity. The file
- * descriptor is stored in the media_entity structure.
+ * Open the V4L2 subdev device node associated with @a entity. The
+ * file descriptor is stored in the media_entity structure. The subdev
+ * will stay open until the last user has closed it using
+ * v4l2_subdev_close(), i.e. the calls are reference-counted.
  *
  * @return 0 on success, or a negative error code on failure.
  */
 int v4l2_subdev_open(struct media_entity *entity);
 
 /**
- * @brief Close a sub-device.
+ * @brief Release a sub-device.
  * @param entity - sub-device media entity.
  *
- * Close the V4L2 subdev device node associated with the @a entity and opened by
- * a previous call to v4l2_subdev_open() (either explicit or implicit).
+ * Releases a reference to a sub-device. The device will stay open
+ * until the last remaining user has released the device.
  */
 void v4l2_subdev_close(struct media_entity *entity);
 
-- 
1.7.10.4

[media-ctl PATCH 2/3] Implement v4l2_subdev_enum_mbus_code()

[Sakari Ailus]

v4l2_subdev_enum_mbus_code() enumerates the media bus pixel codes on a pad
of a given entity. The function returns a struct containing the array of
enumerated pixel codes. Once the user no longer needs the array, its memory
is released using free(3).

The decision not to store the media bus pixel codes in the library's
internal data structures is an informed one: the configuration of the device
could affect the selection of possible pixel codes on the device. One
example of such is the chosen try pixel code on the sink pad which is known
to affect the selection of available pixel codes on the source pad of the
OMAP 3 ISP resizer.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 src/v4l2subdev.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 src/v4l2subdev.h |   20 ++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
index 56964ce..946c030 100644
--- a/src/v4l2subdev.c
+++ b/src/v4l2subdev.c
@@ -27,6 +27,7 @@
 #include <fcntl.h>
 #include <stdbool.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
@@ -64,6 +65,51 @@ void v4l2_subdev_close(struct media_entity *entity)
 	}
 }
 
+struct v4l2_subdev_mbus_codes *
+v4l2_subdev_enum_mbus_code(struct media_entity *entity, unsigned int pad)
+{
+	struct v4l2_subdev_mbus_code_enum c;
+	struct v4l2_subdev_mbus_codes *codes = NULL;
+	int ret;
+
+	ret = v4l2_subdev_open(entity);
+	if (ret < 0)
+		return NULL;
+
+	memset(&c, 0, sizeof(c));
+	c.pad = pad;
+
+	for (; ; c.index++) {
+		void *tmp;
+
+		ret = ioctl(entity->fd, VIDIOC_SUBDEV_ENUM_MBUS_CODE, &c);
+		if (ret == -1) {
+			if (errno == EINVAL)
+				break;
+			else
+				goto out_err;
+		}
+
+		tmp = realloc(codes, sizeof(*codes)
+			      + (c.index + 1) * sizeof(codes->code[0]));
+		if (!tmp)
+			goto out_err;
+		codes = tmp;
+
+		codes->code[c.index] = c.code;
+	}
+
+	codes->ncode = c.index;
+
+	v4l2_subdev_close(entity);
+	return codes;
+
+out_err:
+	v4l2_subdev_close(entity);
+	free(codes);
+	return NULL;
+}
+
 int v4l2_subdev_get_format(struct media_entity *entity,
 	struct v4l2_mbus_framefmt *format, unsigned int pad,
 	enum v4l2_subdev_format_whence which)
diff --git a/src/v4l2subdev.h b/src/v4l2subdev.h
index 43d5c1f..8b42f4d 100644
--- a/src/v4l2subdev.h
+++ b/src/v4l2subdev.h
@@ -48,6 +48,26 @@ int v4l2_subdev_open(struct media_entity *entity);
  */
 void v4l2_subdev_close(struct media_entity *entity);
 
+struct v4l2_subdev_mbus_codes {
+	unsigned int ncode;
+	unsigned int code[0];
+} __packed;
+
+/**
+ * @brief Enumerate mbus pixel codes.
+ * @param entity - subdev-device media entity.
+ * @param pad - the pad of the media entity
+ *
+ * Enumerate media bus pixel codes. This is just a wrapper for
+ * VIDIOC_SUBDEV_ENUM_MBUS_CODE IOCTL.
+ *
+ * @return NULL on error; otherwise struct v4l2_subdev_mbus_code_enum
+ * containing the media bus codes. The user MUST use free(3) to
+ * release the returned struct once the user no longer needs it.
+ */
+struct v4l2_subdev_mbus_codes *
+v4l2_subdev_enum_mbus_code(struct media_entity *entity, unsigned int pad);
+
 /**
  * @brief Retrieve the format on a pad.
  * @param entity - subdev-device media entity.
-- 
1.7.10.4

[media-ctl PATCH 3/3] Sort enumerated media bus codes

[Sakari Ailus]

Sort the enumeration result. This makes e.g. matching formats at both ends
of the link much easier.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 src/v4l2subdev.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/v4l2subdev.c b/src/v4l2subdev.c
index 946c030..d553216 100644
--- a/src/v4l2subdev.c
+++ b/src/v4l2subdev.c
@@ -65,6 +65,19 @@ void v4l2_subdev_close(struct media_entity *entity)
 	}
 }
 
+static int mbus_code_sort(const void *p1, const void *p2)
+{
+	const unsigned int *c1 = p1, *c2 = p2;
+
+	if (*c1 < *c2)
+		return -1;
+
+	if (*c1 > *c2)
+		return 1;
+
+	return 0;
+}
+
 struct v4l2_subdev_mbus_codes *
 v4l2_subdev_enum_mbus_code(struct media_entity *entity, unsigned int pad)
 {
@@ -100,6 +113,7 @@ v4l2_subdev_enum_mbus_code(struct media_entity *entity, unsigned int pad)
 	}
 
 	codes->ncode = c.index;
+	qsort(codes->code, codes->ncode, sizeof(*codes->code), &mbus_code_sort);
 
 	v4l2_subdev_close(entity);
 	return codes;
-- 
1.7.10.4