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

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

[Laurent Pinchart]

Hi everybody,

Here's the second version of the media controller patches. All comments
received on the first version have 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                 |  329 ++++++++++++++
 drivers/media/media-devnode.c                |  339 ++++++++++++++
 drivers/media/media-entity.c                 |  619 ++++++++++++++++++++++++++
 drivers/media/video/v4l2-dev.c               |   35 ++-
 drivers/media/video/v4l2-device.c            |   39 ++-
 drivers/media/video/v4l2-subdev.c            |   27 ++-
 include/linux/media.h                        |   74 +++
 include/media/media-device.h                 |   75 +++
 include/media/media-devnode.h                |   91 ++++
 include/media/media-entity.h                 |  101 +++++
 include/media/v4l2-dev.h                     |    6 +
 include/media/v4l2-device.h                  |    2 +
 include/media/v4l2-subdev.h                  |    7 +
 16 files changed, 2284 insertions(+), 20 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 v2 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 |  339 +++++++++++++++++++++++++++++++++++++++++
 include/media/media-devnode.h |   91 +++++++++++
 3 files changed, 436 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..fa300f8
--- /dev/null
+++ b/drivers/media/media-devnode.c
@@ -0,0 +1,339 @@
+/*
+ * 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);
+
+static inline void media_get(struct media_devnode *mdev)
+{
+	get_device(&mdev->dev);
+}
+
+static inline void media_put(struct media_devnode *mdev)
+{
+	put_device(&mdev->dev);
+}
+
+/* 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 (!media_devnode_is_registered(mdev))
+		return -EIO;
+
+	if (!mdev->fops->unlocked_ioctl)
+		return -ENOTTY;
+
+	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 */
+	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 (mdev == NULL || !media_devnode_is_registered(mdev)) {
+		mutex_unlock(&media_devnode_lock);
+		return -ENODEV;
+	}
+	/* and increase the device refcount */
+	media_get(mdev);
+	mutex_unlock(&media_devnode_lock);
+	if (mdev->fops->open) {
+		ret = mdev->fops->open(filp);
+		if (ret) {
+			media_put(mdev);
+			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. */
+	media_put(mdev);
+	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)
+{
+	void *priv;
+	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) {
+		printk(KERN_ERR "could not get a free minor\n");
+		mutex_unlock(&media_devnode_lock);
+		return -ENFILE;
+	}
+
+	set_bit(mdev->minor, media_devnode_nums);
+	mdev->minor = minor;
+
+	mutex_unlock(&media_devnode_lock);
+
+	/* 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
+	 *
+	 * Zeroing struct device will clear the device's drvdata, so make a
+	 * copy and put it back.
+	 */
+	priv = dev_get_drvdata(&mdev->dev);
+	memset(&mdev->dev, 0, sizeof(mdev->dev));
+	dev_set_drvdata(&mdev->dev, priv);
+	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..5c8f682
--- /dev/null
+++ b/include/media/media-devnode.h
@@ -0,0 +1,91 @@
+/*
+ * 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 (*ioctl) (struct file *, unsigned int, unsigned long);
+	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
+	unsigned long (*get_unmapped_area) (struct file *, unsigned long,
+				unsigned long, unsigned long, unsigned long);
+	int (*mmap) (struct file *, struct vm_area_struct *);
+	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.
+ *
+ * The device node number @num is used to create the kobject name and thus
+ * serves as a hint to udev when creating the device node.
+ */
+struct media_devnode {
+	/* device ops */
+	const struct media_file_operations *fops;
+
+	/* sysfs */
+	struct device dev;		/* v4l 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 v2 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      |   77 +++++++++++++++++++++++++++++++++++++
 include/media/media-device.h      |   53 +++++++++++++++++++++++++
 4 files changed, 199 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..871fdfa
--- /dev/null
+++ b/drivers/media/media-device.c
@@ -0,0 +1,77 @@
+/*
+ *  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).
+ * - 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->name[0]))
+		return 0;
+
+	/* Set name to driver name + device name if it is empty. */
+	if (!mdev->name[0])
+		snprintf(mdev->name, sizeof(mdev->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;
+	strlcpy(mdev->devnode.name, mdev->name, sizeof(mdev->devnode.name));
+	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..0bef8b3
--- /dev/null
+++ b/include/media/media-device.h
@@ -0,0 +1,53 @@
+/*
+ *  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.
+ */
+
+#define MEDIA_DEVICE_NAME_SIZE		32
+
+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;
+
+	/* unique device name, by default the driver name + bus ID */
+	char name[MEDIA_DEVICE_NAME_SIZE];
+};
+
+int __must_check media_device_register(struct media_device *mdev);
+void media_device_unregister(struct media_device *mdev);
+
+#endif
-- 
1.7.1

[RFC/PATCH v2 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 |  127 ++++++++++++++++++++++++++++++++
 drivers/media/Makefile            |    2 +-
 drivers/media/media-device.c      |   53 ++++++++++++++
 drivers/media/media-entity.c      |  145 +++++++++++++++++++++++++++++++++++++
 include/media/media-device.h      |   16 ++++
 include/media/media-entity.h      |   79 ++++++++++++++++++++
 6 files changed, 421 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..1c8779c 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,106 @@ 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_entity_pad *pads, u8 extra_links);
+
+The media_entity name, type and subtype 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_entity_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_entity_pad structure, making the
+media_entity_pad pointer the canonical way to store and pass link references.
+
+Pads have a direction, relative to the entity they belong to:
+
+	pads of direction MEDIA_PAD_DIR_INPUT sink data, while
+	pads of direction MEDIA_PAD_DIR_OUTPUT source data.
+
+- Links
+
+Links are represented by a struct media_entity_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 871fdfa..3e16138 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->name[0]))
 		return 0;
@@ -72,6 +77,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..78c3fd2
--- /dev/null
+++ b/drivers/media/media-entity.c
@@ -0,0 +1,145 @@
+/*
+ *  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_entity_pad *pads, u8 extra_links)
+{
+	struct media_entity_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_entity_link *media_entity_add_link(struct media_entity *entity)
+{
+	if (entity->num_links >= entity->max_links) {
+		struct media_entity_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_entity_link *link;
+	struct media_entity_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 0bef8b3..3a713c0 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.
@@ -43,6 +45,12 @@ struct media_device {
 	struct device *dev;
 	struct media_devnode devnode;
 
+	u32 entity_id;
+	struct list_head entities;
+
+	/* Protects the entities list */
+	spinlock_t lock;
+
 	/* unique device name, by default the driver name + bus ID */
 	char name[MEDIA_DEVICE_NAME_SIZE];
 };
