[RFC/PATCH v3 00/10] Media controller (core and V4L2)

[RFC/PATCH v3 00/10] Media controller (core and V4L2)

[Laurent Pinchart]

Hi everybody,

Here's the third version of the media controller patches. All comments received
on the first and second versions have (hopefully) been incorporated.

The rebased V4L2 API additions and OMAP3 ISP patches will follow. Once again
please consider them as sample code only.

Laurent Pinchart (8):
  media: Media device node support
  media: Media device
  media: Entities, pads and links
  media: Entities, pads and links enumeration
  media: Links setup
  v4l: Add a media_device pointer to the v4l2_device structure
  v4l: Make video_device inherit from media_entity
  v4l: Make v4l2_subdev inherit from media_entity

Sakari Ailus (2):
  media: Entity graph traversal
  media: Reference count and power handling

 Documentation/media-framework.txt            |  481 ++++++++++++++++++++
 Documentation/video4linux/v4l2-framework.txt |   71 +++-
 drivers/media/Makefile                       |    8 +-
 drivers/media/media-device.c                 |  327 ++++++++++++++
 drivers/media/media-devnode.c                |  326 ++++++++++++++
 drivers/media/media-entity.c                 |  613 ++++++++++++++++++++++++++
 drivers/media/video/v4l2-dev.c               |   35 ++-
 drivers/media/video/v4l2-device.c            |   45 ++-
 drivers/media/video/v4l2-subdev.c            |   27 ++-
 include/linux/media.h                        |   78 ++++
 include/media/media-device.h                 |   70 +++
 include/media/media-devnode.h                |   84 ++++
 include/media/media-entity.h                 |  107 +++++
 include/media/v4l2-dev.h                     |    6 +
 include/media/v4l2-device.h                  |    2 +
 include/media/v4l2-subdev.h                  |    7 +
 16 files changed, 2265 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/media-framework.txt
 create mode 100644 drivers/media/media-device.c
 create mode 100644 drivers/media/media-devnode.c
 create mode 100644 drivers/media/media-entity.c
 create mode 100644 include/linux/media.h
 create mode 100644 include/media/media-device.h
 create mode 100644 include/media/media-devnode.h
 create mode 100644 include/media/media-entity.h

-- 
Regards,

Laurent Pinchart

[RFC/PATCH v3 01/10] media: Media device node support

[Laurent Pinchart]

The media_devnode structure provides support for registering and
unregistering character devices using a dynamic major number. Reference
counting is handled internally, making device drivers easier to write
without having to solve the open/disconnect race condition issue over
and over again.

The code is based on video/v4l2-dev.c.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/Makefile        |    8 +-
 drivers/media/media-devnode.c |  326 +++++++++++++++++++++++++++++++++++++++++
 include/media/media-devnode.h |   84 +++++++++++
 3 files changed, 416 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/media-devnode.c
 create mode 100644 include/media/media-devnode.h

diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index 499b081..c1b5938 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -2,7 +2,11 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
+media-objs	:= media-devnode.o
+
+obj-$(CONFIG_MEDIA_SUPPORT)	+= media.o
+
 obj-y += common/ IR/ video/
 