@@ -50,4 +58,12 @@ struct media_device {
 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..fd44647
--- /dev/null
+++ b/include/media/media-entity.h
@@ -0,0 +1,79 @@
+#ifndef _MEDIA_ENTITY_H
+#define _MEDIA_ENTITY_H
+
+#include <linux/list.h>
+
+#define MEDIA_ENTITY_TYPE_NODE				1
+#define MEDIA_ENTITY_TYPE_SUBDEV			2
+
+#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
+#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
+#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
+#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
+
+#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
+#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
+#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
+
+#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
+
+#define MEDIA_PAD_DIR_INPUT				1
+#define MEDIA_PAD_DIR_OUTPUT				2
+
+struct media_entity_link {
+	struct media_entity_pad *source;/* Source pad */
+	struct media_entity_pad *sink;	/* Sink pad  */
+	struct media_entity_link *other;/* Link in the reverse direction */
+	u32 flags;			/* Link flags (MEDIA_LINK_FLAG_*) */
+};
+
+struct media_entity_pad {
+	struct media_entity *entity;	/* Entity this pad belongs to */
+	u32 direction;			/* Pad direction (MEDIA_PAD_DIR_*) */
+	u8 index;			/* Pad index in the entity pads array */
+};
+
+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_*) */
+	u32 subtype;			/* Entity subtype (type-specific) */
+
+	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_entity_pad *pads;	/* Array of pads (num_pads elements) */
+	struct media_entity_link *links;/* Array of links (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 */
+	};
+};
+
+int media_entity_init(struct media_entity *entity, u8 num_pads,
+		struct media_entity_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 v2 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      |  116 +++++++++++++++++++++++++++++++++++++
 include/media/media-entity.h      |   15 +++++
 3 files changed, 171 insertions(+), 0 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 1c8779c..266d80f 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -193,3 +193,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 78c3fd2..ae4e8af 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -81,6 +81,122 @@ 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_entity_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_entity_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_entity_link *media_entity_add_link(struct media_entity *entity)
 {
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index fd44647..15944bb 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -70,10 +70,25 @@ struct media_entity {
 	};
 };
 
+#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_entity_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 v2 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 266d80f..3acc62b 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -233,3 +233,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 3e16138..097419b 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->name[0]))
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index ae4e8af..8fdfabe 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
@@ -194,6 +195,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 (entity->type != 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 (first->type != 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 (entity->type == 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 3a713c0..4d3ad0e 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>
@@ -50,6 +51,8 @@ struct media_device {
 
 	/* Protects the entities list */
 	spinlock_t lock;
+	/* Serializes graph operations. */
+	struct mutex graph_mutex;
 
 	/* unique device name, by default the driver name + bus ID */
 	char name[MEDIA_DEVICE_NAME_SIZE];
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 15944bb..63c30a0 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -34,6 +34,10 @@ struct media_entity_pad {
 	u8 index;			/* Pad index in the entity pads array */
 };
 
+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*/
@@ -52,6 +56,10 @@ struct media_entity {
 	struct media_entity_pad *pads;	/* Array of pads (num_pads elements) */
 	struct media_entity_link *links;/* Array of links (max_links elements)*/
 
+	const struct media_entity_operations *ops;	/* Entity operations */
+
+	int use_count;			/* Use count for the entity. */
+
 	union {
 		/* Node specifications */
 		struct {
@@ -86,9 +94,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 v2 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 |  134 ++++++++++++++++++++++++++++++++
 drivers/media/media-device.c      |  153 +++++++++++++++++++++++++++++++++++++
 include/linux/media.h             |   73 ++++++++++++++++++
 include/media/media-device.h      |    3 +
 include/media/media-entity.h      |   19 +-----
 5 files changed, 364 insertions(+), 18 deletions(-)
 create mode 100644 include/linux/media.h

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 3acc62b..16c0177 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -270,3 +270,137 @@ 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_user_entity *argp);
+
+To query the attributes of an entity, applications set the id field of a
+media_user_entity 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_user_entity structure is defined as
+
+- struct media_user_entity
+
+__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.
+__u32	subtype		Entity subtype.
+__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 - V4L, FB, ALSA or DVB device
+	MEDIA_ENTITY_TYPE_SUBDEV - V4L sub-device
+
+For MEDIA_ENTITY_TYPE_NODE entities, valid entity subtypes are
+
+	MEDIA_ENTITY_SUBTYPE_NODE_V4L - V4L video, radio or vbi device node
+	MEDIA_ENTITY_SUBTYPE_NODE_FB - Frame buffer device node
+	MEDIA_ENTITY_SUBTYPE_NODE_ALSA - ALSA card
+	MEDIA_ENTITY_SUBTYPE_NODE_DVB - DVB card
+
+For MEDIA_ENTITY_TYPE_SUBDEV entities, valid entity subtypes are
+
+	MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER - Video decoder
+	MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER - Video encoder
+	MEDIA_ENTITY_SUBTYPE_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_user_links *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_user_links structure and initialize the media_user_pad and
+media_user_link 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_user_pad, media_user_link and media_user_links structure are defined
+as
+
+- struct media_user_pad
+
+__u32		entity		ID of the entity this pad belongs to.
+__8		index		0-based pad index.
+__u32		direction	Pad direction.
+
+Valid pad directions are
+
+	MEDIA_PAD_DIR_INPUT -	Input pad, relative to the entity. Input pads
+				sink data and are targets of links.
+	MEDIA_PAD_DIR_OUTPUT -	Output pad, relative to the entity. Output
+				pads source data and are origins of links.
+
+- struct media_user_link
+
+struct media_user_pad	source	Pad at the origin of this link.
+struct media_user_pad	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_user_links
+
+__u32			entity	Entity id, set by the application.
+struct media_user_pad	*pads	Pointer to a pads array allocated by the
+				application. Ignored if NULL.
+struct media_user_link	*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 097419b..976a8df 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -20,13 +20,163 @@
 
 #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_user_entity __user *uent)
+{
+	struct media_entity *ent;
+	struct media_user_entity 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.subtype = ent->subtype;
+	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_entity_pad *kpad,
+				      struct media_user_pad *upad)
+{
+	upad->entity = kpad->entity->id;
+	upad->index = kpad->index;
+	upad->direction = kpad->direction;
+}
+
+static long media_device_enum_links(struct media_device *mdev,
+				    struct media_user_links __user *ulinks)
+{
+	struct media_entity *entity;
+	struct media_user_links 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_user_pad 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_user_link __user *ulink;
+		unsigned int l;
+
+		for (l = 0, ulink = links.links; l < entity->num_links; l++) {
+			struct media_user_link 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_user_entity __user *)arg);
+		break;
+
+	case MEDIA_IOC_ENUM_LINKS:
+		mutex_lock(&dev->graph_mutex);
+		ret = media_device_enum_links(dev,
+				(struct media_user_links __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)
@@ -100,6 +250,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..746bdda
--- /dev/null
+++ b/include/linux/media.h
@@ -0,0 +1,73 @@
+#ifndef __LINUX_MEDIA_H
+#define __LINUX_MEDIA_H
+
+#define MEDIA_ENTITY_TYPE_NODE				1
+#define MEDIA_ENTITY_TYPE_SUBDEV			2
+
+#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
+#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
+#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
+#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
+
+#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
+#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
+#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
+
+#define MEDIA_PAD_DIR_INPUT				1
+#define MEDIA_PAD_DIR_OUTPUT				2
+
+#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
+#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
+
+#define MEDIA_ENTITY_ID_FLAG_NEXT	(1 << 31)
+
+struct media_user_pad {
+	__u32 entity;		/* entity ID */
+	__u8 index;		/* pad index */
+	__u32 direction;	/* pad direction */
+};
+
+struct media_user_entity {
+	__u32 id;
+	char name[32];
+	__u32 type;
+	__u32 subtype;
+	__u8 pads;
+	__u32 links;
+
+	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 */
+	};
+};
+
+struct media_user_link {
+	struct media_user_pad source;
+	struct media_user_pad sink;
+	__u32 flags;
+};
+
+struct media_user_links {
+	__u32 entity;
+	/* Should have enough room for pads elements */
+	struct media_user_pad __user *pads;
+	/* Should have enough room for links elements */
+	struct media_user_link __user *links;
+};
+
+#define MEDIA_IOC_ENUM_ENTITIES		_IOWR('M', 1, struct media_user_entity)
+#define MEDIA_IOC_ENUM_LINKS		_IOWR('M', 2, struct media_user_links)
+
+#endif /* __LINUX_MEDIA_H */
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 4d3ad0e..087e788 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -58,6 +58,9 @@ struct media_device {
 	char name[MEDIA_DEVICE_NAME_SIZE];
 };
 
+/* 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 63c30a0..f51128d 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -2,24 +2,7 @@
 #define _MEDIA_ENTITY_H
 
 #include <linux/list.h>
-
-#define MEDIA_ENTITY_TYPE_NODE				1
-#define MEDIA_ENTITY_TYPE_SUBDEV			2
-
-#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
-#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
-#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
-#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
-
-#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
-#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
-#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
-
-#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
-#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
-
-#define MEDIA_PAD_DIR_INPUT				1
-#define MEDIA_PAD_DIR_OUTPUT				2
+#include <linux/media.h>
 
 struct media_entity_link {
 	struct media_entity_pad *source;/* Source pad */
-- 
1.7.1

[RFC/PATCH v2 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      |  212 +++++++++++++++++++++++++++++++++++++
 include/linux/media.h             |    1 +
 include/media/media-entity.h      |    9 ++
 5 files changed, 346 insertions(+), 4 deletions(-)

diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
index 16c0177..4136f25 100644
--- a/Documentation/media-framework.txt
+++ b/Documentation/media-framework.txt
@@ -233,6 +233,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_entity_pad *source,
+			       struct media_entity_pad *sink);
+
+	media_entity_remote_pad(struct media_entity_pad *pad);
+
+Refer to the kerneldoc documentation for more information.
+
 
 Reference counting and power handling
 -------------------------------------
@@ -271,11 +281,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_entity_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
 	-----------------------------------------------------------------
@@ -348,9 +398,6 @@ For MEDIA_ENTITY_TYPE_SUBDEV entities, valid entity subtypes are
 
 	ioctl(int fd, int request, struct media_user_links *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_user_links structure and initialize the media_user_pad and
 media_user_link structure arrays pointed by the pads and links fields. They
@@ -366,6 +413,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_user_pad, media_user_link and media_user_links structure are defined
 as
 
@@ -404,3 +454,28 @@ struct media_user_pad	*pads	Pointer to a pads array allocated by the
 struct media_user_link	*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_user_link *argp);
+
+To change link properties applications fill a media_user_link 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_user_pad and media_user_link 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 976a8df..b4d1f42 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -145,6 +145,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_user_link __user *_ulink)
+{
+	struct media_entity_link *link = NULL;
+	struct media_user_link 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)
 {
@@ -165,6 +203,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_user_link __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 8fdfabe..225d27b 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -198,6 +198,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 (entity->type == 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)
 {
@@ -261,6 +280,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.
@@ -405,3 +450,170 @@ 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_entity_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_entity_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_entity_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_entity_link *
+media_entity_find_link(struct media_entity_pad *source,
+		       struct media_entity_pad *sink)
+{
+	struct media_entity_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_entity_pad *
+media_entity_remote_pad(struct media_entity_pad *pad)
+{
+	unsigned int i;
+
+	for (i = 0; i < pad->entity->num_links; i++) {
+		struct media_entity_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 746bdda..5187851 100644
--- a/include/linux/media.h
+++ b/include/linux/media.h
@@ -69,5 +69,6 @@ struct media_user_links {
 
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('M', 1, struct media_user_entity)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('M', 2, struct media_user_links)
+#define MEDIA_IOC_SETUP_LINK		_IOWR('M', 3, struct media_user_link)
 
 #endif /* __LINUX_MEDIA_H */
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index f51128d..20010ce 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -18,6 +18,9 @@ struct media_entity_pad {
 };
 
 struct media_entity_operations {
+	int (*link_setup)(struct media_entity *entity,
+			  const struct media_entity_pad *local,
+			  const struct media_entity_pad *remote, u32 flags);
 	int (*set_power)(struct media_entity *entity, int power);
 };
 
@@ -76,6 +79,12 @@ 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_entity_link *link, u32 flags);
+int media_entity_setup_link(struct media_entity_link *link, u32 flags);
+struct media_entity_link *media_entity_find_link(
+		struct media_entity_pad *source, struct media_entity_pad *sink);
+struct media_entity_pad *media_entity_remote_pad(
+		struct media_entity_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 v2 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 v2 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..0d9b8ce 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_entity_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, subtype 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..e70424c 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,22 @@ 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) {
+		media_entity_put(entity);
 		video_put(vdev);
+	}
 	return ret;
 }
 
@@ -301,6 +312,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 +577,25 @@ 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;
+		vdev->entity.subtype = MEDIA_ENTITY_SUBTYPE_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 v2 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            |   26 +++++++++++++++++++++---
 drivers/media/video/v4l2-subdev.c            |   27 +++++++++++++++++++++++++-
 include/media/v4l2-subdev.h                  |    7 ++++++
 4 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/Documentation/video4linux/v4l2-framework.txt b/Documentation/video4linux/v4l2-framework.txt
index 0d9b8ce..76ecd43 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_entity_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, subtype 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..ccfa606 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -114,8 +114,9 @@ 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;
 
@@ -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);
@@ -147,22 +157,30 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 			v4l2_device_unregister_subdev(sd);
 	}
 
-	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 v2 00/12] Further V4L2 API additions and OMAP3 ISP driver

[Laurent Pinchart]

Hi everybody,

Here's the OMAP3 ISP driver along with V4L2 API additions/enhancements that
it depends on, rebased on the media controller v2 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            | 1035 ++++++++++++
 drivers/media/video/isp/ispccp2.h            |   89 +
 drivers/media/video/isp/ispcsi2.c            | 1214 ++++++++++++++
 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         | 2263 ++++++++++++++++++++++++++
 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         | 1637 +++++++++++++++++++
 drivers/media/video/isp/ispresizer.h         |  136 ++
 drivers/media/video/isp/ispstat.c            |  965 +++++++++++
 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, 25046 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 v2 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 v2 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 v2 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 v2 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 v2 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 v2 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 v2 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 v2 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 76ecd43..b47adf2 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 v2 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 v2 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 v2 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

[SAMPLE 12/12] OMAP3 ISP driver

[Laurent Pinchart]

[-- Attachment #1: Type: Text/Plain, Size: 98 bytes --]

Patch 12/12 is too big for vger, here it is in a compressed form.

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: 0012-OMAP3-ISP-driver.patch.bz2 --]
[-- Type: application/x-bzip, Size: 98452 bytes --]

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

[Sakari Ailus]

Heippa,

What a nice patch! :-)

Laurent Pinchart wrote:

...

> diff --git a/Documentation/media-framework.txt b/Documentation/media-framework.txt
> index 3acc62b..16c0177 100644
> --- a/Documentation/media-framework.txt
> +++ b/Documentation/media-framework.txt
> @@ -270,3 +270,137 @@ 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_user_entity *argp);
> +
> +To query the attributes of an entity, applications set the id field of a
> +media_user_entity 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_user_entity structure is defined as
> +
> +- struct media_user_entity
> +
> +__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.
> +__u32	subtype		Entity subtype.
> +__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 - V4L, FB, ALSA or DVB device
> +	MEDIA_ENTITY_TYPE_SUBDEV - V4L sub-device
> +
> +For MEDIA_ENTITY_TYPE_NODE entities, valid entity subtypes are
> +
> +	MEDIA_ENTITY_SUBTYPE_NODE_V4L - V4L video, radio or vbi device node
> +	MEDIA_ENTITY_SUBTYPE_NODE_FB - Frame buffer device node
> +	MEDIA_ENTITY_SUBTYPE_NODE_ALSA - ALSA card
> +	MEDIA_ENTITY_SUBTYPE_NODE_DVB - DVB card
> +
> +For MEDIA_ENTITY_TYPE_SUBDEV entities, valid entity subtypes are
> +
> +	MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER - Video decoder
> +	MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER - Video encoder
> +	MEDIA_ENTITY_SUBTYPE_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_user_links *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_user_links structure and initialize the media_user_pad and
> +media_user_link 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_user_pad, media_user_link and media_user_links structure are defined
> +as

I have a comment on naming. These are user space structures, sure, but
do we want that fact to be visible in the names of the structures? I
would just drop the user_ out and make the naming as good as possible in
user space. That is much harder to change later than naming inside the
kernel.

That change causes a lot of clashes in naming since the equivalent
kernel structure is there as well. Those could have _k postfix, for
example, to differentiate them from user space names. I don't really
have a good suggestion how they should be called.

> +- struct media_user_pad
> +
> +__u32		entity		ID of the entity this pad belongs to.
> +__8		index		0-based pad index.

It's possible that 8 bits is enough (I think Hans commented this
already). The compiler will use 4 bytes in any case and I think it's a
good practice not to create holes in the structures, especially not to
the interface ones.

The OMAP 4 has a tiler, could it be that this kind of functionality
might introduce large numbers of pads in the future?

> +__u32		direction	Pad direction.
> +
> +Valid pad directions are
> +
> +	MEDIA_PAD_DIR_INPUT -	Input pad, relative to the entity. Input pads
> +				sink data and are targets of links.
> +	MEDIA_PAD_DIR_OUTPUT -	Output pad, relative to the entity. Output
> +				pads source data and are origins of links.
> +
> +- struct media_user_link
> +
> +struct media_user_pad	source	Pad at the origin of this link.
> +struct media_user_pad	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_user_links
> +
> +__u32			entity	Entity id, set by the application.
> +struct media_user_pad	*pads	Pointer to a pads array allocated by the
> +				application. Ignored if NULL.
> +struct media_user_link	*links	Pointer to a links array allocated by the
> +				application. Ignored if NULL.
> +

Regards,

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

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

[Laurent Pinchart]

Hi Sakari,

On Thursday 22 July 2010 17:10:01 Sakari Ailus wrote:
> Heippa,
> 
> What a nice patch! :-)

Thanks :-)

> Laurent Pinchart wrote:
> 
> ...
> 
> > diff --git a/Documentation/media-framework.txt
> > b/Documentation/media-framework.txt index 3acc62b..16c0177 100644
> > --- a/Documentation/media-framework.txt
> > +++ b/Documentation/media-framework.txt
> > @@ -270,3 +270,137 @@ required, drivers don't need to provide a set_power

[snip]

> > +The media_user_pad, media_user_link and media_user_links structure are 
> > defined
> > +as
> 
> I have a comment on naming. These are user space structures, sure, but
> do we want that fact to be visible in the names of the structures? I
> would just drop the user_ out and make the naming as good as possible in
> user space. That is much harder to change later than naming inside the
> kernel.

I agree.

> That change causes a lot of clashes in naming since the equivalent
> kernel structure is there as well. Those could have _k postfix, for
> example, to differentiate them from user space names. I don't really
> have a good suggestion how they should be called.

Maybe media_k_* ? I'm not very happy with that name either though.

> > +- struct media_user_pad
> > +
> > +__u32		entity		ID of the entity this pad belongs to.
> > +__8		index		0-based pad index.
> 
> It's possible that 8 bits is enough (I think Hans commented this
> already). The compiler will use 4 bytes in any case and I think it's a
> good practice not to create holes in the structures, especially not to
> the interface ones.

The direction could become a 8-bit integer, and a 16-bit attributes/properties 
bitfield would be added to fill the hole (it would be used to store pad 
properties such as a busy flag). I'd rather make that field 32-bits wide 
instead of 16 though.

> The OMAP 4 has a tiler, could it be that this kind of functionality
> might introduce large numbers of pads in the future?

-- 
Regards,

Laurent Pinchart

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

[Sakari Ailus]

Hi,

Laurent Pinchart wrote:
...
> diff --git a/include/linux/media.h b/include/linux/media.h
> new file mode 100644
> index 0000000..746bdda
> --- /dev/null
> +++ b/include/linux/media.h
> @@ -0,0 +1,73 @@
> +#ifndef __LINUX_MEDIA_H
> +#define __LINUX_MEDIA_H
> +
> +#define MEDIA_ENTITY_TYPE_NODE				1
> +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> +
> +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> +
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> +
> +#define MEDIA_PAD_DIR_INPUT				1
> +#define MEDIA_PAD_DIR_OUTPUT				2
> +
> +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> +
> +#define MEDIA_ENTITY_ID_FLAG_NEXT	(1 << 31)
> +
> +struct media_user_pad {
> +	__u32 entity;		/* entity ID */
> +	__u8 index;		/* pad index */
> +	__u32 direction;	/* pad direction */
> +};

Another small comment, I think you mentioned it yourself some time back
:-): how about some reserved fields to these structures?

> +struct media_user_entity {
> +	__u32 id;
> +	char name[32];
> +	__u32 type;
> +	__u32 subtype;
> +	__u8 pads;
> +	__u32 links;
> +
> +	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 */
> +	};
> +};
> +
> +struct media_user_link {
> +	struct media_user_pad source;
> +	struct media_user_pad sink;
> +	__u32 flags;
> +};
> +
> +struct media_user_links {
> +	__u32 entity;
> +	/* Should have enough room for pads elements */
> +	struct media_user_pad __user *pads;
> +	/* Should have enough room for links elements */
> +	struct media_user_link __user *links;
> +};

Cheers,

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

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

[Laurent Pinchart]

Hi Sakari,

On Thursday 22 July 2010 17:26:55 Sakari Ailus wrote:
> Hi,
> 
> Laurent Pinchart wrote:
> ...
> 
> > diff --git a/include/linux/media.h b/include/linux/media.h
> > new file mode 100644
> > index 0000000..746bdda
> > --- /dev/null
> > +++ b/include/linux/media.h
> > @@ -0,0 +1,73 @@

[snip]

> > +
> > +struct media_user_pad {
> > +	__u32 entity;		/* entity ID */
> > +	__u8 index;		/* pad index */
> > +	__u32 direction;	/* pad direction */
> > +};
> 
> Another small comment, I think you mentioned it yourself some time back
> 
> :-): how about some reserved fields to these structures?

Very good point. Reserved fields are needed in media_user_entity and 
media_user_links at least. For media_user_pad and media_user_link, we could do 
without reserved fields if we add fields to media_user_links to store the size 
of those structures.

> > +struct media_user_entity {
> > +	__u32 id;
> > +	char name[32];
> > +	__u32 type;
> > +	__u32 subtype;
> > +	__u8 pads;
> > +	__u32 links;
> > +
> > +	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 */
> > +	};
> > +};
> > +
> > +struct media_user_link {
> > +	struct media_user_pad source;
> > +	struct media_user_pad sink;
> > +	__u32 flags;
> > +};
> > +
> > +struct media_user_links {
> > +	__u32 entity;
> > +	/* Should have enough room for pads elements */
> > +	struct media_user_pad __user *pads;
> > +	/* Should have enough room for links elements */
> > +	struct media_user_link __user *links;
> > +};

-- 
Regards,

Laurent Pinchart

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

[Sakari Ailus]

Laurent Pinchart wrote:
>> That change causes a lot of clashes in naming since the equivalent
>> kernel structure is there as well. Those could have _k postfix, for
>> example, to differentiate them from user space names. I don't really
>> have a good suggestion how they should be called.
> 
> Maybe media_k_* ? I'm not very happy with that name either though.

Sounds better to me.

>>> +- struct media_user_pad
>>> +
>>> +__u32		entity		ID of the entity this pad belongs to.
>>> +__8		index		0-based pad index.
>>
>> It's possible that 8 bits is enough (I think Hans commented this
>> already). The compiler will use 4 bytes in any case and I think it's a
>> good practice not to create holes in the structures, especially not to
>> the interface ones.
> 
> The direction could become a 8-bit integer, and a 16-bit attributes/properties 
> bitfield would be added to fill the hole (it would be used to store pad 
> properties such as a busy flag). I'd rather make that field 32-bits wide 
> instead of 16 though.

I guess you could put more reserved fields to these small holes.

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

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

[Pete Eberlein]