-obj-$(CONFIG_VIDEO_DEV) += radio/
-obj-$(CONFIG_DVB_CORE)  += dvb/
+obj-$(CONFIG_VIDEO_DEV)		+= radio/
+obj-$(CONFIG_DVB_CORE)		+= dvb/
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
new file mode 100644
index 0000000..6f5558c
--- /dev/null
+++ b/drivers/media/media-devnode.c
@@ -0,0 +1,326 @@
+/*
+ * Media device node
+ *
+ * Generic media device node infrastructure to register and unregister
+ * character devices using a dynamic major number and proper reference
+ * counting.
+ *
+ * Copyright 2010 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Based on drivers/media/video/v4l2_dev.c code authored by
+ *
+ *	Mauro Carvalho Chehab <mchehab@infradead.org> (version 2)
+ *	Alan Cox, <alan@lxorguk.ukuu.org.uk> (version 1)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/kmod.h>
+#include <linux/slab.h>
+#include <linux/mm.h>
+#include <linux/smp_lock.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+#include <asm/system.h>
+
+#include <media/media-devnode.h>
+
+#define MEDIA_NUM_DEVICES	256
+#define MEDIA_NAME		"media"
+
+static dev_t media_dev_t;
+
+/*
+ *	sysfs stuff
+ */
+
+static ssize_t show_name(struct device *cd,
+			 struct device_attribute *attr, char *buf)
+{
+	struct media_devnode *mdev = to_media_devnode(cd);
+
+	return sprintf(buf, "%.*s\n", (int)sizeof(mdev->name), mdev->name);
+}
+
+static struct device_attribute media_devnode_attrs[] = {
+	__ATTR(name, S_IRUGO, show_name, NULL),
+	__ATTR_NULL
+};
+
+/*
+ *	Active devices
+ */
+static DEFINE_MUTEX(media_devnode_lock);
+static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
+
+/* Called when the last user of the media device exits. */
+static void media_devnode_release(struct device *cd)
+{
+	struct media_devnode *mdev = to_media_devnode(cd);
+
+	mutex_lock(&media_devnode_lock);
+
+	/* Delete the cdev on this minor as well */
+	cdev_del(&mdev->cdev);
+
+	/* Mark device node number as free */
+	clear_bit(mdev->minor, media_devnode_nums);
+
+	mutex_unlock(&media_devnode_lock);
+
+	/* Release media_devnode and perform other cleanups as needed. */
+	if (mdev->release)
+		mdev->release(mdev);
+}
+
+static struct class media_class = {
+	.name = MEDIA_NAME,
+	.dev_attrs = media_devnode_attrs,
+};
+
+static ssize_t media_read(struct file *filp, char __user *buf,
+		size_t sz, loff_t *off)
+{
+	struct media_devnode *mdev = media_devnode_data(filp);
+
+	if (!mdev->fops->read)
+		return -EINVAL;
+	if (!media_devnode_is_registered(mdev))
+		return -EIO;
+	return mdev->fops->read(filp, buf, sz, off);
+}
+
+static ssize_t media_write(struct file *filp, const char __user *buf,
+		size_t sz, loff_t *off)
+{
+	struct media_devnode *mdev = media_devnode_data(filp);
+
+	if (!mdev->fops->write)
+		return -EINVAL;
+	if (!media_devnode_is_registered(mdev))
+		return -EIO;
+	return mdev->fops->write(filp, buf, sz, off);
+}
+
+static unsigned int media_poll(struct file *filp,
+			       struct poll_table_struct *poll)
+{
+	struct media_devnode *mdev = media_devnode_data(filp);
+
+	if (!mdev->fops->poll || !media_devnode_is_registered(mdev))
+		return DEFAULT_POLLMASK;
+	return mdev->fops->poll(filp, poll);
+}
+
+static long media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	struct media_devnode *mdev = media_devnode_data(filp);
+
+	if (!mdev->fops->unlocked_ioctl)
+		return -ENOTTY;
+
+	if (!media_devnode_is_registered(mdev))
+		return -EIO;
+
+	return mdev->fops->unlocked_ioctl(filp, cmd, arg);
+}
+
+/* Override for the open function */
+static int media_open(struct inode *inode, struct file *filp)
+{
+	struct media_devnode *mdev;
+	int ret;
+
+	/* Check if the media device is available. This needs to be done with
+	 * the media_devnode_lock held to prevent an open/unregister race:
+	 * without the lock, the device could be unregistered and freed between
+	 * the media_devnode_is_registered() and get_device() calls, leading to
+	 * a crash.
+	 */
+	mutex_lock(&media_devnode_lock);
+	mdev = container_of(inode->i_cdev, struct media_devnode, cdev);
+	/* return ENODEV if the media device has been removed
+	   already or if it is not registered anymore. */
+	if (!media_devnode_is_registered(mdev)) {
+		mutex_unlock(&media_devnode_lock);
+		return -ENODEV;
+	}
+	/* and increase the device refcount */
+	get_device(&mdev->dev);
+	mutex_unlock(&media_devnode_lock);
+	if (mdev->fops->open) {
+		ret = mdev->fops->open(filp);
+		if (ret) {
+			put_device(&mdev->dev);
+			return ret;
+		}
+	}
+
+	filp->private_data = mdev;
+	return 0;
+}
+
+/* Override for the release function */
+static int media_release(struct inode *inode, struct file *filp)
+{
+	struct media_devnode *mdev = media_devnode_data(filp);
+	int ret = 0;
+
+	if (mdev->fops->release)
+		mdev->fops->release(filp);
+
+	/* decrease the refcount unconditionally since the release()
+	   return value is ignored. */
+	put_device(&mdev->dev);
+	filp->private_data = NULL;
+	return ret;
+}
+
+static const struct file_operations media_devnode_fops = {
+	.owner = THIS_MODULE,
+	.read = media_read,
+	.write = media_write,
+	.open = media_open,
+	.unlocked_ioctl = media_ioctl,
+	.release = media_release,
+	.poll = media_poll,
+	.llseek = no_llseek,
+};
+
+/**
+ * media_devnode_register - register a media device node
+ * @mdev: media device node structure we want to register
+ *
+ * The registration code assigns minor numbers and registers the new device node
+ * with the kernel. An error is returned if no free minor number can be found,
+ * or if the registration of the device node fails.
+ *
+ * Zero is returned on success.
+ *
+ * Note that if the media_devnode_register call fails, the release() callback of
+ * the media_devnode structure is *not* called, so the caller is responsible for
+ * freeing any data.
+ */
+int __must_check media_devnode_register(struct media_devnode *mdev)
+{
+	int minor;
+	int ret;
+
+	/* Part 1: Find a free minor number */
+	mutex_lock(&media_devnode_lock);
+	minor = find_next_zero_bit(media_devnode_nums, 0, MEDIA_NUM_DEVICES);
+	if (minor == MEDIA_NUM_DEVICES) {
+		mutex_unlock(&media_devnode_lock);
+		printk(KERN_ERR "could not get a free minor\n");
+		return -ENFILE;
+	}
+
+	set_bit(mdev->minor, media_devnode_nums);
+	mutex_unlock(&media_devnode_lock);
+
+	mdev->minor = minor;
+
+	/* Part 2: Initialize and register the character device */
+	cdev_init(&mdev->cdev, &media_devnode_fops);
+	mdev->cdev.owner = mdev->fops->owner;
+
+	ret = cdev_add(&mdev->cdev, MKDEV(MAJOR(media_dev_t), mdev->minor), 1);
+	if (ret < 0) {
+		printk(KERN_ERR "%s: cdev_add failed\n", __func__);
+		goto error;
+	}
+
+	/* Part 3: Register the media device */
+	mdev->dev.class = &media_class;
+	mdev->dev.devt = MKDEV(MAJOR(media_dev_t), mdev->minor);
+	mdev->dev.release = media_devnode_release;
+	if (mdev->parent)
+		mdev->dev.parent = mdev->parent;
+	dev_set_name(&mdev->dev, "media%d", mdev->minor);
+	ret = device_register(&mdev->dev);
+	if (ret < 0) {
+		printk(KERN_ERR "%s: device_register failed\n", __func__);
+		goto error;
+	}
+
+	/* Part 4: Activate this minor. The char device can now be used. */
+	set_bit(MEDIA_FLAG_REGISTERED, &mdev->flags);
+
+	return 0;
+
+error:
+	cdev_del(&mdev->cdev);
+	mutex_lock(&media_devnode_lock);
+	clear_bit(mdev->minor, media_devnode_nums);
+	mutex_unlock(&media_devnode_lock);
+	return ret;
+}
+
+/**
+ * media_devnode_unregister - unregister a media device node
+ * @mdev: the device node to unregister
+ *
+ * This unregisters the passed device. Future open calls will be met with
+ * errors.
+ *
+ * This function can safely be called if the device node has never been
+ * registered or has already been unregistered.
+ */
+void media_devnode_unregister(struct media_devnode *mdev)
+{
+	/* Check if mdev was ever registered at all */
+	if (!media_devnode_is_registered(mdev))
+		return;
+
+	mutex_lock(&media_devnode_lock);
+	clear_bit(MEDIA_FLAG_REGISTERED, &mdev->flags);
+	mutex_unlock(&media_devnode_lock);
+	device_unregister(&mdev->dev);
+}
+
+/*
+ *	Initialise media for linux
+ */
+static int __init media_devnode_init(void)
+{
+	int ret;
+
+	printk(KERN_INFO "Linux media interface: v0.10\n");
+	ret = alloc_chrdev_region(&media_dev_t, 0, MEDIA_NUM_DEVICES,
+				  MEDIA_NAME);
+	if (ret < 0) {
+		printk(KERN_WARNING "media: unable to allcoate major\n");
+		return ret;
+	}
+
+	ret = class_register(&media_class);
+	if (ret < 0) {
+		unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
+		printk(KERN_WARNING "media: class_register failed\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static void __exit media_devnode_exit(void)
+{
+	class_unregister(&media_class);
+	unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
+}
+
+module_init(media_devnode_init)
+module_exit(media_devnode_exit)
+
+MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
+MODULE_DESCRIPTION("Device node registration for media drivers");
+MODULE_LICENSE("GPL");
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
new file mode 100644
index 0000000..ade03b7
--- /dev/null
+++ b/include/media/media-devnode.h
@@ -0,0 +1,84 @@
+/*
+ * Media device node handling
+ *
+ * Copyright (C) 2010  Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * Common functions for media-related drivers to register and unregister media
+ * device nodes.
+ */
+#ifndef _MEDIA_DEVNODE_H
+#define _MEDIA_DEVNODE_H
+
+#include <linux/poll.h>
+#include <linux/fs.h>
+#include <linux/device.h>
+#include <linux/cdev.h>
+
+/*
+ * Flag to mark the media_devnode struct as registered. Drivers must not touch
+ * this flag directly, it will be set and cleared by media_devnode_register and
+ * media_devnode_unregister.
+ */
+#define MEDIA_FLAG_REGISTERED	0
+
+struct media_file_operations {
+	struct module *owner;
+	ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
+	ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);
+	unsigned int (*poll) (struct file *, struct poll_table_struct *);
+	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
+	int (*open) (struct file *);
+	int (*release) (struct file *);
+};
+
+/**
+ * struct media_devnode - Media device node
+ * @parent:	parent device
+ * @name:	media device node name
+ * @minor:	device node minor number
+ * @flags:	flags, combination of the MEDIA_FLAG_* constants
+ *
+ * This structure represents a media-related device node.
+ *
+ * The @parent is a physical device. It must be set by core or device drivers
+ * before registering the node.
+ *
+ * @name is a descriptive name exported through sysfs. It doesn't have to be
+ * unique.
+ */
+struct media_devnode {
+	/* device ops */
+	const struct media_file_operations *fops;
+
+	/* sysfs */
+	struct device dev;		/* media device */
+	struct cdev cdev;		/* character device */
+	struct device *parent;		/* device parent */
+
+	/* device info */
+	char name[32];
+
+	int minor;
+	unsigned long flags;		/* Use bitops to access flags */
+
+	/* callbacks */
+	void (*release)(struct media_devnode *mdev);
+};
+
+/* dev to media_devnode */
+#define to_media_devnode(cd) container_of(cd, struct media_devnode, dev)
+
+int __must_check media_devnode_register(struct media_devnode *mdev);
+void media_devnode_unregister(struct media_devnode *mdev);
+
+static inline struct media_devnode *media_devnode_data(struct file *filp)
+{
+	return filp->private_data;
+}
+
+static inline int media_devnode_is_registered(struct media_devnode *mdev)
+{
+	return test_bit(MEDIA_FLAG_REGISTERED, &mdev->flags);
+}
+
+#endif /* _MEDIA_DEVNODE_H */
-- 
1.7.1

[RFC/PATCH v3 02/10] media: Media device

[Laurent Pinchart]

The media_device structure abstracts functions common to all kind of
media devices (v4l2, dvb, alsa, ...). It manages media entities and
offers a userspace API to discover and configure the media device
internal topology.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/media-framework.txt |   68 +++++++++++++++++++++++++++++++++
 drivers/media/Makefile            |    2 +-
 drivers/media/media-device.c      |   76 +++++++++++++++++++++++++++++++++++++
 include/media/media-device.h      |   48 +++++++++++++++++++++++
 4 files changed, 193 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/media-framework.txt
 create mode 100644 drivers/media/media-device.c
 create mode 100644 include/media/media-device.h

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
new file mode 100644
index 0000000..b942c8f
--- /dev/null
+++ b/Documentation/media-framework.txt
@@ -0,0 +1,68 @@
+Linux kernel media framework
+============================
+
+This document describes the Linux kernel media framework, its data structures,
+functions and their usage.
+
+
+Introduction
+------------
+
+Media devices increasingly handle multiple related functions. Many USB cameras
+include microphones, video capture hardware can also output video, or SoC
+camera interfaces also perform memory-to-memory operations similar to video
+codecs.
+
+Independent functions, even when implemented in the same hardware, can be
+modeled by separate devices. A USB camera with a microphone will be presented
+to userspace applications as V4L2 and ALSA capture devices. The devices
+relationships (when using a webcam, end-users shouldn't have to manually
+select the associated USB microphone), while not made available directly to
+applications by the drivers, can usually be retrieved from sysfs.
+
+With more and more advanced SoC devices being introduced, the current approach
+will not scale. Device topologies are getting increasingly complex and can't
+always be represented by a tree structure. Hardware blocks are shared between
+different functions, creating dependencies between seemingly unrelated
+devices.
+
+Kernel abstraction APIs such as V4L2 and ALSA provide means for applications
+to access hardware parameters. As newer hardware expose an increasingly high
+number of those parameters, drivers need to guess what applications really
+require based on limited information, thereby implementing policies that
+belong to userspace.
+
+The media kernel API aims at solving those problems.
+
+
+Media device
+------------
+
+A media device is represented by a struct media_device instance, defined in
+include/media/media-device.h. Allocation of the structure is handled by the
+media device driver, usually by embedding the media_device instance in a
+larger driver-specific structure.
+
+Drivers register media device instances by calling
+
+	media_device_register(struct media_device *mdev);
+
+The caller is responsible for initializing the media_device structure before
+registration. The following fields must be set:
+
+ - dev should point to the parent device (usually a pci_dev, usb_interface or
+   platform_device instance). In the rare case when no parent device is
+   available (with ISA devices for instance), the field can be set to NULL.
+ - name should be set to the device name. If the name is empty a parent device
+   must be set. In that case the name will be set to the parent device driver
+   name followed by a space and the parent device name.
+
+Upon successful registration a character device named media[0-9]+ is created.
+The device major and minor numbers are dynamic.
+
+Drivers unregister media device instances by calling
+
+	media_device_unregister(struct media_device *mdev);
+
+Unregistering a media device that hasn't been registered is *NOT* safe.
+
diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index c1b5938..f8d8dcb 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
-media-objs	:= media-devnode.o
+media-objs	:= media-device.o media-devnode.o
 
 obj-$(CONFIG_MEDIA_SUPPORT)	+= media.o
 
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
new file mode 100644
index 0000000..75eaccd
--- /dev/null
+++ b/drivers/media/media-device.c
@@ -0,0 +1,76 @@
+/*
+ *  Media device support.
+ *
+ *  Copyright (C) 2010  Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+#include <media/media-device.h>
+#include <media/media-devnode.h>
+
+static const struct media_file_operations media_device_fops = {
+	.owner = THIS_MODULE,
+};
+
+static void media_device_release(struct media_devnode *mdev)
+{
+}
+
+/**
+ * media_device_register - register a media device
+ * @mdev:	The media device
+ *
+ * The caller is responsible for initializing the media device before
+ * registration. The following fields must be set:
+ *
+ * - dev should point to the parent device. The field can be NULL when no
+ *   parent device is available (for instance with ISA devices).
+ * - devnode.name should be set to the device name. If the name is empty a
+ *   parent device must be set. In that case the name will be set to the parent
+ *   device driver name followed by a space and the parent device name.
+ */
+int __must_check media_device_register(struct media_device *mdev)
+{
+	/* If dev == NULL, then name must be filled in by the caller */
+	if (mdev->dev == NULL && WARN_ON(!mdev->devnode.name[0]))
+		return 0;
+
+	/* Set name to driver name + device name if it is empty. */
+	if (!mdev->devnode.name[0])
+		snprintf(mdev->devnode.name, sizeof(mdev->devnode.name),
+			 "%s %s", mdev->dev->driver->name, dev_name(mdev->dev));
+
+	/* Register the device node. */
+	mdev->devnode.fops = &media_device_fops;
+	mdev->devnode.parent = mdev->dev;
+	mdev->devnode.release = media_device_release;
+	return media_devnode_register(&mdev->devnode);
+}
+EXPORT_SYMBOL_GPL(media_device_register);
+
+/**
+ * media_device_unregister - unregister a media device
+ * @mdev:	The media device
+ *
+ */
+void media_device_unregister(struct media_device *mdev)
+{
+	media_devnode_unregister(&mdev->devnode);
+}
+EXPORT_SYMBOL_GPL(media_device_unregister);
diff --git a/include/media/media-device.h b/include/media/media-device.h
new file mode 100644
index 0000000..bd559b0
--- /dev/null
+++ b/include/media/media-device.h
@@ -0,0 +1,48 @@
+/*
+ *  Media device support header.
+ *
+ *  Copyright (C) 2010  Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef _MEDIA_DEVICE_H
+#define _MEDIA_DEVICE_H
+
+#include <linux/device.h>
+#include <linux/list.h>
+
+#include <media/media-devnode.h>
+
+/* Each instance of a media device should create the media_device struct,
+ * either stand-alone or embedded in a larger struct.
+ *
+ * It allows easy access to sub-devices (see v4l2-subdev.h) and provides
+ * basic media device-level support.
+ */
+
+struct media_device {
+	/* dev->driver_data points to this struct.
+	 * Note: dev might be NULL if there is no parent device
+	 * as is the case with e.g. ISA devices.
+	 */
+	struct device *dev;
+	struct media_devnode devnode;
+};
+
+int __must_check media_device_register(struct media_device *mdev);
+void media_device_unregister(struct media_device *mdev);
+
+#endif
-- 
1.7.1

[RFC/PATCH v3 03/10] media: Entities, pads and links

[Laurent Pinchart]

As video hardware pipelines become increasingly complex and
configurable, the current hardware description through v4l2 subdevices
reaches its limits. In addition to enumerating and configuring
subdevices, video camera drivers need a way to discover and modify at
runtime how those subdevices are connected. This is done through new
elements called entities, pads and links.

An entity is a basic media hardware building block. It can correspond to
a large variety of logical blocks such as physical hardware devices
(CMOS sensor for instance), logical hardware devices (a building block
in a System-on-Chip image processing pipeline), DMA channels or physical
connectors.

A pad is a connection endpoint through which an entity can interact with
other entities. Data (not restricted to video) produced by an entity
flows from the entity's output to one or more entity inputs. Pads should
not be confused with physical pins at chip boundaries.

A link is a point-to-point oriented connection between two pads, either
on the same entity or on different entities. Data flows from a source
pad to a sink pad.

Links are stored in the source entity. To make backwards graph walk
faster, a copy of all links is also stored in the sink entity. The copy
is known as a backlink and is only used to help graph traversal.

The entity API is made of three functions:

- media_entity_init() initializes an entity. The caller must provide an
array of pads as well as an estimated number of links. The links array
is allocated dynamically and will be reallocated if it grows beyond the
initial estimate.

- media_entity_cleanup() frees resources allocated for an entity. It
must be called during the cleanup phase after unregistering the entity
and before freeing it.

- media_entity_create_link() creates a link between two entities. An
entry in the link array of each entity is allocated and stores pointers
to source and sink pads.

When a media device is unregistered, all its entities are unregistered
automatically.

The code is based on Hans Verkuil <hverkuil@xs4all.nl> initial work.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 Documentation/media-framework.txt |  130 +++++++++++++++++++++++++++++++++
 drivers/media/Makefile            |    2 +-
 drivers/media/media-device.c      |   53 ++++++++++++++
 drivers/media/media-entity.c      |  144 +++++++++++++++++++++++++++++++++++++
 include/media/media-device.h      |   16 ++++
 include/media/media-entity.h      |   85 ++++++++++++++++++++++
 6 files changed, 429 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/media-entity.c
 create mode 100644 include/media/media-entity.h

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index b942c8f..5bd7216 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -35,6 +35,30 @@ belong to userspace.
 The media kernel API aims at solving those problems.
 
 
+Abstract media device model
+---------------------------
+
+Discovering a device internal topology, and configuring it at runtime, is one
+of the goals of the media framework. To achieve this, hardware devices are
+modeled as an oriented graph of building blocks called entities connected
+through pads.
+
+An entity is a basic media hardware building block. It can correspond to
+a large variety of logical blocks such as physical hardware devices
+(CMOS sensor for instance), logical hardware devices (a building block
+in a System-on-Chip image processing pipeline), DMA channels or physical
+connectors.
+
+A pad is a connection endpoint through which an entity can interact with
+other entities. Data (not restricted to video) produced by an entity
+flows from the entity's output to one or more entity inputs. Pads should
+not be confused with physical pins at chip boundaries.
+
+A link is a point-to-point oriented connection between two pads, either
+on the same entity or on different entities. Data flows from a source
+pad to a sink pad.
+
+
 Media device
 ------------
 
@@ -66,3 +90,109 @@ Drivers unregister media device instances by calling
 
 Unregistering a media device that hasn't been registered is *NOT* safe.
 
+
+Entities, pads and links
+------------------------
+
+- Entities
+
+Entities are represented by a struct media_entity instance, defined in
+include/media/media-entity.h. The structure is usually embedded into a
+higher-level structure, such as a v4l2_subdev or video_device instance,
+although drivers can allocate entities directly.
+
+Drivers initialize entities by calling
+
+	media_entity_init(struct media_entity *entity, u8 num_pads,
+			  struct media_pad *pads, u8 extra_links);
+
+The media_entity name and type fields can be initialized before or after
+calling media_entity_init. Entities embedded in higher-level standard
+structures have those fields set by the higher-level framework.
+
+As the number of pads is known in advance, the pads array is not allocated
+dynamically but is managed by the entity driver. Most drivers will embed the
+pads array in a driver-specific structure, avoiding dynamic allocation.
+
+Drivers must set the direction of every pad in the pads array before calling
+media_entity_init. The function will initialize the other pads fields.
+
+Unlike the number of pads, the total number of links isn't always known in
+advance by the entity driver. As an initial estimate, media_entity_init
+pre-allocates a number of links equal to the number of pads plus an optional
+number of extra links. The links array will be reallocated if it grows beyond
+the initial estimate.
+
+Drivers register entities with a media device by calling
+
+	media_device_register_entity(struct media_device *mdev,
+				     struct media_entity *entity);
+
+When registered the entity is assigned an ID. Entity IDs are positive integers
+and are guaranteed to be unique in the context of the media device. The
+framework doesn't guarantee that IDs will always be continuous.
+
+Drivers unregister entities by calling
+
+	media_device_unregister_entity(struct media_entity *entity);
+
+Unregistering an entity will not change the IDs of the other entities, and the
+ID will never be reused for a newly registered entity.
+
+When a media device is unregistered, all its entities are unregistered
+automatically. No manual entities unregistration is then required.
+
+Drivers free resources associated with an entity by calling
+
+	media_entity_cleanup(struct media_entity *entity);
+
+This function must be called during the cleanup phase after unregistering the
+entity. Note that the media_entity instance itself must be freed explicitly by
+the driver if required.
+
+- Pads
+
+Pads are represented by a struct media_pad instance, defined in
+include/media/media-entity.h. Each entity stores its pads in a pads array
+managed by the entity driver. Drivers usually embed the array in a
+driver-specific structure.
+
+Pads are identified by their entity and their 0-based index in the pads array.
+Both information are stored in the media_pad structure, making the media_pad
+pointer the canonical way to store and pass link references.
+
+Pads have flags that describe the pad capabilities and state.
+
+	MEDIA_PAD_FLAG_INPUT indicates that the pad supports sinking data.
+	MEDIA_PAD_FLAG_OUTPUT indicates that the pad supports sourcing data.
+
+One and only one of MEDIA_PAD_FLAG_INPUT and MEDIA_PAD_FLAG_OUTPUT must be set
+for each pad.
+
+- Links
+
+Links are represented by a struct media_link instance, defined in
+include/media/media-entity.h. Each entity stores all links originating at or
+targetting any of its pads in a links array. A given link is thus stored
+twice, once in the source entity and once in the target entity. The array is
+pre-allocated and grows dynamically as needed.
+
+Drivers create links by calling
+
+	media_entity_create_link(struct media_entity *source, u8 source_pad,
+				 struct media_entity *sink,   u8 sink_pad,
+				 u32 flags);
+
+An entry in the link array of each entity is allocated and stores pointers
+to source and sink pads.
+
+Links have flags that describe the link capabilities and state.
+
+	MEDIA_LINK_FLAG_ACTIVE indicates that the link is active and can be
+	used to transfer media data. When two or more links target a sink pad,
+	only one of them can be active at a time.
+	MEDIA_LINK_FLAG_IMMUTABLE indicates that the link active state can't
+	be modified at runtime. If MEDIA_LINK_FLAG_IMMUTABLE is set, then
+	MEDIA_LINK_FLAG_ACTIVE must also be set since an immutable link is
+	always active.
+
diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index f8d8dcb..a425581 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
-media-objs	:= media-device.o media-devnode.o
+media-objs	:= media-device.o media-devnode.o media-entity.o
 
 obj-$(CONFIG_MEDIA_SUPPORT)	+= media.o
 
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 75eaccd..ed64b4a 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -23,6 +23,7 @@
 
 #include <media/media-device.h>
 #include <media/media-devnode.h>
+#include <media/media-entity.h>
 
 static const struct media_file_operations media_device_fops = {
 	.owner = THIS_MODULE,
@@ -47,6 +48,10 @@ static void media_device_release(struct media_devnode *mdev)
  */
 int __must_check media_device_register(struct media_device *mdev)
 {
+	mdev->entity_id = 1;
+	INIT_LIST_HEAD(&mdev->entities);
+	spin_lock_init(&mdev->lock);
+
 	/* If dev == NULL, then name must be filled in by the caller */
 	if (mdev->dev == NULL && WARN_ON(!mdev->devnode.name[0]))
 		return 0;
@@ -71,6 +76,54 @@ EXPORT_SYMBOL_GPL(media_device_register);
  */
 void media_device_unregister(struct media_device *mdev)
 {
+	struct media_entity *entity;
+	struct media_entity *next;
+
+	list_for_each_entry_safe(entity, next, &mdev->entities, list)
+		media_device_unregister_entity(entity);
+
 	media_devnode_unregister(&mdev->devnode);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
+
+/**
+ * media_device_register_entity - Register an entity with a media device
+ * @mdev:	The media device
+ * @entity:	The entity
+ */
+int __must_check media_device_register_entity(struct media_device *mdev,
+					      struct media_entity *entity)
+{
+	/* Warn if we apparently re-register an entity */
+	WARN_ON(entity->parent != NULL);
+	entity->parent = mdev;
+
+	spin_lock(&mdev->lock);
+	entity->id = mdev->entity_id++;
+	list_add_tail(&entity->list, &mdev->entities);
+	spin_unlock(&mdev->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(media_device_register_entity);
+
+/**
+ * media_device_unregister_entity - Unregister an entity
+ * @entity:	The entity
+ *
+ * If the entity has never been registered this function will return
+ * immediately.
+ */
+void media_device_unregister_entity(struct media_entity *entity)
+{
+	struct media_device *mdev = entity->parent;
+
+	if (mdev == NULL)
+		return;
+
+	spin_lock(&mdev->lock);
+	list_del(&entity->list);
+	spin_unlock(&mdev->lock);
+	entity->parent = NULL;
+}
+EXPORT_SYMBOL_GPL(media_device_unregister_entity);
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
new file mode 100644
index 0000000..a2f9ad9
--- /dev/null
+++ b/drivers/media/media-entity.c
@@ -0,0 +1,144 @@
+/*
+ *  Media Entity support
+ *
+ *  Copyright (C) 2010 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <media/media-entity.h>
+
+/**
+ * media_entity_init - Initialize a media entity
+ *
+ * @num_pads: Total number of input and output pads.
+ * @extra_links: Initial estimate of the number of extra links.
+ * @pads: Array of 'num_pads' pads.
+ *
+ * The total number of pads is an intrinsic property of entities known by the
+ * entity driver, while the total number of links depends on hardware design
+ * and is an extrinsic property unknown to the entity driver. However, in most
+ * use cases the entity driver can guess the number of links which can safely
+ * be assumed to be equal to or larger than the number of pads.
+ *
+ * For those reasons the links array can be preallocated based on the entity
+ * driver guess and will be reallocated later if extra links need to be
+ * created.
+ *
+ * This function allocates a links array with enough space to hold at least
+ * 'num_pads' + 'extra_links' elements. The media_entity::max_links field will
+ * be set to the number of allocated elements.
+ *
+ * The pads array is managed by the entity driver and passed to
+ * media_entity_init() where its pointer will be stored in the entity structure.
+ */
+int
+media_entity_init(struct media_entity *entity, u8 num_pads,
+		  struct media_pad *pads, u8 extra_links)
+{
+	struct media_link *links;
+	unsigned int max_links = num_pads + extra_links;
+	unsigned int i;
+
+	links = kzalloc(max_links * sizeof(links[0]), GFP_KERNEL);
+	if (links == NULL)
+		return -ENOMEM;
+
+	entity->max_links = max_links;
+	entity->num_links = 0;
+	entity->num_backlinks = 0;
+	entity->num_pads = num_pads;
+	entity->pads = pads;
+	entity->links = links;
+
+	for (i = 0; i < num_pads; i++) {
+		pads[i].entity = entity;
+		pads[i].index = i;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(media_entity_init);
+
+void
+media_entity_cleanup(struct media_entity *entity)
+{
+	kfree(entity->links);
+}
+EXPORT_SYMBOL(media_entity_cleanup);
+
+static struct media_link *media_entity_add_link(struct media_entity *entity)
+{
+	if (entity->num_links >= entity->max_links) {
+		struct media_link *links = entity->links;
+		unsigned int max_links = entity->max_links + 2;
+		unsigned int i;
+
+		links = krealloc(links, max_links * sizeof(*links), GFP_KERNEL);
+		if (links == NULL)
+			return NULL;
+
+		for (i = 0; i < entity->num_links; i++)
+			links[i].other->other = &links[i];
+
+		entity->max_links = max_links;
+		entity->links = links;
+	}
+
+	return &entity->links[entity->num_links++];
+}
+
+int
+media_entity_create_link(struct media_entity *source, u8 source_pad,
+			 struct media_entity *sink, u8 sink_pad, u32 flags)
+{
+	struct media_link *link;
+	struct media_link *backlink;
+
+	BUG_ON(source == NULL || sink == NULL);
+	BUG_ON(source_pad >= source->num_pads);
+	BUG_ON(sink_pad >= sink->num_pads);
+
+	link = media_entity_add_link(source);
+	if (link == NULL)
+		return -ENOMEM;
+
+	link->source = &source->pads[source_pad];
+	link->sink = &sink->pads[sink_pad];
+	link->flags = flags;
+
+	/* Create the backlink. Backlinks are used to help graph traversal and
+	 * are not reported to userspace.
+	 */
+	backlink = media_entity_add_link(sink);
+	if (backlink == NULL) {
+		source->num_links--;
+		return -ENOMEM;
+	}
+
+	backlink->source = &source->pads[source_pad];
+	backlink->sink = &sink->pads[sink_pad];
+	backlink->flags = flags;
+
+	link->other = backlink;
+	backlink->other = link;
+
+	sink->num_backlinks++;
+
+	return 0;
+}
+EXPORT_SYMBOL(media_entity_create_link);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index bd559b0..ac96847 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -23,8 +23,10 @@
 
 #include <linux/device.h>
 #include <linux/list.h>
+#include <linux/spinlock.h>
 
 #include <media/media-devnode.h>
+#include <media/media-entity.h>
 
 /* Each instance of a media device should create the media_device struct,
  * either stand-alone or embedded in a larger struct.
@@ -40,9 +42,23 @@ struct media_device {
 	 */
 	struct device *dev;
 	struct media_devnode devnode;
+
+	u32 entity_id;
+	struct list_head entities;
+
+	/* Protects the entities list */
+	spinlock_t lock;
 };
 
 int __must_check media_device_register(struct media_device *mdev);
 void media_device_unregister(struct media_device *mdev);
 
+int __must_check media_device_register_entity(struct media_device *mdev,
+					      struct media_entity *entity);
+void media_device_unregister_entity(struct media_entity *entity);
+
+/* Iterate over all entities. */
+#define media_device_for_each_entity(entity, mdev)			\
+	list_for_each_entry(entity, &(mdev)->entities, list)
+
 #endif
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
new file mode 100644
index 0000000..37a25bf
--- /dev/null
+++ b/include/media/media-entity.h
@@ -0,0 +1,85 @@
+#ifndef _MEDIA_ENTITY_H
+#define _MEDIA_ENTITY_H
+
+#include <linux/list.h>
+
+#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)
+#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
+#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
+#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
+#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
+
+#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV + 1)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
+#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3)
+
+#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
+
+#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
+#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
+
+struct media_link {
+	struct media_pad *source;	/* Source pad */
+	struct media_pad *sink;		/* Sink pad  */
+	struct media_link *other;	/* Link in the reverse direction */
+	unsigned long flags;		/* Link flags (MEDIA_LINK_FLAG_*) */
+};
+
+struct media_pad {
+	struct media_entity *entity;	/* Entity this pad belongs to */
+	u16 index;			/* Pad index in the entity pads array */
+	unsigned long flags;		/* Pad flags (MEDIA_PAD_FLAG_*) */
+};
+
+struct media_entity {
+	struct list_head list;
+	struct media_device *parent;	/* Media device this entity belongs to*/
+	u32 id;				/* Entity ID, unique in the parent media
+					 * device context */
+	const char *name;		/* Entity name */
+	u32 type;			/* Entity type (MEDIA_ENTITY_TYPE_*) */
+
+	u8 num_pads;			/* Number of input and output pads */
+	u8 num_links;			/* Number of existing links, both active
+					 * and inactive */
+	u8 num_backlinks;		/* Number of backlinks */
+	u8 max_links;			/* Maximum number of links */
+
+	struct media_pad *pads;		/* Pads array (num_pads elements) */
+	struct media_link *links;	/* Links array (max_links elements)*/
+
+	union {
+		/* Node specifications */
+		struct {
+			u32 major;
+			u32 minor;
+		} v4l;
+		struct {
+			u32 major;
+			u32 minor;
+		} fb;
+		int alsa;
+		int dvb;
+
+		/* Sub-device specifications */
+		/* Nothing needed yet */
+	};
+};
+
+#define MEDIA_ENTITY_TYPE_MASK		0xffff0000
+#define MEDIA_ENTITY_SUBTYPE_MASK	0x0000ffff
+
+#define media_entity_type(entity) \
+	((entity)->type & MEDIA_ENTITY_TYPE_MASK)
+#define media_entity_subtype(entity) \
+	((entity)->type & MEDIA_ENTITY_SUBTYPE_MASK)
+
+int media_entity_init(struct media_entity *entity, u8 num_pads,
+		struct media_pad *pads, u8 extra_links);
+void media_entity_cleanup(struct media_entity *entity);
+int media_entity_create_link(struct media_entity *source, u8 source_pad,
+		struct media_entity *sink, u8 sink_pad, u32 flags);
+
+#endif
-- 
1.7.1

[RFC/PATCH v3 04/10] media: Entity graph traversal

[Laurent Pinchart]

From: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>

Add media entity graph traversal. The traversal follows active links by
depth first. Traversing graph backwards is prevented by comparing the next
possible entity in the graph with the previous one. Multiply connected
graphs are thus not supported.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Vimarsh Zutshi <vimarsh.zutshi@nokia.com>
---
 Documentation/media-framework.txt |   40 +++++++++++++
 drivers/media/media-entity.c      |  115 +++++++++++++++++++++++++++++++++++++
 include/media/media-entity.h      |   15 +++++
 3 files changed, 170 insertions(+), 0 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 5bd7216..4fe3b32 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -196,3 +196,43 @@ Links have flags that describe the link capabilities and state.
 	MEDIA_LINK_FLAG_ACTIVE must also be set since an immutable link is
 	always active.
 
+
+Graph traversal
+---------------
+
+The media framework provides APIs to iterate over entities in a graph.
+
+To iterate over all entities belonging to a media device, drivers can use the
+media_device_for_each_entity macro, defined in include/media/media-device.h.
+
+	struct media_entity *entity;
+
+	media_device_for_each_entity(entity, mdev) {
+		/* entity will point to each entity in turn */
+		...
+	}
+
+Drivers might also need to iterate over all entities in a graph that can be
+reached only through active links starting at a given entity. The media
+framework provides a depth-first graph traversal API for that purpose.
+
+Note that graphs with cycles (whether directed or undirected) are *NOT*
+supported by the graph traversal API.
+
+Drivers initiate a graph traversal by calling
+
+	media_entity_graph_walk_start(struct media_entity_graph *graph,
+				      struct media_entity *entity);
+
+The graph structure, provided by the caller, is initialized to start graph
+traversal at the given entity.
+
+Drivers can then retrieve the next entity by calling
+
+	media_entity_graph_walk_next(struct media_entity_graph *graph);
+
+When the graph traversal is complete the function will return NULL.
+
+Graph traversal can be interrupted at any moment. No cleanup function call is
+required and the graph structure can be freed normally.
+
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index a2f9ad9..443c5c9 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -81,6 +81,121 @@ media_entity_cleanup(struct media_entity *entity)
 }
 EXPORT_SYMBOL(media_entity_cleanup);
 
+/* -----------------------------------------------------------------------------
+ * Graph traversal
+ */
+
+static struct media_entity *
+media_entity_other(struct media_entity *entity, struct media_link *link)
+{
+	if (link->source->entity == entity)
+		return link->sink->entity;
+	else
+		return link->source->entity;
+}
+
+/* push an entity to traversal stack */
+static void stack_push(struct media_entity_graph *graph,
+		       struct media_entity *entity)
+{
+	if (graph->top == MEDIA_ENTITY_ENUM_MAX_DEPTH - 1) {
+		WARN_ON(1);
+		return;
+	}
+	graph->top++;
+	graph->stack[graph->top].link = 0;
+	graph->stack[graph->top].entity = entity;
+}
+
+static struct media_entity *stack_pop(struct media_entity_graph *graph)
+{
+	struct media_entity *entity;
+
+	entity = graph->stack[graph->top].entity;
+	graph->top--;
+
+	return entity;
+}
+
+#define stack_peek(en)	((en)->stack[(en)->top - 1].entity)
+#define link_top(en)	((en)->stack[(en)->top].link)
+#define stack_top(en)	((en)->stack[(en)->top].entity)
+
+/**
+ * media_entity_graph_walk_start - Start walking the media graph at a given entity
+ * @graph: Media graph structure that will be used to walk the graph
+ * @entity: Starting entity
+ *
+ * This function initializes the graph traversal structure to walk the entities
+ * graph starting at the given entity. The traversal structure must not be
+ * modified by the caller during graph traversal. When done the structure can
+ * safely be freed.
+ */
+void media_entity_graph_walk_start(struct media_entity_graph *graph,
+				   struct media_entity *entity)
+{
+	graph->top = 0;
+	graph->stack[graph->top].entity = NULL;
+	stack_push(graph, entity);
+}
+EXPORT_SYMBOL_GPL(media_entity_graph_walk_start);
+
+/**
+ * media_entity_graph_walk_next - Get the next entity in the graph
+ * @graph: Media graph structure
+ *
+ * Perform a depth-first traversal of the given media entities graph.
+ *
+ * The graph structure must have been previously initialized with a call to
+ * media_entity_graph_walk_start().
+ *
+ * Return the next entity in the graph or NULL if the whole graph have been
+ * traversed.
+ */
+struct media_entity *
+media_entity_graph_walk_next(struct media_entity_graph *graph)
+{
+	if (stack_top(graph) == NULL)
+		return NULL;
+
+	/*
+	 * Depth first search. Push entity to stack and continue from
+	 * top of the stack until no more entities on the level can be
+	 * found.
+	 */
+	while (link_top(graph) < stack_top(graph)->num_links) {
+		struct media_entity *entity = stack_top(graph);
+		struct media_link *link = &entity->links[link_top(graph)];
+		struct media_entity *next;
+
+		/* The link is not active so we do not follow. */
+		if (!(link->flags & MEDIA_LINK_FLAG_ACTIVE)) {
+			link_top(graph)++;
+			continue;
+		}
+
+		/* Get the entity in the other end of the link . */
+		next = media_entity_other(entity, link);
+
+		/* Was it the entity we came here from? */
+		if (next == stack_peek(graph)) {
+			link_top(graph)++;
+			continue;
+		}
+
+		/* Push the new entity to stack and start over. */
+		link_top(graph)++;
+		stack_push(graph, next);
+	}
+
+	return stack_pop(graph);
+}
+EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);
+
+/* -----------------------------------------------------------------------------
+ * Links management
+ */
+
 static struct media_link *media_entity_add_link(struct media_entity *entity)
 {
 	if (entity->num_links >= entity->max_links) {
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 37a25bf..05c37a6 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -76,10 +76,25 @@ struct media_entity {
 #define media_entity_subtype(entity) \
 	((entity)->type & MEDIA_ENTITY_SUBTYPE_MASK)
 
+#define MEDIA_ENTITY_ENUM_MAX_DEPTH	16
+
+struct media_entity_graph {
+	struct {
+		struct media_entity *entity;
+		int link;
+	} stack[MEDIA_ENTITY_ENUM_MAX_DEPTH];
+	int top;
+};
+
 int media_entity_init(struct media_entity *entity, u8 num_pads,
 		struct media_pad *pads, u8 extra_links);
 void media_entity_cleanup(struct media_entity *entity);
 int media_entity_create_link(struct media_entity *source, u8 source_pad,
 		struct media_entity *sink, u8 sink_pad, u32 flags);
 
+void media_entity_graph_walk_start(struct media_entity_graph *graph,
+		struct media_entity *entity);
+struct media_entity *
+media_entity_graph_walk_next(struct media_entity_graph *graph);
+
 #endif
-- 
1.7.1

[RFC/PATCH v3 05/10] media: Reference count and power handling

[Laurent Pinchart]

From: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>

Basically these are the interface functions:

media_entity_get() - acquire entity
media_entity_put() - release entity

	If the entity is of node type, the power change is distributed to
	all connected entities. For non-nodes it only affects that very
	node. A mutex is used to serialise access to the entity graph.

In the background there's a depth-first search algorithm that traverses the
active links in the graph. All these functions parse the graph to implement
whatever they're to do.

The module counters are increased/decreased in media_entity_get/put to
prevent module unloading when an entity is referenced.

Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 Documentation/media-framework.txt |   37 +++++++++
 drivers/media/media-device.c      |    1 +
 drivers/media/media-entity.c      |  146 +++++++++++++++++++++++++++++++++++++
 include/media/media-device.h      |    3 +
 include/media/media-entity.h      |   15 ++++
 5 files changed, 202 insertions(+), 0 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 4fe3b32..6d680c6 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -236,3 +236,40 @@ When the graph traversal is complete the function will return NULL.
 Graph traversal can be interrupted at any moment. No cleanup function call is
 required and the graph structure can be freed normally.
 
+
+Reference counting and power handling
+-------------------------------------
+
+Before accessing type-specific entities operations (such as the V4L2
+sub-device operations), drivers must acquire a reference to the entity. This
+ensures that the entity will be powered on and ready to accept requests.
+Similarly, after being done with an entity, drivers must release the
+reference.
+
+	media_entity_get(struct media_entity *entity)
+
+The function will increase the entity reference count. If the entity is a node
+(MEDIA_ENTITY_TYPE_NODE type), the reference count of all entities it is
+connected to, both directly or indirectly, through active links is increased.
+This ensures that the whole media pipeline will be ready to process
+
+Acquiring a reference to an entity increases the media device module reference
+count to prevent module unloading when an entity is being used.
+
+media_entity_get will return a pointer to the entity if successful, or NULL
+otherwise.
+
+	media_entity_put(struct media_entity *entity)
+
+The function will decrease the entity reference count and, for node entities,
+like media_entity_get, the reference count of all connected entities. Calling
+media_entity_put with a NULL argument is valid and will return immediately.
+
+When the first reference to an entity is acquired, or the last reference
+released, the entity's set_power operation is called. Entity drivers must
+implement the operation if they need to perform any power management task,
+such as turning powers or clocks on or off. If no power management is
+required, drivers don't need to provide a set_power operation. The operation
+is allowed to fail when turning power on, in which case the media_entity_get
+function will return NULL.
+
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index ed64b4a..6fb2e26 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -51,6 +51,7 @@ int __must_check media_device_register(struct media_device *mdev)
 	mdev->entity_id = 1;
 	INIT_LIST_HEAD(&mdev->entities);
 	spin_lock_init(&mdev->lock);
+	mutex_init(&mdev->graph_mutex);
 
 	/* If dev == NULL, then name must be filled in by the caller */
 	if (mdev->dev == NULL && WARN_ON(!mdev->devnode.name[0]))
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 443c5c9..e091bc8 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <media/media-entity.h>
+#include <media/media-device.h>
 
 /**
  * media_entity_init - Initialize a media entity
@@ -193,6 +194,151 @@ media_entity_graph_walk_next(struct media_entity_graph *graph)
 EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);
 
 /* -----------------------------------------------------------------------------
+ * Power state handling
+ */
+
+/* Apply use count to an entity. */
+static void media_entity_use_apply_one(struct media_entity *entity, int change)
+{
+	entity->use_count += change;
+	WARN_ON(entity->use_count < 0);
+}
+
+/*
+ * Apply use count change to an entity and change power state based on
+ * new use count.
+ */
+static int media_entity_power_apply_one(struct media_entity *entity, int change)
+{
+	int ret;
+
+	if (entity->use_count == 0 && change > 0 &&
+	    entity->ops && entity->ops->set_power) {
+		ret = entity->ops->set_power(entity, 1);
+		if (ret)
+			return ret;
+	}
+
+	media_entity_use_apply_one(entity, change);
+
+	if (entity->use_count == 0 && change < 0 &&
+	    entity->ops && entity->ops->set_power)
+		entity->ops->set_power(entity, 0);
+
+	return 0;
+}
+
+/*
+ * Apply power change to all connected entities. This ignores the
+ * nodes.
+ */
+static int media_entity_power_apply(struct media_entity *entity, int change)
+{
+	struct media_entity_graph graph;
+	struct media_entity *first = entity;
+	int ret = 0;
+
+	if (!change)
+		return 0;
+
+	media_entity_graph_walk_start(&graph, entity);
+
+	while (!ret && (entity = media_entity_graph_walk_next(&graph)))
+		if (media_entity_type(entity) != MEDIA_ENTITY_TYPE_NODE)
+			ret = media_entity_power_apply_one(entity, change);
+
+	if (!ret)
+		return 0;
+
+	media_entity_graph_walk_start(&graph, first);
+
+	while ((first = media_entity_graph_walk_next(&graph))
+	       && first != entity)
+		if (media_entity_type(first) != MEDIA_ENTITY_TYPE_NODE)
+			media_entity_power_apply_one(first, -change);
+
+	return ret;
+}
+
+/*
+ * Apply use count change to graph and change power state of entities
+ * accordingly.
+ */
+static int media_entity_node_power_change(struct media_entity *entity,
+					  int change)
+{
+	/* Apply use count to node. */
+	media_entity_use_apply_one(entity, change);
+
+	/* Apply power change to connected non-nodes. */
+	return media_entity_power_apply(entity, change);
+}
+
+/*
+ * Node entity use changes are reflected on power state of all
+ * connected (directly or indirectly) entities whereas non-node entity
+ * use count changes are limited to that very entity.
+ */
+static int media_entity_use_change(struct media_entity *entity, int change)
+{
+	if (media_entity_type(entity) == MEDIA_ENTITY_TYPE_NODE)
+		return media_entity_node_power_change(entity, change);
+	else
+		return media_entity_power_apply_one(entity, change);
+}
+
+static struct media_entity *__media_entity_get(struct media_entity *entity)
+{
+	if (media_entity_use_change(entity, 1))
+		return NULL;
+
+	return entity;
+}
+
+static void __media_entity_put(struct media_entity *entity)
+{
+	media_entity_use_change(entity, -1);
+}
+
+/* user open()s media entity */
+struct media_entity *media_entity_get(struct media_entity *entity)
+{
+	struct media_entity *e;
+
+	if (entity == NULL)
+		return NULL;
+
+	if (entity->parent->dev &&
+	    !try_module_get(entity->parent->dev->driver->owner))
+		return NULL;
+
+	mutex_lock(&entity->parent->graph_mutex);
+	e = __media_entity_get(entity);
+	mutex_unlock(&entity->parent->graph_mutex);
+
+	if (e == NULL && entity->parent->dev)
+		module_put(entity->parent->dev->driver->owner);
+
+	return e;
+}
+EXPORT_SYMBOL_GPL(media_entity_get);
+
+/* user release()s media entity */
+void media_entity_put(struct media_entity *entity)
+{
+	if (entity == NULL)
+		return;
+
+	mutex_lock(&entity->parent->graph_mutex);
+	__media_entity_put(entity);
+	mutex_unlock(&entity->parent->graph_mutex);
+
+	if (entity->parent->dev)
+		module_put(entity->parent->dev->driver->owner);
+}
+EXPORT_SYMBOL_GPL(media_entity_put);
+
+/* -----------------------------------------------------------------------------
  * Links management
  */
 
diff --git a/include/media/media-device.h b/include/media/media-device.h
index ac96847..39a437c 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -23,6 +23,7 @@
 
 #include <linux/device.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
 #include <linux/spinlock.h>
 
 #include <media/media-devnode.h>
@@ -48,6 +49,8 @@ struct media_device {
 
 	/* Protects the entities list */
 	spinlock_t lock;
+	/* Serializes graph operations. */
+	struct mutex graph_mutex;
 };
 
 int __must_check media_device_register(struct media_device *mdev);
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 05c37a6..2205cbc 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -33,6 +33,10 @@ struct media_pad {
 	unsigned long flags;		/* Pad flags (MEDIA_PAD_FLAG_*) */
 };
 
+struct media_entity_operations {
+	int (*set_power)(struct media_entity *entity, int power);
+};
+
 struct media_entity {
 	struct list_head list;
 	struct media_device *parent;	/* Media device this entity belongs to*/
@@ -50,6 +54,10 @@ struct media_entity {
 	struct media_pad *pads;		/* Pads array (num_pads elements) */
 	struct media_link *links;	/* Links array (max_links elements)*/
 
+	const struct media_entity_operations *ops;	/* Entity operations */
+
+	int use_count;			/* Use count for the entity. */
+
 	union {
 		/* Node specifications */
 		struct {
@@ -92,9 +100,16 @@ void media_entity_cleanup(struct media_entity *entity);
 int media_entity_create_link(struct media_entity *source, u8 source_pad,
 		struct media_entity *sink, u8 sink_pad, u32 flags);
 
+struct media_entity *media_entity_get(struct media_entity *entity);
+void media_entity_put(struct media_entity *entity);
+
 void media_entity_graph_walk_start(struct media_entity_graph *graph,
 		struct media_entity *entity);
 struct media_entity *
 media_entity_graph_walk_next(struct media_entity_graph *graph);
 
+#define media_entity_call(entity, operation, args...)			\
+	(((entity)->ops && (entity)->ops->operation) ?			\
+	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
+
 #endif
-- 
1.7.1

[RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Laurent Pinchart]

Create the following two ioctls and implement them at the media device
level to enumerate entities, pads and links.

- MEDIA_IOC_ENUM_ENTITIES: Enumerate entities and their properties
- MEDIA_IOC_ENUM_LINKS: Enumerate all pads and links for a given entity

Entity IDs can be non-contiguous. Userspace applications should
enumerate entities using the MEDIA_ENTITY_ID_FLAG_NEXT flag. When the
flag is set in the entity ID, the MEDIA_IOC_ENUM_ENTITIES will return
the next entity with an ID bigger than the requested one.

Only forward links that originate at one of the entity's source pads are
returned during the enumeration process.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 Documentation/media-framework.txt |  131 +++++++++++++++++++++++++++++++
 drivers/media/media-device.c      |  152 +++++++++++++++++++++++++++++++++++++
 include/linux/media.h             |   77 +++++++++++++++++++
 include/media/media-device.h      |    3 +
 include/media/media-entity.h      |   18 +----
 5 files changed, 364 insertions(+), 17 deletions(-)
 create mode 100644 include/linux/media.h

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 6d680c6..1192feb 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -273,3 +273,134 @@ required, drivers don't need to provide a set_power operation. The operation
 is allowed to fail when turning power on, in which case the media_entity_get
 function will return NULL.
 
+
+Userspace application API
+-------------------------
+
+Media devices offer an API to userspace application to discover the device
+internal topology through ioctls.
+
+	MEDIA_IOC_ENUM_ENTITIES - Enumerate entities and their properties
+	-----------------------------------------------------------------
+
+	ioctl(int fd, int request, struct media_entity_desc *argp);
+
+To query the attributes of an entity, applications set the id field of a
+media_entity_desc structure and call the MEDIA_IOC_ENUM_ENTITIES ioctl with a
+pointer to this structure. The driver fills the rest of the structure or
+returns a EINVAL error code when the id is invalid.
+
+Entities can be enumerated by or'ing the id with the MEDIA_ENTITY_ID_FLAG_NEXT
+flag. The driver will return information about the entity with the smallest id
+strictly larger than the requested one ('next entity'), or EINVAL if there is
+none.
+
+Entity IDs can be non-contiguous. Applications must *not* try to enumerate
+entities by calling MEDIA_IOC_ENUM_ENTITIES with increasing id's until they
+get an error.
+
+The media_entity_desc structure is defined as
+
+- struct media_entity_desc
+
+__u32	id		Entity id, set by the application. When the id is
+			or'ed with MEDIA_ENTITY_ID_FLAG_NEXT, the driver
+			clears the flag and returns the first entity with a
+			larger id.
+char	name[32]	Entity name. UTF-8 NULL-terminated string.
+__u32	type		Entity type.
+__u8	pads		Number of pads.
+__u32	links		Total number of outbound links. Inbound links are not
+			counted in this field.
+/* union */
+	/* struct v4l, Valid for V4L sub-devices and nodes only */
+__u32	major		V4L device node major number. For V4L sub-devices with
+			no device node, set by the driver to 0.
+__u32	minor		V4L device node minor number. For V4L sub-devices with
+			no device node, set by the driver to 0.
+	/* struct fb, Valid for frame buffer nodes only */
+__u32	major		FB device node major number
+__u32	minor		FB device node minor number
+	/* Valid for ALSA devices only */
+int	alsa		ALSA card number
+	/* Valid for DVB devices only */
+int	dvb		DVB card number
+
+Valid entity types are
+
+	MEDIA_ENTITY_TYPE_NODE - Unknown device node
+	MEDIA_ENTITY_TYPE_NODE_V4L - V4L video, radio or vbi device node
+	MEDIA_ENTITY_TYPE_NODE_FB - Frame buffer device node
+	MEDIA_ENTITY_TYPE_NODE_ALSA - ALSA card
+	MEDIA_ENTITY_TYPE_NODE_DVB - DVB card
+
+	MEDIA_ENTITY_TYPE_SUBDEV - Unknown V4L sub-device
+	MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER - Video decoder
+	MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER - Video encoder
+	MEDIA_ENTITY_TYPE_SUBDEV_MISC - Unspecified entity subtype
+
+
+	MEDIA_IOC_ENUM_LINKS - Enumerate all pads and links for a given entity
+	----------------------------------------------------------------------
+
+	ioctl(int fd, int request, struct media_links_enum *argp);
+
+Only forward links that originate at one of the entity's source pads are
+returned during the enumeration process.
+
+To enumerate pads and/or links for a given entity, applications set the entity
+field of a media_links_enum structure and initialize the media_pad_desc and
+media_link_desc structure arrays pointed by the pads and links fields. They then
+call the MEDIA_IOC_ENUM_LINKS ioctl with a pointer to this structure.
+
+If the pads field is not NULL, the driver fills the pads array with
+information about the entity's pads. The array must have enough room to store
+all the entity's pads. The number of pads can be retrieved with the
+MEDIA_IOC_ENUM_ENTITIES ioctl.
+
+If the links field is not NULL, the driver fills the links array with
+information about the entity's outbound links. The array must have enough room
+to store all the entity's outbound links. The number of outbound links can be
+retrieved with the MEDIA_IOC_ENUM_ENTITIES ioctl.
+
+The media_pad_desc, media_link_desc and media_links_enum structures are defined
+as
+
+- struct media_pad_desc
+
+__u32		entity		ID of the entity this pad belongs to.
+__u16		index		0-based pad index.
+__u32		flags		Pad flags.
+
+Valid pad flags are
+
+	MEDIA_PAD_FLAG_INPUT -	Input pad, relative to the entity. Input pads
+				sink data and are targets of links.
+	MEDIA_PAD_FLAG_OUTPUT -	Output pad, relative to the entity. Output
+				pads source data and are origins of links.
+
+One and only one of MEDIA_PAD_FLAG_INPUT and MEDIA_PAD_FLAG_OUTPUT must be set
+for every pad.
+
+- struct media_link_desc
+
+struct media_pad_desc	source	Pad at the origin of this link.
+struct media_pad_desc	sink	Pad at the target of this link.
+__u32			flags	Link flags.
+
+Valid link flags are
+
+	MEDIA_LINK_FLAG_ACTIVE - The link is active and can be used to
+		transfer media data. When two or more links target a sink pad,
+		only one of them can be active at a time.
+	MEDIA_LINK_FLAG_IMMUTABLE - The link active state can't be modified at
+		runtime. An immutable link is always active.
+
+- struct media_links_enum
+
+__u32			entity	Entity id, set by the application.
+struct media_pad_desc	*pads	Pointer to a pads array allocated by the
+				application. Ignored if NULL.
+struct media_link_desc	*links	Pointer to a links array allocated by the
+				application. Ignored if NULL.
+
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 6fb2e26..b32c308 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -20,13 +20,162 @@
 
 #include <linux/types.h>
 #include <linux/ioctl.h>
+#include <linux/media.h>
 
 #include <media/media-device.h>
 #include <media/media-devnode.h>
 #include <media/media-entity.h>
 
+static int media_device_open(struct file *filp)
+{
+	return 0;
+}
+
+static int media_device_close(struct file *filp)
+{
+	return 0;
+}
+
+static struct media_entity *find_entity(struct media_device *mdev, u32 id)
+{
+	struct media_entity *entity;
+	int next = id & MEDIA_ENTITY_ID_FLAG_NEXT;
+
+	id &= ~MEDIA_ENTITY_ID_FLAG_NEXT;
+
+	spin_lock(&mdev->lock);
+
+	media_device_for_each_entity(entity, mdev) {
+		if ((entity->id == id && !next) ||
+		    (entity->id > id && next)) {
+			spin_unlock(&mdev->lock);
+			return entity;
+		}
+	}
+
+	spin_unlock(&mdev->lock);
+
+	return NULL;
+}
+
+static long media_device_enum_entities(struct media_device *mdev,
+				       struct media_entity_desc __user *uent)
+{
+	struct media_entity *ent;
+	struct media_entity_desc u_ent;
+
+	if (copy_from_user(&u_ent.id, &uent->id, sizeof(u_ent.id)))
+		return -EFAULT;
+
+	ent = find_entity(mdev, u_ent.id);
+
+	if (ent == NULL)
+		return -EINVAL;
+
+	u_ent.id = ent->id;
+	u_ent.name[0] = '\0';
+	if (ent->name)
+		strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
+	u_ent.type = ent->type;
+	u_ent.pads = ent->num_pads;
+	u_ent.links = ent->num_links - ent->num_backlinks;
+	u_ent.v4l.major = ent->v4l.major;
+	u_ent.v4l.minor = ent->v4l.minor;
+	if (copy_to_user(uent, &u_ent, sizeof(u_ent)))
+		return -EFAULT;
+	return 0;
+}
+
+static void media_device_kpad_to_upad(const struct media_pad *kpad,
+				      struct media_pad_desc *upad)
+{
+	upad->entity = kpad->entity->id;
+	upad->index = kpad->index;
+	upad->flags = kpad->flags;
+}
+
+static long media_device_enum_links(struct media_device *mdev,
+				    struct media_links_enum __user *ulinks)
+{
+	struct media_entity *entity;
+	struct media_links_enum links;
+
+	if (copy_from_user(&links, ulinks, sizeof(links)))
+		return -EFAULT;
+
+	entity = find_entity(mdev, links.entity);
+	if (entity == NULL)
+		return -EINVAL;
+
+	if (links.pads) {
+		unsigned int p;
+
+		for (p = 0; p < entity->num_pads; p++) {
+			struct media_pad_desc pad;
+			media_device_kpad_to_upad(&entity->pads[p], &pad);
+			if (copy_to_user(&links.pads[p], &pad, sizeof(pad)))
+				return -EFAULT;
+		}
+	}
+
+	if (links.links) {
+		struct media_link_desc __user *ulink;
+		unsigned int l;
+
+		for (l = 0, ulink = links.links; l < entity->num_links; l++) {
+			struct media_link_desc link;
+
+			/* Ignore backlinks. */
+			if (entity->links[l].source->entity != entity)
+				continue;
+
+			media_device_kpad_to_upad(entity->links[l].source,
+						  &link.source);
+			media_device_kpad_to_upad(entity->links[l].sink,
+						  &link.sink);
+			link.flags = entity->links[l].flags;
+			if (copy_to_user(ulink, &link, sizeof(*ulink)))
+				return -EFAULT;
+			ulink++;
+		}
+	}
+	if (copy_to_user(ulinks, &links, sizeof(*ulinks)))
+		return -EFAULT;
+	return 0;
+}
+
+static long media_device_ioctl(struct file *filp, unsigned int cmd,
+			       unsigned long arg)
+{
+	struct media_devnode *devnode = media_devnode_data(filp);
+	struct media_device *dev = to_media_device(devnode);
+	long ret;
+
+	switch (cmd) {
+	case MEDIA_IOC_ENUM_ENTITIES:
+		ret = media_device_enum_entities(dev,
+				(struct media_entity_desc __user *)arg);
+		break;
+
+	case MEDIA_IOC_ENUM_LINKS:
+		mutex_lock(&dev->graph_mutex);
+		ret = media_device_enum_links(dev,
+				(struct media_links_enum __user *)arg);
+		mutex_unlock(&dev->graph_mutex);
+		break;
+
+	default:
+		ret = -ENOIOCTLCMD;
+	}
+
+	return ret;
+}
+
 static const struct media_file_operations media_device_fops = {
 	.owner = THIS_MODULE,
+	.open = media_device_open,
+	.unlocked_ioctl = media_device_ioctl,
+	.release = media_device_close,
 };
 
 static void media_device_release(struct media_devnode *mdev)
@@ -99,6 +248,9 @@ int __must_check media_device_register_entity(struct media_device *mdev,
 	WARN_ON(entity->parent != NULL);
 	entity->parent = mdev;
 
+	/* find_entity() relies on entities being stored in increasing IDs
+	 * order. Don't change that without modifying find_entity().
+	 */
 	spin_lock(&mdev->lock);
 	entity->id = mdev->entity_id++;
 	list_add_tail(&entity->list, &mdev->entities);
diff --git a/include/linux/media.h b/include/linux/media.h
new file mode 100644
index 0000000..9b8acc0
--- /dev/null
+++ b/include/linux/media.h
@@ -0,0 +1,77 @@
+#ifndef __LINUX_MEDIA_H
+#define __LINUX_MEDIA_H
+
+#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)
+#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
+#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
+#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
+#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
+
+#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV + 1)
+#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
+#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3)
+
+#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
+#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
+
+#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
+
+#define MEDIA_ENTITY_ID_FLAG_NEXT		(1 << 31)
+
+struct media_pad_desc {
+	__u32 entity;		/* entity ID */
+	__u16 index;		/* pad index */
+	__u32 flags;		/* pad flags */
+	__u32 reserved[2];
+};
+
+struct media_entity_desc {
+	__u32 id;
+	char name[32];
+	__u32 type;
+	__u8 pads;
+	__u32 links;
+
+	__u32 reserved[4];
+
+	union {
+		/* Node specifications */
+		struct {
+			__u32 major;
+			__u32 minor;
+		} v4l;
+		struct {
+			__u32 major;
+			__u32 minor;
+		} fb;
+		int alsa;
+		int dvb;
+
+		/* Sub-device specifications */
+		/* Nothing needed yet */
+		__u8 raw[64];
+	};
+};
+
+struct media_link_desc {
+	struct media_pad_desc source;
+	struct media_pad_desc sink;
+	__u32 flags;
+	__u32 reserved[2];
+};
+
+struct media_links_enum {
+	__u32 entity;
+	/* Should have enough room for pads elements */
+	struct media_pad_desc __user *pads;
+	/* Should have enough room for links elements */
+	struct media_link_desc __user *links;
+	__u32 reserved[4];
+};
+
+#define MEDIA_IOC_ENUM_ENTITIES		_IOWR('M', 1, struct media_entity_desc)
+#define MEDIA_IOC_ENUM_LINKS		_IOWR('M', 2, struct media_links_enum)
+
+#endif /* __LINUX_MEDIA_H */
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 39a437c..7b09332 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -53,6 +53,9 @@ struct media_device {
 	struct mutex graph_mutex;
 };
 
+/* media_devnode to media_device */
+#define to_media_device(node) container_of(node, struct media_device, devnode)
+
 int __must_check media_device_register(struct media_device *mdev);
 void media_device_unregister(struct media_device *mdev);
 
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 2205cbc..fe8a650 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -2,23 +2,7 @@
 #define _MEDIA_ENTITY_H
 
 #include <linux/list.h>
-
-#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)
-#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
-#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
-#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
-#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
-
-#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
-#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV + 1)
-#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
-#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3)
-
-#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
-#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
-
-#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
-#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
+#include <linux/media.h>
 
 struct media_link {
 	struct media_pad *source;	/* Source pad */
-- 
1.7.1

[RFC/PATCH v3 07/10] media: Links setup

[Laurent Pinchart]

Create the following ioctl and implement it at the media device level to
setup links.

- MEDIA_IOC_SETUP_LINK: Modify the properties of a given link

The only property that can currently be modified is the ACTIVE link flag
to activate/deactivate a link. Links marked with the IMMUTABLE link flag
can not be activated or deactivated.

Activating and deactivating a link has effects on entities' use count.
Those changes are automatically propagated through the graph.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 Documentation/media-framework.txt |   83 ++++++++++++++-
 drivers/media/media-device.c      |   45 ++++++++
 drivers/media/media-entity.c      |  208 +++++++++++++++++++++++++++++++++++++
 include/linux/media.h             |    1 +
 include/media/media-entity.h      |    8 ++
 5 files changed, 341 insertions(+), 4 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 1192feb..7001a94 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -236,6 +236,16 @@ When the graph traversal is complete the function will return NULL.
 Graph traversal can be interrupted at any moment. No cleanup function call is
 required and the graph structure can be freed normally.
 
+Helper functions can be used to find a link between two given pads, or a pad
+connected to another pad through an active link
+
+	media_entity_find_link(struct media_pad *source,
+			       struct media_pad *sink);
+
+	media_entity_remote_pad(struct media_pad *pad);
+
+Refer to the kerneldoc documentation for more information.
+
 
 Reference counting and power handling
 -------------------------------------
@@ -274,11 +284,51 @@ is allowed to fail when turning power on, in which case the media_entity_get
 function will return NULL.
 
 
+Links setup
+-----------
+
+Link properties can be modified at runtime by calling
+
+	media_entity_setup_link(struct media_link *link, u32 flags);
+
+The flags argument contains the requested new link flags.
+
+The only configurable property is the ACTIVE link flag to activate/deactivate
+a link. Links marked with the IMMUTABLE link flag can not be activated or
+deactivated.
+
+When a link is activated or deactivated, the media framework calls the
+link_setup operation for the two entities at the source and sink of the link,
+in that order. If the second link_setup call fails, another link_setup call is
+made on the first entity to restore the original link flags.
+
+Entity drivers must implement the link_setup operation if any of their links
+is non-immutable. The operation must either configure the hardware or store
+the configuration information to be applied later.
+
+Link activation must not have any side effect on other links. If an active
+link at a sink pad prevents another link at the same pad from being
+deactivated, the link_setup operation must return -EBUSY and can't implicitly
+deactivate the first active link.
+
+Activating and deactivating a link has effects on entities' reference counts.
+When two sub-graphs are connected, the reference count of each of them is
+incremented by the total reference count of all node entities in the other
+sub-graph. When two sub-graphs are disconnected, the reverse operation is
+performed. In both cases the set_power operations are called accordingly,
+ensuring that the link_setup calls are made with power active on the source
+and sink entities.
+
+In other words, activating or deactivating a link propagates reference count
+changes through the graph, and the final state is identical to what it would
+have been if the link had been active or inactive from the start.
+
+
 Userspace application API
 -------------------------
 
 Media devices offer an API to userspace application to discover the device
-internal topology through ioctls.
+internal topology and setup links through ioctls.
 
 	MEDIA_IOC_ENUM_ENTITIES - Enumerate entities and their properties
 	-----------------------------------------------------------------
@@ -345,9 +395,6 @@ Valid entity types are
 
 	ioctl(int fd, int request, struct media_links_enum *argp);
 
-Only forward links that originate at one of the entity's source pads are
-returned during the enumeration process.
-
 To enumerate pads and/or links for a given entity, applications set the entity
 field of a media_links_enum structure and initialize the media_pad_desc and
 media_link_desc structure arrays pointed by the pads and links fields. They then
@@ -363,6 +410,9 @@ information about the entity's outbound links. The array must have enough room
 to store all the entity's outbound links. The number of outbound links can be
 retrieved with the MEDIA_IOC_ENUM_ENTITIES ioctl.
 
+Only outbound (forward) links that originate at one of the entity's source
+pads are returned during the enumeration process.
+
 The media_pad_desc, media_link_desc and media_links_enum structures are defined
 as
 
@@ -404,3 +454,28 @@ struct media_pad_desc	*pads	Pointer to a pads array allocated by the
 struct media_link_desc	*links	Pointer to a links array allocated by the
 				application. Ignored if NULL.
 
+
+	MEDIA_IOC_SETUP_LINK - Modify the properties of a link
+	------------------------------------------------------
+
+	ioctl(int fd, int request, struct media_link_desc *argp);
+
+To change link properties applications fill a media_link_desc structure with
+link identification information (source and sink pad) and the new requested link
+flags. They then call the MEDIA_IOC_SETUP_LINK ioctl with a pointer to that
+structure.
+
+The only configurable property is the ACTIVE link flag to activate/deactivate
+a link. Links marked with the IMMUTABLE link flag can not be activated or
+deactivated.
+
+Link activation has no side effect on other links. If an active link at the
+sink pad prevents the link from being activated, the driver returns with a
+EBUSY error code.
+
+If the specified link can't be found the driver returns with a EINVAL error
+code.
+
+The media_pad_desc and media_link_desc structures are described in the
+MEDIA_IOC_ENUM_LINKS ioctl documentation.
+
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index b32c308..157f9bc 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -144,6 +144,44 @@ static long media_device_enum_links(struct media_device *mdev,
 	return 0;
 }
 
+static long media_device_setup_link(struct media_device *mdev,
+				    struct media_link_desc __user *_ulink)
+{
+	struct media_link *link = NULL;
+	struct media_link_desc ulink;
+	struct media_entity *source;
+	struct media_entity *sink;
+	int ret;
+
+	if (copy_from_user(&ulink, _ulink, sizeof(ulink)))
+		return -EFAULT;
+
+	/* Find the source and sink entities and link.
+	 */
+	source = find_entity(mdev, ulink.source.entity);
+	sink = find_entity(mdev, ulink.sink.entity);
+
+	if (source == NULL || sink == NULL)
+		return -EINVAL;
+
+	if (ulink.source.index >= source->num_pads ||
+	    ulink.sink.index >= sink->num_pads)
+		return -EINVAL;
+
+	link = media_entity_find_link(&source->pads[ulink.source.index],
+				      &sink->pads[ulink.sink.index]);
+	if (link == NULL)
+		return -EINVAL;
+
+	/* Setup the link on both entities. */
+	ret = __media_entity_setup_link(link, ulink.flags);
+
+	if (copy_to_user(_ulink, &ulink, sizeof(ulink)))
+		return -EFAULT;
+
+	return ret;
+}
+
 static long media_device_ioctl(struct file *filp, unsigned int cmd,
 			       unsigned long arg)
 {
@@ -164,6 +202,13 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
 		mutex_unlock(&dev->graph_mutex);
 		break;
 
+	case MEDIA_IOC_SETUP_LINK:
+		mutex_lock(&dev->graph_mutex);
+		ret = media_device_setup_link(dev,
+				(struct media_link_desc __user *)arg);
+		mutex_unlock(&dev->graph_mutex);
+		break;
+
 	default:
 		ret = -ENOIOCTLCMD;
 	}
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index e091bc8..587e039 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -197,6 +197,25 @@ EXPORT_SYMBOL_GPL(media_entity_graph_walk_next);
  * Power state handling
  */
 
+/*
+ * Return power count of nodes directly or indirectly connected to
+ * a given entity.
+ */
+static int media_entity_count_node(struct media_entity *entity)
+{
+	struct media_entity_graph graph;
+	int use = 0;
+
+	media_entity_graph_walk_start(&graph, entity);
+
+	while ((entity = media_entity_graph_walk_next(&graph))) {
+		if (media_entity_type(entity) == MEDIA_ENTITY_TYPE_NODE)
+			use += entity->use_count;
+	}
+
+	return use;
+}
+
 /* Apply use count to an entity. */
 static void media_entity_use_apply_one(struct media_entity *entity, int change)
 {
@@ -260,6 +279,32 @@ static int media_entity_power_apply(struct media_entity *entity, int change)
 	return ret;
 }
 
+/* Apply the power state changes when connecting two entities. */
+static int media_entity_power_connect(struct media_entity *one,
+				      struct media_entity *theother)
+{
+	int power_one = media_entity_count_node(one);
+	int power_theother = media_entity_count_node(theother);
+	int ret = 0;
+
+	ret = media_entity_power_apply(one, power_theother);
+	if (ret < 0)
+		return ret;
+
+	return media_entity_power_apply(theother, power_one);
+}
+
+static void media_entity_power_disconnect(struct media_entity *one,
+					  struct media_entity *theother)
+{
+	int power_one = media_entity_count_node(one);
+	int power_theother = media_entity_count_node(theother);
+
+	/* Powering off entities is assumed to never fail. */
+	media_entity_power_apply(one, -power_theother);
+	media_entity_power_apply(theother, -power_one);
+}
+
 /*
  * Apply use count change to graph and change power state of entities
  * accordingly.
@@ -403,3 +448,166 @@ media_entity_create_link(struct media_entity *source, u8 source_pad,
 	return 0;
 }
 EXPORT_SYMBOL(media_entity_create_link);
+
+static int __media_entity_setup_link_notify(struct media_link *link, u32 flags)
+{
+	const u32 mask = MEDIA_LINK_FLAG_ACTIVE;
+	int ret;
+
+	/* Notify both entities. */
+	ret = media_entity_call(link->source->entity, link_setup,
+				link->source, link->sink, flags);
+	if (ret < 0 && ret != -ENOIOCTLCMD)
+		return ret;
+
+	ret = media_entity_call(link->sink->entity, link_setup,
+				link->sink, link->source, flags);
+	if (ret < 0 && ret != -ENOIOCTLCMD) {
+		media_entity_call(link->source->entity, link_setup,
+				  link->source, link->sink, link->flags);
+		return ret;
+	}
+
+	link->flags = (link->flags & ~mask) | (flags & mask);
+	link->other->flags = link->flags;
+
+	return 0;
+}
+
+/**
+ * __media_entity_setup_link - Configure a media link
+ * @link: The link being configured
+ * @flags: Link configuration flags
+ *
+ * The bulk of link setup is handled by the two entities connected through the
+ * link. This function notifies both entities of the link configuration change.
+ *
+ * If the link is immutable or if the current and new configuration are
+ * identical, return immediately.
+ *
+ * The user is expected to hold link->source->parent->mutex. If not,
+ * media_entity_setup_link() should be used instead.
+ */
+int __media_entity_setup_link(struct media_link *link, u32 flags)
+{
+	struct media_entity *source, *sink;
+	int ret = -EBUSY;
+
+	if (link == NULL)
+		return -EINVAL;
+
+	if (link->flags & MEDIA_LINK_FLAG_IMMUTABLE)
+		return link->flags == flags ? 0 : -EINVAL;
+
+	if (link->flags == flags)
+		return 0;
+
+	source = __media_entity_get(link->source->entity);
+	if (!source)
+		return ret;
+
+	sink = __media_entity_get(link->sink->entity);
+	if (!sink)
+		goto err___media_entity_get;
+
+	if (flags & MEDIA_LINK_FLAG_ACTIVE) {
+		ret = media_entity_power_connect(source, sink);
+		if (ret < 0)
+			goto err_media_entity_power_connect;
+	}
+
+	ret = __media_entity_setup_link_notify(link, flags);
+	if (ret < 0)
+		goto err___media_entity_setup_link_notify;
+
+	if (!(flags & MEDIA_LINK_FLAG_ACTIVE))
+		media_entity_power_disconnect(source, sink);
+
+	__media_entity_put(sink);
+	__media_entity_put(source);
+
+	return 0;
+
+err___media_entity_setup_link_notify:
+	if (flags & MEDIA_LINK_FLAG_ACTIVE)
+		media_entity_power_disconnect(source, sink);
+err_media_entity_power_connect:
+	__media_entity_put(sink);
+err___media_entity_get:
+	__media_entity_put(source);
+
+	return ret;
+}
+
+int media_entity_setup_link(struct media_link *link, u32 flags)
+{
+	int ret;
+
+	mutex_lock(&link->source->entity->parent->graph_mutex);
+	ret = __media_entity_setup_link(link, flags);
+	mutex_unlock(&link->source->entity->parent->graph_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(media_entity_setup_link);
+
+/**
+ * media_entity_find_link - Find a link between two pads
+ * @source: Source pad
+ * @sink: Sink pad
+ *
+ * Return a pointer to the link between the two entities. If no such link
+ * exists, return NULL.
+ */
+struct media_link *
+media_entity_find_link(struct media_pad *source, struct media_pad *sink)
+{
+	struct media_link *link;
+	unsigned int i;
+
+	for (i = 0; i < source->entity->num_links; ++i) {
+		link = &source->entity->links[i];
+
+		if (link->source->entity == source->entity &&
+		    link->source->index == source->index &&
+		    link->sink->entity == sink->entity &&
+		    link->sink->index == sink->index)
+			return link;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(media_entity_find_link);
+
+/**
+ * media_entity_remote_pad - Locate the pad at the remote end of a link
+ * @entity: Local entity
+ * @pad: Pad at the local end of the link
+ *
+ * Search for a remote pad connected to the given pad by iterating over all
+ * links originating or terminating at that pad until an active link is found.
+ *
+ * Return a pointer to the pad at the remote end of the first found active link,
+ * or NULL if no active link has been found.
+ */
+struct media_pad *media_entity_remote_pad(struct media_pad *pad)
+{
+	unsigned int i;
+
+	for (i = 0; i < pad->entity->num_links; i++) {
+		struct media_link *link = &pad->entity->links[i];
+
+		if (!(link->flags & MEDIA_LINK_FLAG_ACTIVE))
+			continue;
+
+		if (link->source == pad)
+			return link->sink;
+
+		if (link->sink == pad)
+			return link->source;
+	}
+
+	return NULL;
+
+}
+EXPORT_SYMBOL_GPL(media_entity_remote_pad);
diff --git a/include/linux/media.h b/include/linux/media.h
index 9b8acc0..3457b16 100644
--- a/include/linux/media.h
+++ b/include/linux/media.h
@@ -73,5 +73,6 @@ struct media_links_enum {
 
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('M', 1, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('M', 2, struct media_links_enum)
+#define MEDIA_IOC_SETUP_LINK		_IOWR('M', 3, struct media_link_desc)
 
 #endif /* __LINUX_MEDIA_H */
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index fe8a650..4db0be2 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -18,6 +18,9 @@ struct media_pad {
 };
 
 struct media_entity_operations {
+	int (*link_setup)(struct media_entity *entity,
+			  const struct media_pad *local,
+			  const struct media_pad *remote, u32 flags);
 	int (*set_power)(struct media_entity *entity, int power);
 };
 
@@ -83,6 +86,11 @@ int media_entity_init(struct media_entity *entity, u8 num_pads,
 void media_entity_cleanup(struct media_entity *entity);
 int media_entity_create_link(struct media_entity *source, u8 source_pad,
 		struct media_entity *sink, u8 sink_pad, u32 flags);
+int __media_entity_setup_link(struct media_link *link, u32 flags);
+int media_entity_setup_link(struct media_link *link, u32 flags);
+struct media_link *media_entity_find_link(struct media_pad *source,
+		struct media_pad *sink);
+struct media_pad *media_entity_remote_pad(struct media_pad *pad);
 
 struct media_entity *media_entity_get(struct media_entity *entity);
 void media_entity_put(struct media_entity *entity);
-- 
1.7.1

[RFC/PATCH v3 08/10] v4l: Add a media_device pointer to the v4l2_device structure

[Laurent Pinchart]

The pointer will later be used to register/unregister media entities
when registering/unregistering a v4l2_subdev or a video_device.

With the introduction of media devices, device drivers need to store a
pointer to a driver-specific structure in the device's drvdata.
v4l2_device can't claim ownership of the drvdata anymore.

To maintain compatibility with drivers that rely on v4l2_device storing
a pointer to itself in the device's drvdata, v4l2_device_register() will
keep doing so if the drvdata is NULL.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/video4linux/v4l2-framework.txt |   17 ++++++++++++-----
 drivers/media/video/v4l2-device.c            |   13 +++++++------
 include/media/v4l2-device.h                  |    2 ++
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 89bd881..8a3f14e 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -83,11 +83,17 @@ You must register the device instance:
 
 	v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev);
 
-Registration will initialize the v4l2_device struct and link dev->driver_data
-to v4l2_dev. If v4l2_dev->name is empty then it will be set to a value derived
-from dev (driver name followed by the bus_id, to be precise). If you set it
-up before calling v4l2_device_register then it will be untouched. If dev is
-NULL, then you *must* setup v4l2_dev->name before calling v4l2_device_register.
+Registration will initialize the v4l2_device struct. If the dev->driver_data
+field is NULL, it will be linked to v4l2_dev. Drivers that use the media
+device framework in addition to the V4L2 framework need to set
+dev->driver_data manually to point to the driver-specific device structure
+that embed the struct v4l2_device instance. This is achieved by a
+dev_set_drvdata() call before registering the V4L2 device instance.
+
+If v4l2_dev->name is empty then it will be set to a value derived from dev
+(driver name followed by the bus_id, to be precise). If you set it up before
+calling v4l2_device_register then it will be untouched. If dev is NULL, then
+you *must* setup v4l2_dev->name before calling v4l2_device_register.
 
 You can use v4l2_device_set_name() to set the name based on a driver name and
 a driver-global atomic_t instance. This will generate names like ivtv0, ivtv1,
@@ -108,6 +114,7 @@ You unregister with:
 
 	v4l2_device_unregister(struct v4l2_device *v4l2_dev);
 
+If the dev->driver_data field points to v4l2_dev, it will be reset to NULL.
 Unregistering will also automatically unregister all subdevs from the device.
 
 If you have a hotpluggable device (e.g. a USB device), then when a disconnect
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index b287aaa..91452cd 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -45,9 +45,8 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
 	if (!v4l2_dev->name[0])
 		snprintf(v4l2_dev->name, sizeof(v4l2_dev->name), "%s %s",
 			dev->driver->name, dev_name(dev));
-	if (dev_get_drvdata(dev))
-		v4l2_warn(v4l2_dev, "Non-NULL drvdata on register\n");
-	dev_set_drvdata(dev, v4l2_dev);
+	if (!dev_get_drvdata(dev))
+		dev_set_drvdata(dev, v4l2_dev);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(v4l2_device_register);
@@ -70,10 +69,12 @@ EXPORT_SYMBOL_GPL(v4l2_device_set_name);
 
 void v4l2_device_disconnect(struct v4l2_device *v4l2_dev)
 {
-	if (v4l2_dev->dev) {
+	if (v4l2_dev->dev == NULL)
+		return;
+
+	if (dev_get_drvdata(v4l2_dev->dev) == v4l2_dev)
 		dev_set_drvdata(v4l2_dev->dev, NULL);
-		v4l2_dev->dev = NULL;
-	}
+	v4l2_dev->dev = NULL;
 }
 EXPORT_SYMBOL_GPL(v4l2_device_disconnect);
 
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 5d5d550..83b5966 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -21,6 +21,7 @@
 #ifndef _V4L2_DEVICE_H
 #define _V4L2_DEVICE_H
 
+#include <media/media-device.h>
 #include <media/v4l2-subdev.h>
 
 /* Each instance of a V4L2 device should create the v4l2_device struct,
@@ -37,6 +38,7 @@ struct v4l2_device {
 	   Note: dev might be NULL if there is no parent device
 	   as is the case with e.g. ISA devices. */
 	struct device *dev;
+	struct media_device *mdev;
 	/* used to keep track of the registered subdevs */
 	struct list_head subdevs;
 	/* lock this struct; can be used by the driver as well if this
-- 
1.7.1

[RFC/PATCH v3 09/10] v4l: Make video_device inherit from media_entity

[Laurent Pinchart]

V4L2 devices are media entities. As such they need to inherit from
(include) the media_entity structure.

When registering/unregistering the device, the media entity is
automatically registered/unregistered. The entity is acquired on device
open and released on device close.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 Documentation/video4linux/v4l2-framework.txt |   38 +++++++++++++++++++++++--
 drivers/media/video/v4l2-dev.c               |   35 ++++++++++++++++++++++-
 include/media/v4l2-dev.h                     |    6 ++++
 3 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 8a3f14e..7ff4016 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -71,6 +71,10 @@ sub-device instances, the video_device struct stores V4L2 device node data
 and in the future a v4l2_fh struct will keep track of filehandle instances
 (this is not yet implemented).
 
+The V4L2 framework also optionally integrates with the media framework. If a
+driver sets the struct v4l2_device mdev field, sub-devices and video nodes
+will automatically appear in the media framework as entities.
+
 
 struct v4l2_device
 ------------------
@@ -84,11 +88,14 @@ You must register the device instance:
 	v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev);
 
 Registration will initialize the v4l2_device struct. If the dev->driver_data
-field is NULL, it will be linked to v4l2_dev. Drivers that use the media
-device framework in addition to the V4L2 framework need to set
+field is NULL, it will be linked to v4l2_dev.
+
+Drivers that want integration with the media device framework need to set
 dev->driver_data manually to point to the driver-specific device structure
 that embed the struct v4l2_device instance. This is achieved by a
-dev_set_drvdata() call before registering the V4L2 device instance.
+dev_set_drvdata() call before registering the V4L2 device instance. They must
+also set the struct v4l2_device mdev field to point to a properly initialized
+and registered media_device instance.
 
 If v4l2_dev->name is empty then it will be set to a value derived from dev
 (driver name followed by the bus_id, to be precise). If you set it up before
@@ -523,6 +530,21 @@ If you use v4l2_ioctl_ops, then you should set either .unlocked_ioctl or
 The v4l2_file_operations struct is a subset of file_operations. The main
 difference is that the inode argument is omitted since it is never used.
 
+If integration with the media framework is needed, you must initialize the
+media_entity struct embedded in the video_device struct (entity field) by
+calling media_entity_init():
+
+	struct media_pad *pad = &my_vdev->pad;
+	int err;
+
+	err = media_entity_init(&vdev->entity, 1, pad, 0);
+
+The pads array must have been previously initialized. There is no need to
+manually set the struct media_entity type and name fields.
+
+A reference to the entity will be automatically acquired/released when the
+video device is opened/closed.
+
 
 video_device registration
 -------------------------
@@ -536,6 +558,9 @@ for you.
 		return err;
 	}
 
+If the v4l2_device parent device has a non-NULL mdev field, the video device
+entity will be automatically registered with the media device.
+
 Which device is registered depends on the type argument. The following
 types exist:
 
@@ -613,6 +638,13 @@ those will still be passed on since some buffer ioctls may still be needed.
 When the last user of the video device node exits, then the vdev->release()
 callback is called and you can do the final cleanup there.
 
+Don't forget to cleanup the media entity associated with the video device if
+it has been initialized:
+
+	media_entity_cleanup(&vdev->entity);
+
+This can be done from the release callback.
+
 
 video_device helper functions
 -----------------------------
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index bcd47a0..0cbd3e6 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -269,6 +269,7 @@ static int v4l2_mmap(struct file *filp, struct vm_area_struct *vm)
 static int v4l2_open(struct inode *inode, struct file *filp)
 {
 	struct video_device *vdev;
+	struct media_entity *entity = NULL;
 	int ret = 0;
 
 	/* Check if the video device is available */
@@ -283,12 +284,23 @@ static int v4l2_open(struct inode *inode, struct file *filp)
 	/* and increase the device refcount */
 	video_get(vdev);
 	mutex_unlock(&videodev_lock);
+	if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
+		entity = media_entity_get(&vdev->entity);
+		if (!entity) {
+			ret = -EBUSY;
+			video_put(vdev);
+			return ret;
+		}
+	}
 	if (vdev->fops->open)
 		ret = vdev->fops->open(filp);
 
 	/* decrease the refcount in case of an error */
-	if (ret)
+	if (ret) {
+		if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
+			media_entity_put(entity);
 		video_put(vdev);
+	}
 	return ret;
 }
 
@@ -301,6 +313,9 @@ static int v4l2_release(struct inode *inode, struct file *filp)
 	if (vdev->fops->release)
 		vdev->fops->release(filp);
 
+	if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
+		media_entity_put(&vdev->entity);
+
 	/* decrease the refcount unconditionally since the release()
 	   return value is ignored. */
 	video_put(vdev);
@@ -563,11 +578,24 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
 		printk(KERN_WARNING "%s: requested %s%d, got %s\n", __func__,
 			name_base, nr, video_device_node_name(vdev));
 
-	/* Part 5: Activate this minor. The char device can now be used. */
+	/* Part 5: Register the entity. */
+	if (vdev->v4l2_dev && vdev->v4l2_dev->mdev) {
+		vdev->entity.type = MEDIA_ENTITY_TYPE_NODE_V4L;
+		vdev->entity.name = vdev->name;
+		vdev->entity.v4l.major = VIDEO_MAJOR;
+		vdev->entity.v4l.minor = vdev->minor;
+		ret = media_device_register_entity(vdev->v4l2_dev->mdev,
+			&vdev->entity);
+		if (ret < 0)
+			printk(KERN_ERR "error\n"); /* TODO */
+	}
+
+	/* Part 6: Activate this minor. The char device can now be used. */
 	set_bit(V4L2_FL_REGISTERED, &vdev->flags);
 	mutex_lock(&videodev_lock);
 	video_device[vdev->minor] = vdev;
 	mutex_unlock(&videodev_lock);
+
 	return 0;
 
 cleanup:
@@ -595,6 +623,9 @@ void video_unregister_device(struct video_device *vdev)
 	if (!vdev || !video_is_registered(vdev))
 		return;
 
+	if (vdev->v4l2_dev && vdev->v4l2_dev->mdev)
+		media_device_unregister_entity(&vdev->entity);
+
 	clear_bit(V4L2_FL_REGISTERED, &vdev->flags);
 	device_unregister(&vdev->dev);
 }
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 195fa56..447b154 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -16,6 +16,8 @@
 #include <linux/mutex.h>
 #include <linux/videodev2.h>
 
+#include <media/media-entity.h>
+
 #define VIDEO_MAJOR	81
 
 #define VFL_TYPE_GRABBER	0
@@ -57,6 +59,8 @@ struct v4l2_file_operations {
 
 struct video_device
 {
+	struct media_entity entity;
+
 	/* device ops */
 	const struct v4l2_file_operations *fops;
 
@@ -96,6 +100,8 @@ struct video_device
 	const struct v4l2_ioctl_ops *ioctl_ops;
 };
 
+#define media_entity_to_video_device(entity) \
+	container_of(entity, struct video_device, entity)
 /* dev to video-device */
 #define to_video_device(cd) container_of(cd, struct video_device, dev)
 
-- 
1.7.1

[RFC/PATCH v3 10/10] v4l: Make v4l2_subdev inherit from media_entity

[Laurent Pinchart]

V4L2 subdevices are media entities. As such they need to inherit from
(include) the media_entity structure.

When registering/unregistering the subdevice, the media entity is
automatically registered/unregistered. The entity is acquired on device
open and released on device close.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 Documentation/video4linux/v4l2-framework.txt |   22 +++++++++++++++++
 drivers/media/video/v4l2-device.c            |   32 +++++++++++++++++++++-----
 drivers/media/video/v4l2-subdev.c            |   27 +++++++++++++++++++++-
 include/media/v4l2-subdev.h                  |    7 +++++
 4 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 7ff4016..f7b72a9 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -263,6 +263,25 @@ A sub-device driver initializes the v4l2_subdev struct using:
 Afterwards you need to initialize subdev->name with a unique name and set the
 module owner. This is done for you if you use the i2c helper functions.
 
+If integration with the media framework is needed, you must initialize the
+media_entity struct embedded in the v4l2_subdev struct (entity field) by
+calling media_entity_init():
+
+	struct media_pad *pads = &my_sd->pads;
+	int err;
+
+	err = media_entity_init(&sd->entity, npads, pads, 0);
+
+The pads array must have been previously initialized. There is no need to
+manually set the struct media_entity type and name fields.
+
+A reference to the entity will be automatically acquired/released when the
+subdev device node (if any) is opened/closed.
+
+Don't forget to cleanup the media entity before the sub-device is destroyed:
+
+	media_entity_cleanup(&sd->entity);
+
 A device (bridge) driver needs to register the v4l2_subdev with the
 v4l2_device:
 
@@ -272,6 +291,9 @@ This can fail if the subdev module disappeared before it could be registered.
 After this function was called successfully the subdev->dev field points to
 the v4l2_device.
 
+If the v4l2_device parent device has a non-NULL mdev field, the sub-device
+entity will be automatically registered with the media device.
+
 You can unregister a sub-device using:
 
 	v4l2_device_unregister_subdev(sd);
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index 91452cd..4f74d01 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -114,10 +114,11 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)
 EXPORT_SYMBOL_GPL(v4l2_device_unregister);
 
 int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
-						struct v4l2_subdev *sd)
+				struct v4l2_subdev *sd)
 {
+	struct media_entity *entity = &sd->entity;
 	struct video_device *vdev;
-	int ret = 0;
+	int ret;
 
 	/* Check for valid input */
 	if (v4l2_dev == NULL || sd == NULL || !sd->name[0])
@@ -129,6 +130,15 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 	if (!try_module_get(sd->owner))
 		return -ENODEV;
 
+	/* Register the entity. */
+	if (v4l2_dev->mdev) {
+		ret = media_device_register_entity(v4l2_dev->mdev, entity);
+		if (ret < 0) {
+			module_put(sd->owner);
+			return ret;
+		}
+	}
+
 	sd->v4l2_dev = v4l2_dev;
 	spin_lock(&v4l2_dev->lock);
 	list_add_tail(&sd->list, &v4l2_dev->subdevs);
@@ -143,26 +153,36 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 	if (sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE) {
 		ret = __video_register_device(vdev, VFL_TYPE_SUBDEV, -1, 1,
 					      sd->owner);
-		if (ret < 0)
+		if (ret < 0) {
 			v4l2_device_unregister_subdev(sd);
+			return ret;
+		}
 	}
 
-	return ret;
+	entity->v4l.major = VIDEO_MAJOR;
+	entity->v4l.minor = vdev->minor;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
 
 void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
 {
+	struct v4l2_device *v4l2_dev;
+
 	/* return if it isn't registered */
 	if (sd == NULL || sd->v4l2_dev == NULL)
 		return;
 
-	spin_lock(&sd->v4l2_dev->lock);
+	v4l2_dev = sd->v4l2_dev;
+
+	spin_lock(&v4l2_dev->lock);
 	list_del(&sd->list);
-	spin_unlock(&sd->v4l2_dev->lock);
+	spin_unlock(&v4l2_dev->lock);
 	sd->v4l2_dev = NULL;
 
 	module_put(sd->owner);
+	if (v4l2_dev->mdev)
+		media_device_unregister_entity(&sd->entity);
 	video_unregister_device(&sd->devnode);
 }
 EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index b063195..1efa267 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -32,7 +32,8 @@ static int subdev_open(struct file *file)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
-	struct v4l2_fh *vfh;
+	struct media_entity *entity;
+	struct v4l2_fh *vfh = NULL;
 	int ret;
 
 	if (!sd->initialized)
@@ -59,10 +60,17 @@ static int subdev_open(struct file *file)
 		file->private_data = vfh;
 	}
 
+	entity = media_entity_get(&sd->entity);
+	if (!entity) {
+		ret = -EBUSY;
+		goto err;
+	}
+
 	return 0;
 
 err:
 	if (vfh != NULL) {
+		v4l2_fh_del(vfh);
 		v4l2_fh_exit(vfh);
 		kfree(vfh);
 	}
@@ -72,8 +80,12 @@ err:
 
 static int subdev_close(struct file *file)
 {
+	struct video_device *vdev = video_devdata(file);
+	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
 	struct v4l2_fh *vfh = file->private_data;
 
+	media_entity_put(&sd->entity);
+
 	if (vfh != NULL) {
 		v4l2_fh_del(vfh);
 		v4l2_fh_exit(vfh);
@@ -172,5 +184,18 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
 	sd->grp_id = 0;
 	sd->priv = NULL;
 	sd->initialized = 1;
+	sd->entity.name = sd->name;
+	sd->entity.type = MEDIA_ENTITY_TYPE_SUBDEV;
 }
 EXPORT_SYMBOL(v4l2_subdev_init);
+
+int v4l2_subdev_set_power(struct media_entity *entity, int power)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+
+	dev_dbg(entity->parent->dev,
+		"%s power%s\n", entity->name, power ? "on" : "off");
+
+	return v4l2_subdev_call(sd, core, s_power, power);
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_set_power);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 55a8c93..f9e1897 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -21,6 +21,7 @@
 #ifndef _V4L2_SUBDEV_H
 #define _V4L2_SUBDEV_H
 
+#include <media/media-entity.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-dev.h>
 #include <media/v4l2-mediabus.h>
@@ -421,6 +422,8 @@ struct v4l2_subdev_ops {
    stand-alone or embedded in a larger struct.
  */
 struct v4l2_subdev {
+	struct media_entity entity;
+
 	struct list_head list;
 	struct module *owner;
 	u32 flags;
@@ -439,6 +442,8 @@ struct v4l2_subdev {
 	unsigned int nevents;
 };
 
+#define media_entity_to_v4l2_subdev(ent) \
+	container_of(ent, struct v4l2_subdev, entity)
 #define vdev_to_v4l2_subdev(vdev) \
 	container_of(vdev, struct v4l2_subdev, devnode)
 
@@ -457,6 +462,8 @@ static inline void *v4l2_get_subdevdata(const struct v4l2_subdev *sd)
 void v4l2_subdev_init(struct v4l2_subdev *sd,
 		      const struct v4l2_subdev_ops *ops);
 
+int v4l2_subdev_set_power(struct media_entity *entity, int power);
+
 /* Call an ops of a v4l2_subdev, doing the right checks against
    NULL pointers.
 
-- 
1.7.1

[SAMPLE v3 00/12] V4L2 API additions and OMAP3 ISP driver

[Laurent Pinchart]

Here's the OMAP3 ISP driver along with V4L2 API additions/enhancements that
it depends on, rebased on top of the media controller v3 patches. Once again
please don't review this set, but use it as sample code for the media
controller.

Antti Koskipaa (1):
  v4l: Add crop ioctl to V4L2 subdev API

Laurent Pinchart (8):
  v4l: Move the media/v4l2-mediabus.h header to include/linux
  v4l: Add 16 bit YUYV and SGRBG10 media bus format codes
  v4l-subdev: Add pads operations
  v4l: v4l2_subdev userspace format API
  v4l: Add subdev userspace API to enumerate and configure frame
    interval
  v4l: subdev: Generic ioctl support
  omap34xxcam: Register the ISP platform device during omap34xxcam
    probe
  OMAP3 ISP driver

Stanimir Varbanov (2):
  v4l: Create v4l2 subdev file handle structure
  omap3: Export omap3isp platform device structure

Tuukka Toivonen (1):
  ARM: OMAP3: Update Camera ISP definitions for OMAP3630

 Documentation/video4linux/v4l2-framework.txt |    5 +
 arch/arm/mach-omap2/devices.c                |   46 +-
 arch/arm/mach-omap2/devices.h                |   17 +
 arch/arm/plat-omap/include/mach/isp_user.h   |  637 ++++++++
 arch/arm/plat-omap/include/plat/omap34xx.h   |   16 +-
 drivers/media/video/Kconfig                  |    9 +
 drivers/media/video/Makefile                 |    4 +
 drivers/media/video/isp/Makefile             |   14 +
 drivers/media/video/isp/bluegamma_table.h    | 1040 ++++++++++++
 drivers/media/video/isp/cfa_coef_table.h     |  603 +++++++
 drivers/media/video/isp/greengamma_table.h   | 1040 ++++++++++++
 drivers/media/video/isp/isp.c                | 1686 +++++++++++++++++++
 drivers/media/video/isp/isp.h                |  402 +++++
 drivers/media/video/isp/ispccdc.c            | 2042 +++++++++++++++++++++++
 drivers/media/video/isp/ispccdc.h            |  177 ++
 drivers/media/video/isp/ispccp2.c            | 1036 ++++++++++++
 drivers/media/video/isp/ispccp2.h            |   89 +
 drivers/media/video/isp/ispcsi2.c            | 1215 ++++++++++++++
 drivers/media/video/isp/ispcsi2.h            |  158 ++
 drivers/media/video/isp/ispcsiphy.c          |  245 +++
 drivers/media/video/isp/ispcsiphy.h          |   72 +
 drivers/media/video/isp/isph3a.h             |  111 ++
 drivers/media/video/isp/isph3a_aewb.c        |  307 ++++
 drivers/media/video/isp/isph3a_af.c          |  358 ++++
 drivers/media/video/isp/isphist.c            |  505 ++++++
 drivers/media/video/isp/isphist.h            |   34 +
 drivers/media/video/isp/isppreview.c         | 2264 ++++++++++++++++++++++++++
 drivers/media/video/isp/isppreview.h         |  259 +++
 drivers/media/video/isp/ispqueue.c           | 1074 ++++++++++++
 drivers/media/video/isp/ispqueue.h           |  175 ++
 drivers/media/video/isp/ispreg.h             | 1802 ++++++++++++++++++++
 drivers/media/video/isp/ispresizer.c         | 1638 +++++++++++++++++++
 drivers/media/video/isp/ispresizer.h         |  136 ++
 drivers/media/video/isp/ispstat.c            |  971 +++++++++++
 drivers/media/video/isp/ispstat.h            |  161 ++
 drivers/media/video/isp/ispvideo.c           | 1241 ++++++++++++++
 drivers/media/video/isp/ispvideo.h           |  139 ++
 drivers/media/video/isp/luma_enhance_table.h |  144 ++
 drivers/media/video/isp/noise_filter_table.h |   79 +
 drivers/media/video/isp/redgamma_table.h     | 1040 ++++++++++++
 drivers/media/video/omap34xxcam.c            | 1562 ++++++++++++++++++
 drivers/media/video/omap34xxcam.h            |  137 ++
 drivers/media/video/v4l2-subdev.c            |  177 ++-
 include/linux/v4l2-mediabus.h                |   70 +
 include/linux/v4l2-subdev.h                  |  102 ++
 include/media/soc_mediabus.h                 |    3 +-
 include/media/v4l2-mediabus.h                |   48 +-
 include/media/v4l2-subdev.h                  |   53 +
 48 files changed, 25056 insertions(+), 87 deletions(-)
 create mode 100644 arch/arm/mach-omap2/devices.h
 create mode 100644 arch/arm/plat-omap/include/mach/isp_user.h
 create mode 100644 drivers/media/video/isp/Makefile
 create mode 100644 drivers/media/video/isp/bluegamma_table.h
 create mode 100644 drivers/media/video/isp/cfa_coef_table.h
 create mode 100644 drivers/media/video/isp/greengamma_table.h
 create mode 100644 drivers/media/video/isp/isp.c
 create mode 100644 drivers/media/video/isp/isp.h
 create mode 100644 drivers/media/video/isp/ispccdc.c
 create mode 100644 drivers/media/video/isp/ispccdc.h
 create mode 100644 drivers/media/video/isp/ispccp2.c
 create mode 100644 drivers/media/video/isp/ispccp2.h
 create mode 100644 drivers/media/video/isp/ispcsi2.c
 create mode 100644 drivers/media/video/isp/ispcsi2.h
 create mode 100644 drivers/media/video/isp/ispcsiphy.c
 create mode 100644 drivers/media/video/isp/ispcsiphy.h
 create mode 100644 drivers/media/video/isp/isph3a.h
 create mode 100644 drivers/media/video/isp/isph3a_aewb.c
 create mode 100644 drivers/media/video/isp/isph3a_af.c
 create mode 100644 drivers/media/video/isp/isphist.c
 create mode 100644 drivers/media/video/isp/isphist.h
 create mode 100644 drivers/media/video/isp/isppreview.c
 create mode 100644 drivers/media/video/isp/isppreview.h
 create mode 100644 drivers/media/video/isp/ispqueue.c
 create mode 100644 drivers/media/video/isp/ispqueue.h
 create mode 100644 drivers/media/video/isp/ispreg.h
 create mode 100644 drivers/media/video/isp/ispresizer.c
 create mode 100644 drivers/media/video/isp/ispresizer.h
 create mode 100644 drivers/media/video/isp/ispstat.c
 create mode 100644 drivers/media/video/isp/ispstat.h
 create mode 100644 drivers/media/video/isp/ispvideo.c
 create mode 100644 drivers/media/video/isp/ispvideo.h
 create mode 100644 drivers/media/video/isp/luma_enhance_table.h
 create mode 100644 drivers/media/video/isp/noise_filter_table.h
 create mode 100644 drivers/media/video/isp/redgamma_table.h
 create mode 100644 drivers/media/video/omap34xxcam.c
 create mode 100644 drivers/media/video/omap34xxcam.h
 create mode 100644 include/linux/v4l2-mediabus.h
 create mode 100644 include/linux/v4l2-subdev.h

-- 
Regards,

Laurent Pinchart

[SAMPLE v3 01/12] v4l: Move the media/v4l2-mediabus.h header to include/linux

[Laurent Pinchart]

The header defines the v4l2_mbus_framefmt structure which will be used
by the V4L2 subdevs userspace API.

Change the type of the v4l2_mbus_framefmt::code field to __u32, as enum
sizes can differ between different ABIs on the same architectures.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/linux/v4l2-mediabus.h |   64 +++++++++++++++++++++++++++++++++++++++++
 include/media/soc_mediabus.h  |    3 +-
 include/media/v4l2-mediabus.h |   48 +------------------------------
 3 files changed, 66 insertions(+), 49 deletions(-)
 create mode 100644 include/linux/v4l2-mediabus.h

diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
new file mode 100644
index 0000000..17219c3
--- /dev/null
+++ b/include/linux/v4l2-mediabus.h
@@ -0,0 +1,64 @@
+/*
+ * Media Bus API header
+ *
+ * Copyright (C) 2009, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX_V4L2_MEDIABUS_H
+#define __LINUX_V4L2_MEDIABUS_H
+
+#include <linux/videodev2.h>
+
+/*
+ * These pixel codes uniquely identify data formats on the media bus. Mostly
+ * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is
+ * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where the
+ * data format is fixed. Additionally, "2X8" means that one pixel is transferred
+ * in two 8-bit samples, "BE" or "LE" specify in which order those samples are
+ * transferred over the bus: "LE" means that the least significant bits are
+ * transferred first, "BE" means that the most significant bits are transferred
+ * first, and "PADHI" and "PADLO" define which bits - low or high, in the
+ * incomplete high byte, are filled with padding bits.
+ */
+enum v4l2_mbus_pixelcode {
+	V4L2_MBUS_FMT_FIXED = 1,
+	V4L2_MBUS_FMT_YUYV8_2X8_LE,
+	V4L2_MBUS_FMT_YVYU8_2X8_LE,
+	V4L2_MBUS_FMT_YUYV8_2X8_BE,
+	V4L2_MBUS_FMT_YVYU8_2X8_BE,
+	V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
+	V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
+	V4L2_MBUS_FMT_RGB565_2X8_LE,
+	V4L2_MBUS_FMT_RGB565_2X8_BE,
+	V4L2_MBUS_FMT_SBGGR8_1X8,
+	V4L2_MBUS_FMT_SBGGR10_1X10,
+	V4L2_MBUS_FMT_GREY8_1X8,
+	V4L2_MBUS_FMT_Y10_1X10,
+	V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE,
+	V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE,
+	V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE,
+	V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE,
+	V4L2_MBUS_FMT_SGRBG8_1X8,
+};
+
+/**
+ * struct v4l2_mbus_framefmt - frame format on the media bus
+ * @width:	frame width
+ * @height:	frame height
+ * @code:	data format code
+ * @field:	used interlacing type
+ * @colorspace:	colorspace of the data
+ */
+struct v4l2_mbus_framefmt {
+	__u32				width;
+	__u32				height;
+	__u32				code;
+	enum v4l2_field			field;
+	enum v4l2_colorspace		colorspace;
+};
+
+#endif
diff --git a/include/media/soc_mediabus.h b/include/media/soc_mediabus.h
index 037cd7b..6243147 100644
--- a/include/media/soc_mediabus.h
+++ b/include/media/soc_mediabus.h
@@ -12,8 +12,7 @@
 #define SOC_MEDIABUS_H
 
 #include <linux/videodev2.h>
-
-#include <media/v4l2-mediabus.h>
+#include <linux/v4l2-mediabus.h>
 
 /**
  * enum soc_mbus_packing - data packing types on the media-bus
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 865cda7..971c7fa 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -11,53 +11,7 @@
 #ifndef V4L2_MEDIABUS_H
 #define V4L2_MEDIABUS_H
 
-/*
- * These pixel codes uniquely identify data formats on the media bus. Mostly
- * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is
- * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where the
- * data format is fixed. Additionally, "2X8" means that one pixel is transferred
- * in two 8-bit samples, "BE" or "LE" specify in which order those samples are
- * transferred over the bus: "LE" means that the least significant bits are
- * transferred first, "BE" means that the most significant bits are transferred
- * first, and "PADHI" and "PADLO" define which bits - low or high, in the
- * incomplete high byte, are filled with padding bits.
- */
-enum v4l2_mbus_pixelcode {
-	V4L2_MBUS_FMT_FIXED = 1,
-	V4L2_MBUS_FMT_YUYV8_2X8_LE,
-	V4L2_MBUS_FMT_YVYU8_2X8_LE,
-	V4L2_MBUS_FMT_YUYV8_2X8_BE,
-	V4L2_MBUS_FMT_YVYU8_2X8_BE,
-	V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
-	V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
-	V4L2_MBUS_FMT_RGB565_2X8_LE,
-	V4L2_MBUS_FMT_RGB565_2X8_BE,
-	V4L2_MBUS_FMT_SBGGR8_1X8,
-	V4L2_MBUS_FMT_SBGGR10_1X10,
-	V4L2_MBUS_FMT_GREY8_1X8,
-	V4L2_MBUS_FMT_Y10_1X10,
-	V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE,
-	V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE,
-	V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE,
-	V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE,
-	V4L2_MBUS_FMT_SGRBG8_1X8,
-};
-
-/**
- * struct v4l2_mbus_framefmt - frame format on the media bus
- * @width:	frame width
- * @height:	frame height
- * @code:	data format code
- * @field:	used interlacing type
- * @colorspace:	colorspace of the data
- */
-struct v4l2_mbus_framefmt {
-	__u32				width;
-	__u32				height;
-	enum v4l2_mbus_pixelcode	code;
-	enum v4l2_field			field;
-	enum v4l2_colorspace		colorspace;
-};
+#include <linux/v4l2-mediabus.h>
 
 static inline void v4l2_fill_pix_format(struct v4l2_pix_format *pix_fmt,
 				const struct v4l2_mbus_framefmt *mbus_fmt)
-- 
1.7.1

[SAMPLE v3 02/12] v4l: Add 16 bit YUYV and SGRBG10 media bus format codes

[Laurent Pinchart]

Add the following media bus format code definitions:

- V4L2_MBUS_FMT_SGRBG10_1X10 for 10-bit GRBG Bayer
- V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8 for 10-bit DPCM compressed GRBG Bayer
- V4L2_MBUS_FMT_YUYV16_1X16 for 16-bit YUYV
- V4L2_MBUS_FMT_UYVY16_1X16 for 16-bit UYVY
- V4L2_MBUS_FMT_YVYU16_1X16 for 16-bit YVYU
- V4L2_MBUS_FMT_VYUY16_1X16 for 16-bit VYUY

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/linux/v4l2-mediabus.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
index 17219c3..34dd708 100644
--- a/include/linux/v4l2-mediabus.h
+++ b/include/linux/v4l2-mediabus.h
@@ -43,6 +43,12 @@ enum v4l2_mbus_pixelcode {
 	V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE,
 	V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE,
 	V4L2_MBUS_FMT_SGRBG8_1X8,
+	V4L2_MBUS_FMT_SGRBG10_1X10,
+	V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
+	V4L2_MBUS_FMT_YUYV16_1X16,
+	V4L2_MBUS_FMT_UYVY16_1X16,
+	V4L2_MBUS_FMT_YVYU16_1X16,
+	V4L2_MBUS_FMT_VYUY16_1X16,
 };
 
 /**
-- 
1.7.1

[SAMPLE v3 03/12] v4l: Create v4l2 subdev file handle structure

[Laurent Pinchart]

From: Stanimir Varbanov <svarbanov@mm-sol.com>

Used for storing subdev information per file handle and hold V4L2 file
handle.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/v4l2-subdev.c |   76 ++++++++++++++++++++++++-------------
 include/media/v4l2-subdev.h       |   18 +++++++++
 2 files changed, 67 insertions(+), 27 deletions(-)

diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 1efa267..2fe3818 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -28,38 +28,60 @@
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
 
+static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
+{
+	fh->probe_fmt = kzalloc(sizeof(*fh->probe_fmt) *
+				sd->entity.num_pads, GFP_KERNEL);
+	if (fh->probe_fmt == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void subdev_fh_free(struct v4l2_subdev_fh *fh)
+{
+	kfree(fh->probe_fmt);
+	fh->probe_fmt = NULL;
+}
+
 static int subdev_open(struct file *file)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
+	struct v4l2_subdev_fh *subdev_fh;
 	struct media_entity *entity;
-	struct v4l2_fh *vfh = NULL;
 	int ret;
 
 	if (!sd->initialized)
 		return -EAGAIN;
 
-	if (sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS) {
-		vfh = kzalloc(sizeof(*vfh), GFP_KERNEL);
-		if (vfh == NULL)
-			return -ENOMEM;
+	subdev_fh = kzalloc(sizeof(*subdev_fh), GFP_KERNEL);
+	if (subdev_fh == NULL)
+		return -ENOMEM;
 
-		ret = v4l2_fh_init(vfh, vdev);
-		if (ret)
-			goto err;
+	ret = subdev_fh_init(subdev_fh, sd);
+	if (ret) {
+		kfree(subdev_fh);
+		return ret;
+	}
+
+	ret = v4l2_fh_init(&subdev_fh->vfh, vdev);
+	if (ret)
+		goto err;
 
-		ret = v4l2_event_init(vfh);
+	if (sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS) {
+		ret = v4l2_event_init(&subdev_fh->vfh);
 		if (ret)
 			goto err;
 
-		ret = v4l2_event_alloc(vfh, sd->nevents);
+		ret = v4l2_event_alloc(&subdev_fh->vfh, sd->nevents);
 		if (ret)
 			goto err;
-
-		v4l2_fh_add(vfh);
-		file->private_data = vfh;
 	}
 
+	v4l2_fh_add(&subdev_fh->vfh);
+	file->private_data = &subdev_fh->vfh;
+
 	entity = media_entity_get(&sd->entity);
 	if (!entity) {
 		ret = -EBUSY;
@@ -69,11 +91,10 @@ static int subdev_open(struct file *file)
 	return 0;
 
 err:
-	if (vfh != NULL) {
-		v4l2_fh_del(vfh);
-		v4l2_fh_exit(vfh);
-		kfree(vfh);
-	}
+	v4l2_fh_del(&subdev_fh->vfh);
+	v4l2_fh_exit(&subdev_fh->vfh);
+	subdev_fh_free(subdev_fh);
+	kfree(subdev_fh);
 
 	return ret;
 }
@@ -83,14 +104,15 @@ static int subdev_close(struct file *file)
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
 	struct v4l2_fh *vfh = file->private_data;
+	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
 
 	media_entity_put(&sd->entity);
 
-	if (vfh != NULL) {
-		v4l2_fh_del(vfh);
-		v4l2_fh_exit(vfh);
-		kfree(vfh);
-	}
+	v4l2_fh_del(vfh);
+	v4l2_fh_exit(vfh);
+	subdev_fh_free(subdev_fh);
+	kfree(subdev_fh);
+	file->private_data = NULL;
 
 	return 0;
 }
@@ -99,7 +121,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
-	struct v4l2_fh *fh = file->private_data;
+	struct v4l2_fh *vfh = file->private_data;
 
 	switch (cmd) {
 	case VIDIOC_QUERYCTRL:
@@ -127,13 +149,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_EVENTS))
 			return -ENOIOCTLCMD;
 
-		return v4l2_event_dequeue(fh, arg, file->f_flags & O_NONBLOCK);
+		return v4l2_event_dequeue(vfh, arg, file->f_flags & O_NONBLOCK);
 
 	case VIDIOC_SUBSCRIBE_EVENT:
-		return v4l2_subdev_call(sd, core, subscribe_event, fh, arg);
+		return v4l2_subdev_call(sd, core, subscribe_event, vfh, arg);
 
 	case VIDIOC_UNSUBSCRIBE_EVENT:
-		return v4l2_subdev_call(sd, core, unsubscribe_event, fh, arg);
+		return v4l2_subdev_call(sd, core, unsubscribe_event, vfh, arg);
 
 	default:
 		return -ENOIOCTLCMD;
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index f9e1897..01b4135 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -24,6 +24,7 @@
 #include <media/media-entity.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-dev.h>
+#include <media/v4l2-fh.h>
 #include <media/v4l2-mediabus.h>
 
 /* generic v4l2_device notify callback notification values */
@@ -447,6 +448,23 @@ struct v4l2_subdev {
 #define vdev_to_v4l2_subdev(vdev) \
 	container_of(vdev, struct v4l2_subdev, devnode)
 
+/*
+ * Used for storing subdev information per file handle
+ */
+struct v4l2_subdev_fh {
+	struct v4l2_fh vfh;
+	struct v4l2_mbus_framefmt *probe_fmt;
+};
+
+#define to_v4l2_subdev_fh(fh)	\
+	container_of(fh, struct v4l2_subdev_fh, vfh)
+
+static inline struct v4l2_mbus_framefmt *
+v4l2_subdev_get_probe_format(struct v4l2_subdev_fh *fh, unsigned int pad)
+{
+	return &fh->probe_fmt[pad];
+}
+
 extern const struct v4l2_file_operations v4l2_subdev_fops;
 
 static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
-- 
1.7.1

[SAMPLE v3 04/12] v4l-subdev: Add pads operations

[Laurent Pinchart]

Add a v4l2_subdev_pad_ops structure for the operations that need to be
performed at the pad level such as format-related operations.

The format at the output of a subdev usually depends on the format at
its input(s). The try format operation is thus not suitable for probing
format at individual pads, as it can't modify the device state and thus
can't remember the format probed at the input to compute the output
format.

To fix the problem, pass an extra argument to the get/set format
operations to select the 'probe' or 'active' format.

The probe format is used when probing the subdev. Setting the probe
format must not change the device configuration but can store data for
later reuse. Data storage is provided at the file-handle level so
applications probing the subdev concurently won't interfere with each
other.

The active format is used when configuring the subdev. It's identical to
the format handled by the usual get/set operations.

Pad format-related operations use v4l2_mbus_framefmt instead of
v4l2_format.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/media/v4l2-subdev.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 01b4135..684ab60 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -41,6 +41,7 @@ struct v4l2_device;
 struct v4l2_event_subscription;
 struct v4l2_fh;
 struct v4l2_subdev;
+struct v4l2_subdev_fh;
 struct tuner_setup;
 
 /* decode_vbi_line */
@@ -398,6 +399,25 @@ struct v4l2_subdev_ir_ops {
 				struct v4l2_subdev_ir_parameters *params);
 };
 
+enum v4l2_subdev_format {
+	V4L2_SUBDEV_FORMAT_PROBE = 0,
+	V4L2_SUBDEV_FORMAT_ACTIVE = 1,
+};
+
+struct v4l2_subdev_pad_ops {
+	int (*enum_mbus_code)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
+			      struct v4l2_subdev_pad_mbus_code_enum *code);
+	int (*enum_frame_size)(struct v4l2_subdev *sd,
+			       struct v4l2_subdev_fh *fh,
+			       struct v4l2_subdev_frame_size_enum *fse);
+	int (*get_fmt)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
+		       unsigned int pad, struct v4l2_mbus_framefmt *fmt,
+		       enum v4l2_subdev_format which);
+	int (*set_fmt)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
+		       unsigned int pad, struct v4l2_mbus_framefmt *fmt,
+		       enum v4l2_subdev_format which);
+};
+
 struct v4l2_subdev_ops {
 	const struct v4l2_subdev_core_ops	*core;
 	const struct v4l2_subdev_tuner_ops	*tuner;
@@ -406,6 +426,7 @@ struct v4l2_subdev_ops {
 	const struct v4l2_subdev_vbi_ops	*vbi;
 	const struct v4l2_subdev_ir_ops		*ir;
 	const struct v4l2_subdev_sensor_ops	*sensor;
+	const struct v4l2_subdev_pad_ops	*pad;
 };
 
 #define V4L2_SUBDEV_NAME_SIZE 32
-- 
1.7.1

[SAMPLE v3 05/12] v4l: v4l2_subdev userspace format API

[Laurent Pinchart]

Add a userspace API to get, set and enumerate the media format on a
subdev pad.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/v4l2-subdev.c |   51 ++++++++++++++++++++++++++++
 include/linux/v4l2-subdev.h       |   66 +++++++++++++++++++++++++++++++++++++
 include/media/v4l2-subdev.h       |    6 +---
 3 files changed, 118 insertions(+), 5 deletions(-)
 create mode 100644 include/linux/v4l2-subdev.h

diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 2fe3818..d8b261f 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -122,6 +122,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	struct video_device *vdev = video_devdata(file);
 	struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
 	struct v4l2_fh *vfh = file->private_data;
+	struct v4l2_subdev_fh *subdev_fh = to_v4l2_subdev_fh(vfh);
 
 	switch (cmd) {
 	case VIDIOC_QUERYCTRL:
@@ -157,6 +158,56 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_UNSUBSCRIBE_EVENT:
 		return v4l2_subdev_call(sd, core, unsubscribe_event, vfh, arg);
 
+	case VIDIOC_SUBDEV_G_FMT: {
+		struct v4l2_subdev_pad_format *format = arg;
+
+		if (format->which != V4L2_SUBDEV_FORMAT_PROBE &&
+		    format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+			return -EINVAL;
+
+		if (format->pad >= sd->entity.num_pads)
+			return -EINVAL;
+
+		return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh,
+					format->pad, &format->format,
+					format->which);
+	}
+
+	case VIDIOC_SUBDEV_S_FMT: {
+		struct v4l2_subdev_pad_format *format = arg;
+
+		if (format->which != V4L2_SUBDEV_FORMAT_PROBE &&
+		    format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+			return -EINVAL;
+
+		if (format->pad >= sd->entity.num_pads)
+			return -EINVAL;
+
+		return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh,
+					format->pad, &format->format,
+					format->which);
+	}
+
+	case VIDIOC_SUBDEV_ENUM_MBUS_CODE: {
+		struct v4l2_subdev_pad_mbus_code_enum *code = arg;
+
+		if (code->pad >= sd->entity.num_pads)
+			return -EINVAL;
+
+		return v4l2_subdev_call(sd, pad, enum_mbus_code, subdev_fh,
+					code);
+	}
+
+	case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: {
+		struct v4l2_subdev_frame_size_enum *fse = arg;
+
+		if (fse->pad >= sd->entity.num_pads)
+			return -EINVAL;
+
+		return v4l2_subdev_call(sd, pad, enum_frame_size, subdev_fh,
+					fse);
+	}
+
 	default:
 		return -ENOIOCTLCMD;
 	}
diff --git a/include/linux/v4l2-subdev.h b/include/linux/v4l2-subdev.h
new file mode 100644
index 0000000..6504f22
--- /dev/null
+++ b/include/linux/v4l2-subdev.h
@@ -0,0 +1,66 @@
+/*
+ * V4L2 subdev userspace API
+ *
+ * Copyright (C) 2010 Nokia
+ *
+ * Contributors:
+ *	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * This package is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+ */
+
+#ifndef __LINUX_V4L2_SUBDEV_H
+#define __LINUX_V4L2_SUBDEV_H
+
+#include <linux/ioctl.h>
+#include <linux/v4l2-mediabus.h>
+
+enum v4l2_subdev_format {
+	V4L2_SUBDEV_FORMAT_PROBE = 0,
+	V4L2_SUBDEV_FORMAT_ACTIVE = 1,
+};
+
+/**
+ * struct v4l2_subdev_pad_format
+ */
+struct v4l2_subdev_pad_format {
+	__u32 which;
+	__u32 pad;
+	struct v4l2_mbus_framefmt format;
+};
+
+/**
+ * struct v4l2_subdev_pad_mbus_code_enum
+ */
+struct v4l2_subdev_pad_mbus_code_enum {
+	__u32 pad;
+	__u32 index;
+	__u32 code;
+	__u32 reserved[5];
+};
+
+struct v4l2_subdev_frame_size_enum {
+	__u32 index;
+	__u32 pad;
+	__u32 code;
+	__u32 min_width;
+	__u32 max_width;
+	__u32 min_height;
+	__u32 max_height;
+	__u32 reserved[9];
+};
+
+#define VIDIOC_SUBDEV_G_FMT	_IOWR('V',  4, struct v4l2_subdev_pad_format)
+#define VIDIOC_SUBDEV_S_FMT	_IOWR('V',  5, struct v4l2_subdev_pad_format)
+#define VIDIOC_SUBDEV_ENUM_MBUS_CODE \
+			_IOWR('V', 8, struct v4l2_subdev_pad_mbus_code_enum)
+#define VIDIOC_SUBDEV_ENUM_FRAME_SIZE \
+			_IOWR('V', 9, struct v4l2_subdev_frame_size_enum)
+
+#endif
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 684ab60..acbcd8f 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -21,6 +21,7 @@
 #ifndef _V4L2_SUBDEV_H
 #define _V4L2_SUBDEV_H
 
+#include <linux/v4l2-subdev.h>
 #include <media/media-entity.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-dev.h>
@@ -399,11 +400,6 @@ struct v4l2_subdev_ir_ops {
 				struct v4l2_subdev_ir_parameters *params);
 };
 
-enum v4l2_subdev_format {
-	V4L2_SUBDEV_FORMAT_PROBE = 0,
-	V4L2_SUBDEV_FORMAT_ACTIVE = 1,
-};
-
 struct v4l2_subdev_pad_ops {
 	int (*enum_mbus_code)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
 			      struct v4l2_subdev_pad_mbus_code_enum *code);
-- 
1.7.1

[SAMPLE v3 06/12] v4l: Add subdev userspace API to enumerate and configure frame interval

[Laurent Pinchart]

The three new ioctl VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL,
VIDIOC_SUBDEV_G_FRAME_INTERVAL and VIDIOC_SUBDEV_S_FRAME_INTERVAL can be
used to enumerate and configure a subdev's frame rate from userspace.

Two new video::g/s_frame_interval subdev operations are introduced to
support those ioctls. The existing video::g/s_parm operations are
deprecated and shouldn't be used anymore.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/v4l2-subdev.c |   16 ++++++++++++++++
 include/linux/v4l2-subdev.h       |   24 ++++++++++++++++++++++++
 include/media/v4l2-subdev.h       |    7 +++++++
 3 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index d8b261f..8ddb2fb 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -208,6 +208,22 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 					fse);
 	}
 
+	case VIDIOC_SUBDEV_G_FRAME_INTERVAL:
+		return v4l2_subdev_call(sd, video, g_frame_interval, arg);
+
+	case VIDIOC_SUBDEV_S_FRAME_INTERVAL:
+		return v4l2_subdev_call(sd, video, s_frame_interval, arg);
+
+	case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL: {
+		struct v4l2_subdev_frame_interval_enum *fie = arg;
+
+		if (fie->pad >= sd->entity.num_pads)
+			return -EINVAL;
+
+		return v4l2_subdev_call(sd, pad, enum_frame_interval, subdev_fh,
+					fie);
+	}
+
 	default:
 		return -ENOIOCTLCMD;
 	}
diff --git a/include/linux/v4l2-subdev.h b/include/linux/v4l2-subdev.h
index 6504f22..e3362aa 100644
--- a/include/linux/v4l2-subdev.h
+++ b/include/linux/v4l2-subdev.h
@@ -56,11 +56,35 @@ struct v4l2_subdev_frame_size_enum {
 	__u32 reserved[9];
 };
 
+/**
+ * struct v4l2_subdev_pad_frame_rate
+ */
+struct v4l2_subdev_frame_interval {
+	struct v4l2_fract interval;
+	__u32 reserved[6];
+};
+
+struct v4l2_subdev_frame_interval_enum {
+	__u32 index;
+	__u32 pad;
+	__u32 code;
+	__u32 width;
+	__u32 height;
+	struct v4l2_fract interval;
+	__u32 reserved[9];
+};
+
 #define VIDIOC_SUBDEV_G_FMT	_IOWR('V',  4, struct v4l2_subdev_pad_format)
 #define VIDIOC_SUBDEV_S_FMT	_IOWR('V',  5, struct v4l2_subdev_pad_format)
+#define VIDIOC_SUBDEV_G_FRAME_INTERVAL \
+			_IOWR('V', 6, struct v4l2_subdev_frame_interval)
+#define VIDIOC_SUBDEV_S_FRAME_INTERVAL \
+			_IOWR('V', 7, struct v4l2_subdev_frame_interval)
 #define VIDIOC_SUBDEV_ENUM_MBUS_CODE \
 			_IOWR('V', 8, struct v4l2_subdev_pad_mbus_code_enum)
 #define VIDIOC_SUBDEV_ENUM_FRAME_SIZE \
 			_IOWR('V', 9, struct v4l2_subdev_frame_size_enum)
+#define VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL \
+			_IOWR('V', 10, struct v4l2_subdev_frame_interval_enum)
 
 #endif
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index acbcd8f..58ef923 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -245,6 +245,10 @@ struct v4l2_subdev_video_ops {
 	int (*s_crop)(struct v4l2_subdev *sd, struct v4l2_crop *crop);
 	int (*g_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
 	int (*s_parm)(struct v4l2_subdev *sd, struct v4l2_streamparm *param);
+	int (*g_frame_interval)(struct v4l2_subdev *sd,
+				struct v4l2_subdev_frame_interval *interval);
+	int (*s_frame_interval)(struct v4l2_subdev *sd,
+				struct v4l2_subdev_frame_interval *interval);
 	int (*enum_framesizes)(struct v4l2_subdev *sd, struct v4l2_frmsizeenum *fsize);
 	int (*enum_frameintervals)(struct v4l2_subdev *sd, struct v4l2_frmivalenum *fival);
 	int (*enum_dv_presets) (struct v4l2_subdev *sd,
@@ -406,6 +410,9 @@ struct v4l2_subdev_pad_ops {
 	int (*enum_frame_size)(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_fh *fh,
 			       struct v4l2_subdev_frame_size_enum *fse);
+	int (*enum_frame_interval)(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_fh *fh,
+				   struct v4l2_subdev_frame_interval_enum *fie);
 	int (*get_fmt)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
 		       unsigned int pad, struct v4l2_mbus_framefmt *fmt,
 		       enum v4l2_subdev_format which);
-- 
1.7.1

[SAMPLE v3 07/12] v4l: Add crop ioctl to V4L2 subdev API

[Laurent Pinchart]

From: Antti Koskipaa <antti.koskipaa@nokia.com>

This patch adds the VIDIOC_SUBDEV_S_CROP and G_CROP ioctls to the
userland API. CROPCAP is not implemented because it's redundant.

Signed-off-by: Antti Koskipaa <antti.koskipaa@nokia.com>
---
 drivers/media/video/v4l2-subdev.c |   36 ++++++++++++++++++++++++++++++++++--
 include/linux/v4l2-subdev.h       |   12 ++++++++++++
 include/media/v4l2-subdev.h       |   11 +++++++++++
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index 8ddb2fb..ad4b95e 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -30,11 +30,16 @@
 
 static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
 {
-	fh->probe_fmt = kzalloc(sizeof(*fh->probe_fmt) *
-				sd->entity.num_pads, GFP_KERNEL);
+	/* Allocate probe format and crop in the same memory block */
+	fh->probe_fmt = kzalloc((sizeof(*fh->probe_fmt) +
+				sizeof(*fh->probe_crop)) * sd->entity.num_pads,
+				GFP_KERNEL);
 	if (fh->probe_fmt == NULL)
 		return -ENOMEM;
 
+	fh->probe_crop = (struct v4l2_rect *)
+		(fh->probe_fmt + sd->entity.num_pads);
+
 	return 0;
 }
 
@@ -42,6 +47,7 @@ static void subdev_fh_free(struct v4l2_subdev_fh *fh)
 {
 	kfree(fh->probe_fmt);
 	fh->probe_fmt = NULL;
+	fh->probe_crop = NULL;
 }
 
 static int subdev_open(struct file *file)
@@ -188,6 +194,32 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 					format->which);
 	}
 
+	case VIDIOC_SUBDEV_G_CROP: {
+		struct v4l2_subdev_pad_crop *crop = arg;
+
+		if (crop->which != V4L2_SUBDEV_FORMAT_PROBE &&
+		    crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+			return -EINVAL;
+
+		if (crop->pad >= sd->entity.num_pads)
+			return -EINVAL;
+
+		return v4l2_subdev_call(sd, pad, get_crop, subdev_fh, crop);
+	}
+
+	case VIDIOC_SUBDEV_S_CROP: {
+		struct v4l2_subdev_pad_crop *crop = arg;
+
+		if (crop->which != V4L2_SUBDEV_FORMAT_PROBE &&
+		    crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+			return -EINVAL;
+
+		if (crop->pad >= sd->entity.num_pads)
+			return -EINVAL;
+
+		return v4l2_subdev_call(sd, pad, set_crop, subdev_fh, crop);
+	}
+
 	case VIDIOC_SUBDEV_ENUM_MBUS_CODE: {
 		struct v4l2_subdev_pad_mbus_code_enum *code = arg;
 
diff --git a/include/linux/v4l2-subdev.h b/include/linux/v4l2-subdev.h
index e3362aa..5738e81 100644
--- a/include/linux/v4l2-subdev.h
+++ b/include/linux/v4l2-subdev.h
@@ -36,6 +36,16 @@ struct v4l2_subdev_pad_format {
 };
 
 /**
+ * struct v4l2_subdev_pad_crop
+ */
+struct v4l2_subdev_pad_crop {
+	__u32 pad;
+	__u32 which;
+	struct v4l2_rect rect;
+	__u32 reserved[10];
+};
+
+/**
  * struct v4l2_subdev_pad_mbus_code_enum
  */
 struct v4l2_subdev_pad_mbus_code_enum {
@@ -86,5 +96,7 @@ struct v4l2_subdev_frame_interval_enum {
 			_IOWR('V', 9, struct v4l2_subdev_frame_size_enum)
 #define VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL \
 			_IOWR('V', 10, struct v4l2_subdev_frame_interval_enum)
+#define VIDIOC_SUBDEV_S_CROP	_IOWR('V', 11, struct v4l2_subdev_pad_crop)
+#define VIDIOC_SUBDEV_G_CROP	_IOWR('V', 12, struct v4l2_subdev_pad_crop)
 
 #endif
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 58ef923..41183ab 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -419,6 +419,10 @@ struct v4l2_subdev_pad_ops {
 	int (*set_fmt)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
 		       unsigned int pad, struct v4l2_mbus_framefmt *fmt,
 		       enum v4l2_subdev_format which);
+	int (*set_crop)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
+		       struct v4l2_subdev_pad_crop *crop);
+	int (*get_crop)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
+		       struct v4l2_subdev_pad_crop *crop);
 };
 
 struct v4l2_subdev_ops {
@@ -478,6 +482,7 @@ struct v4l2_subdev {
 struct v4l2_subdev_fh {
 	struct v4l2_fh vfh;
 	struct v4l2_mbus_framefmt *probe_fmt;
+	struct v4l2_rect *probe_crop;
 };
 
 #define to_v4l2_subdev_fh(fh)	\
@@ -489,6 +494,12 @@ v4l2_subdev_get_probe_format(struct v4l2_subdev_fh *fh, unsigned int pad)
 	return &fh->probe_fmt[pad];
 }
 
+static inline struct v4l2_rect *
+v4l2_subdev_get_probe_crop(struct v4l2_subdev_fh *fh, unsigned int pad)
+{
+	return &fh->probe_crop[pad];
+}
+
 extern const struct v4l2_file_operations v4l2_subdev_fops;
 
 static inline void v4l2_set_subdevdata(struct v4l2_subdev *sd, void *p)
-- 
1.7.1

[SAMPLE v3 08/12] v4l: subdev: Generic ioctl support

[Laurent Pinchart]

Instead of returning an error when receiving an ioctl call with an
unsupported command, forward the call to the subdev core::ioctl handler.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/video4linux/v4l2-framework.txt |    5 +++++
 drivers/media/video/v4l2-subdev.c            |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index f7b72a9..bdcf4bb 100644
--- a/Documentation/video4linux/v4l2-framework.txt
+++ b/Documentation/video4linux/v4l2-framework.txt
@@ -401,6 +401,11 @@ VIDIOC_UNSUBSCRIBE_EVENT
 	To properly support events, the poll() file operation is also
 	implemented.
 
+Private ioctls
+
+	All ioctls not in the above list are passed directly to the sub-device
+	driver through the core::ioctl operation.
+
 
 I2C sub-device drivers
 ----------------------
diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
index ad4b95e..887cd88 100644
--- a/drivers/media/video/v4l2-subdev.c
+++ b/drivers/media/video/v4l2-subdev.c
@@ -257,7 +257,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	}
 
 	default:
-		return -ENOIOCTLCMD;
+		return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
 	}
 
 	return 0;
-- 
1.7.1

[SAMPLE v3 09/12] ARM: OMAP3: Update Camera ISP definitions for OMAP3630

[Laurent Pinchart]

From: Tuukka Toivonen <tuukka.o.toivonen@nokia.com>

Add new/changed base address definitions and resources for
OMAP3630 ISP.

The OMAP3430 CSI2PHY block is same as the OMAP3630 CSIPHY2
block. But the later name is chosen as it gives more symmetry
to the names.

Signed-off-by: Tuukka Toivonen <tuukka.o.toivonen@nokia.com>
Signed-off-by: Vimarsh Zutshi <vimarsh.zutshi@nokia.com>
---
 arch/arm/mach-omap2/devices.c              |   28 ++++++++++++++++++++++++----
 arch/arm/plat-omap/include/plat/omap34xx.h |   16 ++++++++++++----
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 03e6c9e..46b0b4b 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -107,13 +107,33 @@ static struct resource omap3isp_resources[] = {
 		.flags		= IORESOURCE_MEM,
 	},
 	{
-		.start		= OMAP3430_ISP_CSI2A_BASE,
-		.end		= OMAP3430_ISP_CSI2A_END,
+		.start		= OMAP3430_ISP_CSI2A_REGS1_BASE,
+		.end		= OMAP3430_ISP_CSI2A_REGS1_END,
 		.flags		= IORESOURCE_MEM,
 	},
 	{
-		.start		= OMAP3430_ISP_CSI2PHY_BASE,
-		.end		= OMAP3430_ISP_CSI2PHY_END,
+		.start		= OMAP3430_ISP_CSIPHY2_BASE,
+		.end		= OMAP3430_ISP_CSIPHY2_END,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3630_ISP_CSI2A_REGS2_BASE,
+		.end		= OMAP3630_ISP_CSI2A_REGS2_END,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3630_ISP_CSI2C_REGS1_BASE,
+		.end		= OMAP3630_ISP_CSI2C_REGS1_END,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3630_ISP_CSIPHY1_BASE,
+		.end		= OMAP3630_ISP_CSIPHY1_END,
+		.flags		= IORESOURCE_MEM,
+	},
+	{
+		.start		= OMAP3630_ISP_CSI2C_REGS2_BASE,
+		.end		= OMAP3630_ISP_CSI2C_REGS2_END,
 		.flags		= IORESOURCE_MEM,
 	},
 	{
diff --git a/arch/arm/plat-omap/include/plat/omap34xx.h b/arch/arm/plat-omap/include/plat/omap34xx.h
index 98fc8b4..b9e8588 100644
--- a/arch/arm/plat-omap/include/plat/omap34xx.h
+++ b/arch/arm/plat-omap/include/plat/omap34xx.h
@@ -56,8 +56,12 @@
 #define OMAP3430_ISP_RESZ_BASE		(OMAP3430_ISP_BASE + 0x1000)
 #define OMAP3430_ISP_SBL_BASE		(OMAP3430_ISP_BASE + 0x1200)
 #define OMAP3430_ISP_MMU_BASE		(OMAP3430_ISP_BASE + 0x1400)
-#define OMAP3430_ISP_CSI2A_BASE		(OMAP3430_ISP_BASE + 0x1800)
-#define OMAP3430_ISP_CSI2PHY_BASE	(OMAP3430_ISP_BASE + 0x1970)
+#define OMAP3430_ISP_CSI2A_REGS1_BASE	(OMAP3430_ISP_BASE + 0x1800)
+#define OMAP3430_ISP_CSIPHY2_BASE	(OMAP3430_ISP_BASE + 0x1970)
+#define OMAP3630_ISP_CSI2A_REGS2_BASE	(OMAP3430_ISP_BASE + 0x19C0)
+#define OMAP3630_ISP_CSI2C_REGS1_BASE	(OMAP3430_ISP_BASE + 0x1C00)
+#define OMAP3630_ISP_CSIPHY1_BASE	(OMAP3430_ISP_BASE + 0x1D70)
+#define OMAP3630_ISP_CSI2C_REGS2_BASE	(OMAP3430_ISP_BASE + 0x1DC0)
 
 #define OMAP3430_ISP_END		(OMAP3430_ISP_BASE         + 0x06F)
 #define OMAP3430_ISP_CBUFF_END		(OMAP3430_ISP_CBUFF_BASE   + 0x077)
@@ -69,8 +73,12 @@
 #define OMAP3430_ISP_RESZ_END		(OMAP3430_ISP_RESZ_BASE    + 0x0AB)
 #define OMAP3430_ISP_SBL_END		(OMAP3430_ISP_SBL_BASE     + 0x0FB)
 #define OMAP3430_ISP_MMU_END		(OMAP3430_ISP_MMU_BASE     + 0x06F)
-#define OMAP3430_ISP_CSI2A_END		(OMAP3430_ISP_CSI2A_BASE   + 0x16F)
-#define OMAP3430_ISP_CSI2PHY_END	(OMAP3430_ISP_CSI2PHY_BASE + 0x007)
+#define OMAP3430_ISP_CSI2A_REGS1_END	(OMAP3430_ISP_CSI2A_REGS1_BASE + 0x16F)
+#define OMAP3430_ISP_CSIPHY2_END	(OMAP3430_ISP_CSIPHY2_BASE + 0x00B)
+#define OMAP3630_ISP_CSI2A_REGS2_END	(OMAP3630_ISP_CSI2A_REGS2_BASE + 0x3F)
+#define OMAP3630_ISP_CSI2C_REGS1_END	(OMAP3630_ISP_CSI2C_REGS1_BASE + 0x16F)
+#define OMAP3630_ISP_CSIPHY1_END	(OMAP3630_ISP_CSIPHY1_BASE + 0x00B)
+#define OMAP3630_ISP_CSI2C_REGS2_END	(OMAP3630_ISP_CSI2C_REGS2_BASE + 0x3F)
 
 #define OMAP34XX_HSUSB_OTG_BASE	(L4_34XX_BASE + 0xAB000)
 #define OMAP34XX_USBTLL_BASE	(L4_34XX_BASE + 0x62000)
-- 
1.7.1

[SAMPLE v3 10/12] omap3: Export omap3isp platform device structure

[Laurent Pinchart]

From: Stanimir Varbanov <svarbanov@mm-sol.com>

omap3isp platform device structure pointer is needed from camera board
files for subdevs registration and calls.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 arch/arm/mach-omap2/devices.c |    5 ++++-
 arch/arm/mach-omap2/devices.h |   17 +++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-omap2/devices.h

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 46b0b4b..ae465ce 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -32,6 +32,8 @@
 
 #include "mux.h"
 
+#include "devices.h"
+
 #if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE)
 
 static struct resource cam_resources[] = {
@@ -142,12 +144,13 @@ static struct resource omap3isp_resources[] = {
 	}
 };
 
-static struct platform_device omap3isp_device = {
+struct platform_device omap3isp_device = {
 	.name		= "omap3isp",
 	.id		= -1,
 	.num_resources	= ARRAY_SIZE(omap3isp_resources),
 	.resource	= omap3isp_resources,
 };
+EXPORT_SYMBOL_GPL(omap3isp_device);
 
 static inline void omap_init_camera(void)
 {
diff --git a/arch/arm/mach-omap2/devices.h b/arch/arm/mach-omap2/devices.h
new file mode 100644
index 0000000..f312d49
--- /dev/null
+++ b/arch/arm/mach-omap2/devices.h
@@ -0,0 +1,17 @@
+/*
+ * arch/arm/mach-omap2/devices.h
+ *
+ * OMAP2 platform device setup/initialization
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __ARCH_ARM_MACH_OMAP_DEVICES_H
+#define __ARCH_ARM_MACH_OMAP_DEVICES_H
+
+extern struct platform_device omap3isp_device;
+
+#endif
-- 
1.7.1

[SAMPLE v3 11/12] omap34xxcam: Register the ISP platform device during omap34xxcam probe

[Laurent Pinchart]

In order to properly clean up all resources allocated by the isp-mod
driver, the ISP platform device needs to be unregistered when the
omap34xxcam driver is unloaded.

Move the ISP platform device registration from omap_init_camera to
omap34xxcam_probe. This fixes many memory leaks when unloading and
reloading the omap34xxcam driver.

Platform device registration should be moved back to omap_init_camera
when (if) the omap34xxcam and isp-mod drivers will be merged.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 arch/arm/mach-omap2/devices.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index ae465ce..61e5136 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -144,17 +144,28 @@ static struct resource omap3isp_resources[] = {
 	}
 };
 
+static void omap3isp_release(struct device *dev)
+{
+	/* Zero the device structure to avoid re-initialization complaints from
+	 * kobject when the device will be re-registered.
+	 */
+	memset(dev, 0, sizeof(*dev));
+	dev->release = omap3isp_release;
+}
+
 struct platform_device omap3isp_device = {
 	.name		= "omap3isp",
 	.id		= -1,
 	.num_resources	= ARRAY_SIZE(omap3isp_resources),
 	.resource	= omap3isp_resources,
+	.dev = {
+		.release	= omap3isp_release,
+	},
 };
 EXPORT_SYMBOL_GPL(omap3isp_device);
 
 static inline void omap_init_camera(void)
 {
-	platform_device_register(&omap3isp_device);
 }
 #else
 static inline void omap_init_camera(void)
-- 
1.7.1

Re: [RFC/PATCH v3 03/10] media: Entities, pads and links

[Sakari Ailus]

Hi Laurent,

And thanks for the patch!

Laurent Pinchart wrote:
...
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index bd559b0..ac96847 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -23,8 +23,10 @@
>  
>  #include <linux/device.h>
>  #include <linux/list.h>
> +#include <linux/spinlock.h>
>  
>  #include <media/media-devnode.h>
> +#include <media/media-entity.h>
>  
>  /* Each instance of a media device should create the media_device struct,
>   * either stand-alone or embedded in a larger struct.
> @@ -40,9 +42,23 @@ struct media_device {
>  	 */
>  	struct device *dev;
>  	struct media_devnode devnode;
> +
> +	u32 entity_id;
> +	struct list_head entities;
> +
> +	/* Protects the entities list */
> +	spinlock_t lock;
>  };
>  
>  int __must_check media_device_register(struct media_device *mdev);
>  void media_device_unregister(struct media_device *mdev);
>  
> +int __must_check media_device_register_entity(struct media_device *mdev,
> +					      struct media_entity *entity);
> +void media_device_unregister_entity(struct media_entity *entity);
> +
> +/* Iterate over all entities. */
> +#define media_device_for_each_entity(entity, mdev)			\
> +	list_for_each_entry(entity, &(mdev)->entities, list)
> +
>  #endif
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> new file mode 100644
> index 0000000..37a25bf
> --- /dev/null
> +++ b/include/media/media-entity.h
> @@ -0,0 +1,85 @@
> +#ifndef _MEDIA_ENTITY_H
> +#define _MEDIA_ENTITY_H
> +
> +#include <linux/list.h>
> +
> +#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)

About that 16 there, could that be replaced by a #define instead? Like
MEDIA_ENTITY_TYPE_SHIFT? (I don't think we'd need
MEDIA_ENTITY_SUBTYPE_SHIFT.)

> +#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
> +#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
> +#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
> +#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
> +
> +#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV + 1)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3)
> +
> +#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
> +#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
> +
> +#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
> +#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
> +
> +struct media_link {
> +	struct media_pad *source;	/* Source pad */
> +	struct media_pad *sink;		/* Sink pad  */
> +	struct media_link *other;	/* Link in the reverse direction */
> +	unsigned long flags;		/* Link flags (MEDIA_LINK_FLAG_*) */
> +};
> +
> +struct media_pad {
> +	struct media_entity *entity;	/* Entity this pad belongs to */
> +	u16 index;			/* Pad index in the entity pads array */
> +	unsigned long flags;		/* Pad flags (MEDIA_PAD_FLAG_*) */
> +};
> +
> +struct media_entity {
> +	struct list_head list;
> +	struct media_device *parent;	/* Media device this entity belongs to*/
> +	u32 id;				/* Entity ID, unique in the parent media
> +					 * device context */
> +	const char *name;		/* Entity name */
> +	u32 type;			/* Entity type (MEDIA_ENTITY_TYPE_*) */
> +
> +	u8 num_pads;			/* Number of input and output pads */

Hans suggested u16 for pads. This is a kernel structure but still it'd
be good to keep them the same IMO, even if that u16 was there for the
future.

u8 is used on some function arguments as well for these purposes.

> +	u8 num_links;			/* Number of existing links, both active
> +					 * and inactive */
> +	u8 num_backlinks;		/* Number of backlinks */
> +	u8 max_links;			/* Maximum number of links */

Same for these.

> +	struct media_pad *pads;		/* Pads array (num_pads elements) */
> +	struct media_link *links;	/* Links array (max_links elements)*/
> +
> +	union {
> +		/* Node specifications */
> +		struct {
> +			u32 major;
> +			u32 minor;

How about dev_t here?

> +		} v4l;
> +		struct {
> +			u32 major;
> +			u32 minor;

And here.

> +		} fb;
> +		int alsa;
> +		int dvb;
> +
> +		/* Sub-device specifications */
> +		/* Nothing needed yet */
> +	};
> +};
> +
> +#define MEDIA_ENTITY_TYPE_MASK		0xffff0000
> +#define MEDIA_ENTITY_SUBTYPE_MASK	0x0000ffff
> +
> +#define media_entity_type(entity) \
> +	((entity)->type & MEDIA_ENTITY_TYPE_MASK)
> +#define media_entity_subtype(entity) \
> +	((entity)->type & MEDIA_ENTITY_SUBTYPE_MASK)
> +
> +int media_entity_init(struct media_entity *entity, u8 num_pads,
> +		struct media_pad *pads, u8 extra_links);
> +void media_entity_cleanup(struct media_entity *entity);
> +int media_entity_create_link(struct media_entity *source, u8 source_pad,
> +		struct media_entity *sink, u8 sink_pad, u32 flags);
> +
> +#endif

Regards,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [RFC/PATCH v3 01/10] media: Media device node support

[Hans Verkuil]

On Thursday 29 July 2010 18:06:34 Laurent Pinchart wrote:
> The media_devnode structure provides support for registering and
> unregistering character devices using a dynamic major number. Reference
> counting is handled internally, making device drivers easier to write
> without having to solve the open/disconnect race condition issue over
> and over again.
> 
> The code is based on video/v4l2-dev.c.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/Makefile        |    8 +-
>  drivers/media/media-devnode.c |  326 +++++++++++++++++++++++++++++++++++++++++
>  include/media/media-devnode.h |   84 +++++++++++
>  3 files changed, 416 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/media/media-devnode.c
>  create mode 100644 include/media/media-devnode.h
> 

<snip>

> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> new file mode 100644
> index 0000000..6f5558c
> --- /dev/null
> +++ b/drivers/media/media-devnode.c

<snip>

> +/* Override for the open function */
> +static int media_open(struct inode *inode, struct file *filp)
> +{
> +	struct media_devnode *mdev;
> +	int ret;
> +
> +	/* Check if the media device is available. This needs to be done with
> +	 * the media_devnode_lock held to prevent an open/unregister race:
> +	 * without the lock, the device could be unregistered and freed between
> +	 * the media_devnode_is_registered() and get_device() calls, leading to
> +	 * a crash.
> +	 */
> +	mutex_lock(&media_devnode_lock);
> +	mdev = container_of(inode->i_cdev, struct media_devnode, cdev);
> +	/* return ENODEV if the media device has been removed
> +	   already or if it is not registered anymore. */
> +	if (!media_devnode_is_registered(mdev)) {
> +		mutex_unlock(&media_devnode_lock);
> +		return -ENODEV;
> +	}
> +	/* and increase the device refcount */
> +	get_device(&mdev->dev);
> +	mutex_unlock(&media_devnode_lock);
> +	if (mdev->fops->open) {
> +		ret = mdev->fops->open(filp);
> +		if (ret) {
> +			put_device(&mdev->dev);
> +			return ret;
> +		}
> +	}
> +
> +	filp->private_data = mdev;

This line should be moved up to before the if: that way the open op can rely
on private_data being setup correctly.

> +	return 0;
> +}

<snip>

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

Re: [RFC/PATCH v3 03/10] media: Entities, pads and links

[Hans Verkuil]

On Thursday 29 July 2010 18:06:36 Laurent Pinchart wrote:

<snip>

> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index bd559b0..ac96847 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -23,8 +23,10 @@
>  
>  #include <linux/device.h>
>  #include <linux/list.h>
> +#include <linux/spinlock.h>
>  
>  #include <media/media-devnode.h>
> +#include <media/media-entity.h>
>  
>  /* Each instance of a media device should create the media_device struct,
>   * either stand-alone or embedded in a larger struct.
> @@ -40,9 +42,23 @@ struct media_device {
>  	 */
>  	struct device *dev;
>  	struct media_devnode devnode;
> +
> +	u32 entity_id;
> +	struct list_head entities;
> +
> +	/* Protects the entities list */
> +	spinlock_t lock;
>  };
>  
>  int __must_check media_device_register(struct media_device *mdev);
>  void media_device_unregister(struct media_device *mdev);
>  
> +int __must_check media_device_register_entity(struct media_device *mdev,
> +					      struct media_entity *entity);
> +void media_device_unregister_entity(struct media_entity *entity);
> +
> +/* Iterate over all entities. */
> +#define media_device_for_each_entity(entity, mdev)			\
> +	list_for_each_entry(entity, &(mdev)->entities, list)
> +
>  #endif
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> new file mode 100644
> index 0000000..37a25bf
> --- /dev/null
> +++ b/include/media/media-entity.h
> @@ -0,0 +1,85 @@
> +#ifndef _MEDIA_ENTITY_H
> +#define _MEDIA_ENTITY_H
> +
> +#include <linux/list.h>
> +
> +#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)

I agree with Sakari that '16' should be a define.

> +#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
> +#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
> +#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
> +#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
> +
> +#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV + 1)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3)
> +
> +#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
> +#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
> +
> +#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
> +#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
> +
> +struct media_link {
> +	struct media_pad *source;	/* Source pad */
> +	struct media_pad *sink;		/* Sink pad  */
> +	struct media_link *other;	/* Link in the reverse direction */

Why not rename 'other' to 'back' or 'backlink'? 'other' is not a good name IMHO.

> +	unsigned long flags;		/* Link flags (MEDIA_LINK_FLAG_*) */
> +};
> +
> +struct media_pad {
> +	struct media_entity *entity;	/* Entity this pad belongs to */
> +	u16 index;			/* Pad index in the entity pads array */
> +	unsigned long flags;		/* Pad flags (MEDIA_PAD_FLAG_*) */
> +};
> +
> +struct media_entity {
> +	struct list_head list;
> +	struct media_device *parent;	/* Media device this entity belongs to*/
> +	u32 id;				/* Entity ID, unique in the parent media
> +					 * device context */
> +	const char *name;		/* Entity name */
> +	u32 type;			/* Entity type (MEDIA_ENTITY_TYPE_*) */
> +
> +	u8 num_pads;			/* Number of input and output pads */
> +	u8 num_links;			/* Number of existing links, both active
> +					 * and inactive */
> +	u8 num_backlinks;		/* Number of backlinks */
> +	u8 max_links;			/* Maximum number of links */
> +
> +	struct media_pad *pads;		/* Pads array (num_pads elements) */
> +	struct media_link *links;	/* Links array (max_links elements)*/
> +
> +	union {
> +		/* Node specifications */
> +		struct {
> +			u32 major;
> +			u32 minor;
> +		} v4l;
> +		struct {
> +			u32 major;
> +			u32 minor;
> +		} fb;
> +		int alsa;
> +		int dvb;
> +
> +		/* Sub-device specifications */
> +		/* Nothing needed yet */
> +	};
> +};
> +
> +#define MEDIA_ENTITY_TYPE_MASK		0xffff0000
> +#define MEDIA_ENTITY_SUBTYPE_MASK	0x0000ffff
> +
> +#define media_entity_type(entity) \
> +	((entity)->type & MEDIA_ENTITY_TYPE_MASK)
> +#define media_entity_subtype(entity) \
> +	((entity)->type & MEDIA_ENTITY_SUBTYPE_MASK)

Make these inline functions to ensure correct typechecking.
Since bit 31 of the entity ID is reserved for MEDIA_ENTITY_ID_FLAG_NEXT you
probably should change the TYPE_MASK to 0x7fff0000. Actually, I would
change the type mask to 0x00ff0000, that way all top 8 bits are available
for the future.

> +
> +int media_entity_init(struct media_entity *entity, u8 num_pads,
> +		struct media_pad *pads, u8 extra_links);
> +void media_entity_cleanup(struct media_entity *entity);
> +int media_entity_create_link(struct media_entity *source, u8 source_pad,
> +		struct media_entity *sink, u8 sink_pad, u32 flags);
> +
> +#endif
> 

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Hans Verkuil]

On Thursday 29 July 2010 18:06:39 Laurent Pinchart wrote:
> Create the following two ioctls and implement them at the media device
> level to enumerate entities, pads and links.
> 
> - MEDIA_IOC_ENUM_ENTITIES: Enumerate entities and their properties
> - MEDIA_IOC_ENUM_LINKS: Enumerate all pads and links for a given entity
> 
> Entity IDs can be non-contiguous. Userspace applications should
> enumerate entities using the MEDIA_ENTITY_ID_FLAG_NEXT flag. When the
> flag is set in the entity ID, the MEDIA_IOC_ENUM_ENTITIES will return
> the next entity with an ID bigger than the requested one.
> 
> Only forward links that originate at one of the entity's source pads are
> returned during the enumeration process.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> ---
>  Documentation/media-framework.txt |  131 +++++++++++++++++++++++++++++++
>  drivers/media/media-device.c      |  152 +++++++++++++++++++++++++++++++++++++
>  include/linux/media.h             |   77 +++++++++++++++++++
>  include/media/media-device.h      |    3 +
>  include/media/media-entity.h      |   18 +----
>  5 files changed, 364 insertions(+), 17 deletions(-)
>  create mode 100644 include/linux/media.h
> 
> diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
> index 6d680c6..1192feb 100644
> --- a/Documentation/media-framework.txt
> +++ b/Documentation/media-framework.txt
> @@ -273,3 +273,134 @@ required, drivers don't need to provide a set_power operation. The operation
>  is allowed to fail when turning power on, in which case the media_entity_get
>  function will return NULL.
>  
> +
> +Userspace application API
> +-------------------------
> +
> +Media devices offer an API to userspace application to discover the device
> +internal topology through ioctls.
> +
> +	MEDIA_IOC_ENUM_ENTITIES - Enumerate entities and their properties
> +	-----------------------------------------------------------------
> +
> +	ioctl(int fd, int request, struct media_entity_desc *argp);
> +
> +To query the attributes of an entity, applications set the id field of a
> +media_entity_desc structure and call the MEDIA_IOC_ENUM_ENTITIES ioctl with a
> +pointer to this structure. The driver fills the rest of the structure or
> +returns a EINVAL error code when the id is invalid.
> +
> +Entities can be enumerated by or'ing the id with the MEDIA_ENTITY_ID_FLAG_NEXT
> +flag. The driver will return information about the entity with the smallest id
> +strictly larger than the requested one ('next entity'), or EINVAL if there is
> +none.
> +
> +Entity IDs can be non-contiguous. Applications must *not* try to enumerate
> +entities by calling MEDIA_IOC_ENUM_ENTITIES with increasing id's until they
> +get an error.
> +
> +The media_entity_desc structure is defined as
> +
> +- struct media_entity_desc
> +
> +__u32	id		Entity id, set by the application. When the id is
> +			or'ed with MEDIA_ENTITY_ID_FLAG_NEXT, the driver
> +			clears the flag and returns the first entity with a
> +			larger id.
> +char	name[32]	Entity name. UTF-8 NULL-terminated string.
> +__u32	type		Entity type.
> +__u8	pads		Number of pads.

Should be u16.

> +__u32	links		Total number of outbound links. Inbound links are not
> +			counted in this field.
> +/* union */
> +	/* struct v4l, Valid for V4L sub-devices and nodes only */
> +__u32	major		V4L device node major number. For V4L sub-devices with
> +			no device node, set by the driver to 0.
> +__u32	minor		V4L device node minor number. For V4L sub-devices with
> +			no device node, set by the driver to 0.
> +	/* struct fb, Valid for frame buffer nodes only */
> +__u32	major		FB device node major number
> +__u32	minor		FB device node minor number
> +	/* Valid for ALSA devices only */
> +int	alsa		ALSA card number
> +	/* Valid for DVB devices only */
> +int	dvb		DVB card number
> +
> +Valid entity types are
> +
> +	MEDIA_ENTITY_TYPE_NODE - Unknown device node
> +	MEDIA_ENTITY_TYPE_NODE_V4L - V4L video, radio or vbi device node
> +	MEDIA_ENTITY_TYPE_NODE_FB - Frame buffer device node
> +	MEDIA_ENTITY_TYPE_NODE_ALSA - ALSA card
> +	MEDIA_ENTITY_TYPE_NODE_DVB - DVB card
> +
> +	MEDIA_ENTITY_TYPE_SUBDEV - Unknown V4L sub-device
> +	MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER - Video decoder
> +	MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER - Video encoder
> +	MEDIA_ENTITY_TYPE_SUBDEV_MISC - Unspecified entity subtype
> +
> +
> +	MEDIA_IOC_ENUM_LINKS - Enumerate all pads and links for a given entity
> +	----------------------------------------------------------------------
> +
> +	ioctl(int fd, int request, struct media_links_enum *argp);
> +
> +Only forward links that originate at one of the entity's source pads are
> +returned during the enumeration process.
> +
> +To enumerate pads and/or links for a given entity, applications set the entity
> +field of a media_links_enum structure and initialize the media_pad_desc and
> +media_link_desc structure arrays pointed by the pads and links fields. They then
> +call the MEDIA_IOC_ENUM_LINKS ioctl with a pointer to this structure.
> +
> +If the pads field is not NULL, the driver fills the pads array with
> +information about the entity's pads. The array must have enough room to store
> +all the entity's pads. The number of pads can be retrieved with the
> +MEDIA_IOC_ENUM_ENTITIES ioctl.
> +
> +If the links field is not NULL, the driver fills the links array with
> +information about the entity's outbound links. The array must have enough room
> +to store all the entity's outbound links. The number of outbound links can be
> +retrieved with the MEDIA_IOC_ENUM_ENTITIES ioctl.
> +
> +The media_pad_desc, media_link_desc and media_links_enum structures are defined
> +as
> +
> +- struct media_pad_desc
> +
> +__u32		entity		ID of the entity this pad belongs to.
> +__u16		index		0-based pad index.
> +__u32		flags		Pad flags.
> +
> +Valid pad flags are
> +
> +	MEDIA_PAD_FLAG_INPUT -	Input pad, relative to the entity. Input pads
> +				sink data and are targets of links.
> +	MEDIA_PAD_FLAG_OUTPUT -	Output pad, relative to the entity. Output
> +				pads source data and are origins of links.
> +
> +One and only one of MEDIA_PAD_FLAG_INPUT and MEDIA_PAD_FLAG_OUTPUT must be set
> +for every pad.
> +
> +- struct media_link_desc
> +
> +struct media_pad_desc	source	Pad at the origin of this link.
> +struct media_pad_desc	sink	Pad at the target of this link.
> +__u32			flags	Link flags.
> +
> +Valid link flags are
> +
> +	MEDIA_LINK_FLAG_ACTIVE - The link is active and can be used to
> +		transfer media data. When two or more links target a sink pad,
> +		only one of them can be active at a time.
> +	MEDIA_LINK_FLAG_IMMUTABLE - The link active state can't be modified at
> +		runtime. An immutable link is always active.
> +
> +- struct media_links_enum
> +
> +__u32			entity	Entity id, set by the application.
> +struct media_pad_desc	*pads	Pointer to a pads array allocated by the
> +				application. Ignored if NULL.
> +struct media_link_desc	*links	Pointer to a links array allocated by the
> +				application. Ignored if NULL.
> +
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 6fb2e26..b32c308 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -20,13 +20,162 @@
>  
>  #include <linux/types.h>
>  #include <linux/ioctl.h>
> +#include <linux/media.h>
>  
>  #include <media/media-device.h>
>  #include <media/media-devnode.h>
>  #include <media/media-entity.h>
>  
> +static int media_device_open(struct file *filp)
> +{
> +	return 0;
> +}
> +
> +static int media_device_close(struct file *filp)
> +{
> +	return 0;
> +}
> +
> +static struct media_entity *find_entity(struct media_device *mdev, u32 id)
> +{
> +	struct media_entity *entity;
> +	int next = id & MEDIA_ENTITY_ID_FLAG_NEXT;
> +
> +	id &= ~MEDIA_ENTITY_ID_FLAG_NEXT;
> +
> +	spin_lock(&mdev->lock);
> +
> +	media_device_for_each_entity(entity, mdev) {
> +		if ((entity->id == id && !next) ||
> +		    (entity->id > id && next)) {
> +			spin_unlock(&mdev->lock);
> +			return entity;
> +		}
> +	}
> +
> +	spin_unlock(&mdev->lock);
> +
> +	return NULL;
> +}
> +
> +static long media_device_enum_entities(struct media_device *mdev,
> +				       struct media_entity_desc __user *uent)
> +{
> +	struct media_entity *ent;
> +	struct media_entity_desc u_ent;
> +
> +	if (copy_from_user(&u_ent.id, &uent->id, sizeof(u_ent.id)))
> +		return -EFAULT;
> +
> +	ent = find_entity(mdev, u_ent.id);
> +
> +	if (ent == NULL)
> +		return -EINVAL;
> +
> +	u_ent.id = ent->id;
> +	u_ent.name[0] = '\0';
> +	if (ent->name)
> +		strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
> +	u_ent.type = ent->type;
> +	u_ent.pads = ent->num_pads;
> +	u_ent.links = ent->num_links - ent->num_backlinks;
> +	u_ent.v4l.major = ent->v4l.major;
> +	u_ent.v4l.minor = ent->v4l.minor;
> +	if (copy_to_user(uent, &u_ent, sizeof(u_ent)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static void media_device_kpad_to_upad(const struct media_pad *kpad,
> +				      struct media_pad_desc *upad)
> +{
> +	upad->entity = kpad->entity->id;
> +	upad->index = kpad->index;
> +	upad->flags = kpad->flags;
> +}
> +
> +static long media_device_enum_links(struct media_device *mdev,
> +				    struct media_links_enum __user *ulinks)
> +{
> +	struct media_entity *entity;
> +	struct media_links_enum links;
> +
> +	if (copy_from_user(&links, ulinks, sizeof(links)))
> +		return -EFAULT;
> +
> +	entity = find_entity(mdev, links.entity);
> +	if (entity == NULL)
> +		return -EINVAL;
> +
> +	if (links.pads) {
> +		unsigned int p;
> +
> +		for (p = 0; p < entity->num_pads; p++) {
> +			struct media_pad_desc pad;
> +			media_device_kpad_to_upad(&entity->pads[p], &pad);
> +			if (copy_to_user(&links.pads[p], &pad, sizeof(pad)))
> +				return -EFAULT;
> +		}
> +	}
> +
> +	if (links.links) {
> +		struct media_link_desc __user *ulink;
> +		unsigned int l;
> +
> +		for (l = 0, ulink = links.links; l < entity->num_links; l++) {
> +			struct media_link_desc link;
> +
> +			/* Ignore backlinks. */
> +			if (entity->links[l].source->entity != entity)
> +				continue;
> +
> +			media_device_kpad_to_upad(entity->links[l].source,
> +						  &link.source);
> +			media_device_kpad_to_upad(entity->links[l].sink,
> +						  &link.sink);
> +			link.flags = entity->links[l].flags;
> +			if (copy_to_user(ulink, &link, sizeof(*ulink)))
> +				return -EFAULT;
> +			ulink++;
> +		}
> +	}
> +	if (copy_to_user(ulinks, &links, sizeof(*ulinks)))
> +		return -EFAULT;
> +	return 0;
> +}
> +
> +static long media_device_ioctl(struct file *filp, unsigned int cmd,
> +			       unsigned long arg)
> +{
> +	struct media_devnode *devnode = media_devnode_data(filp);
> +	struct media_device *dev = to_media_device(devnode);
> +	long ret;
> +
> +	switch (cmd) {
> +	case MEDIA_IOC_ENUM_ENTITIES:
> +		ret = media_device_enum_entities(dev,
> +				(struct media_entity_desc __user *)arg);
> +		break;
> +
> +	case MEDIA_IOC_ENUM_LINKS:
> +		mutex_lock(&dev->graph_mutex);
> +		ret = media_device_enum_links(dev,
> +				(struct media_links_enum __user *)arg);
> +		mutex_unlock(&dev->graph_mutex);
> +		break;
> +
> +	default:
> +		ret = -ENOIOCTLCMD;
> +	}
> +
> +	return ret;
> +}
> +
>  static const struct media_file_operations media_device_fops = {
>  	.owner = THIS_MODULE,
> +	.open = media_device_open,
> +	.unlocked_ioctl = media_device_ioctl,
> +	.release = media_device_close,
>  };
>  
>  static void media_device_release(struct media_devnode *mdev)
> @@ -99,6 +248,9 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  	WARN_ON(entity->parent != NULL);
>  	entity->parent = mdev;
>  
> +	/* find_entity() relies on entities being stored in increasing IDs
> +	 * order. Don't change that without modifying find_entity().
> +	 */
>  	spin_lock(&mdev->lock);
>  	entity->id = mdev->entity_id++;
>  	list_add_tail(&entity->list, &mdev->entities);
> diff --git a/include/linux/media.h b/include/linux/media.h
> new file mode 100644
> index 0000000..9b8acc0
> --- /dev/null
> +++ b/include/linux/media.h
> @@ -0,0 +1,77 @@
> +#ifndef __LINUX_MEDIA_H
> +#define __LINUX_MEDIA_H
> +
> +#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)
> +#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
> +#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
> +#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
> +#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
> +
> +#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV + 1)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
> +#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3)
> +
> +#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
> +#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
> +
> +#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
> +#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
> +
> +#define MEDIA_ENTITY_ID_FLAG_NEXT		(1 << 31)
> +
> +struct media_pad_desc {
> +	__u32 entity;		/* entity ID */
> +	__u16 index;		/* pad index */
> +	__u32 flags;		/* pad flags */
> +	__u32 reserved[2];
> +};
> +
> +struct media_entity_desc {
> +	__u32 id;
> +	char name[32];
> +	__u32 type;
> +	__u8 pads;

u16.

> +	__u32 links;
> +
> +	__u32 reserved[4];
> +
> +	union {
> +		/* Node specifications */
> +		struct {
> +			__u32 major;
> +			__u32 minor;
> +		} v4l;
> +		struct {
> +			__u32 major;
> +			__u32 minor;
> +		} fb;
> +		int alsa;
> +		int dvb;
> +
> +		/* Sub-device specifications */
> +		/* Nothing needed yet */
> +		__u8 raw[64];
> +	};
> +};

Would there be anything else that we want to describe with these pad_desc and
entity_desc structs?

For subdevs you want to return a chip ident and revision field (same as
VIDIOC_DBG_G_CHIP_IDENT does).

Should we allow (possibly optional) names for pads? Or 'tooltip'-type descriptions
that can be a lot longer than 32 chars? (Just brainstorming here).

I am of course thinking of apps where the user can setup the media flow using a
GUI. If the driver can provide more extensive descriptions of the various
entities/pads, then that would make it much easier for the user to experiment.

Note that I also think that obtaining such detailed information might be better
done through separate ioctls (e.g. MEDIA_IOC_G_PAD_INFO, etc.).

What is definitely missing and *must* be added is a QUERYCAP type ioctl that
provides driver/versioning info.

Another thing that we need to figure out is how to tell the application which
audio and video nodes belong together. Not only that, but we need to be able
to inform the driver how audio is hooked up: through an audio loopback cable,
an alsa device, part of an mpeg stream, or as a V4L2 audio device (ivtv can do
that, and I think pvrusb2 does the same for radio). I'm not entirely sure we
want to expose that last option as it is not really spec compliant.

Other things we may want to expose: is the video stream raw or compressed? What
are the default video/audio/vbi streams? (That allows an app to find the default
video device node if a driver has lots of them).

Some of this information should perhaps be exposed through the v4l2 API, but
other parts definitely belong here.

I've not thought about this in detail, but we need to set some time aside to
brainstorm on how to provide this information in a logical and consistent manner.

Regards,

	Hans

> +
> +struct media_link_desc {
> +	struct media_pad_desc source;
> +	struct media_pad_desc sink;
> +	__u32 flags;
> +	__u32 reserved[2];
> +};
> +
> +struct media_links_enum {
> +	__u32 entity;
> +	/* Should have enough room for pads elements */
> +	struct media_pad_desc __user *pads;
> +	/* Should have enough room for links elements */
> +	struct media_link_desc __user *links;
> +	__u32 reserved[4];
> +};
> +
> +#define MEDIA_IOC_ENUM_ENTITIES		_IOWR('M', 1, struct media_entity_desc)
> +#define MEDIA_IOC_ENUM_LINKS		_IOWR('M', 2, struct media_links_enum)
> +
> +#endif /* __LINUX_MEDIA_H */
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 39a437c..7b09332 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -53,6 +53,9 @@ struct media_device {
>  	struct mutex graph_mutex;
>  };
>  
> +/* media_devnode to media_device */
> +#define to_media_device(node) container_of(node, struct media_device, devnode)
> +
>  int __must_check media_device_register(struct media_device *mdev);
>  void media_device_unregister(struct media_device *mdev);
>  
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 2205cbc..fe8a650 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -2,23 +2,7 @@
>  #define _MEDIA_ENTITY_H
>  
>  #include <linux/list.h>
> -
> -#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)
> -#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
> -#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
> -#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
> -#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
> -
> -#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV + 1)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
> -#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3)
> -
> -#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
> -#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
> -
> -#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
> -#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
> +#include <linux/media.h>
>  
>  struct media_link {
>  	struct media_pad *source;	/* Source pad */
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

Re: [RFC/PATCH v3 01/10] media: Media device node support

[Laurent Pinchart]

Hi Hans,

On Sunday 01 August 2010 13:14:18 Hans Verkuil wrote:
> On Thursday 29 July 2010 18:06:34 Laurent Pinchart wrote:
> > The media_devnode structure provides support for registering and
> > unregistering character devices using a dynamic major number. Reference
> > counting is handled internally, making device drivers easier to write
> > without having to solve the open/disconnect race condition issue over
> > and over again.
> > 
> > The code is based on video/v4l2-dev.c.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/media/Makefile        |    8 +-
> >  drivers/media/media-devnode.c |  326
> >  +++++++++++++++++++++++++++++++++++++++++ include/media/media-devnode.h
> >  |   84 +++++++++++
> >  3 files changed, 416 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/media/media-devnode.c
> >  create mode 100644 include/media/media-devnode.h
> 
> <snip>
> 
> > diff --git a/drivers/media/media-devnode.c
> > b/drivers/media/media-devnode.c new file mode 100644
> > index 0000000..6f5558c
> > --- /dev/null
> > +++ b/drivers/media/media-devnode.c
> 
> <snip>
> 
> > +/* Override for the open function */
> > +static int media_open(struct inode *inode, struct file *filp)
> > +{
> > +	struct media_devnode *mdev;
> > +	int ret;
> > +
> > +	/* Check if the media device is available. This needs to be done with
> > +	 * the media_devnode_lock held to prevent an open/unregister race:
> > +	 * without the lock, the device could be unregistered and freed between
> > +	 * the media_devnode_is_registered() and get_device() calls, leading to
> > +	 * a crash.
> > +	 */
> > +	mutex_lock(&media_devnode_lock);
> > +	mdev = container_of(inode->i_cdev, struct media_devnode, cdev);
> > +	/* return ENODEV if the media device has been removed
> > +	   already or if it is not registered anymore. */
> > +	if (!media_devnode_is_registered(mdev)) {
> > +		mutex_unlock(&media_devnode_lock);
> > +		return -ENODEV;
> > +	}
> > +	/* and increase the device refcount */
> > +	get_device(&mdev->dev);
> > +	mutex_unlock(&media_devnode_lock);
> > +	if (mdev->fops->open) {
> > +		ret = mdev->fops->open(filp);
> > +		if (ret) {
> > +			put_device(&mdev->dev);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	filp->private_data = mdev;
> 
> This line should be moved up to before the if: that way the open op can
> rely on private_data being setup correctly.

Agreed, I'll fix that.

> > +	return 0;
> > +}

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 03/10] media: Entities, pads and links

[Laurent Pinchart]

Hi Sakari,

On Friday 30 July 2010 16:00:31 Sakari Ailus wrote:
> Laurent Pinchart wrote:

[snip]

> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > new file mode 100644
> > index 0000000..37a25bf
> > --- /dev/null
> > +++ b/include/media/media-entity.h
> > @@ -0,0 +1,85 @@
> > +#ifndef _MEDIA_ENTITY_H
> > +#define _MEDIA_ENTITY_H
> > +
> > +#include <linux/list.h>
> > +
> > +#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)
> 
> About that 16 there, could that be replaced by a #define instead? Like
> MEDIA_ENTITY_TYPE_SHIFT? (I don't think we'd need
> MEDIA_ENTITY_SUBTYPE_SHIFT.)

Agreed.

> > +#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
> > +#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
> > +#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
> > +#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
> > +
> > +#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
> > +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV +
> > 1) +#define
> > MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV + 2)
> > +#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3) 
+
> > +#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
> > +#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
> > +
> > +#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
> > +#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
> > +
> > +struct media_link {
> > +	struct media_pad *source;	/* Source pad */
> > +	struct media_pad *sink;		/* Sink pad  */
> > +	struct media_link *other;	/* Link in the reverse direction */
> > +	unsigned long flags;		/* Link flags (MEDIA_LINK_FLAG_*) */
> > +};
> > +
> > +struct media_pad {
> > +	struct media_entity *entity;	/* Entity this pad belongs to */
> > +	u16 index;			/* Pad index in the entity pads array */
> > +	unsigned long flags;		/* Pad flags (MEDIA_PAD_FLAG_*) */
> > +};
> > +
> > +struct media_entity {
> > +	struct list_head list;
> > +	struct media_device *parent;	/* Media device this entity belongs to*/
> > +	u32 id;				/* Entity ID, unique in the parent media
> > +					 * device context */
> > +	const char *name;		/* Entity name */
> > +	u32 type;			/* Entity type (MEDIA_ENTITY_TYPE_*) */
> > +
> > +	u8 num_pads;			/* Number of input and output pads */
> 
> Hans suggested u16 for pads. This is a kernel structure but still it'd
> be good to keep them the same IMO, even if that u16 was there for the
> future.
> 
> u8 is used on some function arguments as well for these purposes.

Will fix.

> > +	u8 num_links;			/* Number of existing links, both active
> > +					 * and inactive */
> > +	u8 num_backlinks;		/* Number of backlinks */
> > +	u8 max_links;			/* Maximum number of links */
> 
> Same for these.

Agreed. The number of links is expected to be >= number of pads. I'll change 
num_links, num_backlinks and max_links to u16 as well.

> > +	struct media_pad *pads;		/* Pads array (num_pads elements) */
> > +	struct media_link *links;	/* Links array (max_links elements)*/
> > +
> > +	union {
> > +		/* Node specifications */
> > +		struct {
> > +			u32 major;
> > +			u32 minor;
> 
> How about dev_t here?

dev_t doesn't seem to be a public kernel type.

> > +		} v4l;
> > +		struct {
> > +			u32 major;
> > +			u32 minor;
> 
> And here.
> 
> > +		} fb;
> > +		int alsa;
> > +		int dvb;
> > +
> > +		/* Sub-device specifications */
> > +		/* Nothing needed yet */
> > +	};
> > +};

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 03/10] media: Entities, pads and links

[Laurent Pinchart]

Hi Hans,

On Sunday 01 August 2010 13:32:50 Hans Verkuil wrote:
> On Thursday 29 July 2010 18:06:36 Laurent Pinchart wrote:

<snip>

> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > new file mode 100644
> > index 0000000..37a25bf
> > --- /dev/null
> > +++ b/include/media/media-entity.h
> > @@ -0,0 +1,85 @@
> > +#ifndef _MEDIA_ENTITY_H
> > +#define _MEDIA_ENTITY_H
> > +
> > +#include <linux/list.h>
> > +
> > +#define MEDIA_ENTITY_TYPE_NODE			(1 << 16)
> 
> I agree with Sakari that '16' should be a define.

Will do.

> > +#define MEDIA_ENTITY_TYPE_NODE_V4L		(MEDIA_ENTITY_TYPE_NODE + 1)
> > +#define MEDIA_ENTITY_TYPE_NODE_FB		(MEDIA_ENTITY_TYPE_NODE + 2)
> > +#define MEDIA_ENTITY_TYPE_NODE_ALSA		(MEDIA_ENTITY_TYPE_NODE + 3)
> > +#define MEDIA_ENTITY_TYPE_NODE_DVB		(MEDIA_ENTITY_TYPE_NODE + 4)
> > +
> > +#define MEDIA_ENTITY_TYPE_SUBDEV		(2 << 16)
> > +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER	(MEDIA_ENTITY_TYPE_SUBDEV +
> > 1)
> > +#define MEDIA_ENTITY_TYPE_SUBDEV_VID_ENCODER	(MEDIA_ENTITY_TYPE_SUBDEV +
> > 2)
> > +#define MEDIA_ENTITY_TYPE_SUBDEV_MISC		(MEDIA_ENTITY_TYPE_SUBDEV + 3) 
+
> > +#define MEDIA_LINK_FLAG_ACTIVE			(1 << 0)
> > +#define MEDIA_LINK_FLAG_IMMUTABLE		(1 << 1)
> > +
> > +#define MEDIA_PAD_FLAG_INPUT			(1 << 0)
> > +#define MEDIA_PAD_FLAG_OUTPUT			(1 << 1)
> > +
> > +struct media_link {
> > +	struct media_pad *source;	/* Source pad */
> > +	struct media_pad *sink;		/* Sink pad  */
> > +	struct media_link *other;	/* Link in the reverse direction */
> 
> Why not rename 'other' to 'back' or 'backlink'? 'other' is not a good name
> IMHO.

For forward links other is the backlink, but for backlinks other is the 
forward link. I'll rename it to reverse.

> > +	unsigned long flags;		/* Link flags (MEDIA_LINK_FLAG_*) */
> > +};
> > +
> > +struct media_pad {
> > +	struct media_entity *entity;	/* Entity this pad belongs to */
> > +	u16 index;			/* Pad index in the entity pads array */
> > +	unsigned long flags;		/* Pad flags (MEDIA_PAD_FLAG_*) */
> > +};
> > +
> > +struct media_entity {
> > +	struct list_head list;
> > +	struct media_device *parent;	/* Media device this entity belongs to*/
> > +	u32 id;				/* Entity ID, unique in the parent media
> > +					 * device context */
> > +	const char *name;		/* Entity name */
> > +	u32 type;			/* Entity type (MEDIA_ENTITY_TYPE_*) */
> > +
> > +	u8 num_pads;			/* Number of input and output pads */
> > +	u8 num_links;			/* Number of existing links, both active
> > +					 * and inactive */
> > +	u8 num_backlinks;		/* Number of backlinks */
> > +	u8 max_links;			/* Maximum number of links */
> > +
> > +	struct media_pad *pads;		/* Pads array (num_pads elements) */
> > +	struct media_link *links;	/* Links array (max_links elements)*/
> > +
> > +	union {
> > +		/* Node specifications */
> > +		struct {
> > +			u32 major;
> > +			u32 minor;
> > +		} v4l;
> > +		struct {
> > +			u32 major;
> > +			u32 minor;
> > +		} fb;
> > +		int alsa;
> > +		int dvb;
> > +
> > +		/* Sub-device specifications */
> > +		/* Nothing needed yet */
> > +	};
> > +};
> > +
> > +#define MEDIA_ENTITY_TYPE_MASK		0xffff0000
> > +#define MEDIA_ENTITY_SUBTYPE_MASK	0x0000ffff
> > +
> > +#define media_entity_type(entity) \
> > +	((entity)->type & MEDIA_ENTITY_TYPE_MASK)
> > +#define media_entity_subtype(entity) \
> > +	((entity)->type & MEDIA_ENTITY_SUBTYPE_MASK)
> 
> Make these inline functions to ensure correct typechecking.

OK.

> Since bit 31 of the entity ID is reserved for MEDIA_ENTITY_ID_FLAG_NEXT you
> probably should change the TYPE_MASK to 0x7fff0000. Actually, I would
> change the type mask to 0x00ff0000, that way all top 8 bits are available
> for the future.

Agreed.

> > +
> > +int media_entity_init(struct media_entity *entity, u8 num_pads,
> > +		struct media_pad *pads, u8 extra_links);
> > +void media_entity_cleanup(struct media_entity *entity);
> > +int media_entity_create_link(struct media_entity *source, u8 source_pad,
> > +		struct media_entity *sink, u8 sink_pad, u32 flags);
> > +
> > +#endif

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Laurent Pinchart]

Hi Hans,

On Sunday 01 August 2010 13:58:20 Hans Verkuil wrote:
> On Thursday 29 July 2010 18:06:39 Laurent Pinchart wrote:

[snip]

> > diff --git a/Documentation/media-framework.txt
> > b/Documentation/media-framework.txt index 6d680c6..1192feb 100644
> > --- a/Documentation/media-framework.txt
> > +++ b/Documentation/media-framework.txt
> > @@ -273,3 +273,134 @@ required, drivers don't need to provide a set_power

[snip]

> > +- struct media_entity_desc
> > +
> > +__u32	id		Entity id, set by the application. When the id is
> > +			or'ed with MEDIA_ENTITY_ID_FLAG_NEXT, the driver
> > +			clears the flag and returns the first entity with a
> > +			larger id.
> > +char	name[32]	Entity name. UTF-8 NULL-terminated string.
> > +__u32	type		Entity type.
> > +__u8	pads		Number of pads.
> 
> Should be u16.

Thanks. Will fix.

[snip]

> > diff --git a/include/linux/media.h b/include/linux/media.h
> > new file mode 100644
> > index 0000000..9b8acc0
> > --- /dev/null
> > +++ b/include/linux/media.h
> > @@ -0,0 +1,77 @@

[snip]

> > +struct media_entity_desc {
> > +	__u32 id;
> > +	char name[32];
> > +	__u32 type;
> > +	__u8 pads;
> 
> u16.

Thanks. Will fix.

[snip]

> 
> > +	__u32 links;
> > +
> > +	__u32 reserved[4];
> > +
> > +	union {
> > +		/* Node specifications */
> > +		struct {
> > +			__u32 major;
> > +			__u32 minor;
> > +		} v4l;
> > +		struct {
> > +			__u32 major;
> > +			__u32 minor;
> > +		} fb;
> > +		int alsa;
> > +		int dvb;
> > +
> > +		/* Sub-device specifications */
> > +		/* Nothing needed yet */
> > +		__u8 raw[64];
> > +	};
> > +};
> 
> Would there be anything else that we want to describe with these pad_desc
> and entity_desc structs?

Definitely. Thanks for reminding me :-)

> For subdevs you want to return a chip ident and revision field (same as
> VIDIOC_DBG_G_CHIP_IDENT does).

Do we still need a chip ID when we now have a name ? Keeping the chip ID 
registry updated is painful, it would be nice if we could do away with it.

A revision field is a very good idea, I'll add it.

> Should we allow (possibly optional) names for pads? Or 'tooltip'-type
> descriptions that can be a lot longer than 32 chars? (Just brainstorming
> here).
>
> I am of course thinking of apps where the user can setup the media flow
> using a GUI. If the driver can provide more extensive descriptions of the
> various entities/pads, then that would make it much easier for the user to
> experiment.

It would be nice to have, yes. Some kind of pad capabilities would be 
interesting too.

> Note that I also think that obtaining such detailed information might be
> better done through separate ioctls (e.g. MEDIA_IOC_G_PAD_INFO, etc.).

I agree. So we can leave the additional pad information out for now and add it 
later if needed :-)
 
> What is definitely missing and *must* be added is a QUERYCAP type ioctl
> that provides driver/versioning info.

I'll create one.

> Another thing that we need to figure out is how to tell the application
> which audio and video nodes belong together.

What about adding a group ID field in media_entity ?

> Not only that, but we need to be able to inform the driver how audio is
> hooked up: through an audio loopback cable, an alsa device,

Doesn't the loopback cable connect the audio signal to audio hardware that 
exposes an ALSA device ? How will drivers be able to tell if the user has 
connected a loopback cable and what he has connected it to ?

> part of an mpeg stream,

In that case there will be no audio device.

> or as a V4L2 audio device (ivtv can do that, and I think pvrusb2 does the
> same for radio). I'm not entirely sure we want to expose that last option as
> it is not really spec compliant.

I'm not sure either :-) Why doesn't ivtv use an ALSA device ?

> Other things we may want to expose: is the video stream raw or compressed?

I think that belongs to V4L2.

> What are the default video/audio/vbi streams? (That allows an app to find
> the default video device node if a driver has lots of them).

What about adding a __u32 flags field to media_entity, and defining a 
MEDIA_ENTITY_FLAG_DEFAULT bit ?

> Some of this information should perhaps be exposed through the v4l2 API,
> but other parts definitely belong here.
> 
> I've not thought about this in detail, but we need to set some time aside
> to brainstorm on how to provide this information in a logical and
> consistent manner.

IRC ? A real meeting would be better, but the next scheduled one is in 
November and that's a bit too far away.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Hans Verkuil]

On Monday 02 August 2010 16:35:54 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Sunday 01 August 2010 13:58:20 Hans Verkuil wrote:
> > On Thursday 29 July 2010 18:06:39 Laurent Pinchart wrote:
> 
> [snip]

> > For subdevs you want to return a chip ident and revision field (same as
> > VIDIOC_DBG_G_CHIP_IDENT does).
> 
> Do we still need a chip ID when we now have a name ? Keeping the chip ID 
> registry updated is painful, it would be nice if we could do away with it.

It's not that easy I think. The name of the subdev entity is set by the host
(bridge) driver and that doesn't know the exact chip ID. E.g. the audio msp3400
subdev driver actually supports dozens of msp variants. Knowing which variant
it is may actually be important information on an embedded system.

Note also that the chip ID is optional: it is only required when using the
debug ioctls to get/set registers and to obtain the chip ident.
 
> A revision field is a very good idea, I'll add it.
> 
> > Should we allow (possibly optional) names for pads? Or 'tooltip'-type
> > descriptions that can be a lot longer than 32 chars? (Just brainstorming
> > here).
> >
> > I am of course thinking of apps where the user can setup the media flow
> > using a GUI. If the driver can provide more extensive descriptions of the
> > various entities/pads, then that would make it much easier for the user to
> > experiment.
> 
> It would be nice to have, yes. Some kind of pad capabilities would be 
> interesting too.

What sort of caps do you have in mind?
 
> > Note that I also think that obtaining such detailed information might be
> > better done through separate ioctls (e.g. MEDIA_IOC_G_PAD_INFO, etc.).
> 
> I agree. So we can leave the additional pad information out for now and add it 
> later if needed :-)
>  
> > What is definitely missing and *must* be added is a QUERYCAP type ioctl
> > that provides driver/versioning info.
> 
> I'll create one.
> 
> > Another thing that we need to figure out is how to tell the application
> > which audio and video nodes belong together.
> 
> What about adding a group ID field in media_entity ?

It's a possibility, but it's always a bit of a hassle in an application to
work with group IDs. I wonder if there is a more elegant method.
 
> > Not only that, but we need to be able to inform the driver how audio is
> > hooked up: through an audio loopback cable, an alsa device,
> 
> Doesn't the loopback cable connect the audio signal to audio hardware that 
> exposes an ALSA device ? How will drivers be able to tell if the user has 
> connected a loopback cable and what he has connected it to ?

If the audio is passed out from the card through a loopback cable, then the
application knows that it has to ask the user for the alsa device. It is
obviously impossible to know in advance which alsa device the card is hooked up
to.
 
> > part of an mpeg stream,
> 
> In that case there will be no audio device.
> 
> > or as a V4L2 audio device (ivtv can do that, and I think pvrusb2 does the
> > same for radio). I'm not entirely sure we want to expose that last option as
> > it is not really spec compliant.
> 
> I'm not sure either :-) Why doesn't ivtv use an ALSA device ?

With earlier firmwares the audio was out of sync with the video so you needed
timestamps to sync them up. And alsa didn't (and doesn't) support this.
 
> > Other things we may want to expose: is the video stream raw or compressed?
> 
> I think that belongs to V4L2.

V4L2 has something along these lines: VIDIOC_ENUM_FMT can set a COMPRESSED flag
when enumerating formats. After thinking about this a bit more I've come to the
conclusion that for now at least we should keep this in V4L2.
 
> > What are the default video/audio/vbi streams? (That allows an app to find
> > the default video device node if a driver has lots of them).
> 
> What about adding a __u32 flags field to media_entity, and defining a 
> MEDIA_ENTITY_FLAG_DEFAULT bit ?

I had the same idea. Not just useful for alsa, v4l and dvb nodes, but also for
IR inputs. We should look at IR as well: it is nice if you can easily discover
the IR input associated with the board.
 
> > Some of this information should perhaps be exposed through the v4l2 API,
> > but other parts definitely belong here.
> > 
> > I've not thought about this in detail, but we need to set some time aside
> > to brainstorm on how to provide this information in a logical and
> > consistent manner.
> 
> IRC ? A real meeting would be better, but the next scheduled one is in 
> November and that's a bit too far away.

If there is enough to discuss, then an interim meeting might well be useful.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco

Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Laurent Pinchart]

Hi Hans,

On Monday 02 August 2010 23:01:55 Hans Verkuil wrote:
> On Monday 02 August 2010 16:35:54 Laurent Pinchart wrote:
> > On Sunday 01 August 2010 13:58:20 Hans Verkuil wrote:
> > > On Thursday 29 July 2010 18:06:39 Laurent Pinchart wrote:
> > [snip]
> > 
> > > For subdevs you want to return a chip ident and revision field (same as
> > > VIDIOC_DBG_G_CHIP_IDENT does).
> > 
> > Do we still need a chip ID when we now have a name ? Keeping the chip ID
> > registry updated is painful, it would be nice if we could do away with
> > it.
> 
> It's not that easy I think. The name of the subdev entity is set by the
> host (bridge) driver

Is it ? It should be set by the subdev driver.

> and that doesn't know the exact chip ID. E.g. the audio msp3400 subdev
> driver actually supports dozens of msp variants. Knowing which variant it is
> may actually be important information on an embedded system.
> 
> Note also that the chip ID is optional: it is only required when using the
> debug ioctls to get/set registers and to obtain the chip ident.
> 
> > A revision field is a very good idea, I'll add it.
> > 
> > > Should we allow (possibly optional) names for pads? Or 'tooltip'-type
> > > descriptions that can be a lot longer than 32 chars? (Just
> > > brainstorming here).
> > > 
> > > I am of course thinking of apps where the user can setup the media flow
> > > using a GUI. If the driver can provide more extensive descriptions of
> > > the various entities/pads, then that would make it much easier for the
> > > user to experiment.
> > 
> > It would be nice to have, yes. Some kind of pad capabilities would be
> > interesting too.
> 
> What sort of caps do you have in mind?

I'm not sure yet. Maybe the kind of media streams the pad supports, things 
like that.

> > > Note that I also think that obtaining such detailed information might
> > > be better done through separate ioctls (e.g. MEDIA_IOC_G_PAD_INFO,
> > > etc.).
> > 
> > I agree. So we can leave the additional pad information out for now and
> > add it later if needed :-)
> > 
> > > What is definitely missing and *must* be added is a QUERYCAP type ioctl
> > > that provides driver/versioning info.
> > 
> > I'll create one.
> > 
> > > Another thing that we need to figure out is how to tell the application
> > > which audio and video nodes belong together.
> > 
> > What about adding a group ID field in media_entity ?
> 
> It's a possibility, but it's always a bit of a hassle in an application to
> work with group IDs. I wonder if there is a more elegant method.

The problem is a bit broader than just showing relationships between video 
nodes and ALSA devices. We also need to show relationships between lens/flash 
controllers and sensors for instance. Group IDs sound easy, but I'm open to 
suggestions.

> > > Not only that, but we need to be able to inform the driver how audio is
> > > hooked up: through an audio loopback cable, an alsa device,
> > 
> > Doesn't the loopback cable connect the audio signal to audio hardware
> > that exposes an ALSA device ? How will drivers be able to tell if the
> > user has connected a loopback cable and what he has connected it to ?
> 
> If the audio is passed out from the card through a loopback cable, then the
> application knows that it has to ask the user for the alsa device. It is
> obviously impossible to know in advance which alsa device the card is
> hooked up to.
> 
> > > part of an mpeg stream,
> > 
> > In that case there will be no audio device.
> > 
> > > or as a V4L2 audio device (ivtv can do that, and I think pvrusb2 does
> > > the same for radio). I'm not entirely sure we want to expose that last
> > > option as it is not really spec compliant.
> > 
> > I'm not sure either :-) Why doesn't ivtv use an ALSA device ?
> 
> With earlier firmwares the audio was out of sync with the video so you
> needed timestamps to sync them up. And alsa didn't (and doesn't) support
> this.

Still not ? :-S

> > > Other things we may want to expose: is the video stream raw or
> > > compressed?
> > 
> > I think that belongs to V4L2.
> 
> V4L2 has something along these lines: VIDIOC_ENUM_FMT can set a COMPRESSED
> flag when enumerating formats. After thinking about this a bit more I've
> come to the conclusion that for now at least we should keep this in V4L2.

OK.

> > > What are the default video/audio/vbi streams? (That allows an app to
> > > find the default video device node if a driver has lots of them).
> > 
> > What about adding a __u32 flags field to media_entity, and defining a
> > MEDIA_ENTITY_FLAG_DEFAULT bit ?
> 
> I had the same idea. Not just useful for alsa, v4l and dvb nodes, but also
> for IR inputs. We should look at IR as well: it is nice if you can easily
> discover the IR input associated with the board.

So a default flag sounds good.

> > > Some of this information should perhaps be exposed through the v4l2
> > > API, but other parts definitely belong here.
> > > 
> > > I've not thought about this in detail, but we need to set some time
> > > aside to brainstorm on how to provide this information in a logical
> > > and consistent manner.
> > 
> > IRC ? A real meeting would be better, but the next scheduled one is in
> > November and that's a bit too far away.
> 
> If there is enough to discuss, then an interim meeting might well be
> useful.

Anther crazy idea: reporting physical location information for subdevs, such 
as "this sensor is located on the back of the case" or "this sensor faces the 
user".

Some brainstorming is indeed needed.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Marko Ristola]

Hi Hans and Laurent.

I hope my thoughts help you further.

03.08.2010 12:22, Laurent Pinchart wrote:
> Hi Hans,
>
> On Monday 02 August 2010 23:01:55 Hans Verkuil wrote:
>> On Monday 02 August 2010 16:35:54 Laurent Pinchart wrote:
>>> On Sunday 01 August 2010 13:58:20 Hans Verkuil wrote:
>>>> On Thursday 29 July 2010 18:06:39 Laurent Pinchart wrote:
>>> [snip]
>>>
[snip]
>> It's a possibility, but it's always a bit of a hassle in an application to
>> work with group IDs. I wonder if there is a more elegant method.
> The problem is a bit broader than just showing relationships between video
> nodes and ALSA devices. We also need to show relationships between lens/flash
> controllers and sensors for instance. Group IDs sound easy, but I'm open to
> suggestions.

Low level example

DVB I2C bus is easy: get all I2C devices from an entity (DVB demuxer).
Some external chip (entity, the tuner) might be behind some I2C bridge 
device.

With I2C you need to know the characteristics, how you talk with
the destination device via the bus (extra sleeps, clock speed,
quiesce the whole bus for 50ms after talking to the slave device).
I'd like that each device would describe how it should be
talked to via the bus.

On i2c_transfer you could hide opening and closing the I2C bridge, and hide
the callbacks for extra sleeps so that the main driver and core 
framework code is free from such ugly details.
By storing entity's special requirements inside of it, you could reuse 
the callbacks with another product variant.

With I2C, an array of I2C slave devices that are reachable via I2C bus 
would work for controlling the device
rather nicely.

Higher abstraction level

So detailed descriptions and bus knowledge is needed for controlling 
each entity and pad.
That hierarchy is a bit different than optimal hierarchy of how the 
streams can flow
into, within and out from the entity (the driver). Buses are the 
gateways for the data stream flows,
shared by two or more entities/pads by links.

Thus I'd suggest to separate these two hierarchies (initialization time 
hierarchy and
stream flow capability hierarchy) at necessary points, and use buses
to bind the entities/pads by links to each other.

A single wire with just two end points can also be thought like a bus.

Regards,
Marko Ristola

[ snip ]

Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Laurent Pinchart]

Hi Marko,

On Wednesday 04 August 2010 21:28:22 Marko Ristola wrote:
> Hi Hans and Laurent.
> 
> I hope my thoughts help you further.

Thank you for sharing them.

> 03.08.2010 12:22, Laurent Pinchart wrote:
> > On Monday 02 August 2010 23:01:55 Hans Verkuil wrote:
> >> On Monday 02 August 2010 16:35:54 Laurent Pinchart wrote:
> >>> On Sunday 01 August 2010 13:58:20 Hans Verkuil wrote:
> >>>> On Thursday 29 July 2010 18:06:39 Laurent Pinchart wrote:
> >>> [snip]
> 
> [snip]
> 
> >> It's a possibility, but it's always a bit of a hassle in an application
> >> to work with group IDs. I wonder if there is a more elegant method.
> > 
> > The problem is a bit broader than just showing relationships between
> > video nodes and ALSA devices. We also need to show relationships between
> > lens/flash controllers and sensors for instance. Group IDs sound easy,
> > but I'm open to suggestions.
> 
> Low level example
> 
> DVB I2C bus is easy: get all I2C devices from an entity (DVB demuxer).
> Some external chip (entity, the tuner) might be behind some I2C bridge
> device.
> 
> With I2C you need to know the characteristics, how you talk with
> the destination device via the bus (extra sleeps, clock speed,
> quiesce the whole bus for 50ms after talking to the slave device).
> I'd like that each device would describe how it should be
> talked to via the bus.

There's probably some confusion here.

The media controller aims at giving userspace the ability to discover the 
internal device topology. This includes various information about the 
entities, such as a name, a version number, ...

The I2C data you mention are low-level information required by the kernel to 
talk to the I2C device, but I don't think they're useful to userspace at all. 
That kind of information come either from platform data or from the I2C device 
driver and get used internally in the kernel.

Do you see any need to expose such information to userspace ?

> On i2c_transfer you could hide opening and closing the I2C bridge, and hide
> the callbacks for extra sleeps so that the main driver and core framework
> code is free from such ugly details. By storing entity's special
> requirements inside of it, you could reuse the callbacks with another
> product variant.

The callbacks are implemented by I2C device drivers that should know about the 
specific device requirements (either hardcoded in the driver, or provided 
through platform data). I'm not sure to understand the exact problem here.

> With I2C, an array of I2C slave devices that are reachable via I2C bus
> would work for controlling the device rather nicely.
> 
> Higher abstraction level
> 
> So detailed descriptions and bus knowledge is needed for controlling each
> entity and pad.

It's needed to control them inside the kernel, but I don't think it's needed 
in userspace.

> That hierarchy is a bit different than optimal hierarchy of how the streams
> can flow into, within and out from the entity (the driver). Buses are the
> gateways for the data stream flows, shared by two or more entities/pads by
> links.
> 
> Thus I'd suggest to separate these two hierarchies (initialization time
> hierarchy and stream flow capability hierarchy) at necessary points, and use
> buses to bind the entities/pads by links to each other.

I don't get it, sorry.

> A single wire with just two end points can also be thought like a bus.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v3 06/10] media: Entities, pads and links enumeration

[Marko Ristola]

  05.08.2010 12:38, Laurent Pinchart wrote:
> Hi Marko,
>
> On Wednesday 04 August 2010 21:28:22 Marko Ristola wrote:
>> Hi Hans and Laurent.
>>
>> I hope my thoughts help you further.
> Thank you for sharing them.
Thank you for taking time to answer for me.

[ snip ]

> There's probably some confusion here.
>
> The media controller aims at giving userspace the ability to discover the
> internal device topology. This includes various information about the
> entities, such as a name, a version number, ...
>
> The I2C data you mention are low-level information required by the kernel to
> talk to the I2C device, but I don't think they're useful to userspace at all.
> That kind of information come either from platform data or from the I2C device
> driver and get used internally in the kernel.
>
> Do you see any need to expose such information to userspace ?
>
I don't see any needs to expose such information into userspace.
I have some thoughts about I2C hardware problems,
but that is of course off topic. I wrote them here only
because I tried to draw some bigger picture how this entity, pads
and links thing relates to the whole driver.

[ snip ]

>> With I2C, an array of I2C slave devices that are reachable via I2C bus
>> would work for controlling the device rather nicely.
>>
>> Higher abstraction level
>>
>> So detailed descriptions and bus knowledge is needed for controlling each
>> entity and pad.
> It's needed to control them inside the kernel, but I don't think it's needed
> in userspace.
>
I agree: The detailed information needs to be internal to the driver,
and userspace needs only the information that is needed for being
able to decide how to configure the streams over many drivers,
and how each driver must be asked to do the configuration for it,
without driver needing knowledge about other related drivers.

Maybe a pad could have a list of pads that it can connect to.
Each destination pad reference could have a link as a property
if the link is mandatory for binding the pads together.

A list of open links without pads might be usable too, enabling binding
different drivers to pass data to each other with pad->link->pad binding.
Driver could be a property of a pad for being able to do binding over 
drivers.

You have of course a better picture and understanding about these,
and you will need to come up with the best solution together on your own.

Maybe I just need to do something else and go and buy another motherboard
from Helsinki Ruoholahti to replace my overheated one, walking by some
Nokia Research Center buildings.

Best regards from the summerish Finland.
Marko Ristola

RE: [RFC/PATCH v3 00/10] Media controller (core and V4L2)

[Aguirre, Sergio]

Hi Laurent,

> -----Original Message-----
> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> owner@vger.kernel.org] On Behalf Of Laurent Pinchart
> Sent: Thursday, July 29, 2010 11:07 AM
> To: linux-media@vger.kernel.org
> Cc: sakari.ailus@maxwell.research.nokia.com
> Subject: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
> 
> Hi everybody,
> 
> Here's the third version of the media controller patches. All comments
> received
> on the first and second versions have (hopefully) been incorporated.
> 
> The rebased V4L2 API additions and OMAP3 ISP patches will follow. Once
> again
> please consider them as sample code only.
> 
> Laurent Pinchart (8):
>   media: Media device node support
>   media: Media device
>   media: Entities, pads and links
>   media: Entities, pads and links enumeration
>   media: Links setup
>   v4l: Add a media_device pointer to the v4l2_device structure
>   v4l: Make video_device inherit from media_entity
>   v4l: Make v4l2_subdev inherit from media_entity