On Thu, 2010-07-22 at 17:20 +0200, Laurent Pinchart wrote:
> > Laurent Pinchart wrote:
> > 
> > ...
> > 
> > > diff --git a/Documentation/media-framework.txt
> > > b/Documentation/media-framework.txt index 3acc62b..16c0177 100644
> > > --- a/Documentation/media-framework.txt
> > > +++ b/Documentation/media-framework.txt
> > > @@ -270,3 +270,137 @@ required, drivers don't need to provide a set_power
> 
> [snip]
> 
> > > +The media_user_pad, media_user_link and media_user_links structure are 
> > > defined
> > > +as
> > 
> > I have a comment on naming. These are user space structures, sure, but
> > do we want that fact to be visible in the names of the structures? I
> > would just drop the user_ out and make the naming as good as possible in
> > user space. That is much harder to change later than naming inside the
> > kernel.
> 
> I agree.
> 
> > That change causes a lot of clashes in naming since the equivalent
> > kernel structure is there as well. Those could have _k postfix, for
> > example, to differentiate them from user space names. I don't really
> > have a good suggestion how they should be called.
> 
> Maybe media_k_* ? I'm not very happy with that name either though.

What do you think about a single underscore prefix for the kernel
structures, used commonly to indicate that a declaration is limited?

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

[Sakari Ailus]

Laurent Pinchart wrote:
> Hi Sakari,

Hello Laurent,

...
>>> +
>>> +struct media_user_pad {
>>> +	__u32 entity;		/* entity ID */
>>> +	__u8 index;		/* pad index */
>>> +	__u32 direction;	/* pad direction */
>>> +};
>>
>> Another small comment, I think you mentioned it yourself some time back
>>
>> :-): how about some reserved fields to these structures?
> 
> Very good point. Reserved fields are needed in media_user_entity and 
> media_user_links at least. For media_user_pad and media_user_link, we could do 
> without reserved fields if we add fields to media_user_links to store the size 
> of those structures.

The structure size is part of the ioctl number defined by the _IOC macro
so I'd go with reserved fields even for these structures. Otherwise
special handling would be required for these ioctls in a few places.

>>> +struct media_user_entity {
>>> +	__u32 id;
>>> +	char name[32];
>>> +	__u32 type;
>>> +	__u32 subtype;
>>> +	__u8 pads;
>>> +	__u32 links;
>>> +
>>> +	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 */

This union could have a defined size as well, e.g. u8 blob[64] or something.

Regards,

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

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

[Karicheri, Muralidharan]

Laurent,

Could you explain the probe and active usage using an example such as
below?

            Link1    Link2 
input sensor -> ccdc -> video node.

Assume Link2 we can have either format 1 or format 2 for capture.

Thanks.

Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
email: m-karicheri2@ti.com

>-----Original Message-----
>From: linux-media-owner@vger.kernel.org [mailto:linux-media-
>owner@vger.kernel.org] On Behalf Of Laurent Pinchart
>Sent: Wednesday, July 21, 2010 10:42 AM
>To: linux-media@vger.kernel.org
>Cc: sakari.ailus@maxwell.research.nokia.com
>Subject: [SAMPLE v2 04/12] v4l-subdev: Add pads operations
>
>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
>
>--
>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 v2 01/10] media: Media device node support

[Hans Verkuil]

On Wednesday 21 July 2010 16:35:26 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 |  339 +++++++++++++++++++++++++++++++++++++++++
>  include/media/media-devnode.h |   91 +++++++++++
>  3 files changed, 436 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..fa300f8
> --- /dev/null
> +++ b/drivers/media/media-devnode.c
> @@ -0,0 +1,339 @@
> +/*
> + * 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);

I don't think this lock is actually needed. The bit operations are already
atomic, so we can do without this lock.

> +static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
> +
> +static inline void media_get(struct media_devnode *mdev)
> +{
> +	get_device(&mdev->dev);
> +}
> +
> +static inline void media_put(struct media_devnode *mdev)
> +{
> +	put_device(&mdev->dev);
> +}

These functions are used quite rarely. I'd remove these inlines and just
call get/put_device directly.

> +
> +/* 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 (!media_devnode_is_registered(mdev))
> +		return -EIO;
> +
> +	if (!mdev->fops->unlocked_ioctl)
> +		return -ENOTTY;

These two if's should be swapper (just like in media_read and write).

> +
> +	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 */
> +	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 (mdev == NULL || !media_devnode_is_registered(mdev)) {

mdev can never be NULL.

> +		mutex_unlock(&media_devnode_lock);
> +		return -ENODEV;
> +	}
> +	/* and increase the device refcount */
> +	media_get(mdev);
> +	mutex_unlock(&media_devnode_lock);
> +	if (mdev->fops->open) {
> +		ret = mdev->fops->open(filp);
> +		if (ret) {
> +			media_put(mdev);
> +			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. */
> +	media_put(mdev);
> +	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)
> +{
> +	void *priv;
> +	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);

When media_devnode_lock is removed we can't use find_next_zero_bit anymore.
Instead, we have to loop over all bits and call test_and_set_bit() to ensure
atomicity.

> +	if (minor == MEDIA_NUM_DEVICES) {
> +		printk(KERN_ERR "could not get a free minor\n");
> +		mutex_unlock(&media_devnode_lock);
> +		return -ENFILE;
> +	}
> +
> +	set_bit(mdev->minor, media_devnode_nums);
> +	mdev->minor = minor;
> +
> +	mutex_unlock(&media_devnode_lock);
> +
> +	/* 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
> +	 *
> +	 * Zeroing struct device will clear the device's drvdata, so make a
> +	 * copy and put it back.
> +	 */
> +	priv = dev_get_drvdata(&mdev->dev);
> +	memset(&mdev->dev, 0, sizeof(mdev->dev));
> +	dev_set_drvdata(&mdev->dev, priv);

Don't zero mdev->dev. We should assume that mdev has been zeroed by the caller.
This construct is actually a bug in v4l2-dev.c. I've seen patches that fixed it,
but I don't know the status of those patches. The problem is that dev_set_drvdata
these days allocates memory. So if dev_set_drvdata was called before this
function, then this construct will leak a bit of memory.

I would just remove this and require that mdev was initialized correctly
when it was allocated.

> +	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);

Wouldn't mediactlX be a better name? Just plain 'media' is awfully general.

> +	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..5c8f682
> --- /dev/null
> +++ b/include/media/media-devnode.h
> @@ -0,0 +1,91 @@
> +/*
> + * 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 (*ioctl) (struct file *, unsigned int, unsigned long);

I think you forgot to remove the ioctl, mmap and get_unmapped_area ops from this
struct.

> +	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> +	unsigned long (*get_unmapped_area) (struct file *, unsigned long,
> +				unsigned long, unsigned long, unsigned long);
> +	int (*mmap) (struct file *, struct vm_area_struct *);
> +	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.
> + *
> + * The device node number @num is used to create the kobject name and thus
> + * serves as a hint to udev when creating the device node.

'num' no longer exists.

> + */
> +struct media_devnode {
> +	/* device ops */
> +	const struct media_file_operations *fops;
> +
> +	/* sysfs */
> +	struct device dev;		/* v4l 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 */
> 

Regards,

	Hans

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

Re: [RFC/PATCH v2 02/10] media: Media device

[Hans Verkuil]

On Wednesday 21 July 2010 16:35:27 Laurent Pinchart wrote:
> 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      |   77 +++++++++++++++++++++++++++++++++++++
>  include/media/media-device.h      |   53 +++++++++++++++++++++++++
>  4 files changed, 199 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
> 

<snip>

As discussed on IRC: I would merge media-device and media-devnode. I see no
benefit in separating them at this time.

Regards,

	Hans

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

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

[Hans Verkuil]

On Wednesday 21 July 2010 16:35:28 Laurent Pinchart wrote:

<snip>

> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> new file mode 100644
> index 0000000..fd44647
> --- /dev/null
> +++ b/include/media/media-entity.h
> @@ -0,0 +1,79 @@
> +#ifndef _MEDIA_ENTITY_H
> +#define _MEDIA_ENTITY_H
> +
> +#include <linux/list.h>
> +
> +#define MEDIA_ENTITY_TYPE_NODE				1
> +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> +
> +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> +
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3

These names are too awkward.

I see two options:

1) Rename the type field to 'entity' and the macros to MEDIA_ENTITY_NODE/SUBDEV.
   Also rename subtype to type and the macros to MEDIA_ENTITY_TYPE_NODE_V4L
   and MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER. We might even get away with dropping
   _TYPE from the macro name.

2) Merge type and subtype to a single entity field. The top 16 bits are the entity
   type, the bottom 16 bits are the subtype. That way you end up with:

#define MEDIA_ENTITY_NODE			(1 << 16)
#define MEDIA_ENTITY_SUBDEV			(2 << 16)

#define MEDIA_ENTITY_NODE_V4L			(MEDIA_ENTITY_NODE + 1)

#define MEDIA_ENTITY_SUBDEV_VID_DECODER		(MEDIA_ENTITY_SUBDEV + 1)

I rather like this option myself.

> +
> +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> +
> +#define MEDIA_PAD_DIR_INPUT				1
> +#define MEDIA_PAD_DIR_OUTPUT				2
> +
> +struct media_entity_link {
> +	struct media_entity_pad *source;/* Source pad */
> +	struct media_entity_pad *sink;	/* Sink pad  */
> +	struct media_entity_link *other;/* Link in the reverse direction */
> +	u32 flags;			/* Link flags (MEDIA_LINK_FLAG_*) */
> +};
> +
> +struct media_entity_pad {
> +	struct media_entity *entity;	/* Entity this pad belongs to */
> +	u32 direction;			/* Pad direction (MEDIA_PAD_DIR_*) */
> +	u8 index;			/* Pad index in the entity pads array */

We can use bitfields for direction and index. That way we can also easily add
other flags/attributes.

> +};
> +
> +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_*) */
> +	u32 subtype;			/* Entity subtype (type-specific) */
> +
> +	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_entity_pad *pads;	/* Array of pads (num_pads elements) */
> +	struct media_entity_link *links;/* Array of links (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 */
> +	};
> +};
> +
> +int media_entity_init(struct media_entity *entity, u8 num_pads,
> +		struct media_entity_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 v2 06/10] media: Entities, pads and links enumeration

[Hans Verkuil]

On Wednesday 21 July 2010 16:35:31 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 |  134 ++++++++++++++++++++++++++++++++
>  drivers/media/media-device.c      |  153 +++++++++++++++++++++++++++++++++++++
>  include/linux/media.h             |   73 ++++++++++++++++++
>  include/media/media-device.h      |    3 +
>  include/media/media-entity.h      |   19 +-----
>  5 files changed, 364 insertions(+), 18 deletions(-)
>  create mode 100644 include/linux/media.h

<snip>

> diff --git a/include/linux/media.h b/include/linux/media.h
> new file mode 100644
> index 0000000..746bdda
> --- /dev/null
> +++ b/include/linux/media.h
> @@ -0,0 +1,73 @@
> +#ifndef __LINUX_MEDIA_H
> +#define __LINUX_MEDIA_H
> +
> +#define MEDIA_ENTITY_TYPE_NODE				1
> +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> +
> +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> +
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> +
> +#define MEDIA_PAD_DIR_INPUT				1
> +#define MEDIA_PAD_DIR_OUTPUT				2
> +
> +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> +
> +#define MEDIA_ENTITY_ID_FLAG_NEXT	(1 << 31)
> +
> +struct media_user_pad {
> +	__u32 entity;		/* entity ID */
> +	__u8 index;		/* pad index */
> +	__u32 direction;	/* pad direction */
> +};

How about:

struct media_pad {
	__u32 entity;		/* entity ID */
	__u16 index;		/* pad index */
	__u16 flags;		/* pad flags (includes direction) */
	u32 reserved;
};

I think u16 for the number of pads might be safer than a u8.

> +
> +struct media_user_entity {
> +	__u32 id;
> +	char name[32];
> +	__u32 type;
> +	__u32 subtype;
> +	__u8 pads;
> +	__u32 links;

Need reserved fields.

> +
> +	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 */

Add something like:

		__u8 raw[64];

> +	};
> +};
> +
> +struct media_user_link {
> +	struct media_user_pad source;
> +	struct media_user_pad sink;
> +	__u32 flags;

Add a single __u32 reserved.

> +};
> +
> +struct media_user_links {
> +	__u32 entity;
> +	/* Should have enough room for pads elements */
> +	struct media_user_pad __user *pads;
> +	/* Should have enough room for links elements */
> +	struct media_user_link __user *links;

Add a  __u32 reserved[4].

> +};
> +
> +#define MEDIA_IOC_ENUM_ENTITIES		_IOWR('M', 1, struct media_user_entity)
> +#define MEDIA_IOC_ENUM_LINKS		_IOWR('M', 2, struct media_user_links)

We also need a MEDIA_IOC_QUERYCAP or MEDIA_IOC_VERSION or something like that.

> +
> +#endif /* __LINUX_MEDIA_H */
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index 4d3ad0e..087e788 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -58,6 +58,9 @@ struct media_device {
>  	char name[MEDIA_DEVICE_NAME_SIZE];
>  };
>  
> +/* 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 63c30a0..f51128d 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -2,24 +2,7 @@
>  #define _MEDIA_ENTITY_H
>  
>  #include <linux/list.h>
> -
> -#define MEDIA_ENTITY_TYPE_NODE				1
> -#define MEDIA_ENTITY_TYPE_SUBDEV			2
> -
> -#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> -#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> -#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> -#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> -
> -#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> -#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> -#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> -
> -#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> -#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> -
> -#define MEDIA_PAD_DIR_INPUT				1
> -#define MEDIA_PAD_DIR_OUTPUT				2
> +#include <linux/media.h>
>  
>  struct media_entity_link {
>  	struct media_entity_pad *source;/* Source pad */
> 

Regards,

	Hans

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

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

[Laurent Pinchart]

Hi Hans,

Thanks for the review.

On Saturday 24 July 2010 13:59:11 Hans Verkuil wrote:
> On Wednesday 21 July 2010 16:35:26 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.

[snip]

> > diff --git a/drivers/media/media-devnode.c
> > b/drivers/media/media-devnode.c new file mode 100644
> > index 0000000..fa300f8
> > --- /dev/null
> > +++ b/drivers/media/media-devnode.c
> > @@ -0,0 +1,339 @@

[snip]

> > +/*
> > + *	Active devices
> > + */
> > +static DEFINE_MUTEX(media_devnode_lock);
> 
> I don't think this lock is actually needed. The bit operations are already
> atomic, so we can do without this lock.

The mutex is needed to avoid an open/unregister race. Without it the device 
could be unregistered during media_open, after the media_devnode_is_registered 
check but before the call to get_device.

> > +static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
> > +
> > +static inline void media_get(struct media_devnode *mdev)
> > +{
> > +	get_device(&mdev->dev);
> > +}
> > +
> > +static inline void media_put(struct media_devnode *mdev)
> > +{
> > +	put_device(&mdev->dev);
> > +}
> 
> These functions are used quite rarely. I'd remove these inlines and just
> call get/put_device directly.

Agreed. I was thinking of removing them and thought you would like it better 
if I kept them :-)

[snip]

> > +static long media_ioctl(struct file *filp, unsigned int cmd, unsigned
> > long arg) +{
> > +	struct media_devnode *mdev = media_devnode_data(filp);
> > +
> > +	if (!media_devnode_is_registered(mdev))
> > +		return -EIO;
> > +
> > +	if (!mdev->fops->unlocked_ioctl)
> > +		return -ENOTTY;
> 
> These two if's should be swapper (just like in media_read and write).

I'm not sure why, but OK.

> > +
> > +	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 */
> > +	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 (mdev == NULL || !media_devnode_is_registered(mdev)) {
> 
> mdev can never be NULL.

Indeed, thanks.

> > +		mutex_unlock(&media_devnode_lock);
> > +		return -ENODEV;
> > +	}
> > +	/* and increase the device refcount */
> > +	media_get(mdev);
> > +	mutex_unlock(&media_devnode_lock);
> > +	if (mdev->fops->open) {
> > +		ret = mdev->fops->open(filp);
> > +		if (ret) {
> > +			media_put(mdev);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	filp->private_data = mdev;
> > +	return 0;
> > +}

[snip]

> > +/**
> > + * 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)
> > +{
> > +	void *priv;
> > +	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);
> 
> When media_devnode_lock is removed we can't use find_next_zero_bit anymore.
> Instead, we have to loop over all bits and call test_and_set_bit() to
> ensure atomicity.

I think the lock is still needed (see above), so find_next_zero_bit can be 
kept.

> > +	if (minor == MEDIA_NUM_DEVICES) {
> > +		printk(KERN_ERR "could not get a free minor\n");
> > +		mutex_unlock(&media_devnode_lock);
> > +		return -ENFILE;
> > +	}
> > +
> > +	set_bit(mdev->minor, media_devnode_nums);
> > +	mdev->minor = minor;
> > +
> > +	mutex_unlock(&media_devnode_lock);
> > +
> > +	/* 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
> > +	 *
> > +	 * Zeroing struct device will clear the device's drvdata, so make a
> > +	 * copy and put it back.
> > +	 */
> > +	priv = dev_get_drvdata(&mdev->dev);
> > +	memset(&mdev->dev, 0, sizeof(mdev->dev));
> > +	dev_set_drvdata(&mdev->dev, priv);
> 
> Don't zero mdev->dev. We should assume that mdev has been zeroed by the
> caller. This construct is actually a bug in v4l2-dev.c. I've seen patches
> that fixed it, but I don't know the status of those patches. The problem
> is that dev_set_drvdata these days allocates memory. So if dev_set_drvdata
> was called before this function, then this construct will leak a bit of
> memory.
> 
> I would just remove this and require that mdev was initialized correctly
> when it was allocated.

OK.

> > +	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);
> 
> Wouldn't mediactlX be a better name? Just plain 'media' is awfully general.

Good question.

> > +	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;
> > +}

[snip]

> > diff --git a/include/media/media-devnode.h
> > b/include/media/media-devnode.h new file mode 100644
> > index 0000000..5c8f682
> > --- /dev/null
> > +++ b/include/media/media-devnode.h
> > @@ -0,0 +1,91 @@

[snip]

> > +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 (*ioctl) (struct file *, unsigned int, unsigned long);
> 
> I think you forgot to remove the ioctl, mmap and get_unmapped_area ops from
> this struct.

Yes, thanks.

> > +	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> > +	unsigned long (*get_unmapped_area) (struct file *, unsigned long,
> > +				unsigned long, unsigned long, unsigned long);
> > +	int (*mmap) (struct file *, struct vm_area_struct *);
> > +	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.
> > + *
> > + * The device node number @num is used to create the kobject name and
> > thus
> > + * serves as a hint to udev when creating the device node.
> 
> 'num' no longer exists.

Oops :-)

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v2 02/10] media: Media device

[Laurent Pinchart]

Hi Hans,

On Saturday 24 July 2010 14:02:50 Hans Verkuil wrote:
> On Wednesday 21 July 2010 16:35:27 Laurent Pinchart wrote:
> > 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      |   77
> >  +++++++++++++++++++++++++++++++++++++ include/media/media-device.h     
> >  |   53 +++++++++++++++++++++++++ 4 files changed, 199 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
> 
> <snip>
> 
> As discussed on IRC: I would merge media-device and media-devnode. I see no
> benefit in separating them at this time.

I'm not too sure about it. I still think the separation gives us cleaner, 
easier to understand code. My opinion on it isn't that strong, so I could be 
convinced to merge the two, but Sakari seemed to think they shouldn't be 
merged last time I talked to him about it. I'll let him answer.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH v2 02/10] media: Media device

[Sakari Ailus]

Hans Verkuil wrote:
> On Wednesday 21 July 2010 16:35:27 Laurent Pinchart wrote:
>> 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      |   77 +++++++++++++++++++++++++++++++++++++
>>  include/media/media-device.h      |   53 +++++++++++++++++++++++++
>>  4 files changed, 199 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
>>
> 
> <snip>
> 
> As discussed on IRC: I would merge media-device and media-devnode. I see no
> benefit in separating them at this time.

I have to say I like the current separation of registration / node
handling and the actual implementation, as in V4L2. There's more code to
both files in the following patches. It think the result is easier to
understand the way it is.

You do have a point there that there's no need to separate them since
media_devnode is only used in media_device, at the moment at least. Or
is there a chance we would get different kind of control devices that
would use media_devnode in the future? I don't see a clear need for
such, though.

Could media_devnode and media_device be combined without breaking this
nice separation in the code too much?

Regards,

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

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

[Laurent Pinchart]