This patch (#0010) doesn't apply to mainline, after this commit:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b74c0aac357e5c71ee6de98b9887fe478bc73cf4

Am I missing something here?

Regards,
Sergio

> 
> Sakari Ailus (2):
>   media: Entity graph traversal
>   media: Reference count and power handling
> 
>  Documentation/media-framework.txt            |  481 ++++++++++++++++++++
>  Documentation/video4linux/v4l2-framework.txt |   71 +++-
>  drivers/media/Makefile                       |    8 +-
>  drivers/media/media-device.c                 |  327 ++++++++++++++
>  drivers/media/media-devnode.c                |  326 ++++++++++++++
>  drivers/media/media-entity.c                 |  613
> ++++++++++++++++++++++++++
>  drivers/media/video/v4l2-dev.c               |   35 ++-
>  drivers/media/video/v4l2-device.c            |   45 ++-
>  drivers/media/video/v4l2-subdev.c            |   27 ++-
>  include/linux/media.h                        |   78 ++++
>  include/media/media-device.h                 |   70 +++
>  include/media/media-devnode.h                |   84 ++++
>  include/media/media-entity.h                 |  107 +++++
>  include/media/v4l2-dev.h                     |    6 +
>  include/media/v4l2-device.h                  |    2 +
>  include/media/v4l2-subdev.h                  |    7 +
>  16 files changed, 2265 insertions(+), 22 deletions(-)
>  create mode 100644 Documentation/media-framework.txt
>  create mode 100644 drivers/media/media-device.c
>  create mode 100644 drivers/media/media-devnode.c
>  create mode 100644 drivers/media/media-entity.c
>  create mode 100644 include/linux/media.h
>  create mode 100644 include/media/media-device.h
>  create mode 100644 include/media/media-devnode.h
>  create mode 100644 include/media/media-entity.h
> 
> --
> Regards,
> 
> Laurent Pinchart
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH v3 00/10] Media controller (core and V4L2)