On Friday 23 July 2010 17:56:02 Karicheri, Muralidharan wrote:
> Laurent,
> 
> Could you explain the probe and active usage using an example such as
> below?
> 
>             Link1    Link2
> input sensor -> ccdc -> video node.
> 
> Assume Link2 we can have either format 1 or format 2 for capture.

Sure.

The probe and active formats are used to probe supported formats and 
getting/setting active formats.

* Enumerating supported formats on the CCDC input and output would be done 
with the following calls

ENUM_FMT(CCDC input pad)

for the input, and

S_FMT(PROBE, CCDC input pad, format)
ENUM_FMT(CCDC output pad)

for the output.

Setting the probe format on the input pad is required, as the format on an 
output pad usually depends on the format on input pads.

* Trying a format on the CCDC input and output would be done with

S_FMT(PROBE, CCDC input pad, format)

for the input, and

S_FMT(PROBE, CCDC input pad, format)
S_FMT(PROBE, CCDC output pad, format)

on the output. The S_FMT call will mangle the format given format if it can't 
be supported exactly, so there's no need to call G_FMT after S_FMT (a G_FMT 
call following a S_FMT call will return the same format as the S_FMT call).

* Setting the active format is done with

S_FMT(ACTIVE, CCDC input pad, format)
S_FMT(ACTIVE, CCDC output pad, format)

The formats will be applied to the hardware (possibly with a delay, drivers 
can delay register writes until STREAMON for instance).

Probe formats are stored in the subdev file handles, so two applications 
trying formats at the same time will not interfere with each other. Active 
formats are stored in the device structure, so modifications done by an 
application are visible to other applications.

Hope this helps clarifying the API.

-- 
Regards,

Laurent Pinchart

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

[Karicheri, Muralidharan]

Laurent,

Thanks for clarifying this. I guess this will also get documented in the
v4l2 specs (if not already done) as part of this patch.

Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
email: m-karicheri2@ti.com

>-----Original Message-----
>From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>Sent: Monday, July 26, 2010 12:13 PM
>To: Karicheri, Muralidharan
>Cc: linux-media@vger.kernel.org; sakari.ailus@maxwell.research.nokia.com
>Subject: Re: [SAMPLE v2 04/12] v4l-subdev: Add pads operations
>
>On Friday 23 July 2010 17:56:02 Karicheri, Muralidharan wrote:
>> Laurent,
>>
>> Could you explain the probe and active usage using an example such as
>> below?
>>
>>             Link1    Link2
>> input sensor -> ccdc -> video node.
>>
>> Assume Link2 we can have either format 1 or format 2 for capture.
>
>Sure.
>
>The probe and active formats are used to probe supported formats and
>getting/setting active formats.
>
>* Enumerating supported formats on the CCDC input and output would be done
>with the following calls
>
>ENUM_FMT(CCDC input pad)
>
>for the input, and
>
>S_FMT(PROBE, CCDC input pad, format)
>ENUM_FMT(CCDC output pad)
>
>for the output.
>
>Setting the probe format on the input pad is required, as the format on an
>output pad usually depends on the format on input pads.
>
>* Trying a format on the CCDC input and output would be done with
>
>S_FMT(PROBE, CCDC input pad, format)
>
>for the input, and
>
>S_FMT(PROBE, CCDC input pad, format)
>S_FMT(PROBE, CCDC output pad, format)
>
>on the output. The S_FMT call will mangle the format given format if it
>can't
>be supported exactly, so there's no need to call G_FMT after S_FMT (a G_FMT
>call following a S_FMT call will return the same format as the S_FMT call).
>
>* Setting the active format is done with
>
>S_FMT(ACTIVE, CCDC input pad, format)
>S_FMT(ACTIVE, CCDC output pad, format)
>
>The formats will be applied to the hardware (possibly with a delay, drivers
>can delay register writes until STREAMON for instance).
>
>Probe formats are stored in the subdev file handles, so two applications
>trying formats at the same time will not interfere with each other. Active
>formats are stored in the device structure, so modifications done by an
>application are visible to other applications.
>
>Hope this helps clarifying the API.
>
>--
>Regards,
>
>Laurent Pinchart

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

[Laurent Pinchart]

Hi Hans,

On Monday 26 July 2010 11:07:14 Laurent Pinchart wrote:
> Hi Hans,
> On Saturday 24 July 2010 13:59:11 Hans Verkuil wrote:
> > On Wednesday 21 July 2010 16:35:26 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.

[snip]

> > > +	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);
> > 
> > Wouldn't mediactlX be a better name? Just plain 'media' is awfully
> > general.
> 
> Good question.

Oops, I forgot to elaborate on that :-)

Plain "media" is indeed very generic. "mediactl" would be more specific, but 
I've never really liked the term "media controller". If you look at the media 
framework documentation, there's no reference to "controller" :-) I think it's 
"just" a media device.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Pete,

On Thursday 22 July 2010 18:36:51 Pete Eberlein wrote:
> On Thu, 2010-07-22 at 17:20 +0200, Laurent Pinchart wrote:
> > > Laurent Pinchart wrote:
> > > 
> > > ...
> > > 
> > > > diff --git a/Documentation/media-framework.txt
> > > > b/Documentation/media-framework.txt index 3acc62b..16c0177 100644
> > > > --- a/Documentation/media-framework.txt
> > > > +++ b/Documentation/media-framework.txt
> > > > @@ -270,3 +270,137 @@ required, drivers don't need to provide a
> > > > set_power
> > 
> > [snip]
> > 
> > > > +The media_user_pad, media_user_link and media_user_links structure
> > > > are defined
> > > > +as
> > > 
> > > I have a comment on naming. These are user space structures, sure, but
> > > do we want that fact to be visible in the names of the structures? I
> > > would just drop the user_ out and make the naming as good as possible
> > > in user space. That is much harder to change later than naming inside
> > > the kernel.
> > 
> > I agree.
> > 
> > > That change causes a lot of clashes in naming since the equivalent
> > > kernel structure is there as well. Those could have _k postfix, for
> > > example, to differentiate them from user space names. I don't really
> > > have a good suggestion how they should be called.
> > 
> > Maybe media_k_* ? I'm not very happy with that name either though.
> 
> What do you think about a single underscore prefix for the kernel
> structures, used commonly to indicate that a declaration is limited?

The underscore is usually used for internal functions/variables. I'd rather go 
for _k, I think it's more obvious.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Sakari,

On Thursday 22 July 2010 19:30:21 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> >>> +
> >>> +struct media_user_pad {
> >>> +	__u32 entity;		/* entity ID */
> >>> +	__u8 index;		/* pad index */
> >>> +	__u32 direction;	/* pad direction */
> >>> +};
> >> 
> >> Another small comment, I think you mentioned it yourself some time back
> >> 
> >> :-): how about some reserved fields to these structures?
> > 
> > Very good point. Reserved fields are needed in media_user_entity and
> > media_user_links at least. For media_user_pad and media_user_link, we
> > could do without reserved fields if we add fields to media_user_links to
> > store the size of those structures.
> 
> The structure size is part of the ioctl number defined by the _IOC macro

Not in this case, the ioctl uses a structure that stores pointers to the links 
and pads arrays.

> so I'd go with reserved fields even for these structures. Otherwise
> special handling would be required for these ioctls in a few places.

A few reserved links would still be good, yes.

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Hans,

On Saturday 24 July 2010 14:45:39 Hans Verkuil wrote:
> On Wednesday 21 July 2010 16:35:31 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 |  134
> >  ++++++++++++++++++++++++++++++++ drivers/media/media-device.c      | 
> >  153 +++++++++++++++++++++++++++++++++++++ include/linux/media.h        
> >      |   73 ++++++++++++++++++
> >  include/media/media-device.h      |    3 +
> >  include/media/media-entity.h      |   19 +-----
> >  5 files changed, 364 insertions(+), 18 deletions(-)
> >  create mode 100644 include/linux/media.h
> 
> <snip>
> 
> > diff --git a/include/linux/media.h b/include/linux/media.h
> > new file mode 100644
> > index 0000000..746bdda
> > --- /dev/null
> > +++ b/include/linux/media.h
> > @@ -0,0 +1,73 @@
> > +#ifndef __LINUX_MEDIA_H
> > +#define __LINUX_MEDIA_H
> > +
> > +#define MEDIA_ENTITY_TYPE_NODE				1
> > +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> > +
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> > +
> > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> > +
> > +#define MEDIA_PAD_DIR_INPUT				1
> > +#define MEDIA_PAD_DIR_OUTPUT				2
> > +
> > +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> > +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> > +
> > +#define MEDIA_ENTITY_ID_FLAG_NEXT	(1 << 31)
> > +
> > +struct media_user_pad {
> > +	__u32 entity;		/* entity ID */
> > +	__u8 index;		/* pad index */
> > +	__u32 direction;	/* pad direction */
> > +};
> 
> How about:
> 
> struct media_pad {
> 	__u32 entity;		/* entity ID */
> 	__u16 index;		/* pad index */
> 	__u16 flags;		/* pad flags (includes direction) */

Just to be sure, I suppose I should combine flags + direction in the 
media_k_pad structure as well, right ?

> 	u32 reserved;
> };

OK.

> I think u16 for the number of pads might be safer than a u8.

it should definitely be enough, otherwise we'll have a big issue anyway.

> > +
> > +struct media_user_entity {
> > +	__u32 id;
> > +	char name[32];
> > +	__u32 type;
> > +	__u32 subtype;
> > +	__u8 pads;
> > +	__u32 links;
> 
> Need reserved fields.

OK.

> > +
> > +	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 */
> 
> Add something like:
> 
> 		__u8 raw[64];

OK.

> 
> > +	};
> > +};
> > +
> > +struct media_user_link {
> > +	struct media_user_pad source;
> > +	struct media_user_pad sink;
> > +	__u32 flags;
> 
> Add a single __u32 reserved.

OK.

> > +};
> > +
> > +struct media_user_links {
> > +	__u32 entity;
> > +	/* Should have enough room for pads elements */
> > +	struct media_user_pad __user *pads;
> > +	/* Should have enough room for links elements */
> > +	struct media_user_link __user *links;
> 
> Add a  __u32 reserved[4].

OK.

> > +};
> > +
> > +#define MEDIA_IOC_ENUM_ENTITIES	_IOWR('M', 1, struct media_user_entity)
> > +#define MEDIA_IOC_ENUM_LINKS		_IOWR('M', 2, struct media_user_links)
> 
> We also need a MEDIA_IOC_QUERYCAP or MEDIA_IOC_VERSION or something like
> that.



-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Hans,

On Saturday 24 July 2010 14:18:11 Hans Verkuil wrote:
> On Wednesday 21 July 2010 16:35:28 Laurent Pinchart wrote:
> 
> <snip>
> 
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > new file mode 100644
> > index 0000000..fd44647
> > --- /dev/null
> > +++ b/include/media/media-entity.h
> > @@ -0,0 +1,79 @@
> > +#ifndef _MEDIA_ENTITY_H
> > +#define _MEDIA_ENTITY_H
> > +
> > +#include <linux/list.h>
> > +
> > +#define MEDIA_ENTITY_TYPE_NODE				1
> > +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> > +
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> > +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> > +
> > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> 
> These names are too awkward.
> 
> I see two options:
> 
> 1) Rename the type field to 'entity' and the macros to
> MEDIA_ENTITY_NODE/SUBDEV. Also rename subtype to type and the macros to
> MEDIA_ENTITY_TYPE_NODE_V4L and MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER. We
> might even get away with dropping _TYPE from the macro name.
> 
> 2) Merge type and subtype to a single entity field. The top 16 bits are the
> entity type, the bottom 16 bits are the subtype. That way you end up with:
> 
> #define MEDIA_ENTITY_NODE			(1 << 16)
> #define MEDIA_ENTITY_SUBDEV			(2 << 16)
> 
> #define MEDIA_ENTITY_NODE_V4L			(MEDIA_ENTITY_NODE + 1)
> 
> #define MEDIA_ENTITY_SUBDEV_VID_DECODER		(MEDIA_ENTITY_SUBDEV + 1)
> 
> I rather like this option myself.

I like option 2 better, but I would keep the field name "type" instead of 
"entity". Constants could start with MEDIA_ENTITY_TYPE_, or just MEDIA_ENTITY_ 
(I think I would prefer MEDIA_ENTITY_TYPE_).

> > +
> > +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> > +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> > +
> > +#define MEDIA_PAD_DIR_INPUT				1
> > +#define MEDIA_PAD_DIR_OUTPUT				2
> > +
> > +struct media_entity_link {
> > +	struct media_entity_pad *source;/* Source pad */
> > +	struct media_entity_pad *sink;	/* Sink pad  */
> > +	struct media_entity_link *other;/* Link in the reverse direction */
> > +	u32 flags;			/* Link flags (MEDIA_LINK_FLAG_*) */
> > +};
> > +
> > +struct media_entity_pad {
> > +	struct media_entity *entity;	/* Entity this pad belongs to */
> > +	u32 direction;			/* Pad direction (MEDIA_PAD_DIR_*) */
> > +	u8 index;			/* Pad index in the entity pads array */
> 
> We can use bitfields for direction and index. That way we can also easily
> add other flags/attributes.

You proposed to merge the direction field into a new flags field, I suppose 
that should be done here too for consistency. Having 16 flags might be a bit 
low though, 32 would be better. If you want to keep 16 bits for now, maybe we 
should have 2 reserved __u32 instead of one.

> > +};
> > +

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Murali,

On Monday 26 July 2010 18:19:05 Karicheri, Muralidharan wrote:
> Laurent,
> 
> Thanks for clarifying this. I guess this will also get documented in the
> v4l2 specs (if not already done) as part of this patch.

Sure it will. I haven't written the documentation yet, that's why those 
patches are posted as sample code only for now.

-- 
Regards,

Laurent Pinchart

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

[Sakari Ailus]

Laurent Pinchart wrote:
> Hi Hans,

Hi,

> On Saturday 24 July 2010 14:18:11 Hans Verkuil wrote:
>> On Wednesday 21 July 2010 16:35:28 Laurent Pinchart wrote:
...
>>> +
>>> +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
>>> +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
>>> +
>>> +#define MEDIA_PAD_DIR_INPUT				1
>>> +#define MEDIA_PAD_DIR_OUTPUT				2
>>> +
>>> +struct media_entity_link {
>>> +	struct media_entity_pad *source;/* Source pad */
>>> +	struct media_entity_pad *sink;	/* Sink pad  */
>>> +	struct media_entity_link *other;/* Link in the reverse direction */
>>> +	u32 flags;			/* Link flags (MEDIA_LINK_FLAG_*) */
>>> +};
>>> +
>>> +struct media_entity_pad {
>>> +	struct media_entity *entity;	/* Entity this pad belongs to */
>>> +	u32 direction;			/* Pad direction (MEDIA_PAD_DIR_*) */
>>> +	u8 index;			/* Pad index in the entity pads array */
>>
>> We can use bitfields for direction and index. That way we can also easily
>> add other flags/attributes.
> 
> You proposed to merge the direction field into a new flags field, I suppose 
> that should be done here too for consistency. Having 16 flags might be a bit 
> low though, 32 would be better. If you want to keep 16 bits for now, maybe we 
> should have 2 reserved __u32 instead of one.

I think we could have some more reserved fields than just one or two.
Nothing can replace reserved fields when you do need them.

Think of supporting dynamic format changes across a streaming pipeline,
for example. That might not happen ever, but once the hardware support
is there it might be something to think about.

Haven't exactly heard of problems of having too many of the reserved
fields, too few, yes! :-)

Regards,

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

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

[Hans Verkuil]

On Monday 26 July 2010 18:12:55 Laurent Pinchart wrote:
> On Friday 23 July 2010 17:56:02 Karicheri, Muralidharan wrote:
> > Laurent,
> > 
> > Could you explain the probe and active usage using an example such as
> > below?
> > 
> >             Link1    Link2
> > input sensor -> ccdc -> video node.
> > 
> > Assume Link2 we can have either format 1 or format 2 for capture.
> 
> Sure.
> 
> The probe and active formats are used to probe supported formats and 
> getting/setting active formats.

Just to verify: we are dealing with mediabus formats here, right?

One thing that I don't like is the name 'probe'. I would prefer the name
'proposed'. You propose an input format and based on that you can enumerate
a list of proposed output format. I think that name fits better than 'probe'
which I find confusing in this context.

> 
> * Enumerating supported formats on the CCDC input and output would be done 
> with the following calls
> 
> ENUM_FMT(CCDC input pad)
> 
> for the input, and
> 
> S_FMT(PROBE, CCDC input pad, format)
> ENUM_FMT(CCDC output pad)
> 
> for the output.

How does ENUM_FMT know if it has to use the proposed or actual input formats?
Or is it supposed to always act on proposed formats?

> 
> Setting the probe format on the input pad is required, as the format on an 
> output pad usually depends on the format on input pads.
> 
> * Trying a format on the CCDC input and output would be done with
> 
> S_FMT(PROBE, CCDC input pad, format)
> 
> for the input, and
> 
> S_FMT(PROBE, CCDC input pad, format)
> S_FMT(PROBE, CCDC output pad, format)
> 
> on the output. The S_FMT call will mangle the format given format if it can't 
> be supported exactly, so there's no need to call G_FMT after S_FMT (a G_FMT 
> call following a S_FMT call will return the same format as the S_FMT call).
> 
> * Setting the active format is done with
> 
> S_FMT(ACTIVE, CCDC input pad, format)
> S_FMT(ACTIVE, CCDC output pad, format)
> 
> The formats will be applied to the hardware (possibly with a delay, drivers 
> can delay register writes until STREAMON for instance).
> 
> Probe formats are stored in the subdev file handles, so two applications 
> trying formats at the same time will not interfere with each other. Active 
> formats are stored in the device structure, so modifications done by an 
> application are visible to other applications.
> 
> Hope this helps clarifying the API.

You know that I have never been happy with this approach, but I also have to
admit that I haven't found a better way of doing it.

Regards,

	Hans

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

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

[Laurent Pinchart]

Hi Hans,

On Monday 26 July 2010 21:42:49 Hans Verkuil wrote:
> On Monday 26 July 2010 18:12:55 Laurent Pinchart wrote:
> > On Friday 23 July 2010 17:56:02 Karicheri, Muralidharan wrote:
> > > Laurent,
> > > 
> > > Could you explain the probe and active usage using an example such as
> > > below?
> > > 
> > >             Link1    Link2
> > > 
> > > input sensor -> ccdc -> video node.
> > > 
> > > Assume Link2 we can have either format 1 or format 2 for capture.
> > 
> > Sure.
> > 
> > The probe and active formats are used to probe supported formats and
> > getting/setting active formats.
> 
> Just to verify: we are dealing with mediabus formats here, right?

That's correct.

> One thing that I don't like is the name 'probe'. I would prefer the name
> 'proposed'. You propose an input format and based on that you can enumerate
> a list of proposed output format. I think that name fits better than
> 'probe' which I find confusing in this context.

I'm open to all kind of suggestions. Those patches were not posted for review, 
they're not final yet. They're just sample code.

> > * Enumerating supported formats on the CCDC input and output would be
> > done with the following calls
> > 
> > ENUM_FMT(CCDC input pad)
> > 
> > for the input, and
> > 
> > S_FMT(PROBE, CCDC input pad, format)
> > ENUM_FMT(CCDC output pad)
> > 
> > for the output.
> 
> How does ENUM_FMT know if it has to use the proposed or actual input
> formats? Or is it supposed to always act on proposed formats?

Enumeration always uses the probe/proposed formats.

> > Setting the probe format on the input pad is required, as the format on
> > an output pad usually depends on the format on input pads.
> > 
> > * Trying a format on the CCDC input and output would be done with
> > 
> > S_FMT(PROBE, CCDC input pad, format)
> > 
> > for the input, and
> > 
> > S_FMT(PROBE, CCDC input pad, format)
> > S_FMT(PROBE, CCDC output pad, format)
> > 
> > on the output. The S_FMT call will mangle the format given format if it
> > can't be supported exactly, so there's no need to call G_FMT after S_FMT
> > (a G_FMT call following a S_FMT call will return the same format as the
> > S_FMT call).
> > 
> > * Setting the active format is done with
> > 
> > S_FMT(ACTIVE, CCDC input pad, format)
> > S_FMT(ACTIVE, CCDC output pad, format)
> > 
> > The formats will be applied to the hardware (possibly with a delay,
> > drivers can delay register writes until STREAMON for instance).
> > 
> > Probe formats are stored in the subdev file handles, so two applications
> > trying formats at the same time will not interfere with each other.
> > Active formats are stored in the device structure, so modifications done
> > by an application are visible to other applications.
> > 
> > Hope this helps clarifying the API.
> 
> You know that I have never been happy with this approach, but I also have
> to admit that I haven't found a better way of doing it.