[Laurent Pinchart]

Hi Sergio,

On Thursday 19 August 2010 21:09:30 Aguirre, Sergio wrote:
> > -----Original Message-----
> > From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> > owner@vger.kernel.org] On Behalf Of Laurent Pinchart
> > Sent: Thursday, July 29, 2010 11:07 AM
> > To: linux-media@vger.kernel.org
> > Cc: sakari.ailus@maxwell.research.nokia.com
> > Subject: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
> > 
> > Hi everybody,
> > 
> > Here's the third version of the media controller patches. All comments
> > received on the first and second versions have (hopefully) been
> > incorporated.
> > 
> > The rebased V4L2 API additions and OMAP3 ISP patches will follow. Once
> > again please consider them as sample code only.
> > 
> > Laurent Pinchart (8):
> >   media: Media device node support
> >   media: Media device
> >   media: Entities, pads and links
> >   media: Entities, pads and links enumeration
> >   media: Links setup
> >   v4l: Add a media_device pointer to the v4l2_device structure
> >   v4l: Make video_device inherit from media_entity
> >   v4l: Make v4l2_subdev inherit from media_entity
> 
> This patch (#0010) doesn't apply to mainline, after this commit:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h
> =b74c0aac357e5c71ee6de98b9887fe478bc73cf4
> 
> Am I missing something here?

Yes, you're missing the next version of the patches :-) I'll probably send 
them tomorrow.

-- 
Regards,

Laurent Pinchart

RE: [RFC/PATCH v3 00/10] Media controller (core and V4L2)

[Aguirre, Sergio]

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Thursday, August 19, 2010 2:12 PM
> To: Aguirre, Sergio
> Cc: linux-media@vger.kernel.org; sakari.ailus@maxwell.research.nokia.com
> Subject: Re: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
> 
> Hi Sergio,
> 
> On Thursday 19 August 2010 21:09:30 Aguirre, Sergio wrote:
> > > -----Original Message-----
> > > From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> > > owner@vger.kernel.org] On Behalf Of Laurent Pinchart
> > > Sent: Thursday, July 29, 2010 11:07 AM
> > > To: linux-media@vger.kernel.org
> > > Cc: sakari.ailus@maxwell.research.nokia.com
> > > Subject: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
> > >
> > > Hi everybody,
> > >
> > > Here's the third version of the media controller patches. All comments
> > > received on the first and second versions have (hopefully) been
> > > incorporated.
> > >
> > > The rebased V4L2 API additions and OMAP3 ISP patches will follow. Once
> > > again please consider them as sample code only.
> > >
> > > Laurent Pinchart (8):
> > >   media: Media device node support
> > >   media: Media device
> > >   media: Entities, pads and links
> > >   media: Entities, pads and links enumeration
> > >   media: Links setup
> > >   v4l: Add a media_device pointer to the v4l2_device structure
> > >   v4l: Make video_device inherit from media_entity
> > >   v4l: Make v4l2_subdev inherit from media_entity
> >
> > This patch (#0010) doesn't apply to mainline, after this commit:
> >
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
> 2.6.git;a=commit;h
> > =b74c0aac357e5c71ee6de98b9887fe478bc73cf4
> >
> > Am I missing something here?
> 
> Yes, you're missing the next version of the patches :-) I'll probably send
> them tomorrow.

Ohh ok, :)

Thanks!

Regards,
Sergio

> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [RFC/PATCH v3 00/10] Media controller (core and V4L2)

[Laurent Pinchart]

Hi Sergio,

On Thursday 19 August 2010 21:12:12 Laurent Pinchart wrote:
> On Thursday 19 August 2010 21:09:30 Aguirre, Sergio wrote:
> > > -----Original Message-----
> > > From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> > > owner@vger.kernel.org] On Behalf Of Laurent Pinchart
> > > Sent: Thursday, July 29, 2010 11:07 AM
> > > To: linux-media@vger.kernel.org
> > > Cc: sakari.ailus@maxwell.research.nokia.com
> > > Subject: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
> > > 
> > > Hi everybody,
> > > 
> > > Here's the third version of the media controller patches. All comments
> > > received on the first and second versions have (hopefully) been
> > > incorporated.
> > > 
> > > The rebased V4L2 API additions and OMAP3 ISP patches will follow. Once
> > > again please consider them as sample code only.
> > > 
> > > Laurent Pinchart (8):
> > >   media: Media device node support
> > >   media: Media device
> > >   media: Entities, pads and links
> > >   media: Entities, pads and links enumeration
> > >   media: Links setup
> > >   v4l: Add a media_device pointer to the v4l2_device structure
> > >   v4l: Make video_device inherit from media_entity
> > >   v4l: Make v4l2_subdev inherit from media_entity
> > 
> > This patch (#0010) doesn't apply to mainline, after this commit:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit
> > ;h =b74c0aac357e5c71ee6de98b9887fe478bc73cf4
> > 
> > Am I missing something here?
> 
> Yes, you're missing the next version of the patches :-) I'll probably send
> them tomorrow.

On second thought, you're probably missing the V4L2 subdev device node 
patches. b74c0aac357e5c71ee6de98b9887fe478bc73cf4 is very old (between 2.6.29 
and 2.6.30) and isn't related.

-- 
Regards,

Laurent Pinchart

RE: [RFC/PATCH v3 00/10] Media controller (core and V4L2)

[Aguirre, Sergio]

Hi Laurent,

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Friday, August 20, 2010 10:25 AM
> To: Aguirre, Sergio
> Cc: linux-media@vger.kernel.org; sakari.ailus@maxwell.research.nokia.com
> Subject: Re: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
> 
> Hi Sergio,
> 
> On Thursday 19 August 2010 21:12:12 Laurent Pinchart wrote:
> > On Thursday 19 August 2010 21:09:30 Aguirre, Sergio wrote:
> > > > -----Original Message-----
> > > > From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> > > > owner@vger.kernel.org] On Behalf Of Laurent Pinchart
> > > > Sent: Thursday, July 29, 2010 11:07 AM
> > > > To: linux-media@vger.kernel.org
> > > > Cc: sakari.ailus@maxwell.research.nokia.com
> > > > Subject: [RFC/PATCH v3 00/10] Media controller (core and V4L2)
> > > >
> > > > Hi everybody,
> > > >
> > > > Here's the third version of the media controller patches. All
> comments
> > > > received on the first and second versions have (hopefully) been
> > > > incorporated.
> > > >
> > > > The rebased V4L2 API additions and OMAP3 ISP patches will follow.
> Once
> > > > again please consider them as sample code only.
> > > >
> > > > Laurent Pinchart (8):
> > > >   media: Media device node support
> > > >   media: Media device
> > > >   media: Entities, pads and links
> > > >   media: Entities, pads and links enumeration
> > > >   media: Links setup
> > > >   v4l: Add a media_device pointer to the v4l2_device structure
> > > >   v4l: Make video_device inherit from media_entity
> > > >   v4l: Make v4l2_subdev inherit from media_entity
> > >
> > > This patch (#0010) doesn't apply to mainline, after this commit:
> > >
> > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
> 2.6.git;a=commit
> > > ;h =b74c0aac357e5c71ee6de98b9887fe478bc73cf4
> > >
> > > Am I missing something here?
> >
> > Yes, you're missing the next version of the patches :-) I'll probably
> send
> > them tomorrow.
> 
> On second thought, you're probably missing the V4L2 subdev device node
> patches. b74c0aac357e5c71ee6de98b9887fe478bc73cf4 is very old (between
> 2.6.29
> and 2.6.30) and isn't related.

Ok..

But where can I find those? In what tree?

Sorry for the ignorance.

Regards,
Sergio

> 
> --
> Regards,
> 
> Laurent Pinchart

Re: [RFC/PATCH v3 00/10] Media controller (core and V4L2)

[Laurent Pinchart]

Hi Sergio,

On Friday 20 August 2010 17:26:49 Aguirre, Sergio wrote:
> > -----Original Message-----
> > From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> > Sent: Friday, August 20, 2010 10:25 AM

[snip]

> > On second thought, you're probably missing the V4L2 subdev device node
> > patches. b74c0aac357e5c71ee6de98b9887fe478bc73cf4 is very old (between
> > 2.6.29 and 2.6.30) and isn't related.
> 
> Ok..
> 
> But where can I find those? In what tree?
> 
> Sorry for the ignorance.

They have been posted to the linux-media list on 2010-07-12 under the subject 
"[RFC/PATCH v3 0/7] V4L2 subdev userspace API".

I will prepare a tree based on mainline with the media controller patches and 
the ISP driver.

-- 
Regards,

Laurent Pinchart