Once again I'm open to proposals/discussions/whatever (well, mostly whatever 
:-)). Let's concentrate on the media controller patches first, I'll then 
submit the next set and we'll discuss it.

-- 
Regards,

Laurent Pinchart

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

[Hans Verkuil]

On Monday 26 July 2010 18:34:42 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Saturday 24 July 2010 14:45:39 Hans Verkuil wrote:
> > On Wednesday 21 July 2010 16:35:31 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 |  134
> > >  ++++++++++++++++++++++++++++++++ drivers/media/media-device.c      | 
> > >  153 +++++++++++++++++++++++++++++++++++++ include/linux/media.h        
> > >      |   73 ++++++++++++++++++
> > >  include/media/media-device.h      |    3 +
> > >  include/media/media-entity.h      |   19 +-----
> > >  5 files changed, 364 insertions(+), 18 deletions(-)
> > >  create mode 100644 include/linux/media.h
> > 
> > <snip>
> > 
> > > diff --git a/include/linux/media.h b/include/linux/media.h
> > > new file mode 100644
> > > index 0000000..746bdda
> > > --- /dev/null
> > > +++ b/include/linux/media.h
> > > @@ -0,0 +1,73 @@
> > > +#ifndef __LINUX_MEDIA_H
> > > +#define __LINUX_MEDIA_H
> > > +
> > > +#define MEDIA_ENTITY_TYPE_NODE				1
> > > +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> > > +
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> > > +
> > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> > > +
> > > +#define MEDIA_PAD_DIR_INPUT				1
> > > +#define MEDIA_PAD_DIR_OUTPUT				2
> > > +
> > > +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> > > +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> > > +
> > > +#define MEDIA_ENTITY_ID_FLAG_NEXT	(1 << 31)
> > > +
> > > +struct media_user_pad {
> > > +	__u32 entity;		/* entity ID */
> > > +	__u8 index;		/* pad index */
> > > +	__u32 direction;	/* pad direction */
> > > +};
> > 
> > How about:
> > 
> > struct media_pad {
> > 	__u32 entity;		/* entity ID */
> > 	__u16 index;		/* pad index */
> > 	__u16 flags;		/* pad flags (includes direction) */
> 
> Just to be sure, I suppose I should combine flags + direction in the 
> media_k_pad structure as well, right ?

Yes. I think we should just make a u32 flags, use bits 0 and 1 for the direction
and add a simple inline like this:

static inline u8 media_dir(struct media_pad *pad)
{
	return pad->flags & MEDIA_PAD_MASK_DIR);
}

And we should use the same for media_k_pad (and make a media_k_dir inline).

> 
> > 	u32 reserved;
> > };
> 
> OK.
> 
> > I think u16 for the number of pads might be safer than a u8.
> 
> it should definitely be enough, otherwise we'll have a big issue anyway.

Is u8 definitely enough or u16? I assume u16.

<snip>

Regards,

	Hans

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

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

[Hans Verkuil]

On Monday 26 July 2010 18:38:28 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Saturday 24 July 2010 14:18:11 Hans Verkuil wrote:
> > On Wednesday 21 July 2010 16:35:28 Laurent Pinchart wrote:
> > 
> > <snip>
> > 
> > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > > new file mode 100644
> > > index 0000000..fd44647
> > > --- /dev/null
> > > +++ b/include/media/media-entity.h
> > > @@ -0,0 +1,79 @@
> > > +#ifndef _MEDIA_ENTITY_H
> > > +#define _MEDIA_ENTITY_H
> > > +
> > > +#include <linux/list.h>
> > > +
> > > +#define MEDIA_ENTITY_TYPE_NODE				1
> > > +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> > > +
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> > > +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> > > +
> > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> > 
> > These names are too awkward.
> > 
> > I see two options:
> > 
> > 1) Rename the type field to 'entity' and the macros to
> > MEDIA_ENTITY_NODE/SUBDEV. Also rename subtype to type and the macros to
> > MEDIA_ENTITY_TYPE_NODE_V4L and MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER. We
> > might even get away with dropping _TYPE from the macro name.
> > 
> > 2) Merge type and subtype to a single entity field. The top 16 bits are the
> > entity type, the bottom 16 bits are the subtype. That way you end up with:
> > 
> > #define MEDIA_ENTITY_NODE			(1 << 16)
> > #define MEDIA_ENTITY_SUBDEV			(2 << 16)
> > 
> > #define MEDIA_ENTITY_NODE_V4L			(MEDIA_ENTITY_NODE + 1)
> > 
> > #define MEDIA_ENTITY_SUBDEV_VID_DECODER		(MEDIA_ENTITY_SUBDEV + 1)
> > 
> > I rather like this option myself.
> 
> I like option 2 better, but I would keep the field name "type" instead of 
> "entity". Constants could start with MEDIA_ENTITY_TYPE_, or just MEDIA_ENTITY_ 
> (I think I would prefer MEDIA_ENTITY_TYPE_).

Yes, I realized that later as well. Keep the 'type' field name.
I'm not sure about the macro name. I still think MEDIA_ENTITY_TYPE_SUBDEV_VID_DECODER
is too much of a mouthful.

> 
> > > +
> > > +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> > > +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> > > +
> > > +#define MEDIA_PAD_DIR_INPUT				1
> > > +#define MEDIA_PAD_DIR_OUTPUT				2
> > > +
> > > +struct media_entity_link {
> > > +	struct media_entity_pad *source;/* Source pad */
> > > +	struct media_entity_pad *sink;	/* Sink pad  */
> > > +	struct media_entity_link *other;/* Link in the reverse direction */
> > > +	u32 flags;			/* Link flags (MEDIA_LINK_FLAG_*) */
> > > +};
> > > +
> > > +struct media_entity_pad {
> > > +	struct media_entity *entity;	/* Entity this pad belongs to */
> > > +	u32 direction;			/* Pad direction (MEDIA_PAD_DIR_*) */
> > > +	u8 index;			/* Pad index in the entity pads array */
> > 
> > We can use bitfields for direction and index. That way we can also easily
> > add other flags/attributes.
> 
> You proposed to merge the direction field into a new flags field, I suppose 
> that should be done here too for consistency. Having 16 flags might be a bit 
> low though, 32 would be better. If you want to keep 16 bits for now, maybe we 
> should have 2 reserved __u32 instead of one.

Yes, let's use a u32 flags field for this.

Regards,

	Hans

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

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

[Laurent Pinchart]

Hi Hans,

On Monday 26 July 2010 21:48:28 Hans Verkuil wrote:
> On Monday 26 July 2010 18:34:42 Laurent Pinchart wrote:
> > On Saturday 24 July 2010 14:45:39 Hans Verkuil wrote:
> > > On Wednesday 21 July 2010 16:35:31 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 |  134
> > > >  ++++++++++++++++++++++++++++++++ drivers/media/media-device.c      |
> > > >  153 +++++++++++++++++++++++++++++++++++++ include/linux/media.h
> > > >  
> > > >      |   73 ++++++++++++++++++
> > > >  
> > > >  include/media/media-device.h      |    3 +
> > > >  include/media/media-entity.h      |   19 +-----
> > > >  5 files changed, 364 insertions(+), 18 deletions(-)
> > > >  create mode 100644 include/linux/media.h
> > > 
> > > <snip>
> > > 
> > > > diff --git a/include/linux/media.h b/include/linux/media.h
> > > > new file mode 100644
> > > > index 0000000..746bdda
> > > > --- /dev/null
> > > > +++ b/include/linux/media.h
> > > > @@ -0,0 +1,73 @@
> > > > +#ifndef __LINUX_MEDIA_H
> > > > +#define __LINUX_MEDIA_H
> > > > +
> > > > +#define MEDIA_ENTITY_TYPE_NODE				1
> > > > +#define MEDIA_ENTITY_TYPE_SUBDEV			2
> > > > +
> > > > +#define MEDIA_ENTITY_SUBTYPE_NODE_V4L			1
> > > > +#define MEDIA_ENTITY_SUBTYPE_NODE_FB			2
> > > > +#define MEDIA_ENTITY_SUBTYPE_NODE_ALSA			3
> > > > +#define MEDIA_ENTITY_SUBTYPE_NODE_DVB			4
> > > > +
> > > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_DECODER		1
> > > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_VID_ENCODER		2
> > > > +#define MEDIA_ENTITY_SUBTYPE_SUBDEV_MISC		3
> > > > +
> > > > +#define MEDIA_PAD_DIR_INPUT				1
> > > > +#define MEDIA_PAD_DIR_OUTPUT				2
> > > > +
> > > > +#define MEDIA_LINK_FLAG_ACTIVE				(1 << 0)
> > > > +#define MEDIA_LINK_FLAG_IMMUTABLE			(1 << 1)
> > > > +
> > > > +#define MEDIA_ENTITY_ID_FLAG_NEXT	(1 << 31)
> > > > +
> > > > +struct media_user_pad {
> > > > +	__u32 entity;		/* entity ID */
> > > > +	__u8 index;		/* pad index */
> > > > +	__u32 direction;	/* pad direction */
> > > > +};
> > > 
> > > How about:
> > > 
> > > struct media_pad {
> > > 
> > > 	__u32 entity;		/* entity ID */
> > > 	__u16 index;		/* pad index */
> > > 	__u16 flags;		/* pad flags (includes direction) */
> > 
> > Just to be sure, I suppose I should combine flags + direction in the
> > media_k_pad structure as well, right ?
> 
> Yes. I think we should just make a u32 flags, use bits 0 and 1 for the
> direction and add a simple inline like this:
> 
> static inline u8 media_dir(struct media_pad *pad)
> {
> 	return pad->flags & MEDIA_PAD_MASK_DIR);
> }
> 
> And we should use the same for media_k_pad (and make a media_k_dir inline).

OK.

> > > 	u32 reserved;
> > > 
> > > };
> > 
> > OK.
> > 
> > > I think u16 for the number of pads might be safer than a u8.
> > 
> > it should definitely be enough, otherwise we'll have a big issue anyway.
> 
> Is u8 definitely enough or u16? I assume u16.

u16. u8 should be enough, u16 should definitely be enough :-)

-- 
Regards,

Laurent Pinchart